Message ID | 1453966147-10568-1-git-send-email-kys@microsoft.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
On Wed, 2016-01-27 at 23:29 -0800, K. Y. Srinivasan wrote: > tree: https://na01.safelinks.protection.outlook.com/?url=https%3a%2 > f%2fgit.kernel.org%2fpub%2fscm%2flinux%2fkernel%2fgit%2ftorvalds%2fli > nux.git&data=01%7c01%7ckys%40microsoft.com%7ce2e0622715844b79ad7108d3 > 2796ec3c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=ubr4GbBaNS%2ftO > z%2buJBk0CL9N0UNG9x2TidLgy6Yovg4%3d master > head: 03c21cb775a313f1ff19be59c5d02df3e3526471 > commit: dac582417bc449b1f7f572d3f1dd9d23eec15cc9 storvsc: Properly > support Fibre Channel devices > date: 3 weeks ago > config: x86_64-randconfig-s3-01281016 (attached as .config) > reproduce: > git checkout dac582417bc449b1f7f572d3f1dd9d23eec15cc9 > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > > drivers/built-in.o: In function `storvsc_remove': > > > storvsc_drv.c:(.text+0x213af7): undefined reference to > > > `fc_remove_host' > drivers/built-in.o: In function `storvsc_drv_init': > > > storvsc_drv.c:(.init.text+0xcbcc): undefined reference to > > > `fc_attach_transport' > > > storvsc_drv.c:(.init.text+0xcc06): undefined reference to > > > `fc_release_transport' > drivers/built-in.o: In function `storvsc_drv_exit': > > > storvsc_drv.c:(.exit.text+0x123c): undefined reference to > > > `fc_release_transport' > > With this commit, the storvsc driver depends on FC atttributes. Make > this > dependency explicit. > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > --- > drivers/scsi/Kconfig | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 64eed87..24365c3 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -594,6 +594,7 @@ config XEN_SCSI_FRONTEND > config HYPERV_STORAGE > tristate "Microsoft Hyper-V virtual storage driver" > depends on SCSI && HYPERV > + depends on SCSI_FC_ATTRS > default HYPERV > help > Select this option to enable the Hyper-V virtual storage > driver. OK, so I thought Hannes requested that you not make the hyperv driver depend on the FC attrs and you said you would ... has this changed? James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com] > Sent: Wednesday, January 27, 2016 10:03 PM > To: KY Srinivasan <kys@microsoft.com>; gregkh@linuxfoundation.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; ohering@suse.com; > jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org; > apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; > martin.petersen@oracle.com; hare@suse.de > Subject: Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test > robot > > On Wed, 2016-01-27 at 23:29 -0800, K. Y. Srinivasan wrote: > > tree: https://na01.safelinks.protection.outlook.com/?url=https%3a%2 > > f%2fgit.kernel.org%2fpub%2fscm%2flinux%2fkernel%2fgit%2ftorvalds%2fli > > > nux.git&data=01%7c01%7ckys%40microsoft.com%7ce2e0622715844b79ad71 > 08d3 > > > 2796ec3c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=ubr4GbBaNS > %2ftO > > z%2buJBk0CL9N0UNG9x2TidLgy6Yovg4%3d master > > head: 03c21cb775a313f1ff19be59c5d02df3e3526471 > > commit: dac582417bc449b1f7f572d3f1dd9d23eec15cc9 storvsc: Properly > > support Fibre Channel devices > > date: 3 weeks ago > > config: x86_64-randconfig-s3-01281016 (attached as .config) > > reproduce: > > git checkout dac582417bc449b1f7f572d3f1dd9d23eec15cc9 > > # save the attached .config to linux build tree > > make ARCH=x86_64 > > > > All errors (new ones prefixed by >>): > > > > drivers/built-in.o: In function `storvsc_remove': > > > > storvsc_drv.c:(.text+0x213af7): undefined reference to > > > > `fc_remove_host' > > drivers/built-in.o: In function `storvsc_drv_init': > > > > storvsc_drv.c:(.init.text+0xcbcc): undefined reference to > > > > `fc_attach_transport' > > > > storvsc_drv.c:(.init.text+0xcc06): undefined reference to > > > > `fc_release_transport' > > drivers/built-in.o: In function `storvsc_drv_exit': > > > > storvsc_drv.c:(.exit.text+0x123c): undefined reference to > > > > `fc_release_transport' > > > > With this commit, the storvsc driver depends on FC atttributes. Make > > this > > dependency explicit. > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > > --- > > drivers/scsi/Kconfig | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > > index 64eed87..24365c3 100644 > > --- a/drivers/scsi/Kconfig > > +++ b/drivers/scsi/Kconfig > > @@ -594,6 +594,7 @@ config XEN_SCSI_FRONTEND > > config HYPERV_STORAGE > > tristate "Microsoft Hyper-V virtual storage driver" > > depends on SCSI && HYPERV > > + depends on SCSI_FC_ATTRS > > default HYPERV > > help > > Select this option to enable the Hyper-V virtual storage > > driver. > > OK, so I thought Hannes requested that you not make the hyperv driver > depend on the FC attrs and you said you would ... has this changed? Since 99% of the code would be identical, Hannes agreed that it would not be good to have a separate FC driver. Given that, this is the only option we have. Regards, K. Y > > James
On Wed, Jan 27, K. Y. Srinivasan wrote:
> + depends on SCSI_FC_ATTRS
I think 'depends' instead of 'select' will cause HYPERV_STORAGE to
disapepar during make oldconfig if SCSI_FC_ATTRS was not set before.
Not sure what the policy of 'depends' vs. 'select' actually is. If
SCSI_FC_ATTRS is supposed to be a library kind of thing then 'select'
might be the correct way.
Olaf
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogT2xhZiBIZXJpbmcgW21h aWx0bzpvbGFmQGFlcGZsZS5kZV0NCj4gU2VudDogVGh1cnNkYXksIEphbnVhcnkgMjgsIDIwMTYg Nzo1NiBBTQ0KPiBUbzogS1kgU3Jpbml2YXNhbiA8a3lzQG1pY3Jvc29mdC5jb20+DQo+IENjOiBn cmVna2hAbGludXhmb3VuZGF0aW9uLm9yZzsgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsN Cj4gZGV2ZWxAbGludXhkcml2ZXJwcm9qZWN0Lm9yZzsgb2hlcmluZ0BzdXNlLmNvbTsNCj4gamJv dHRvbWxleUBwYXJhbGxlbHMuY29tOyBoY2hAaW5mcmFkZWFkLm9yZzsgbGludXgtc2NzaUB2Z2Vy Lmtlcm5lbC5vcmc7DQo+IGFwd0BjYW5vbmljYWwuY29tOyB2a3V6bmV0c0ByZWRoYXQuY29tOyBq YXNvd2FuZ0ByZWRoYXQuY29tOw0KPiBtYXJ0aW4ucGV0ZXJzZW5Ab3JhY2xlLmNvbTsgaGFyZUBz dXNlLmRlDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMS8xXSBzY3NpOiBzdG9ydnNjOiBGaXggYSBi dWlsZCBpc3N1ZSByZXBvcnRlZCBieSBrYnVpbGQgdGVzdA0KPiByb2JvdA0KPiANCj4gT24gV2Vk LCBKYW4gMjcsIEsuIFkuIFNyaW5pdmFzYW4gd3JvdGU6DQo+IA0KPiA+ICsJZGVwZW5kcyBvbiBT Q1NJX0ZDX0FUVFJTDQo+IA0KPiBJIHRoaW5rICdkZXBlbmRzJyBpbnN0ZWFkIG9mICdzZWxlY3Qn IHdpbGwgY2F1c2UgSFlQRVJWX1NUT1JBR0UgdG8NCj4gZGlzYXBlcGFyIGR1cmluZyBtYWtlIG9s ZGNvbmZpZyBpZiBTQ1NJX0ZDX0FUVFJTIHdhcyBub3Qgc2V0IGJlZm9yZS4NCj4gTm90IHN1cmUg d2hhdCB0aGUgcG9saWN5IG9mICdkZXBlbmRzJyB2cy4gICdzZWxlY3QnIGFjdHVhbGx5IGlzLiAg SWYNCj4gU0NTSV9GQ19BVFRSUyBpcyBzdXBwb3NlZCB0byBiZSBhIGxpYnJhcnkga2luZCBvZiB0 aGluZyB0aGVuICdzZWxlY3QnDQo+IG1pZ2h0IGJlIHRoZSBjb3JyZWN0IHdheS4NCg0KVGhlIGN1 cnJlbnQgYnVpbGQgaXNzdWUgaXMgYmVjYXVzZSB0aGUgSHlwZXItViBzdG9yYWdlIGlzIGNvbmZp Z3VyZWQgdG8gYmUgYnVpbHQgd2l0aA0KdGhlIGtlcm5lbCB3aGlsZSB0aGUgU0NTSV9GQ19BQVRS UyBpcyBjb25maWd1cmVkIGFzIGEgbW9kdWxlLiBUaGlzIHBhdGNoIGZpeGVzIHRoYXQgaXNzdWUu DQpJIHRvbyBhbSBub3Qgc3VyZSB3aGF0IHRoZSBwb2xpY3kgb2YgdXNpbmcgImRlcGVuZHMiIHZz ICIgc2VsZWN0IiBpcy4NCkphbWVzLCB3aGF0IHdvdWxkIGJlIHlvdXIgcmVjb21tZW5kYXRpb24g aGVyZS4NCg0KUmVnYXJkcywNCg0KSy4gWQ0KPiANCj4gT2xhZg0K -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2016-01-28 at 19:07 +0000, KY Srinivasan wrote: > > > -----Original Message----- > > From: Olaf Hering [mailto:olaf@aepfle.de] > > Sent: Thursday, January 28, 2016 7:56 AM > > To: KY Srinivasan <kys@microsoft.com> > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > devel@linuxdriverproject.org; ohering@suse.com; > > jbottomley@parallels.com; hch@infradead.org; > > linux-scsi@vger.kernel.org; > > apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; > > martin.petersen@oracle.com; hare@suse.de > > Subject: Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported > > by kbuild test > > robot > > > > On Wed, Jan 27, K. Y. Srinivasan wrote: > > > > > + depends on SCSI_FC_ATTRS > > > > I think 'depends' instead of 'select' will cause HYPERV_STORAGE to > > disapepar during make oldconfig if SCSI_FC_ATTRS was not set > > before. > > Not sure what the policy of 'depends' vs. 'select' actually is. > > If > > SCSI_FC_ATTRS is supposed to be a library kind of thing then > > 'select' > > might be the correct way. > > The current build issue is because the Hyper-V storage is configured > to be built with > the kernel while the SCSI_FC_AATRS is configured as a module. This > patch fixes that issue. > I too am not sure what the policy of using "depends" vs " select" is. > James, what would be your recommendation here. Oh, you don't care if FC_ATTRS are enabled, but if they are you need to exclude the case where storvsc is built in but FC_ATTRS is a module? This is the accepted way of doing it: depends on m || SCSI_FC_ATTRS != m James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com] > Sent: Thursday, January 28, 2016 11:22 AM > To: KY Srinivasan <kys@microsoft.com>; Olaf Hering <olaf@aepfle.de> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; > jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org; > apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; > martin.petersen@oracle.com; hare@suse.de > Subject: Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test > robot > > On Thu, 2016-01-28 at 19:07 +0000, KY Srinivasan wrote: > > > > > -----Original Message----- > > > From: Olaf Hering [mailto:olaf@aepfle.de] > > > Sent: Thursday, January 28, 2016 7:56 AM > > > To: KY Srinivasan <kys@microsoft.com> > > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > > devel@linuxdriverproject.org; ohering@suse.com; > > > jbottomley@parallels.com; hch@infradead.org; > > > linux-scsi@vger.kernel.org; > > > apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; > > > martin.petersen@oracle.com; hare@suse.de > > > Subject: Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported > > > by kbuild test > > > robot > > > > > > On Wed, Jan 27, K. Y. Srinivasan wrote: > > > > > > > + depends on SCSI_FC_ATTRS > > > > > > I think 'depends' instead of 'select' will cause HYPERV_STORAGE to > > > disapepar during make oldconfig if SCSI_FC_ATTRS was not set > > > before. > > > Not sure what the policy of 'depends' vs. 'select' actually is. > > > If > > > SCSI_FC_ATTRS is supposed to be a library kind of thing then > > > 'select' > > > might be the correct way. > > > > The current build issue is because the Hyper-V storage is configured > > to be built with > > the kernel while the SCSI_FC_AATRS is configured as a module. This > > patch fixes that issue. > > I too am not sure what the policy of using "depends" vs " select" is. > > James, what would be your recommendation here. > > Oh, you don't care if FC_ATTRS are enabled, but if they are you need to > exclude the case where storvsc is built in but FC_ATTRS is a module? > This is the accepted way of doing it: > > depends on m || SCSI_FC_ATTRS != m Thanks James, I will resubmit this patch with the change you have recommended. K. Y
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 64eed87..24365c3 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -594,6 +594,7 @@ config XEN_SCSI_FRONTEND config HYPERV_STORAGE tristate "Microsoft Hyper-V virtual storage driver" depends on SCSI && HYPERV + depends on SCSI_FC_ATTRS default HYPERV help Select this option to enable the Hyper-V virtual storage driver.