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);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
