Fixes to EMC scsi device handler
Subsystems (2)
PATCH 1/1 Fixes to EMC scsi device handler
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
- 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.
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.
- The ->activate function should be able to be triggered manually via sysfs.
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 :-)
Cheers,
hannes
Signature · 4 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg)
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
drivers/scsi/device_handler/scsi_dh_emc.c +7 -3
@@ -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);
Quoted 9 lines · Hannes Reinecke
Hi Chandra,
based on your latest patchset there are some issues which should be fixed:
- When
SYSFS_DEPRECATEDis not set alsoscsi_targetandscsi_hostdevices 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 ?
Quoted 8 lines
- 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.
Quoted 6 lines
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) ?
Quoted 2 lines
- The ->activate function should be able to be triggered manually via sysfs.
Why would you need this ?
Quoted 11 lines
James, seeing that the
scsi_dhpointer is already present in the sdev structure, would you accept a patch which addsscsi_dhattributes todrivers/scsi/scsi_sysfs.c? Having them encapsulated intodrivers/scsi/device_handlersomewhere 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.
Quoted 61 lines
Cheers,
hannes
plain text document attachment (scsi-dh-emc-fixes) Fixes to EMC scsi device handlerSeveral 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 callSigned-off-by Hannes Reinecke <[email protected]>drivers/scsi/device_handler/scsi_dh_emc.c +7
-3@@ -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);
FYI: The VRAID type is for the CLARiiON implementation of RAID6.
Regards,
Wayne.
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Chandra Seetharaman
Sent: Monday, May 05, 2008 9:34 PM
To: Hannes Reinecke
Cc: James Bottomley; dm-devel; SCSI Mailing List
Subject: [dm-devel] Re: [PATCH] Fixes to EMC scsi device handler
Quoted 9 lines · Hannes Reinecke
Hi Chandra,
based on your latest patchset there are some issues which should be fixed:
- When
SYSFS_DEPRECATEDis not set alsoscsi_targetandscsi_hostdevices 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 ?
Quoted 8 lines
- 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.
Quoted 6 lines
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) ?
Quoted 2 lines
- The ->activate function should be able to be triggered manually via sysfs.
Why would you need this ?
Quoted 11 lines
James, seeing that the
scsi_dhpointer is already present in the sdev structure, would you accept a patch which addsscsi_dhattributes todrivers/scsi/scsi_sysfs.c? Having them encapsulated intodrivers/scsi/device_handlersomewhere 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.
Quoted 7 lines
Cheers,
hannes
plain text document attachment (scsi-dh-emc-fixes) Fixes to EMC scsi device handler
Review
Quoted 8 lines
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 callSigned-off-by Hannes Reinecke <[email protected]>(quoted diff) +0
-0
b/drivers/scsi/device_handler/scsi_dh_emc.c
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_check_sense(struct
Quoted 13 lines
index 1b18989..0aded96 100644
drivers/scsi/device_handler/scsi_dh_emc.c +1
-1@@ -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,
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_bus_notify(struct
Quoted 15 lines
drivers/scsi/device_handler/scsi_dh_emc.c +2
-1return 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,
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_bus_notify(struct notifier_block *nb,
Quoted 9 lines
drivers/scsi/device_handler/scsi_dh_emc.c +3
-0int 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,
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_bus_notify(struct
Quoted 1 line
drivers/scsi/device_handler/scsi_dh_emc.c +0
-0@@ -465,7 +469,7 @@ static int clariion_bus_notify(struct
notifier_block *nb,
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_bus_notify(struct · line 470
Quoted 2 lines
drivers/scsi/device_handler/scsi_dh_emc.c +0
-0sdev->scsi_dh_data =NULL ; spin_unlock_irqrestore(sdev->request_queue->queue_lock,
flags);
drivers/scsi/device_handler/scsi_dh_emc.c static int clariion_bus_notify(struct notifier_block *nb, · line 475
Quoted 6 lines
drivers/scsi/device_handler/scsi_dh_emc.c +1
-1-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
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel