linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-02-26 23:45 [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot K. Y. Srinivasan
@ 2016-02-26 22:25 ` James Bottomley
  2016-02-26 23:22   ` KY Srinivasan
  0 siblings, 1 reply; 12+ messages in thread
From: James Bottomley @ 2016-02-26 22:25 UTC (permalink / raw)
  To: K. Y. Srinivasan, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare

On Fri, 2016-02-26 at 15:45 -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

Well, I suppose continually sending the wrong patch until I get annoyed
enough to send the right one is one way of doing it.  This patch is
wrong. what you want is below.

You want HYPERV_STORAGE to be built in if the FC attributes are,
otherwise you don't care because if they're N the FC code will be
compiled out.

James

---

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e2f31c9..5ecabdb 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -596,6 +596,7 @@ config XEN_SCSI_FRONTEND
 config HYPERV_STORAGE
 	tristate "Microsoft Hyper-V virtual storage driver"
 	depends on SCSI && HYPERV
+	depends on m || SCSI_FC_ATTRS
 	default HYPERV
 	help
 	  Select this option to enable the Hyper-V virtual storage driver.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-02-26 22:25 ` James Bottomley
@ 2016-02-26 23:22   ` KY Srinivasan
  2016-02-26 23:33     ` James Bottomley
  0 siblings, 1 reply; 12+ messages in thread
From: KY Srinivasan @ 2016-02-26 23:22 UTC (permalink / raw)
  To: James Bottomley, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Friday, February 26, 2016 2:25 PM
> To: KY Srinivasan <kys@microsoft.com>; 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 Fri, 2016-02-26 at 15:45 -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
> 
> Well, I suppose continually sending the wrong patch until I get annoyed
> enough to send the right one is one way of doing it.  This patch is
> wrong. what you want is below.

James, that was not my intent; although it is a tempting strategy!
Here is an excerpt of your comments on v2 of this patch:
(dated Jan 29 of this year):

"No ... if you want to depend on the FC_ATTRS then a simple depend
works.  If you want to be able to build without them or with them, then
that line must read

depends on m || SCSI_FC_ATTRS != m"

Since all I wanted was to depend on FC_ATTRS I chose to go with your recommendation -
which also happened to be what I had initially sent (v1 of this patch).

> 
> You want HYPERV_STORAGE to be built in if the FC attributes are,
> otherwise you don't care because if they're N the FC code will be
> compiled out.

If FC attributes are built in, I have no issue - I want to be able to build the storvsc
driver both as a module as well as built in. However, if storvsc is built as part of the kernel,
I want to make sure that FC attributes are also built as part of the kernel. The build test failure
that was reported was this case. With this patch, the build issue reported by kbuild cannot
happen since if the FC_ATTRS are built as a module, storvsc cannot be builtin.

Regards,

K. Y 
> 
> James
> 
> ---
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index e2f31c9..5ecabdb 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -596,6 +596,7 @@ config XEN_SCSI_FRONTEND
>  config HYPERV_STORAGE
>  	tristate "Microsoft Hyper-V virtual storage driver"
>  	depends on SCSI && HYPERV
> +	depends on m || SCSI_FC_ATTRS
>  	default HYPERV
>  	help
>  	  Select this option to enable the Hyper-V virtual storage driver.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-02-26 23:22   ` KY Srinivasan
@ 2016-02-26 23:33     ` James Bottomley
  2016-02-26 23:58       ` KY Srinivasan
  0 siblings, 1 reply; 12+ messages in thread
From: James Bottomley @ 2016-02-26 23:33 UTC (permalink / raw)
  To: KY Srinivasan, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare

On Fri, 2016-02-26 at 23:22 +0000, KY Srinivasan wrote:
> 
> > -----Original Message-----
> > From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com
> > ]
> > Sent: Friday, February 26, 2016 2:25 PM
> > To: KY Srinivasan <kys@microsoft.com>; 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 Fri, 2016-02-26 at 15:45 -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
> > 
> > Well, I suppose continually sending the wrong patch until I get
> > annoyed
> > enough to send the right one is one way of doing it.  This patch is
> > wrong. what you want is below.
> 
> James, that was not my intent; although it is a tempting strategy!
> Here is an excerpt of your comments on v2 of this patch:
> (dated Jan 29 of this year):
> 
> "No ... if you want to depend on the FC_ATTRS then a simple depend
> works.  If you want to be able to build without them or with them,
> then
> that line must read
> 
> depends on m || SCSI_FC_ATTRS != m"
> 
> Since all I wanted was to depend on FC_ATTRS I chose to go with your
> recommendation -
> which also happened to be what I had initially sent (v1 of this
> patch).

If you're going to depend on the FC_ATTRS then you don't need all of
the 

#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)

In the driver, because you're never building without it.  I thought
those were put in to satisfy Hannes' request that the driver be
buildable without the attributes?

> > You want HYPERV_STORAGE to be built in if the FC attributes are,
> > otherwise you don't care because if they're N the FC code will be
> > compiled out.
> 
> If FC attributes are built in, I have no issue - I want to be able to 
> build the storvsc driver both as a module as well as built in. 
> However, if storvsc is built as part of the kernel, I want to make 
> sure that FC attributes are also built as part of the kernel. The 
> build test failure that was reported was this case. With this patch, 
> the build issue reported by kbuild cannot happen since if the 
> FC_ATTRS are built as a module, storvsc cannot be builtin.

Oh, right, the line should read

depends on m || SCSI_FC_ATTRS != m

for that case.

James

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
@ 2016-02-26 23:45 K. Y. Srinivasan
  2016-02-26 22:25 ` James Bottomley
  0 siblings, 1 reply; 12+ messages in thread
From: K. Y. Srinivasan @ 2016-02-26 23:45 UTC (permalink / raw)
  To: linux-kernel, devel, ohering, jbottomley, hch, linux-scsi, apw,
	vkuznets, jasowang, martin.petersen, hare
  Cc: K. Y. Srinivasan

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(-)

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.
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-02-26 23:33     ` James Bottomley
@ 2016-02-26 23:58       ` KY Srinivasan
  0 siblings, 0 replies; 12+ messages in thread
From: KY Srinivasan @ 2016-02-26 23:58 UTC (permalink / raw)
  To: James Bottomley, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Friday, February 26, 2016 3:33 PM
> To: KY Srinivasan <kys@microsoft.com>; 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 Fri, 2016-02-26 at 23:22 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: James Bottomley
> [mailto:James.Bottomley@HansenPartnership.com
> > > ]
> > > Sent: Friday, February 26, 2016 2:25 PM
> > > To: KY Srinivasan <kys@microsoft.com>; 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 Fri, 2016-02-26 at 15:45 -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
> > >
> > > Well, I suppose continually sending the wrong patch until I get
> > > annoyed
> > > enough to send the right one is one way of doing it.  This patch is
> > > wrong. what you want is below.
> >
> > James, that was not my intent; although it is a tempting strategy!
> > Here is an excerpt of your comments on v2 of this patch:
> > (dated Jan 29 of this year):
> >
> > "No ... if you want to depend on the FC_ATTRS then a simple depend
> > works.  If you want to be able to build without them or with them,
> > then
> > that line must read
> >
> > depends on m || SCSI_FC_ATTRS != m"
> >
> > Since all I wanted was to depend on FC_ATTRS I chose to go with your
> > recommendation -
> > which also happened to be what I had initially sent (v1 of this
> > patch).
> 
> If you're going to depend on the FC_ATTRS then you don't need all of
> the
> 
> #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
> 
> In the driver, because you're never building without it.  I thought
> those were put in to satisfy Hannes' request that the driver be
> buildable without the attributes?
> 
> > > You want HYPERV_STORAGE to be built in if the FC attributes are,
> > > otherwise you don't care because if they're N the FC code will be
> > > compiled out.
> >
> > If FC attributes are built in, I have no issue - I want to be able to
> > build the storvsc driver both as a module as well as built in.
> > However, if storvsc is built as part of the kernel, I want to make
> > sure that FC attributes are also built as part of the kernel. The
> > build test failure that was reported was this case. With this patch,
> > the build issue reported by kbuild cannot happen since if the
> > FC_ATTRS are built as a module, storvsc cannot be builtin.
> 
> Oh, right, the line should read
> 
> depends on m || SCSI_FC_ATTRS != m
> 
> for that case.

Thanks James. I tested your suggestion and I will send out the updated version of the patch.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28 19:22     ` James Bottomley
@ 2016-01-28 19:28       ` KY Srinivasan
  0 siblings, 0 replies; 12+ messages in thread
From: KY Srinivasan @ 2016-01-28 19:28 UTC (permalink / raw)
  To: James Bottomley, Olaf Hering
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare



> -----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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28 19:07   ` KY Srinivasan
@ 2016-01-28 19:22     ` James Bottomley
  2016-01-28 19:28       ` KY Srinivasan
  0 siblings, 1 reply; 12+ messages in thread
From: James Bottomley @ 2016-01-28 19:22 UTC (permalink / raw)
  To: KY Srinivasan, Olaf Hering
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28 15:56 ` Olaf Hering
@ 2016-01-28 19:07   ` KY Srinivasan
  2016-01-28 19:22     ` James Bottomley
  0 siblings, 1 reply; 12+ messages in thread
From: KY Srinivasan @ 2016-01-28 19:07 UTC (permalink / raw)
  To: Olaf Hering
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare



> -----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.

Regards,

K. Y
> 
> Olaf

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28  7:29 K. Y. Srinivasan
  2016-01-28  6:02 ` James Bottomley
@ 2016-01-28 15:56 ` Olaf Hering
  2016-01-28 19:07   ` KY Srinivasan
  1 sibling, 1 reply; 12+ messages in thread
From: Olaf Hering @ 2016-01-28 15:56 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28  6:02 ` James Bottomley
@ 2016-01-28 15:46   ` KY Srinivasan
  0 siblings, 0 replies; 12+ messages in thread
From: KY Srinivasan @ 2016-01-28 15:46 UTC (permalink / raw)
  To: James Bottomley, gregkh, linux-kernel, devel, ohering,
	jbottomley, hch, linux-scsi, apw, vkuznets, jasowang,
	martin.petersen, hare



> -----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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
@ 2016-01-28  7:29 K. Y. Srinivasan
  2016-01-28  6:02 ` James Bottomley
  2016-01-28 15:56 ` Olaf Hering
  0 siblings, 2 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2016-01-28  7:29 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, martin.petersen, hare
  Cc: K. Y. Srinivasan

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(-)

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.
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot
  2016-01-28  7:29 K. Y. Srinivasan
@ 2016-01-28  6:02 ` James Bottomley
  2016-01-28 15:46   ` KY Srinivasan
  2016-01-28 15:56 ` Olaf Hering
  1 sibling, 1 reply; 12+ messages in thread
From: James Bottomley @ 2016-01-28  6:02 UTC (permalink / raw)
  To: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering,
	jbottomley, hch, linux-scsi, apw, vkuznets, jasowang,
	martin.petersen, hare

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-02-26 23:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 23:45 [PATCH 1/1] scsi: storvsc: Fix a build issue reported by kbuild test robot K. Y. Srinivasan
2016-02-26 22:25 ` James Bottomley
2016-02-26 23:22   ` KY Srinivasan
2016-02-26 23:33     ` James Bottomley
2016-02-26 23:58       ` KY Srinivasan
  -- strict thread matches above, loose matches on Subject: below --
2016-01-28  7:29 K. Y. Srinivasan
2016-01-28  6:02 ` James Bottomley
2016-01-28 15:46   ` KY Srinivasan
2016-01-28 15:56 ` Olaf Hering
2016-01-28 19:07   ` KY Srinivasan
2016-01-28 19:22     ` James Bottomley
2016-01-28 19:28       ` KY Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).