From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chandra Seetharaman Subject: Re: [PATCH] Fixes to EMC scsi device handler Date: Mon, 05 May 2008 18:34:18 -0700 Message-ID: <1210037658.21974.125.camel@chandra-ubuntu> References: <481EF5AA.7090304@suse.de> Reply-To: sekharan@us.ibm.com, device-mapper development Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <481EF5AA.7090304@suse.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Hannes Reinecke Cc: James Bottomley , dm-devel , SCSI Mailing List List-Id: linux-scsi@vger.kernel.org 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 > > 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 > #include > > -#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);