On Mon, 2008-05-05 at 13:55 +0200, Hannes Reinecke wrote: > Hi Chandra, > > based on your latest patchset there are some issues which > should be fixed: > > - When SYSFS_DEPRECATED is not set also scsi_target and > scsi_host devices will be attached to the scsi bus. > So we have to check with 'scsi_is_sdev_device' before > processing the notifier call This need to be done for all the handlers, right ? > > - Newer EMC flarecode also provide for a 'VRAID' device > model; this should be added to the device map > - The name should be 'emc', not 'emc_clariion', for > integration with existing multipath-tools > - It's spelled 'detached' :-) > > Patch attached. Looks good to me. > > Some issues which I'd like to see eventually: > > - We should have a way of adding/modifying the internal > device map. Currently we have to recompile the module > every time a new device definition is found. Yeah, very true. it will become an annoyance. Is this the reason why you were asking for the sysfs interface (below) ? > - The ->activate function should be able to be triggered > manually via sysfs. Why would you need this ? > > James, seeing that the scsi_dh pointer is already present > in the sdev structure, would you accept a patch which adds > scsi_dh attributes to drivers/scsi/scsi_sysfs.c? > Having them encapsulated into drivers/scsi/device_handler > somewhere is really painful due to the asynchronous nature > of this whole thing ... > > Apart from this: Great work! > I'll check the other device handlers, too; already noticed > that the device map for HP is wrong :-) Can you please help fix it. > > Cheers, > > hannes > plain text document attachment (scsi-dh-emc-fixes) > Fixes to EMC scsi device handler > > Several fixes to the EMC scsi device handler: > - The name should be 'emc' for seamless integration with multipathing > - Newer EMC flarecode also has a 'VRAID' type > - Check if this is really a sdev before processing the notifier call > > Signed-off-by: Hannes Reinecke <hare@xxxxxxx> > > diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c > index 1b18989..0aded96 100644 > --- a/drivers/scsi/device_handler/scsi_dh_emc.c > +++ b/drivers/scsi/device_handler/scsi_dh_emc.c > @@ -25,7 +25,7 @@ > #include <scsi/scsi_dh.h> > #include <scsi/scsi_device.h> > > -#define CLARIION_NAME "emc_clariion" > +#define CLARIION_NAME "emc" > > #define CLARIION_TRESPASS_PAGE 0x22 > #define CLARIION_BUFFER_SIZE 0x80 > @@ -390,12 +390,13 @@ static int clariion_check_sense(struct scsi_device *sdev, > return SUCCESS; > } > > -static const struct { > +static const struct clariion_dev_list_t { > char *vendor; > char *model; > } clariion_dev_list[] = { > {"DGC", "RAID"}, > {"DGC", "DISK"}, > + {"DGC", "VRAID"}, > {NULL, NULL}, > }; > > @@ -422,6 +423,9 @@ static int clariion_bus_notify(struct notifier_block *nb, > int i, found = 0; > unsigned long flags; > > + if (!scsi_is_sdev_device(dev)) > + return 0; > + > if (action == BUS_NOTIFY_ADD_DEVICE) { > for (i = 0; clariion_dev_list[i].vendor; i++) { > if (!strncmp(sdev->vendor, clariion_dev_list[i].vendor, > @@ -465,7 +469,7 @@ static int clariion_bus_notify(struct notifier_block *nb, > sdev->scsi_dh_data = NULL; > spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags); > > - sdev_printk(KERN_NOTICE, sdev, "Dettached %s.\n", > + sdev_printk(KERN_NOTICE, sdev, "Detached %s.\n", > CLARIION_NAME); > > kfree(scsi_dh_data); -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel