add fc transport events
RFC PATCH 1/3 add fc transport events
01-add-host-transport-classdev.patch - adds the transport class_device to the scsi_host structure.
drivers/scsi/hosts.c | 22 +++++++++++++++++++++- include/scsi/scsi_host.h | 4 ++++ 2 files changed, 25 insertions(+), 1 deletion(-)
drivers/scsi/hosts.c +21 -1
@@ -83,6 +83,8 @@ void scsi_remove_host(struct Scsi_Host *set_bit (SHOST_DEL , &shost->shost_state); +if (shost->transportt->class) +class_device_unregister (&shost->transport_classdev);class_device_unregister (&shost->shost_classdev);device_del (&shost->shost_gendev); }
@@ -123,15 +125,28 @@ int scsi_add_host(struct Scsi_Host *shosif (error) goto out_del_gendev; +if (shost->transportt->class) { + shost->transport_classdev.class = shost->transportt->class; + + error =class_device_add (&shost->transport_classdev); +if (error) + goto out_del_classdev; +/* take a reference for the transport_classdev; this +* is released by the transport_class .release */ +get_device (&shost->shost_gendev); + } +get_device (&shost->shost_gendev); error =scsi_sysfs_add_host (shost);if (error) - goto out_del_classdev; + goto out_del_transport;scsi_proc_host_add (shost);return error; + out_del_transport: +class_device_del (&shost->transport_classdev); out_del_classdev:class_device_del (&shost->shost_classdev); out_del_gendev:
@@ -280,6 +295,11 @@ struct Scsi_Host *scsi_host_alloc(structsnprintf (shost->shost_classdev.class_id,BUS_ID_SIZE ,"host%d" , shost->host_no); +class_device_initialize (&shost->transport_classdev); + shost->transport_classdev.dev = &shost->shost_gendev; +snprintf (shost->transport_classdev.class_id,BUS_ID_SIZE ,"host%d" , + shost->host_no); + shost->eh_notify = &complete; rval =kernel_thread (scsi_error_handler, shost,0 );if (rval <0 )
include/scsi/scsi_host.h +4 -0
@@ -475,6 +475,7 @@ struct Scsi_Host {/* ldm bits */ struct device shost_gendev;struct class_device shost_classdev; +struct class_device transport_classdev;/* * List of hosts per template.
@@ -497,6 +498,9 @@ struct Scsi_Host { container_of(d, struct Scsi_Host, shost_gendev)#define class_to_shost (d) \ container_of(d, struct Scsi_Host, shost_classdev) +#define transport_class_to_shost (_class_dev) \ + container_of(_class_dev, struct Scsi_Host, transport_classdev) +extern struct Scsi_Host *scsi_host_alloc (struct scsi_host_template *,int );extern int scsi_add_host (struct Scsi_Host *,struct device *);
Quoted 2 lines · Mike Christie
01-add-host-transport-classdev.patch - adds the transport
class_deviceto thescsi_hoststructure.
I've been thinking about this quite a bit, and I like the general idea, I just have a quibble with the implementation.
The main problem is the adding of a kobject to complement your host transport class (and the device transport class). I don't think we need to do this since, like the transport classes fall into either host or device objects, so the events are similarly divided; thus, we should be able to trigger the hotplug events through the host or device kobject (that come with the embedded struct device) rather than adding another kobject for the purpose.
What would be the disadvantage of doing what you propose this way instead of using a separate kobject?
James
Quoted 11 lines · James Bottomley
Quoted 2 lines · Mike Christie
01-add-host-transport-classdev.patch - adds the transport
class_deviceto thescsi_hoststructure.I've been thinking about this quite a bit, and I like the general idea, I just have a quibble with the implementation.
The main problem is the adding of a kobject to complement your host transport class (and the device transport class). I don't think we need
I am not adding a host and a device transport class. I am structuring things so there is a single fc transport class.
Quoted 5 lines
to do this since, like the transport classes fall into either host or device objects, so the events are similarly divided; thus, we should be able to trigger the hotplug events through the host or device kobject (that come with the embedded struct device) rather than adding another kobject for the purpose.
The kobject I added to the scsi_device replaced the class_device (and its kobject) we were previously using for the device oriented transport class. I did this only because I wanted the scsi device's parent to be the host. In my patch, I then added a class_device to the host becuase the host's parent was the "FC Class".
There is no technical argument why they couldn't be coded the way you described. My patch just has the FC Class, where under it the device, host and whatever objects arise are set up as parent/children to reflect how SCSI/FC and the kernel structures really were.
It does not make a difference to me. It is easier to code just having a fc_host_transport_class and a fc_device_transport_class. It seemed like a waste to add more classes and doing the symlinks when you can just restructure things though.
Mike
Quoted 18 lines · Mike Christie
Quoted 7 lines · James Bottomley
Quoted 2 lines · Mike Christie
01-add-host-transport-classdev.patch - adds the transport
class_deviceto thescsi_hoststructure.Quoted 5 lines
I've been thinking about this quite a bit, and I like the general idea, I just have a quibble with the implementation.
The main problem is the adding of a kobject to complement your host transport class (and the device transport class). I don't think we need
I am not adding a host and a device transport class. I am structuring things so there is a single fc transport class.
Quoted 8 lines
Quoted 5 lines
to do this since, like the transport classes fall into either host or device objects, so the events are similarly divided; thus, we should be able to trigger the hotplug events through the host or device kobject (that come with the embedded struct device) rather than adding another kobject for the purpose.
The kobject I added to the
scsi_devicereplaced theclass_device(and its kobject) we were previously using for the device oriented transport class. I did this only because I wanted the scsi device's parent to be the host. In my patch, I then added aclass_deviceto the host becuase the host's parent was the "FC Class".
Oh yeah, with this type of setup the host and device hotplug events were all coming through the single FC class. Host events can be distinguished from device events by their DEVPATH (one ends in scsiN the other ends with W:X:Y:Z and has the scsiN prefixed to it).
Quoted 11 lines
There is no technical argument why they couldn't be coded the way you described. My patch just has the FC Class, where under it the device, host and whatever objects arise are set up as parent/children to reflect how SCSI/FC and the kernel structures really were.
It does not make a difference to me. It is easier to code just having a
fc_host_transport_classand afc_device_transport_class. It seemed like a waste to add more classes and doing the symlinks when you can just restructure things though.Mike
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 2 lines · Mike Christie
I am not adding a host and a device transport class. I am structuring things so there is a single fc transport class.
I don't think that's such a good idea. A class is supposed to represent an interface on a device. The host and scsi device should have separate interfaces. The only reason we don't have any host interfaces in the transport classes is because no-one has yet had a reason to add one. However, since the loop status is definitely a host property, you do...part of what's missing is an attribute showing the loop state. SPI has a similar need; the host property there is LVD or SE, and we might be interested in transitions between them.
Quoted 11 lines
The kobject I added to the
scsi_devicereplaced theclass_device(and its kobject) we were previously using for the device oriented transport class. I did this only because I wanted the scsi device's parent to be the host. In my patch, I then added aclass_deviceto the host becuase the host's parent was the "FC Class".There is no technical argument why they couldn't be coded the way you described. My patch just has the FC Class, where under it the device, host and whatever objects arise are set up as parent/children to reflect how SCSI/FC and the kernel structures really were.
Quoted 4 lines
It does not make a difference to me. It is easier to code just having a
fc_host_transport_classand afc_device_transport_class. It seemed like a waste to add more classes and doing the symlinks when you can just restructure things though.
Well do it as two separate classes then. Kobjects and Ksets are a bit of a minefield that I'd rather we didn't get into unless it's really, really necessary. The abstraction we care about is the interface, which to the generic device stuff is the class. If we stick to what we need, we'll get broken by generic device changes less often...
James
Quoted 7 lines · James Bottomley
Quoted 1 line · Mike Christie
I am not adding a host and a device transport class. I am structuring things so there is a single fc transport class.
I don't think that's such a good idea. A class is supposed to represent an interface on a device.
Ok. I saw the class as representing a "thingy" not necessarily a device, becuase at one point the mainatiner had taken out the coupling of the "struct device" to the class_device which I thought this was for. For example the io scheduler or request queue could and probably should be a class if you wanted to abe able to hot swap the schedluer. In my case I was representing the transport itself as a class (the class_device should have gone into the fc_transport object and a transport_kobject should have been placed into the host though).
The host and scsi device should have separate
Quoted 11 lines
interfaces. The only reason we don't have any host interfaces in the
transport classes is because no-one has yet had a reason to add one. However, since the loop status is definitely a host property, you do...part of what's missing is an attribute showing the loop state. SPI
has a similar need; the host property there is LVD or SE, and we might
be interested in transitions between them.Quoted 4 lines
The kobject I added to the
scsi_devicereplaced theclass_device(and its kobject) we were previously using for the device oriented transport class. I did this only because I wanted the scsi device's parent to be the host. In my patch, I then added aclass_deviceto the host becuase the host's parent was the "FC Class".There is no technical argument why they couldn't be coded the way you described. My patch just has the FC Class, where under it the device, host and whatever objects arise are set up as parent/children to reflect how SCSI/FC and the kernel structures really were.
Quoted 9 lines
Quoted 1 line
It does not make a difference to me. It is easier to code just having a
fc_host_transport_classand afc_device_transport_class. It seemed like a waste to add more classes and doing the symlinks when you can just restructure things though.Well do it as two separate classes then. Kobjects and Ksets are a bit of a minefield that I'd rather we didn't get into unless it's really, really necessary. The abstraction we care about is the interface, which to the generic device stuff is the class. If we stick to what we need, we'll get broken by generic device changes less often...
Ok then.
Will this mean we will end up with device and host specific error handler issues that the classes will have to coordinate? For example, if I got a link down I know that cmds are going to be timing out for every device on the host, but depending on the setup if just the device's port went down I will not get a link down and may want to do something different. Or maybe not?
Mike
Quoted 5 lines · James Bottomley
Quoted 1 line · Mike Christie
I am not adding a host and a device transport class. I am structuring things so there is a single fc transport class.
Quoted 7 lines
I don't think that's such a good idea. A class is supposed to represent
an interface on a device. The host and scsi device should have separate
interfaces. The only reason we don't have any host interfaces in the
transport classes is because no-one has yet had a reason to add one. However, since the loop status is definitely a host property, you do...part of what's missing is an attribute showing the loop state. SPI
has a similar need; the host property there is LVD or SE, and we might
be interested in transitions between them.
There are 4 categories of "devices" that we might try to address:
a) host (PCI side and SCSI transport side)
b) transport service delivery subsystem
c) target device server
d) logical unit
Only a, b + c are associated with the transport in question. In category b could be FC switches and SAS expanders. In category d could be sATA, SPI or SAS disks behind a FC target. So the logical unit doesn't necessarily belong to the same transport as the initiator (especially in iSCSI) as noted above by James.
So the end point of the transport is category c and according to SPC-3 a target device server is addressed via "well known" logical units (see section 8). [The standard requirement for a device server to respond to lun==0 for INQUIRY and REPORT LUNS is a related hack that allows a SCSI disk to double as a target device server and a lu.]
Is our transport class design flexible enough for this level of complexity?
Doug Gilbert
Quoted 6 lines · Douglas Gilbert
There are 4 categories of "devices" that we might try to address:
a) host (PCI side and SCSI transport side)
b) transport service delivery subsystem
c) target device server
d) logical unit
a) is really two (which we currently have represented) with the bus side device and the host device.
b,c and d are all scsi devices.
Quoted 6 lines
Only a, b + c are associated with the transport in question. In category b could be FC switches and SAS expanders. In category d could be sATA, SPI or SAS disks behind a FC target. So the logical unit doesn't necessarily belong to the same transport as the initiator (especially in iSCSI) as noted above by James.
The idea of the transport class is that it be invisible to the mid-layer. The services it provides are either to the LLD or to the user (via sysfs).
Quoted 9 lines
So the end point of the transport is category c and according to SPC-3 a target device server is addressed via "well known" logical units (see section 8). [The standard requirement for a device server to respond to lun==0 for INQUIRY and REPORT LUNS is a related hack that allows a SCSI disk to double as a target device server and a lu.]
Is our transport class design flexible enough for this level of complexity?
Well, in the SAM 4 level system:
1. Device specific command sets
2. Shared command sets (all device types)
3. SCSI transport protocols
4. Interconnects
Our ULDs do 1, the mid-layer does 2 and I'm hoping to expand the transport classes into 3 and 4.
Enumeration really isn't addressed by SAM, but we're already moving towards having enumeration done from user level (i.e. removing simple scanning from the mid-layer), so that should be easily able to do complex topology probes.
James