Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1567444 > unrolled thread

[PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.  Cathy Avery <cavery@redhat.com> - 2017-01-26 14:50 +0100
    Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight  host. Christoph Hellwig <hch@infradead.org> - 2017-01-26 16:00 +0100
      RE: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight  host. KY Srinivasan <kys@microsoft.com> - 2017-01-29 02:20 +0100
        Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight  host. Christoph Hellwig <hch@infradead.org> - 2017-01-29 10:50 +0100

#1567444 — [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

This patch set is based on the following patch submission
and email exchange:

[PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on Hyper-V
K. Y. Srinivasan kys at microsoft.com
Sat Mar 12 21:52:48 UTC 2016

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-March/087116.html

Included in the current storvsc driver for Hyper-V is the ability
to access luns on an FC fabric via a virtualized fiber channel
adapter exposed by the Hyper-V host. This was done to provide an
interface for existing customer tools that was more consistent with
a conventional FC device. The driver attaches to the FC transport 
to allow host and port names to be published under 
/sys/class/fc_host/hostX.

A problem arose when attaching to the FC transport. The scsi_scan code
attempts to call fc_user_scan which has basically become a no-op 
due to the virtualized nature of the FC host 
( missing rports, vports, etc ). At this point you cannot refresh
the scsi bus after mapping or unmapping luns on the SAN without
a reboot.

The patch above attempted to address the problem of not being
able to scan FC hosts on a Hyper-V guest by setting
fc_transport_template->user_scan = NULL but it was rejected
in favor of a new "lightweight" version of the FC transport that
only provides the bare minimum functionality of the standard FC model.
This new transport option would be more suitable for FC transports
running on a VM and provide some flexibility in the future.

The patches below offer a method to incorporate the new 
lightweight FC option into the existing transport
and storvsc drivers.

Patch 1: scsi_transport_fc.h, scsi_transport_fc.c

1) Adds the lightweight_transport option to fc_function_template.
Based on this selection the transport will either be lightweight
or default to heavyweight.

2) Divides the applicable export functions into two sets.
The lightweight functions involve FC attributes port_name and 
node name. The functions that deal with targets, rports, etc
are not used. The heavyweight default contains the original
standard physical FC hba attribute set. 

3) All top level FC class directories such fc_remote_ports,
fc_transport, and fc_vports are still created when the transport
driver is loaded. They are just not populated when running in
lightweight mode. Conceptually both lightweight and heavyweight
clients could coexist.

4) fc_transport_template->user_scan is now null and the bus 
can be scanned.

Patch 2: storvsc.c

1) storvsc elects to use the new lightweight FC host option 
by enabling it in fc_function_template. 

2) Removes an original workaround dealing with replacing
the eh_timed_out function. Patch 1 will not set the 
scsi_transport_template.eh_timed_out function directly during
lightweight fc_attach_transport(). It instead relies on
whatever was indicated as the scsi_host_template timeout handler
during scsi_times_out() scsi_error.c. So the workaround is 
no longer necessary.


It has been suggested that the word lightweight may not be
the best choice of terms when describing the new FC transport
option.  I can offer a few new ones but I am not particularly
imaginative. 

Virtual FC
Mini FC
Host only FC 


Changes from V1:

Added more comments and documentation in the code regarding 
the lightweight feature.


Cathy Avery (2):
  scsi: scsi_transport_fc: Provide a lightweight option for Virtual FC
    Hosts.
  scsi: storvsc: Add support for FC lightweight host.

 drivers/scsi/scsi_transport_fc.c | 144 +++++++++++++++++++++++++++++++++++++--
 drivers/scsi/storvsc_drv.c       |  12 ++--
 include/scsi/scsi_transport_fc.h |   2 +
 3 files changed, 149 insertions(+), 9 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]

#1567481 — Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

On Thu, Jan 26, 2017 at 08:38:58AM -0500, Cathy Avery wrote:
> Included in the current storvsc driver for Hyper-V is the ability
> to access luns on an FC fabric via a virtualized fiber channel
> adapter exposed by the Hyper-V host. This was done to provide an
> interface for existing customer tools that was more consistent with
> a conventional FC device. The driver attaches to the FC transport 
> to allow host and port names to be published under 
> /sys/class/fc_host/hostX.
> 
> A problem arose when attaching to the FC transport. The scsi_scan code
> attempts to call fc_user_scan which has basically become a no-op 
> due to the virtualized nature of the FC host 
> ( missing rports, vports, etc ). At this point you cannot refresh
> the scsi bus after mapping or unmapping luns on the SAN without
> a reboot.

I don't think a device without rports or vports is a FC device, plain and
simple.  So as far as I'm concerned we should remove the code from storvsc
that pretends to be FC, and not add it to virtio to start with.

And again I think leightweight is a very confusing name -
what exactly is leight or heavy?   It's really fake or dummy
in the current version.

> 
> 2) Removes an original workaround dealing with replacing
> the eh_timed_out function. Patch 1 will not set the 
> scsi_transport_template.eh_timed_out function directly during
> lightweight fc_attach_transport(). It instead relies on
> whatever was indicated as the scsi_host_template timeout handler
> during scsi_times_out() scsi_error.c. So the workaround is 
> no longer necessary.

Can you send a patch that gets rid of the transport class timeout handler
entirely?  I think it's simply the wrong layering we have here - the
driver needs to be in control of timeouts, and if it wants it can
optionally call into library code in the transport class.


FYI, all the long-term relevant explanation need to go into the patches
themselves (patch description or code comments), not in the cover
letter.

[toc] | [prev] | [next] | [standalone]

#1569151 — RE: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, January 26, 2017 6:52 AM
> To: Cathy Avery <cavery@redhat.com>
> Cc: KY Srinivasan <kys@microsoft.com>; hch@infradead.org; Haiyang Zhang
> <haiyangz@microsoft.com>; jejb@linux.vnet.ibm.com;
> martin.petersen@oracle.com; dan.carpenter@oracle.com;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; linux-
> scsi@vger.kernel.org; famz@redhat.com
> Subject: Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.
> 
> On Thu, Jan 26, 2017 at 08:38:58AM -0500, Cathy Avery wrote:
> > Included in the current storvsc driver for Hyper-V is the ability
> > to access luns on an FC fabric via a virtualized fiber channel
> > adapter exposed by the Hyper-V host. This was done to provide an
> > interface for existing customer tools that was more consistent with
> > a conventional FC device. The driver attaches to the FC transport
> > to allow host and port names to be published under
> > /sys/class/fc_host/hostX.
> >
> > A problem arose when attaching to the FC transport. The scsi_scan code
> > attempts to call fc_user_scan which has basically become a no-op
> > due to the virtualized nature of the FC host
> > ( missing rports, vports, etc ). At this point you cannot refresh
> > the scsi bus after mapping or unmapping luns on the SAN without
> > a reboot.
> 
> I don't think a device without rports or vports is a FC device, plain and
> simple.  So as far as I'm concerned we should remove the code from storvsc
> that pretends to be FC, and not add it to virtio to start with.
> 
> And again I think leightweight is a very confusing name -
> what exactly is leight or heavy?   It's really fake or dummy
> in the current version.

Windows has chosen this model for virtualizing FC devices to the guest -
without rports (or vports). As I noted in my earlier email, James came up with
this notion of a lightweight template almost a year ago. We can certainly pick a 
more appropriate name and include better documentation. 
> 
> >
> > 2) Removes an original workaround dealing with replacing
> > the eh_timed_out function. Patch 1 will not set the
> > scsi_transport_template.eh_timed_out function directly during
> > lightweight fc_attach_transport(). It instead relies on
> > whatever was indicated as the scsi_host_template timeout handler
> > during scsi_times_out() scsi_error.c. So the workaround is
> > no longer necessary.
> 
> Can you send a patch that gets rid of the transport class timeout handler
> entirely?  I think it's simply the wrong layering we have here - the
> driver needs to be in control of timeouts, and if it wants it can
> optionally call into library code in the transport class.

We will address this concern.
> 
> 
> FYI, all the long-term relevant explanation need to go into the patches
> themselves (patch description or code comments), not in the cover
> letter.

We will address this.

Regards,

K. Y

[toc] | [prev] | [next] | [standalone]

#1569169 — Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

On Sun, Jan 29, 2017 at 12:35:32AM +0000, KY Srinivasan wrote:
> Windows has chosen this model for virtualizing FC devices to the guest -
> without rports (or vports). As I noted in my earlier email, James came
> up with this notion of a lightweight template almost a year ago. We can
> certainly pick a  more appropriate name and include better documentation. 

Can we take a step back and figure out what you're trying to
archive here.

storsvc is a paravirtualized device interface, and whatever underlies
it should be of no relevance for the guest.

Despite that fact Microsoft apparently wants to expose a FC-like
port_name and node_name to guests for some virtual disks.  Can you
please explain what the guest is supposed to use them for?

And second I'd like to understand what the fascination with the FC
transport class is to expose these two attributes.  Given that
your sysfs layout will be entirely different from real FC devices
I simply don't see any need for that.

Why can't this whole thing simply be solved by adding sdev_attrs
for the port_name and node_name to storsvc directly?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web