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

[KY Srinivasan]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxxxxxxxxxxx]
> Sent: Friday, February 26, 2016 2:25 PM
> To: KY Srinivasan <kys@xxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> devel@xxxxxxxxxxxxxxxxxxxxxx; ohering@xxxxxxxx;
> jbottomley@xxxxxxxxxxxxx; hch@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx;
> apw@xxxxxxxxxxxxx; vkuznets@xxxxxxxxxx; jasowang@xxxxxxxxxx;
> martin.petersen@xxxxxxxxxx; hare@xxxxxxx
> 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@xxxxxxxxxxxxx>
> > Reported-by: Fengguang Wu <fengguang.wu@xxxxxxxxx>
> > ---
> >  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.