diff mbox

[1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot

Message ID 1453966147-10568-1-git-send-email-kys@microsoft.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

KY Srinivasan Jan. 28, 2016, 7:29 a.m. UTC
tree:   https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgit.kernel.org%2fpub%2fscm%2flinux%2fkernel%2fgit%2ftorvalds%2flinux.git&data=01%7c01%7ckys%40microsoft.com%7ce2e0622715844b79ad7108d32796ec3c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=ubr4GbBaNS%2ftOz%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(-)

Comments

James Bottomley Jan. 28, 2016, 6:02 a.m. UTC | #1
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
KY Srinivasan Jan. 28, 2016, 3:46 p.m. UTC | #2
> -----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
Olaf Hering Jan. 28, 2016, 3:56 p.m. UTC | #3
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
KY Srinivasan Jan. 28, 2016, 7:07 p.m. UTC | #4
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
James Bottomley Jan. 28, 2016, 7:22 p.m. UTC | #5
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
KY Srinivasan Jan. 28, 2016, 7:28 p.m. UTC | #6
> -----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 mbox

Patch

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.