[PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

Hello,

Currently, SCSI targets doesn't have modalias support. It wasn't an issue
until SCSI device handler came along.

We want the SCSI device handler modules to be insmodded automatically
when the specific SCSI targets are probed and found.

This set of patches adds the modalias support for SCSI targets and
also makes the relevant changes to SCSI device handler modules to 
make use of it.

Applies cleanly on 2.6.30-rc3 and is tested on the same.

Please review and consider this for inclusion.

Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).

Resending after testing on 2.6.30-rc3 and with an ack from Hannes.

Thanks & Regards,

chandra

[PATCH 1/3] scsi_dh: Add modalias support for SCSI targets

[Chandra Seetharaman]

From: Peter Jones <pjones@redhat.com>

This patch allows the use of modaliases on scsi targets to correctly
load scsi device handler modules when the devices are found.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Acked-by: Hannes Reinecke <hare@suse.de>

---
 drivers/scsi/scsi_sysfs.c       |   55 +++++++++++++++++++++++++++++++++++++++-
 include/linux/mod_devicetable.h |    6 ++++
 include/linux/string_helpers.h  |    2 +
 include/scsi/scsi.h             |    1 +
 include/scsi/scsi_device.h      |    9 +-----
 lib/string_helpers.c            |   32 +++++++++++++++++++++++
 scripts/mod/file2alias.c        |   38 ++++++++++++++++++++++++++++
 7 files changed, 134 insertions(+), 9 deletions(-)

Index: linux-2.6.29/drivers/scsi/scsi_sysfs.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/scsi_sysfs.c
+++ linux-2.6.29/drivers/scsi/scsi_sysfs.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/blkdev.h>
 #include <linux/device.h>
+#include <linux/string_helpers.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -362,16 +363,63 @@ static int scsi_bus_match(struct device
 	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
 }
 
+static ssize_t format_scsi_modalias(struct scsi_device *sdev, char *buffer,
+				    ssize_t len)
+{
+	char vendor[9];
+	char *hex_vendor;
+	char model[17];
+	char *hex_model;
+	int i;
+
+	strncpy(vendor, sdev->vendor, 8);
+	vendor[8] = '\0';
+	for (i = strlen(vendor) - 1; i >= 0; i--) {
+		if (vendor[i] != ' ')
+			break;
+		vendor[i] = '\0';
+	}
+	hex_vendor = string_to_hex(vendor);
+	if (!hex_vendor)
+		return -ENOMEM;
+
+	strncpy(model, sdev->model, 16);
+	model[8] = '\0';
+	for (i = strlen(model) - 1; i >= 0; i--) {
+		if (model[i] != ' ')
+			break;
+		model[i] = '\0';
+	}
+	hex_model = string_to_hex(model);
+	if (!hex_model) {
+		kfree(hex_vendor);
+		return -ENOMEM;
+	}
+
+	i = snprintf(buffer, len, "scsi:t-0x%02xv%.16sm%.32s", sdev->type,
+		 hex_vendor, hex_model);
+	kfree(hex_vendor);
+	kfree(hex_model);
+	return i;
+}
+
 static int scsi_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct scsi_device *sdev;
+	char buffer[501];
+	int rc;
 
 	if (dev->type != &scsi_dev_type)
 		return 0;
 
 	sdev = to_scsi_device(dev);
 
-	add_uevent_var(env, "MODALIAS=" SCSI_DEVICE_MODALIAS_FMT, sdev->type);
+	buffer[500] = '\0';
+	rc = format_scsi_modalias(sdev, buffer, 500);
+	if (rc < 0)
+		return rc;
+
+	add_uevent_var(env, "MODALIAS=%s", buffer);
 	return 0;
 }
 
@@ -697,8 +745,11 @@ static ssize_t
 sdev_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct scsi_device *sdev;
+	ssize_t rc;
+
 	sdev = to_scsi_device(dev);
-	return snprintf (buf, 20, SCSI_DEVICE_MODALIAS_FMT "\n", sdev->type);
+	rc = format_scsi_modalias(sdev, buf, 500);
+	return rc;
 }
 static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
 
Index: linux-2.6.29/include/linux/mod_devicetable.h
===================================================================
--- linux-2.6.29.orig/include/linux/mod_devicetable.h
+++ linux-2.6.29/include/linux/mod_devicetable.h
@@ -454,6 +454,12 @@ struct dmi_system_id {
 
 #define DMI_MATCH(a, b)	{ a, b }
 
+struct scsi_dh_device_id {
+	unsigned char type;
+	char vendor[9];
+	char model[17];
+};
+
 #define PLATFORM_NAME_SIZE	20
 #define PLATFORM_MODULE_PREFIX	"platform:"
 
Index: linux-2.6.29/include/linux/string_helpers.h
===================================================================
--- linux-2.6.29.orig/include/linux/string_helpers.h
+++ linux-2.6.29/include/linux/string_helpers.h
@@ -13,4 +13,6 @@ enum string_size_units {
 int string_get_size(u64 size, enum string_size_units units,
 		    char *buf, int len);
 
+unsigned char *string_to_hex(const unsigned char *s);
+
 #endif
Index: linux-2.6.29/include/scsi/scsi.h
===================================================================
--- linux-2.6.29.orig/include/scsi/scsi.h
+++ linux-2.6.29/include/scsi/scsi.h
@@ -266,6 +266,7 @@ static inline int scsi_status_is_good(in
 #define TYPE_RBC	    0x0e
 #define TYPE_OSD            0x11
 #define TYPE_NO_LUN         0x7f
+#define TYPE_ANY	    0xff
 
 /* SCSI protocols; these are taken from SPC-3 section 7.5 */
 enum scsi_protocol {
Index: linux-2.6.29/include/scsi/scsi_device.h
===================================================================
--- linux-2.6.29.orig/include/scsi/scsi_device.h
+++ linux-2.6.29/include/scsi/scsi_device.h
@@ -1,6 +1,7 @@
 #ifndef _SCSI_SCSI_DEVICE_H
 #define _SCSI_SCSI_DEVICE_H
 
+#include <linux/mod_devicetable.h>
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
@@ -169,11 +170,6 @@ struct scsi_device {
 	unsigned long		sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long))));
 
-struct scsi_dh_devlist {
-	char *vendor;
-	char *model;
-};
-
 struct scsi_device_handler {
 	/* Used by the infrastructure */
 	struct list_head list; /* list of scsi_device_handlers */
@@ -181,7 +177,7 @@ struct scsi_device_handler {
 	/* Filled by the hardware handler */
 	struct module *module;
 	const char *name;
-	const struct scsi_dh_devlist *devlist;
+	const struct scsi_dh_device_id *devlist;
 	int (*check_sense)(struct scsi_device *, struct scsi_sense_hdr *);
 	int (*attach)(struct scsi_device *);
 	void (*detach)(struct scsi_device *);
@@ -452,6 +448,5 @@ static inline int scsi_device_protection
 
 #define MODULE_ALIAS_SCSI_DEVICE(type) \
 	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
-#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
 
 #endif /* _SCSI_SCSI_DEVICE_H */
Index: linux-2.6.29/lib/string_helpers.c
===================================================================
--- linux-2.6.29.orig/lib/string_helpers.c
+++ linux-2.6.29/lib/string_helpers.c
@@ -66,3 +66,35 @@ int string_get_size(u64 size, const enum
 	return 0;
 }
 EXPORT_SYMBOL(string_get_size);
+
+/**
+ * string_to_hex - convert a string to a series of hexidecimal values
+ * @s:	The string to operate on
+ *
+ * This function returns a GFP_KERNEL allocated buffer filled with
+ * the hexidecimal representation of the value of each character in @s .
+ * Returns a pointer to the allocated string on success and NULL on error,
+ * and the returned string is zero terminated.
+ *
+ */
+unsigned char *string_to_hex(const unsigned char *s)
+{
+	unsigned char *ret, *ptr;
+	static const unsigned char *hex = "0123456789ABCDEF";
+	int len;
+
+	len = strlen(s);
+
+	ret = ptr = kmalloc(len * 2 + 1, GFP_KERNEL);
+	if (!ret)
+		return NULL;
+
+	for (; *s; s++) {
+		*ptr++ = hex[(*s & 0xf0)>>4];
+		*ptr++ = hex[*s & 0x0f];
+	}
+	*ptr = '\0';
+
+	return ret;
+}
+EXPORT_SYMBOL(string_to_hex);
Index: linux-2.6.29/scripts/mod/file2alias.c
===================================================================
--- linux-2.6.29.orig/scripts/mod/file2alias.c
+++ linux-2.6.29/scripts/mod/file2alias.c
@@ -51,6 +51,22 @@ do {
                 sprintf(str + strlen(str), "*");                \
 } while(0)
 
+#define ADD_HEX_STR(str, sep, cond, field)				\
+do {									\
+	strcat(str, sep);						\
+	if (cond) {							\
+		char * _s = str + strlen(str);				\
+		char * _f = field;					\
+		static const char *_hex = "0123456789ABCDEF";		\
+									\
+		for (; *_f; _f++) {					\
+			*_s++ = _hex[(*_f & 0xf0)>>4];			\
+			*_s++ = _hex[*_f & 0xf];			\
+		}							\
+	} else								\
+		strcat(str, "*");					\
+} while(0)
+
 /* Always end in a wildcard, for future extension */
 static inline void add_wildcard(char *str)
 {
@@ -718,6 +734,23 @@ static int do_platform_entry(const char
 	return 1;
 }
 
+
+/* Looks like: scsi:t-NvSmS */
+/* defining TYPE_ANY here is a gross hack to avoid moving all the scsi.h
+ * TYPE_ definitions into mod_devicetable.h */
+#define TYPE_ANY 0xff
+static int do_scsi_entry(const char *filename,
+			struct scsi_dh_device_id *id, char *alias)
+{
+	strcpy(alias, "scsi:");
+	ADD(alias, "t-", id->type != TYPE_ANY, id->type);
+	ADD_HEX_STR(alias, "v", id->vendor[0] != '\0', id->vendor);
+	ADD_HEX_STR(alias, "m", id->model[0] != '\0', id->model);
+
+	add_wildcard(alias);
+	return 1;
+}
+
 /* Ignore any prefix, eg. some architectures prepend _ */
 static inline int sym_is(const char *symbol, const char *name)
 {
@@ -744,6 +777,7 @@ static void do_table(void *symval, unsig
 	size -= id_size;
 
 	for (i = 0; i < size; i += id_size) {
+		memset(alias, '\0', 500);
 		if (do_entry(mod->name, symval+i, alias)) {
 			buf_printf(&mod->dev_table_buf,
 				   "MODULE_ALIAS(\"%s\");\n", alias);
@@ -861,6 +895,10 @@ void handle_moddevtable(struct module *m
 		do_table(symval, sym->st_size,
 			 sizeof(struct platform_device_id), "platform",
 			 do_platform_entry, mod);
+	else if (sym_is(symname, "__mod_scsi_dh_device_table"))
+		do_table(symval, sym->st_size,
+			 sizeof(struct scsi_dh_device_id), "scsi",
+			 do_scsi_entry, mod);
 	free(zeros);
 }
 

[PATCH 2/3] scsi_dh: Change scsi device handler modules to utilize modalias

[Chandra Seetharaman]

From: Peter Jones <pjones@redhat.com>

This patch changes the scsi_dh files to make use of the modalias feature.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---

 drivers/scsi/device_handler/scsi_dh.c       |    5 +++-
 drivers/scsi/device_handler/scsi_dh_alua.c  |   23 ++++++++-------
 drivers/scsi/device_handler/scsi_dh_emc.c   |   11 ++++---
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |   13 +++++----
 drivers/scsi/device_handler/scsi_dh_rdac.c  |   42 +++++++++++++++-------------
 5 files changed, 51 insertions(+), 43 deletions(-)

Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh.c
@@ -75,7 +75,10 @@ static int scsi_dh_handler_lookup(struct
 {
 	int i, found = 0;
 
-	for(i = 0; scsi_dh->devlist[i].vendor; i++) {
+	for(i = 0; scsi_dh->devlist[i].vendor[0]; i++) {
+		if ((scsi_dh->devlist[i].type != TYPE_ANY) &&
+				(scsi_dh->devlist[i].type != sdev->type))
+			continue;
 		if (!strncmp(sdev->vendor, scsi_dh->devlist[i].vendor,
 			     strlen(scsi_dh->devlist[i].vendor)) &&
 		    !strncmp(sdev->model, scsi_dh->devlist[i].model,
Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh_alua.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -691,19 +691,21 @@ static int alua_prep_fn(struct scsi_devi
 }
 
 static const struct scsi_dh_devlist alua_dev_list[] = {
-	{"HP", "MSA VOLUME" },
-	{"HP", "HSV101" },
-	{"HP", "HSV111" },
-	{"HP", "HSV200" },
-	{"HP", "HSV210" },
-	{"HP", "HSV300" },
-	{"IBM", "2107900" },
-	{"IBM", "2145" },
-	{"Pillar", "Axiom" },
-	{"Intel", "Multi-Flex"},
-	{NULL, NULL}
+	{TYPE_ANY, "HP", "MSA VOLUME" },
+	{TYPE_ANY, "HP", "HSV101" },
+	{TYPE_ANY, "HP", "HSV111" },
+	{TYPE_ANY, "HP", "HSV200" },
+	{TYPE_ANY, "HP", "HSV210" },
+	{TYPE_ANY, "HP", "HSV300" },
+	{TYPE_ANY, "IBM", "2107900" },
+	{TYPE_ANY, "IBM", "2145" },
+	{TYPE_ANY, "Pillar", "Axiom" },
+	{TYPE_ANY, "Intel", "Multi-Flex"},
+	{0, "", ""},
 };
 
+MODULE_DEVICE_TABLE(scsi_dh, alua_dev_list);
+
 static int alua_bus_attach(struct scsi_device *sdev);
 static void alua_bus_detach(struct scsi_device *sdev);
 
Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh_emc.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -562,12 +562,13 @@ done:
 	return result;
 }
 
-static const struct scsi_dh_devlist clariion_dev_list[] = {
-	{"DGC", "RAID"},
-	{"DGC", "DISK"},
-	{"DGC", "VRAID"},
-	{NULL, NULL},
+static const struct scsi_dh_device_id clariion_dev_list[] = {
+	{TYPE_ANY, "DGC", "RAID"},
+	{TYPE_ANY, "DGC", "DISK"},
+	{TYPE_ANY, "DGC", "VRAID"},
+	{0, "", ""},
 };
+MODULE_DEVICE_TABLE(scsi_dh, clariion_dev_list);
 
 static int clariion_bus_attach(struct scsi_device *sdev);
 static void clariion_bus_detach(struct scsi_device *sdev);
Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -286,13 +286,14 @@ static int hp_sw_activate(struct scsi_de
 	return ret;
 }
 
-static const struct scsi_dh_devlist hp_sw_dh_data_list[] = {
-	{"COMPAQ", "MSA1000 VOLUME"},
-	{"COMPAQ", "HSV110"},
-	{"HP", "HSV100"},
-	{"DEC", "HSG80"},
-	{NULL, NULL},
+static const struct scsi_dh_device_id hp_sw_dh_data_list[] = {
+	{TYPE_ANY, "COMPAQ", "MSA1000 VOLUME"},
+	{TYPE_ANY, "COMPAQ", "HSV110"},
+	{TYPE_ANY, "HP", "HSV100"},
+	{TYPE_ANY, "DEC", "HSG80"},
+	{0, "", ""},
 };
+MODULE_DEVICE_TABLE(scsi_dh, hp_sw_dh_data_list);
 
 static int hp_sw_bus_attach(struct scsi_device *sdev);
 static void hp_sw_bus_detach(struct scsi_device *sdev);
Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -608,28 +608,30 @@ static int rdac_check_sense(struct scsi_
 	return SCSI_RETURN_NOT_HANDLED;
 }
 
-static const struct scsi_dh_devlist rdac_dev_list[] = {
-	{"IBM", "1722"},
-	{"IBM", "1724"},
-	{"IBM", "1726"},
-	{"IBM", "1742"},
-	{"IBM", "1814"},
-	{"IBM", "1815"},
-	{"IBM", "1818"},
-	{"IBM", "3526"},
-	{"SGI", "TP9400"},
-	{"SGI", "TP9500"},
-	{"SGI", "IS"},
-	{"STK", "OPENstorage D280"},
-	{"SUN", "CSM200_R"},
-	{"SUN", "LCSM100_F"},
-	{"DELL", "MD3000"},
-	{"DELL", "MD3000i"},
-	{"LSI", "INF-01-00"},
-	{"ENGENIO", "INF-01-00"},
-	{NULL, NULL},
+static const struct scsi_dh_device_id rdac_dev_list[] = {
+	{TYPE_ANY, "IBM", "1722"},
+	{TYPE_ANY, "IBM", "1724"},
+	{TYPE_ANY, "IBM", "1726"},
+	{TYPE_ANY, "IBM", "1742"},
+	{TYPE_ANY, "IBM", "1814"},
+	{TYPE_ANY, "IBM", "1815"},
+	{TYPE_ANY, "IBM", "1818"},
+	{TYPE_ANY, "IBM", "3526"},
+	{TYPE_ANY, "SGI", "TP9400"},
+	{TYPE_ANY, "SGI", "TP9500"},
+	{TYPE_ANY, "SGI", "IS"},
+	{TYPE_ANY, "STK", "OPENstorage D280"},
+	{TYPE_ANY, "SUN", "CSM200_R"},
+	{TYPE_ANY, "SUN", "LCSM100_F"},
+	{TYPE_ANY, "DELL", "MD3000"},
+	{TYPE_ANY, "DELL", "MD3000i"},
+	{TYPE_ANY, "LSI", "INF-01-00"},
+	{TYPE_ANY, "ENGENIO", "INF-01-00"},
+	{0, "", ""},
 };
 
+MODULE_DEVICE_TABLE(scsi_dh, rdac_dev_list);
+
 static int rdac_bus_attach(struct scsi_device *sdev);
 static void rdac_bus_detach(struct scsi_device *sdev);
 

[PATCH 3/3] scsi_dh: Workaround a race condition in module insertion

[Chandra Seetharaman]

From: Chandra Seetharaman <sekharan@us.ibm.com>

scsi_dh module is getting inserted as soon as the first device is seen.
But, the first device is not seen by the module as we were past the 
ADD_DEVICE handling in the module.

Catch the first device by handling BUS_NOTIFY_BOUND_DRIVER event,
and not handle that event for any of the future devices (as they would
have been handled by ADD_DEVICE event).

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Signed-off-by: Peter Jones <pjones@redhat.com>

---
 drivers/scsi/device_handler/scsi_dh.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Index: linux-2.6.29/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.29.orig/drivers/scsi/device_handler/scsi_dh.c
+++ linux-2.6.29/drivers/scsi/device_handler/scsi_dh.c
@@ -294,7 +294,16 @@ static int scsi_dh_notifier(struct notif
 
 	sdev = to_scsi_device(dev);
 
-	if (action == BUS_NOTIFY_ADD_DEVICE) {
+	if ((action == BUS_NOTIFY_ADD_DEVICE) ||
+			(action == BUS_NOTIFY_BOUND_DRIVER)) {
+		/*
+		 * This can happen if device was configured already
+		 * with BUS_NOTIFY_ADD_DEVICE and we are called
+		 * now with BUS_NOTIFY_BOUND_DRIVER
+		 */
+		if (sdev->scsi_dh_data)
+			goto out;
+
 		devinfo = device_handler_match(NULL, sdev);
 		if (!devinfo)
 			goto out;

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 04/27/2009 02:06 PM, Chandra Seetharaman wrote:
> Hello,
> 
> Currently, SCSI targets doesn't have modalias support. It wasn't an issue
> until SCSI device handler came along.
> 
> We want the SCSI device handler modules to be insmodded automatically
> when the specific SCSI targets are probed and found.
> 
> This set of patches adds the modalias support for SCSI targets and
> also makes the relevant changes to SCSI device handler modules to 
> make use of it.
> 
> Applies cleanly on 2.6.30-rc3 and is tested on the same.
> 
> Please review and consider this for inclusion.
> 
> Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).
> 
> Resending after testing on 2.6.30-rc3 and with an ack from Hannes.

Was there ever any followup to this?

-- 
        Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

On Mon, 2009-06-15 at 14:29 -0400, Peter Jones wrote:
> On 04/27/2009 02:06 PM, Chandra Seetharaman wrote:
> > Hello,
> > 
> > Currently, SCSI targets doesn't have modalias support. It wasn't an issue
> > until SCSI device handler came along.
> > 
> > We want the SCSI device handler modules to be insmodded automatically
> > when the specific SCSI targets are probed and found.
> > 
> > This set of patches adds the modalias support for SCSI targets and
> > also makes the relevant changes to SCSI device handler modules to 
> > make use of it.
> > 
> > Applies cleanly on 2.6.30-rc3 and is tested on the same.
> > 
> > Please review and consider this for inclusion.
> > 
> > Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).
> > 
> > Resending after testing on 2.6.30-rc3 and with an ack from Hannes.
> 
> Was there ever any followup to this?

Nope.
> 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Mon, 2009-06-15 at 14:29 -0400, Peter Jones wrote:
> On 04/27/2009 02:06 PM, Chandra Seetharaman wrote:
> > Hello,
> > 
> > Currently, SCSI targets doesn't have modalias support. It wasn't an issue
> > until SCSI device handler came along.
> > 
> > We want the SCSI device handler modules to be insmodded automatically
> > when the specific SCSI targets are probed and found.
> > 
> > This set of patches adds the modalias support for SCSI targets and
> > also makes the relevant changes to SCSI device handler modules to 
> > make use of it.
> > 
> > Applies cleanly on 2.6.30-rc3 and is tested on the same.
> > 
> > Please review and consider this for inclusion.
> > 
> > Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).
> > 
> > Resending after testing on 2.6.30-rc3 and with an ack from Hannes.
> 
> Was there ever any followup to this?

OK, since I've forgotten where we are, let me summarise what I think the
situation is (correct me if I misstate any of the facts):

This code adds no functional value to the kernel because dm already
autoloads the correct handlers based on the inquiry strings

The only value it adds is that by overloading the module table with the
inquiry strings, mkinitrd pulls in the correct dm handlers for the state
the system was in.

the unaddressed problems are:

The kernel now tries to load the dm handler for the device dynamically
whether or not the user is actually deploying multi-path (previously dm
does this and if it's not loaded, that doesn't happen).  It's entirely
unclear whether this would interfere with proprietary multipath handlers
or even cause problems in single path systems which were designed that
way.

So as I see it, the functional benefit to a running kernel is zero and
the functional risk exists but is unquantified, so it seems far better
simply to solve this issue in mkinitrd.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 06/18/2009 06:48 PM, James Bottomley wrote:
> On Mon, 2009-06-15 at 14:29 -0400, Peter Jones wrote:
>> On 04/27/2009 02:06 PM, Chandra Seetharaman wrote:
>>> Hello,
>>>
>>> Currently, SCSI targets doesn't have modalias support. It wasn't an issue
>>> until SCSI device handler came along.
>>>
>>> We want the SCSI device handler modules to be insmodded automatically
>>> when the specific SCSI targets are probed and found.
>>>
>>> This set of patches adds the modalias support for SCSI targets and
>>> also makes the relevant changes to SCSI device handler modules to 
>>> make use of it.
>>>
>>> Applies cleanly on 2.6.30-rc3 and is tested on the same.
>>>
>>> Please review and consider this for inclusion.
>>>
>>> Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).
>>>
>>> Resending after testing on 2.6.30-rc3 and with an ack from Hannes.
>> Was there ever any followup to this?
> 
> OK, since I've forgotten where we are, let me summarise what I think the
> situation is (correct me if I misstate any of the facts):
> 
> This code adds no functional value to the kernel because dm already
> autoloads the correct handlers based on the inquiry strings

I don't agree here.  It adds functional value to the entire system by making the
loading of device drivers use the same method no matter which kernel subsystem
the driver is part of.  This is a very tangible benefit in terms of maintainability.

> The only value it adds is that by overloading the module table with the
> inquiry strings, mkinitrd pulls in the correct dm handlers for the state
> the system was in.

You keep on saying this, and to me it seems very strange.  The patch does no
"overloading" of vendor strings -- it uses them just like the PCI vendor/device ID or
USB idVendor/idDevice.  Fundamentally they are the same thing: an identifier to
tell us what device we're looking at.  There's no overloading here, it's just
hooking them up to the kernel's generalized mechanism for loading modules based on
this kind of data.

> the unaddressed problems are:
> 
> The kernel now tries to load the dm handler for the device dynamically
> whether or not the user is actually deploying multi-path (previously dm
> does this and if it's not loaded, that doesn't happen).  It's entirely
> unclear whether this would interfere with proprietary multipath handlers
> or even cause problems in single path systems which were designed that
> way.

I just don't see how this is a real concern -- the whole point of the drivers
which we're autoloading here is to work with specific hardware that's designed to
work in this fashion.  What's more, if that hardware is being used for single-path,
and the driver is loaded and it decides to reconfigure to prefer a path that's not
plugged in, surely that's a bug in the device handler driver, and should be fixed
there, rather than blocking the auto-loading of such drivers?

-- 
        Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

Hi James,

Thanks for getting back on this.

On Thu, 2009-06-18 at 17:48 -0500, James Bottomley wrote:

> > 
> > Was there ever any followup to this?
> 
> OK, since I've forgotten where we are, let me summarise what I think the
> situation is (correct me if I misstate any of the facts):
> 
> This code adds no functional value to the kernel because dm already
> autoloads the correct handlers based on the inquiry strings

First point of clarification:

The main purpose of moving the device handlers from the dm-layer to scsi
layer was that the time at which dm-layer comes in is too late.

SCSI-DH was added mainly to make sure that scsi layer recognize that
these are special devices and has to be handled differently.

(Original problem was that during device probing lot of ios are sent to
passive paths which caused boot time delays and tons of errors messages.
These increase linearly with the number of luns and the number of
passive paths, i.e more redundant the system is, more time it takes to
boot and more error messages it spits out).

To paraphrase, if we were willing to wait till dm layer loads
appropriate device handler modules, there would be no need for us to
move the device handlers to SCSI layer.

With that clarification....

Having device handlers in SCSI is useful _only_ if we have the
appropriate device handler modules in initrd. Otherwise, the user will
go thru all the sufferings (boot delay and error messages) that they
went thru when the device handlers were in dm-layer voiding the very
benefit of moving the handlers to SCSI layer.

As of today (actually since scsi dh was in the kernel), I suggest the
users of scsi device handler modules to create a new initrd with the
required scsi_dh module
(http://sources.redhat.com/lvm2/wiki/MultipathUsageGuide#head-fb3efbb82fa69ca86b7db26423c235ae6c280caa)

Ideal way to solve this problem is to make sure the
appropriate/necessary modules are built into the initrd image
automatically (as is the case with all other devices that need a
driver), without the user/admin doing it .

Hence this patch.
> 
> The only value it adds is that by overloading the module table with the
> inquiry strings, mkinitrd pulls in the correct dm handlers for the state
> the system was in.
> 
> the unaddressed problems are:
> 
> The kernel now tries to load the dm handler for the device dynamically
> whether or not the user is actually deploying multi-path (previously dm
> does this and if it's not loaded, that doesn't happen).  It's entirely
> unclear whether this would interfere with proprietary multipath handlers
> or even cause problems in single path systems which were designed that
> way.

This is by design (of SCSI DH). We do want the device to be attached to
its handler _irrespective_ of whether multipath comes along or not.

BTW, there is _no_ infrastructure in multipath for handlers. They were
removed from multipath when scsi dh came along. So, no worries about
proprietary multipath handlers. Also, multipath _can_ attach a device to
a different (SCSI) device handler if it finds that the one that is
already attached is not the right one.

SCSI DH is implemented to make accessing the devices better (as is the
case with any device specific driver over generic one). So, the effect
of scsi dh handler on single path system is towards betterment
than being problematic.

> So as I see it, the functional benefit to a running kernel is zero and
> the functional risk exists but is unquantified, so it seems far better
> simply to solve this issue in mkinitrd.

I do not agree. This functionality makes the scsi devices behave the
same way as the other devices, and hence make the system admin's life
easier.

I hope you consider this feature in a better light now :)

chandra
> 
> James
> 
> 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

James, I think Chandra and I have responded to most if not all of your points, 
and would appreciate your thoughts on what we've said.

Peter Jones wrote:
> On 06/18/2009 06:48 PM, James Bottomley wrote:
>> On Mon, 2009-06-15 at 14:29 -0400, Peter Jones wrote:
>>> On 04/27/2009 02:06 PM, Chandra Seetharaman wrote:
>>>> Hello,
>>>>
>>>> Currently, SCSI targets doesn't have modalias support. It wasn't an issue
>>>> until SCSI device handler came along.
>>>>
>>>> We want the SCSI device handler modules to be insmodded automatically
>>>> when the specific SCSI targets are probed and found.
>>>>
>>>> This set of patches adds the modalias support for SCSI targets and
>>>> also makes the relevant changes to SCSI device handler modules to 
>>>> make use of it.
>>>>
>>>> Applies cleanly on 2.6.30-rc3 and is tested on the same.
>>>>
>>>> Please review and consider this for inclusion.
>>>>
>>>> Originally sent on March 17 2009 (http://marc.info/?l=linux-scsi&m=123734001009654&w=2).
>>>>
>>>> Resending after testing on 2.6.30-rc3 and with an ack from Hannes.
>>> Was there ever any followup to this?
>> OK, since I've forgotten where we are, let me summarise what I think the
>> situation is (correct me if I misstate any of the facts):
>>
>> This code adds no functional value to the kernel because dm already
>> autoloads the correct handlers based on the inquiry strings
> 
> I don't agree here.  It adds functional value to the entire system by making the
> loading of device drivers use the same method no matter which kernel subsystem
> the driver is part of.  This is a very tangible benefit in terms of maintainability.
> 
>> The only value it adds is that by overloading the module table with the
>> inquiry strings, mkinitrd pulls in the correct dm handlers for the state
>> the system was in.
> 
> You keep on saying this, and to me it seems very strange.  The patch does no
> "overloading" of vendor strings -- it uses them just like the PCI vendor/device ID or
> USB idVendor/idDevice.  Fundamentally they are the same thing: an identifier to
> tell us what device we're looking at.  There's no overloading here, it's just
> hooking them up to the kernel's generalized mechanism for loading modules based on
> this kind of data.
> 
>> the unaddressed problems are:
>>
>> The kernel now tries to load the dm handler for the device dynamically
>> whether or not the user is actually deploying multi-path (previously dm
>> does this and if it's not loaded, that doesn't happen).  It's entirely
>> unclear whether this would interfere with proprietary multipath handlers
>> or even cause problems in single path systems which were designed that
>> way.
> 
> I just don't see how this is a real concern -- the whole point of the drivers
> which we're autoloading here is to work with specific hardware that's designed to
> work in this fashion.  What's more, if that hardware is being used for single-path,
> and the driver is loaded and it decides to reconfigure to prefer a path that's not
> plugged in, surely that's a bug in the device handler driver, and should be fixed
> there, rather than blocking the auto-loading of such drivers?
> 


-- 
   Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

James,

Any updates on this ?

thanks

chandra
On Fri, 2009-06-19 at 12:37 -0700, Chandra Seetharaman wrote:
> Hi James,
> 
> Thanks for getting back on this.
> 
> On Thu, 2009-06-18 at 17:48 -0500, James Bottomley wrote:
> 
> > > 
> > > Was there ever any followup to this?
> > 
> > OK, since I've forgotten where we are, let me summarise what I think the
> > situation is (correct me if I misstate any of the facts):
> > 
> > This code adds no functional value to the kernel because dm already
> > autoloads the correct handlers based on the inquiry strings
> 
> First point of clarification:
> 
> The main purpose of moving the device handlers from the dm-layer to scsi
> layer was that the time at which dm-layer comes in is too late.
> 
> SCSI-DH was added mainly to make sure that scsi layer recognize that
> these are special devices and has to be handled differently.
> 
> (Original problem was that during device probing lot of ios are sent to
> passive paths which caused boot time delays and tons of errors messages.
> These increase linearly with the number of luns and the number of
> passive paths, i.e more redundant the system is, more time it takes to
> boot and more error messages it spits out).
> 
> To paraphrase, if we were willing to wait till dm layer loads
> appropriate device handler modules, there would be no need for us to
> move the device handlers to SCSI layer.
> 
> With that clarification....
> 
> Having device handlers in SCSI is useful _only_ if we have the
> appropriate device handler modules in initrd. Otherwise, the user will
> go thru all the sufferings (boot delay and error messages) that they
> went thru when the device handlers were in dm-layer voiding the very
> benefit of moving the handlers to SCSI layer.
> 
> As of today (actually since scsi dh was in the kernel), I suggest the
> users of scsi device handler modules to create a new initrd with the
> required scsi_dh module
> (http://sources.redhat.com/lvm2/wiki/MultipathUsageGuide#head-fb3efbb82fa69ca86b7db26423c235ae6c280caa)
> 
> Ideal way to solve this problem is to make sure the
> appropriate/necessary modules are built into the initrd image
> automatically (as is the case with all other devices that need a
> driver), without the user/admin doing it .
> 
> Hence this patch.
> > 
> > The only value it adds is that by overloading the module table with the
> > inquiry strings, mkinitrd pulls in the correct dm handlers for the state
> > the system was in.
> > 
> > the unaddressed problems are:
> > 
> > The kernel now tries to load the dm handler for the device dynamically
> > whether or not the user is actually deploying multi-path (previously dm
> > does this and if it's not loaded, that doesn't happen).  It's entirely
> > unclear whether this would interfere with proprietary multipath handlers
> > or even cause problems in single path systems which were designed that
> > way.
> 
> This is by design (of SCSI DH). We do want the device to be attached to
> its handler _irrespective_ of whether multipath comes along or not.
> 
> BTW, there is _no_ infrastructure in multipath for handlers. They were
> removed from multipath when scsi dh came along. So, no worries about
> proprietary multipath handlers. Also, multipath _can_ attach a device to
> a different (SCSI) device handler if it finds that the one that is
> already attached is not the right one.
> 
> SCSI DH is implemented to make accessing the devices better (as is the
> case with any device specific driver over generic one). So, the effect
> of scsi dh handler on single path system is towards betterment
> than being problematic.
> 
> > So as I see it, the functional benefit to a running kernel is zero and
> > the functional risk exists but is unquantified, so it seems far better
> > simply to solve this issue in mkinitrd.
> 
> I do not agree. This functionality makes the scsi devices behave the
> same way as the other devices, and hence make the system admin's life
> easier.
> 
> I hope you consider this feature in a better light now :)
> 
> chandra
> > 
> > James
> > 
> > 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
> James, I think Chandra and I have responded to most if not all of your points, 
> and would appreciate your thoughts on what we've said.

Well, you didn't respond to the important one:

You're seeking to change the binding of these helpers from manual to
automatic.  This would mean that the modules are loaded in the single
controller case, for which they may not be wanted and also when some
multi path tool other than dm-mp is managing the device, in which case
they may actively interfere with operations.  Your basic contention is
that you "don't see any concern here".  When I ask what testing you've
done for either of these, the studied silence eloquently illustrates
"none".  A policy change like this can't be made without being
incredibly sure we're not going to screw up existing installations.

The second point I made speaks to the technical ugliness of this: what
you're basically doing is open coding multiple binding for a device
handler specific case.  If you can persuade me the policy above is
correct, then technically all this should be done correctly via multiple
binding in the generic device core ... before this interface nastiness
you're constructing propagates outwards and becomes part of the user
ABI.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 07/07/2009 01:12 PM, James Bottomley wrote:
> On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
>> James, I think Chandra and I have responded to most if not all of your points, 
>> and would appreciate your thoughts on what we've said.
> 
> Well, you didn't respond to the important one:
> 
> You're seeking to change the binding of these helpers from manual to
> automatic.  This would mean that the modules are loaded in the single
> controller case, for which they may not be wanted and also when some
> multi path tool other than dm-mp is managing the device, in which case
> they may actively interfere with operations.  Your basic contention is
> that you "don't see any concern here".

I think Chandra addressed this in his reply to your previous email: 

[From him]
> This is by design (of SCSI DH). We do want the device to be attached to
> its handler _irrespective_ of whether multipath comes along or not.
> 
> BTW, there is _no_ infrastructure in multipath for handlers. They were
> removed from multipath when scsi dh came along. So, no worries about
> proprietary multipath handlers. Also, multipath _can_ attach a device to
> a different (SCSI) device handler if it finds that the one that is
> already attached is not the right one.

[From you again]
> When I ask what testing you've done for either of these, the studied
> silence eloquently illustrates "none". A policy change like this
> can't be made without being incredibly sure we're not going to screw
> up existing installations.

I'll let Chandra address this, as it is my understanding that he has
hardware and has tested this code with it.

> The second point I made speaks to the technical ugliness of this: what
> you're basically doing is open coding multiple binding for a device
> handler specific case.  If you can persuade me the policy above is
> correct, then technically all this should be done correctly via multiple
> binding in the generic device core ... before this interface nastiness
> you're constructing propagates outwards and becomes part of the user
> ABI.

I'm willing to re-implement the functionality with a different mechanism
if it has your blessing, if you can be specific about what it is you think
would be better.  Obviously I'll hold off on that until we've come to some
agreement about the other aspects.

-- 
        Peter

For some reason it has always seemed to me that the term software 
engineering contains some very optimistic assumptions about the 
nature of reality.

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Tue, 2009-07-07 at 13:51 -0400, Peter Jones wrote:
> On 07/07/2009 01:12 PM, James Bottomley wrote:
> > On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
> >> James, I think Chandra and I have responded to most if not all of your points, 
> >> and would appreciate your thoughts on what we've said.
> > 
> > Well, you didn't respond to the important one:
> > 
> > You're seeking to change the binding of these helpers from manual to
> > automatic.  This would mean that the modules are loaded in the single
> > controller case, for which they may not be wanted and also when some
> > multi path tool other than dm-mp is managing the device, in which case
> > they may actively interfere with operations.  Your basic contention is
> > that you "don't see any concern here".
> 
> I think Chandra addressed this in his reply to your previous email: 

I don't think so:

> [From him]
> > This is by design (of SCSI DH). We do want the device to be attached to
> > its handler _irrespective_ of whether multipath comes along or not.
> > 
> > BTW, there is _no_ infrastructure in multipath for handlers. They were
> > removed from multipath when scsi dh came along. So, no worries about
> > proprietary multipath handlers. Also, multipath _can_ attach a device to
> > a different (SCSI) device handler if it finds that the one that is
> > already attached is not the right one.

I want to know what happens when some multipath system other than dm-mp
tries to use a system with a device handler attached ... I don't see how
that addresses the issue at all. The device handlers, when they're
attached can alter sense and return code processing .. this has the
potential to interfere with how the other multipath code is expecting
things to happen

If we have to get really concrete: what happens with something like
powerpath and scsi_dh_emc attached?

> [From you again]
> > When I ask what testing you've done for either of these, the studied
> > silence eloquently illustrates "none". A policy change like this
> > can't be made without being incredibly sure we're not going to screw
> > up existing installations.
> 
> I'll let Chandra address this, as it is my understanding that he has
> hardware and has tested this code with it.
> 
> > The second point I made speaks to the technical ugliness of this: what
> > you're basically doing is open coding multiple binding for a device
> > handler specific case.  If you can persuade me the policy above is
> > correct, then technically all this should be done correctly via multiple
> > binding in the generic device core ... before this interface nastiness
> > you're constructing propagates outwards and becomes part of the user
> > ABI.
> 
> I'm willing to re-implement the functionality with a different mechanism
> if it has your blessing, if you can be specific about what it is you think
> would be better.  Obviously I'll hold off on that until we've come to some
> agreement about the other aspects.

Well, the two things aren't so dependent: this dh_state attribute that
hannes just NAK'd is precisely a binding attribute for your hand rolled
multiple driver attachment, so actually getting generic device multiple
binding sorted out would help out regardless of what the final
attachment policy decision is.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

On Tue, 2009-07-07 at 18:14 +0000, James Bottomley wrote:
> On Tue, 2009-07-07 at 13:51 -0400, Peter Jones wrote:
> > On 07/07/2009 01:12 PM, James Bottomley wrote:
> > > On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
> > >> James, I think Chandra and I have responded to most if not all of your points, 
> > >> and would appreciate your thoughts on what we've said.
> > > 
> > > Well, you didn't respond to the important one:
> > > 
> > > You're seeking to change the binding of these helpers from manual to
> > > automatic.  This would mean that the modules are loaded in the single
> > > controller case, for which they may not be wanted and also when some
> > > multi path tool other than dm-mp is managing the device, in which case
> > > they may actively interfere with operations.  Your basic contention is
> > > that you "don't see any concern here".
> > 
> > I think Chandra addressed this in his reply to your previous email: 
> 
> I don't think so:
> 
> > [From him]
> > > This is by design (of SCSI DH). We do want the device to be attached to
> > > its handler _irrespective_ of whether multipath comes along or not.
> > > 
> > > BTW, there is _no_ infrastructure in multipath for handlers. They were
> > > removed from multipath when scsi dh came along. So, no worries about
> > > proprietary multipath handlers. Also, multipath _can_ attach a device to
> > > a different (SCSI) device handler if it finds that the one that is
> > > already attached is not the right one.
> 
> I want to know what happens when some multipath system other than dm-mp
> tries to use a system with a device handler attached ... I don't see how
> that addresses the issue at all. The device handlers, when they're
> attached can alter sense and return code processing .. this has the
> potential to interfere with how the other multipath code is expecting
> things to happen

When I responded to your earlier email I thought you meant the
interaction between dm-mp and the hardware handler ( and not other
out-of-tree multipath solutions). My answer was in that context alone.

Yes, your conclusion is correct. There are few functionalities in the
hardware handler interface that might affect the above layers (block and
above). One is the prep_fn() function, second is the check_sense()
function, and third is the activate() function.

Since scsi_dh is designed for dm-mp :). 

I do not know of all the out-of-tree multipath solutions and how they
behave and at what layer they interact. In effect, I haven't tested
hardware handler with other multipath solutions.

> 
> If we have to get really concrete: what happens with something like
> powerpath and scsi_dh_emc attached?
> 

They _might_ be affected as mentioned above.

> > [From you again]
> > > When I ask what testing you've done for either of these, the studied
> > > silence eloquently illustrates "none". A policy change like this
> > > can't be made without being incredibly sure we're not going to screw
> > > up existing installations.
> > 
> > I'll let Chandra address this, as it is my understanding that he has
> > hardware and has tested this code with it.
> > 
> > > The second point I made speaks to the technical ugliness of this: what
> > > you're basically doing is open coding multiple binding for a device
> > > handler specific case.  If you can persuade me the policy above is
> > > correct, then technically all this should be done correctly via multiple
> > > binding in the generic device core ... before this interface nastiness
> > > you're constructing propagates outwards and becomes part of the user
> > > ABI.
> > 
> > I'm willing to re-implement the functionality with a different mechanism
> > if it has your blessing, if you can be specific about what it is you think
> > would be better.  Obviously I'll hold off on that until we've come to some
> > agreement about the other aspects.
> 
> Well, the two things aren't so dependent: this dh_state attribute that
> hannes just NAK'd is precisely a binding attribute for your hand rolled

Just to clarify:

dh_state attribute is not added new, it was originally added by Hannes. 

I just fixed a problem to make it work as it is intended, and sent an
explanation (about my fix) for Hannes to reconsider is NAK.

> multiple driver attachment, so actually getting generic device multiple
> binding sorted out would help out regardless of what the final
> attachment policy decision is.
> 
> James
> 
> 

RE: [PATCH 0/3] scsi_dh: Make scsi device handler modulesautomatically inserted

[berthiaume_wayne]

Hi Chandra.

	I can speak for EMC PowerPath in that it is affect by DM if both
are trying to control the same device. Another large multipathing
solution that may be affected would be Symantec's DMP.

Regards,
Wayne. 

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org
[mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Chandra
Seetharaman
Sent: Tuesday, July 07, 2009 3:37 PM
To: James Bottomley
Cc: Peter Jones; linux-scsi@vger.kernel.org; michaelc@cs.wisc.edu;
hare@suse.de
Subject: Re: [PATCH 0/3] scsi_dh: Make scsi device handler
modulesautomatically inserted


On Tue, 2009-07-07 at 18:14 +0000, James Bottomley wrote:
> On Tue, 2009-07-07 at 13:51 -0400, Peter Jones wrote:
> > On 07/07/2009 01:12 PM, James Bottomley wrote:
> > > On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
> > >> James, I think Chandra and I have responded to most if not all of
your points, 
> > >> and would appreciate your thoughts on what we've said.
> > > 
> > > Well, you didn't respond to the important one:
> > > 
> > > You're seeking to change the binding of these helpers from manual
to
> > > automatic.  This would mean that the modules are loaded in the
single
> > > controller case, for which they may not be wanted and also when
some
> > > multi path tool other than dm-mp is managing the device, in which
case
> > > they may actively interfere with operations.  Your basic
contention is
> > > that you "don't see any concern here".
> > 
> > I think Chandra addressed this in his reply to your previous email: 
> 
> I don't think so:
> 
> > [From him]
> > > This is by design (of SCSI DH). We do want the device to be
attached to
> > > its handler _irrespective_ of whether multipath comes along or
not.
> > > 
> > > BTW, there is _no_ infrastructure in multipath for handlers. They
were
> > > removed from multipath when scsi dh came along. So, no worries
about
> > > proprietary multipath handlers. Also, multipath _can_ attach a
device to
> > > a different (SCSI) device handler if it finds that the one that is
> > > already attached is not the right one.
> 
> I want to know what happens when some multipath system other than
dm-mp
> tries to use a system with a device handler attached ... I don't see
how
> that addresses the issue at all. The device handlers, when they're
> attached can alter sense and return code processing .. this has the
> potential to interfere with how the other multipath code is expecting
> things to happen

When I responded to your earlier email I thought you meant the
interaction between dm-mp and the hardware handler ( and not other
out-of-tree multipath solutions). My answer was in that context alone.

Yes, your conclusion is correct. There are few functionalities in the
hardware handler interface that might affect the above layers (block and
above). One is the prep_fn() function, second is the check_sense()
function, and third is the activate() function.

Since scsi_dh is designed for dm-mp :). 

I do not know of all the out-of-tree multipath solutions and how they
behave and at what layer they interact. In effect, I haven't tested
hardware handler with other multipath solutions.

> 
> If we have to get really concrete: what happens with something like
> powerpath and scsi_dh_emc attached?
> 

They _might_ be affected as mentioned above.

> > [From you again]
> > > When I ask what testing you've done for either of these, the
studied
> > > silence eloquently illustrates "none". A policy change like this
> > > can't be made without being incredibly sure we're not going to
screw
> > > up existing installations.
> > 
> > I'll let Chandra address this, as it is my understanding that he has
> > hardware and has tested this code with it.
> > 
> > > The second point I made speaks to the technical ugliness of this:
what
> > > you're basically doing is open coding multiple binding for a
device
> > > handler specific case.  If you can persuade me the policy above is
> > > correct, then technically all this should be done correctly via
multiple
> > > binding in the generic device core ... before this interface
nastiness
> > > you're constructing propagates outwards and becomes part of the
user
> > > ABI.
> > 
> > I'm willing to re-implement the functionality with a different
mechanism
> > if it has your blessing, if you can be specific about what it is you
think
> > would be better.  Obviously I'll hold off on that until we've come
to some
> > agreement about the other aspects.
> 
> Well, the two things aren't so dependent: this dh_state attribute that
> hannes just NAK'd is precisely a binding attribute for your hand
rolled

Just to clarify:

dh_state attribute is not added new, it was originally added by Hannes. 

I just fixed a problem to make it work as it is intended, and sent an
explanation (about my fix) for Hannes to reconsider is NAK.

> multiple driver attachment, so actually getting generic device
multiple
> binding sorted out would help out regardless of what the final
> attachment policy decision is.
> 
> James
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Christoph Hellwig]

On Tue, Jul 07, 2009 at 12:36:41PM -0700, Chandra Seetharaman wrote:
> I do not know of all the out-of-tree multipath solutions and how they
> behave and at what layer they interact. In effect, I haven't tested
> hardware handler with other multipath solutions.

It doesn't matter anyway.  We've never cared about these out of tree
problems and they'll have to find a way by themselves to deal with any
changes we do to offer better multipath support upstream.

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

On Wed, 2009-07-08 at 11:58 -0400, Christoph Hellwig wrote:
> On Tue, Jul 07, 2009 at 12:36:41PM -0700, Chandra Seetharaman wrote:
> > I do not know of all the out-of-tree multipath solutions and how they
> > behave and at what layer they interact. In effect, I haven't tested
> > hardware handler with other multipath solutions.
> 
> It doesn't matter anyway.  We've never cared about these out of tree
> problems and they'll have to find a way by themselves to deal with any
> changes we do to offer better multipath support upstream.

Agree.

One simple way the out-of-tree multipath solutions can circumvent this
feature is by blacklisting the hardware handler modules in their
modprobe.conf file.

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/3] scsi_dh: Make scsi device handler modulesautomatically inserted

[Chandra Seetharaman]

On Wed, 2009-07-08 at 11:53 -0400, berthiaume_wayne@emc.com wrote:
> Hi Chandra.
> 
> 	I can speak for EMC PowerPath in that it is affect by DM if both
> are trying to control the same device. Another large multipathing
> solution that may be affected would be Symantec's DMP.

Wayne,

James's concern was, "what happens when the scsi hardware handler is
attached to the device _and_ powerpath is the multipathing solution (and
not DM)". (since we will be attaching the handler automatically - with
this feature).

Anyways, I do not think we can practically take care of _all_
out-of-tree solutions, so it doesn't really matter what the answer for
the above question is :)
 
> 
> Regards,
> Wayne. 
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Chandra
> Seetharaman
> Sent: Tuesday, July 07, 2009 3:37 PM
> To: James Bottomley
> Cc: Peter Jones; linux-scsi@vger.kernel.org; michaelc@cs.wisc.edu;
> hare@suse.de
> Subject: Re: [PATCH 0/3] scsi_dh: Make scsi device handler
> modulesautomatically inserted
> 
> 
> On Tue, 2009-07-07 at 18:14 +0000, James Bottomley wrote:
> > On Tue, 2009-07-07 at 13:51 -0400, Peter Jones wrote:
> > > On 07/07/2009 01:12 PM, James Bottomley wrote:
> > > > On Fri, 2009-06-26 at 09:56 -0400, Peter Jones wrote:
> > > >> James, I think Chandra and I have responded to most if not all of
> your points, 
> > > >> and would appreciate your thoughts on what we've said.
> > > > 
> > > > Well, you didn't respond to the important one:
> > > > 
> > > > You're seeking to change the binding of these helpers from manual
> to
> > > > automatic.  This would mean that the modules are loaded in the
> single
> > > > controller case, for which they may not be wanted and also when
> some
> > > > multi path tool other than dm-mp is managing the device, in which
> case
> > > > they may actively interfere with operations.  Your basic
> contention is
> > > > that you "don't see any concern here".
> > > 
> > > I think Chandra addressed this in his reply to your previous email: 
> > 
> > I don't think so:
> > 
> > > [From him]
> > > > This is by design (of SCSI DH). We do want the device to be
> attached to
> > > > its handler _irrespective_ of whether multipath comes along or
> not.
> > > > 
> > > > BTW, there is _no_ infrastructure in multipath for handlers. They
> were
> > > > removed from multipath when scsi dh came along. So, no worries
> about
> > > > proprietary multipath handlers. Also, multipath _can_ attach a
> device to
> > > > a different (SCSI) device handler if it finds that the one that is
> > > > already attached is not the right one.
> > 
> > I want to know what happens when some multipath system other than
> dm-mp
> > tries to use a system with a device handler attached ... I don't see
> how
> > that addresses the issue at all. The device handlers, when they're
> > attached can alter sense and return code processing .. this has the
> > potential to interfere with how the other multipath code is expecting
> > things to happen
> 
> When I responded to your earlier email I thought you meant the
> interaction between dm-mp and the hardware handler ( and not other
> out-of-tree multipath solutions). My answer was in that context alone.
> 
> Yes, your conclusion is correct. There are few functionalities in the
> hardware handler interface that might affect the above layers (block and
> above). One is the prep_fn() function, second is the check_sense()
> function, and third is the activate() function.
> 
> Since scsi_dh is designed for dm-mp :). 
> 
> I do not know of all the out-of-tree multipath solutions and how they
> behave and at what layer they interact. In effect, I haven't tested
> hardware handler with other multipath solutions.
> 
> > 
> > If we have to get really concrete: what happens with something like
> > powerpath and scsi_dh_emc attached?
> > 
> 
> They _might_ be affected as mentioned above.
> 
> > > [From you again]
> > > > When I ask what testing you've done for either of these, the
> studied
> > > > silence eloquently illustrates "none". A policy change like this
> > > > can't be made without being incredibly sure we're not going to
> screw
> > > > up existing installations.
> > > 
> > > I'll let Chandra address this, as it is my understanding that he has
> > > hardware and has tested this code with it.
> > > 
> > > > The second point I made speaks to the technical ugliness of this:
> what
> > > > you're basically doing is open coding multiple binding for a
> device
> > > > handler specific case.  If you can persuade me the policy above is
> > > > correct, then technically all this should be done correctly via
> multiple
> > > > binding in the generic device core ... before this interface
> nastiness
> > > > you're constructing propagates outwards and becomes part of the
> user
> > > > ABI.
> > > 
> > > I'm willing to re-implement the functionality with a different
> mechanism
> > > if it has your blessing, if you can be specific about what it is you
> think
> > > would be better.  Obviously I'll hold off on that until we've come
> to some
> > > agreement about the other aspects.
> > 
> > Well, the two things aren't so dependent: this dh_state attribute that
> > hannes just NAK'd is precisely a binding attribute for your hand
> rolled
> 
> Just to clarify:
> 
> dh_state attribute is not added new, it was originally added by Hannes. 
> 
> I just fixed a problem to make it work as it is intended, and sent an
> explanation (about my fix) for Hannes to reconsider is NAK.
> 
> > multiple driver attachment, so actually getting generic device
> multiple
> > binding sorted out would help out regardless of what the final
> > attachment policy decision is.
> > 
> > James
> > 
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 07/08/2009 11:58 AM, Christoph Hellwig wrote:
> On Tue, Jul 07, 2009 at 12:36:41PM -0700, Chandra Seetharaman wrote:
>> I do not know of all the out-of-tree multipath solutions and how they
>> behave and at what layer they interact. In effect, I haven't tested
>> hardware handler with other multipath solutions.
> 
> It doesn't matter anyway.  We've never cared about these out of tree
> problems and they'll have to find a way by themselves to deal with any
> changes we do to offer better multipath support upstream.

That's definitely how I feel about it, but James apparently disagrees.
Anyway, to alleviate some of his concerns, here's a different starting
point that maybe we could work from.  James, tell me how you feel about
this?

From: Peter Jones <pjones@redhat.com>
Date: Tue, 7 Jul 2009 15:08:00 -0400
Subject: [PATCH] Add scsi device and vendor IDs to scsi target modaliases.

This patch adds scsi device and vendor IDs to scsi target modaliases,
including the corresponding uevents and sysfs modalias file.

This behavior is conditional on the module parameter
scsi_mod.target_modalias_has_vendor, which has its default set by the
enabling or disabling of CONFIG_SCSI_TARGET_MODALIAS_WITH_ID .
---
 drivers/scsi/Kconfig           |   10 +++++
 drivers/scsi/scsi_sysfs.c      |   72 ++++++++++++++++++++++++++++++++++++++-
 include/linux/string_helpers.h |    2 +
 include/scsi/scsi_device.h     |    1 -
 lib/string_helpers.c           |   32 ++++++++++++++++++
 5 files changed, 114 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 9c23122..b18c329 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -239,6 +239,16 @@ config SCSI_LOGGING
 	  there should be no noticeable performance impact as long as you have
 	  logging turned off.
 
+config SCSI_TARGET_MODALIAS_WITH_ID
+	bool "Include vendor/device IDs in SCSI target modalias strings"
+	default n
+	depends on SCSI
+	help
+	  The SCSI subsystem can present modalias strings for SCSI target
+	  devices either with or without vendor and device ID data.
+	  This is experimental, and make cause undesirable interactions with
+	  some device management programs.
+
 config SCSI_SCAN_ASYNC
 	bool "Asynchronous SCSI scanning"
 	depends on SCSI
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 91482f2..af4030c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -10,6 +10,7 @@
 #include <linux/init.h>
 #include <linux/blkdev.h>
 #include <linux/device.h>
+#include <linux/string_helpers.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -362,16 +363,80 @@ static int scsi_bus_match(struct device *dev, struct device_driver *gendrv)
 	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
 }
 
+#ifdef CONFIG_SCSI_TARGET_MODALIAS_WITH_ID
+static unsigned int scsi_target_modalias_has_vendor = 1;
+#else
+static unsigned int scsi_target_modalias_has_vendor = 0;
+#endif
+
+module_param_named(target_modalias_has_vendor, scsi_target_modalias_has_vendor,
+		   uint, S_IRUGO|S_IWUSR);
+MODULE_PARAM_DESC(target_modalias_has_vendor, "control presense of vendor and device ID in scsi target modaliases");
+
+static ssize_t format_scsi_modalias(struct scsi_device *sdev, char *buffer,
+				    ssize_t len)
+{
+	int i = 0;
+
+	if (scsi_target_modalias_has_vendor) {
+		int j;
+		char vendor[9];
+		char *hex_vendor;
+		char model[17];
+		char *hex_model;
+	
+		strncpy(vendor, sdev->vendor, 8);
+		vendor[8] = '\0';
+		for (j = strlen(vendor) - 1; j >= 0; j--) {
+			if (vendor[j] != ' ')
+				break;
+			vendor[j] = '\0';
+		}
+		hex_vendor = string_to_hex(vendor);
+		if (!hex_vendor)
+			return -ENOMEM;
+	
+		strncpy(model, sdev->model, 16);
+		model[8] = '\0';
+		for (j = strlen(model) - 1; j >= 0; j--) {
+			if (model[j] != ' ')
+				break;
+			model[j] = '\0';
+		}
+		hex_model = string_to_hex(model);
+		if (!hex_model) {
+			kfree(hex_vendor);
+			return -ENOMEM;
+		}
+	
+		i = snprintf(buffer, len, "scsi:t-0x%02xv%.16sm%.32s",
+			sdev->type, hex_vendor, hex_model);
+		kfree(hex_vendor);
+		kfree(hex_model);
+	} else {
+		i = snprintf(buffer, len, "scsi:t-0x%02x" sdev->type);
+	}
+
+	return i;
+}
+
 static int scsi_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct scsi_device *sdev;
+	char buffer[501];
+	int rc;
 
 	if (dev->type != &scsi_dev_type)
 		return 0;
 
 	sdev = to_scsi_device(dev);
 
-	add_uevent_var(env, "MODALIAS=" SCSI_DEVICE_MODALIAS_FMT, sdev->type);
+	buffer[500] = '\0';
+	rc = format_scsi_modalias(sdev, buffer, 500);
+	if (rc < 0)
+		return rc;
+
+	add_uevent_var(env, "MODALIAS=%s", buffer);
 	return 0;
 }
 
@@ -680,8 +745,11 @@ static ssize_t
 sdev_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct scsi_device *sdev;
+	ssize_t rc;
+
 	sdev = to_scsi_device(dev);
-	return snprintf (buf, 20, SCSI_DEVICE_MODALIAS_FMT "\n", sdev->type);
+	rc = format_scsi_modalias(sdev, buf, 500);
+	return rc;
 }
 static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
 
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index a3eb2f6..46a7594 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -13,4 +13,6 @@ enum string_size_units {
 int string_get_size(u64 size, enum string_size_units units,
 		    char *buf, int len);
 
+unsigned char *string_to_hex(const unsigned char *s);
+
 #endif
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 3f566af..ffa7746 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -452,6 +452,5 @@ static inline int scsi_device_protection(struct scsi_device *sdev)
 
 #define MODULE_ALIAS_SCSI_DEVICE(type) \
 	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
-#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
 
 #endif /* _SCSI_SCSI_DEVICE_H */
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index ab431d4..2ecd500 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -66,3 +66,35 @@ int string_get_size(u64 size, const enum string_size_units units,
 	return 0;
 }
 EXPORT_SYMBOL(string_get_size);
+
+/**
+ * string_to_hex - convert a string to a series of hexidecimal values
+ * @s:	The string to operate on
+ *
+ * This function returns a GFP_KERNEL allocated buffer filled with
+ * the hexidecimal representation of the value of each character in @s .
+ * Returns a pointer to the allocated string on success and NULL on error,
+ * and the returned string is zero terminated.
+ *
+ */
+unsigned char *string_to_hex(const unsigned char *s)
+{
+	unsigned char *ret, *ptr;
+	static const unsigned char *hex = "0123456789ABCDEF";
+	int len;
+
+	len = strlen(s);
+
+	ret = ptr = kmalloc(len * 2 + 1, GFP_KERNEL);
+	if (!ret)
+		return NULL;
+
+	for (; *s; s++) {
+		*ptr++ = hex[(*s & 0xf0)>>4];
+		*ptr++ = hex[*s & 0x0f];
+	}
+	*ptr = '\0';
+
+	return ret;
+}
+EXPORT_SYMBOL(string_to_hex);
-- 
1.6.2.2

-- 
        Peter

Growth for the sake of growth is the ideology of the cancer cell.

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Christoph Hellwig]

On Wed, Jul 08, 2009 at 02:33:01PM -0400, Peter Jones wrote:
> This behavior is conditional on the module parameter
> scsi_mod.target_modalias_has_vendor, which has its default set by the
> enabling or disabling of CONFIG_SCSI_TARGET_MODALIAS_WITH_ID .

Sorry, but that's really stupid.  If people have the arrays that have
device handlers there are two possible cases:

 - just a single port configured on the array. In this case the device
   handlers are useless but also completely unharmful.
 - multiple ports configured on the array.  In this case we desperately
   want the device handlers loaded early so that we do the right thing
   for the inactive ports and avoid the horrible probing delays, not
   matter if we actually do end up using multipath later or no.

No need to make this configurable.  If people really do not want the
handler (e.g. becuase the may end up beeing buggy for a new array
matching the old idea) they can just blacklist it in the modules
configuration.

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 07/08/2009 02:40 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2009 at 02:33:01PM -0400, Peter Jones wrote:
>> This behavior is conditional on the module parameter
>> scsi_mod.target_modalias_has_vendor, which has its default set by the
>> enabling or disabling of CONFIG_SCSI_TARGET_MODALIAS_WITH_ID .
> 
> Sorry, but that's really stupid.  If people have the arrays that have
> device handlers there are two possible cases:
> 
>  - just a single port configured on the array. In this case the device
>    handlers are useless but also completely unharmful.
>  - multiple ports configured on the array.  In this case we desperately
>    want the device handlers loaded early so that we do the right thing
>    for the inactive ports and avoid the horrible probing delays, not
>    matter if we actually do end up using multipath later or no.
> 
> No need to make this configurable.  If people really do not want the
> handler (e.g. becuase the may end up beeing buggy for a new array
> matching the old idea) they can just blacklist it in the modules
> configuration.

Which is what I had in mind when I wrote the original version of this,
but which James does not agree with.

-- 
        Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

James,

Please let us know which way you want us to proceed ?

We have been pursuing this for more than three months now.

Thanks,

chandra
On Wed, 2009-07-08 at 14:33 -0400, Peter Jones wrote:
> On 07/08/2009 11:58 AM, Christoph Hellwig wrote:
> > On Tue, Jul 07, 2009 at 12:36:41PM -0700, Chandra Seetharaman wrote:
> >> I do not know of all the out-of-tree multipath solutions and how they
> >> behave and at what layer they interact. In effect, I haven't tested
> >> hardware handler with other multipath solutions.
> > 
> > It doesn't matter anyway.  We've never cared about these out of tree
> > problems and they'll have to find a way by themselves to deal with any
> > changes we do to offer better multipath support upstream.
> 
> That's definitely how I feel about it, but James apparently disagrees.
> Anyway, to alleviate some of his concerns, here's a different starting
> point that maybe we could work from.  James, tell me how you feel about
> this?
> 
> From: Peter Jones <pjones@redhat.com>
> Date: Tue, 7 Jul 2009 15:08:00 -0400
> Subject: [PATCH] Add scsi device and vendor IDs to scsi target modaliases.
> 
> This patch adds scsi device and vendor IDs to scsi target modaliases,
> including the corresponding uevents and sysfs modalias file.
> 
> This behavior is conditional on the module parameter
> scsi_mod.target_modalias_has_vendor, which has its default set by the
> enabling or disabling of CONFIG_SCSI_TARGET_MODALIAS_WITH_ID .
> ---
>  drivers/scsi/Kconfig           |   10 +++++
>  drivers/scsi/scsi_sysfs.c      |   72 ++++++++++++++++++++++++++++++++++++++-
>  include/linux/string_helpers.h |    2 +
>  include/scsi/scsi_device.h     |    1 -
>  lib/string_helpers.c           |   32 ++++++++++++++++++
>  5 files changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 9c23122..b18c329 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -239,6 +239,16 @@ config SCSI_LOGGING
>  	  there should be no noticeable performance impact as long as you have
>  	  logging turned off.
> 
> +config SCSI_TARGET_MODALIAS_WITH_ID
> +	bool "Include vendor/device IDs in SCSI target modalias strings"
> +	default n
> +	depends on SCSI
> +	help
> +	  The SCSI subsystem can present modalias strings for SCSI target
> +	  devices either with or without vendor and device ID data.
> +	  This is experimental, and make cause undesirable interactions with
> +	  some device management programs.
> +
>  config SCSI_SCAN_ASYNC
>  	bool "Asynchronous SCSI scanning"
>  	depends on SCSI
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 91482f2..af4030c 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/blkdev.h>
>  #include <linux/device.h>
> +#include <linux/string_helpers.h>
> 
>  #include <scsi/scsi.h>
>  #include <scsi/scsi_device.h>
> @@ -362,16 +363,80 @@ static int scsi_bus_match(struct device *dev, struct device_driver *gendrv)
>  	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
>  }
> 
> +#ifdef CONFIG_SCSI_TARGET_MODALIAS_WITH_ID
> +static unsigned int scsi_target_modalias_has_vendor = 1;
> +#else
> +static unsigned int scsi_target_modalias_has_vendor = 0;
> +#endif
> +
> +module_param_named(target_modalias_has_vendor, scsi_target_modalias_has_vendor,
> +		   uint, S_IRUGO|S_IWUSR);
> +MODULE_PARAM_DESC(target_modalias_has_vendor, "control presense of vendor and device ID in scsi target modaliases");
> +
> +static ssize_t format_scsi_modalias(struct scsi_device *sdev, char *buffer,
> +				    ssize_t len)
> +{
> +	int i = 0;
> +
> +	if (scsi_target_modalias_has_vendor) {
> +		int j;
> +		char vendor[9];
> +		char *hex_vendor;
> +		char model[17];
> +		char *hex_model;
> +	
> +		strncpy(vendor, sdev->vendor, 8);
> +		vendor[8] = '\0';
> +		for (j = strlen(vendor) - 1; j >= 0; j--) {
> +			if (vendor[j] != ' ')
> +				break;
> +			vendor[j] = '\0';
> +		}
> +		hex_vendor = string_to_hex(vendor);
> +		if (!hex_vendor)
> +			return -ENOMEM;
> +	
> +		strncpy(model, sdev->model, 16);
> +		model[8] = '\0';
> +		for (j = strlen(model) - 1; j >= 0; j--) {
> +			if (model[j] != ' ')
> +				break;
> +			model[j] = '\0';
> +		}
> +		hex_model = string_to_hex(model);
> +		if (!hex_model) {
> +			kfree(hex_vendor);
> +			return -ENOMEM;
> +		}
> +	
> +		i = snprintf(buffer, len, "scsi:t-0x%02xv%.16sm%.32s",
> +			sdev->type, hex_vendor, hex_model);
> +		kfree(hex_vendor);
> +		kfree(hex_model);
> +	} else {
> +		i = snprintf(buffer, len, "scsi:t-0x%02x" sdev->type);
> +	}
> +
> +	return i;
> +}
> +
>  static int scsi_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {
>  	struct scsi_device *sdev;
> +	char buffer[501];
> +	int rc;
> 
>  	if (dev->type != &scsi_dev_type)
>  		return 0;
> 
>  	sdev = to_scsi_device(dev);
> 
> -	add_uevent_var(env, "MODALIAS=" SCSI_DEVICE_MODALIAS_FMT, sdev->type);
> +	buffer[500] = '\0';
> +	rc = format_scsi_modalias(sdev, buffer, 500);
> +	if (rc < 0)
> +		return rc;
> +
> +	add_uevent_var(env, "MODALIAS=%s", buffer);
>  	return 0;
>  }
> 
> @@ -680,8 +745,11 @@ static ssize_t
>  sdev_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct scsi_device *sdev;
> +	ssize_t rc;
> +
>  	sdev = to_scsi_device(dev);
> -	return snprintf (buf, 20, SCSI_DEVICE_MODALIAS_FMT "\n", sdev->type);
> +	rc = format_scsi_modalias(sdev, buf, 500);
> +	return rc;
>  }
>  static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
> 
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index a3eb2f6..46a7594 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -13,4 +13,6 @@ enum string_size_units {
>  int string_get_size(u64 size, enum string_size_units units,
>  		    char *buf, int len);
> 
> +unsigned char *string_to_hex(const unsigned char *s);
> +
>  #endif
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 3f566af..ffa7746 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -452,6 +452,5 @@ static inline int scsi_device_protection(struct scsi_device *sdev)
> 
>  #define MODULE_ALIAS_SCSI_DEVICE(type) \
>  	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
> -#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
> 
>  #endif /* _SCSI_SCSI_DEVICE_H */
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index ab431d4..2ecd500 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -66,3 +66,35 @@ int string_get_size(u64 size, const enum string_size_units units,
>  	return 0;
>  }
>  EXPORT_SYMBOL(string_get_size);
> +
> +/**
> + * string_to_hex - convert a string to a series of hexidecimal values
> + * @s:	The string to operate on
> + *
> + * This function returns a GFP_KERNEL allocated buffer filled with
> + * the hexidecimal representation of the value of each character in @s .
> + * Returns a pointer to the allocated string on success and NULL on error,
> + * and the returned string is zero terminated.
> + *
> + */
> +unsigned char *string_to_hex(const unsigned char *s)
> +{
> +	unsigned char *ret, *ptr;
> +	static const unsigned char *hex = "0123456789ABCDEF";
> +	int len;
> +
> +	len = strlen(s);
> +
> +	ret = ptr = kmalloc(len * 2 + 1, GFP_KERNEL);
> +	if (!ret)
> +		return NULL;
> +
> +	for (; *s; s++) {
> +		*ptr++ = hex[(*s & 0xf0)>>4];
> +		*ptr++ = hex[*s & 0x0f];
> +	}
> +	*ptr = '\0';
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(string_to_hex);
> -- 
> 1.6.2.2
> 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
> James,
> 
> Please let us know which way you want us to proceed ?

Yes, propose a mechanism that keeps manual binding but allows the dm-mp
user an exception.  For the actual binding I still think it should be
part of generic driver multiple binding.

> We have been pursuing this for more than three months now.

Well, the concerns I flagged at the outset haven't really changed ...
they just seem finally to be better understood.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

On Thu, 2009-07-16 at 01:16 +0000, James Bottomley wrote:
> On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
> > James,
> > 
> > Please let us know which way you want us to proceed ?
> 
> Yes, propose a mechanism that keeps manual binding but allows the dm-mp
> user an exception. 

James, this is the current behavior. We wanted to make the binding
automatic, hence the patches.

>  For the actual binding I still think it should be
> part of generic driver multiple binding.

I don't quite understand what does this mean. Can you elaborate, please.

> 
> > We have been pursuing this for more than three months now.
> 
> Well, the concerns I flagged at the outset haven't really changed ...
> they just seem finally to be better understood.
> 
> James
> 
> 

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Thu, 2009-07-16 at 18:01 -0700, Chandra Seetharaman wrote:
> On Thu, 2009-07-16 at 01:16 +0000, James Bottomley wrote:
> > On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
> > > James,
> > > 
> > > Please let us know which way you want us to proceed ?
> > 
> > Yes, propose a mechanism that keeps manual binding but allows the dm-mp
> > user an exception. 
> 
> James, this is the current behavior. We wanted to make the binding
> automatic, hence the patches.

OK, well then no ... I'm not breaking an unknown number of enterprise
configurations by forcing a binding where none is wanted or needed.
Find a way to do what you want while not breaking anyone else.

> >  For the actual binding I still think it should be
> > part of generic driver multiple binding.
> 
> I don't quite understand what does this mean. Can you elaborate, please.

http://marc.info/?l=linux-scsi&m=124699047711732

Specifically the second half.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 07/17/2009 12:19 AM, James Bottomley wrote:
> On Thu, 2009-07-16 at 18:01 -0700, Chandra Seetharaman wrote:
>> On Thu, 2009-07-16 at 01:16 +0000, James Bottomley wrote:
>>> On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
>>>> James,
>>>>
>>>> Please let us know which way you want us to proceed ?
>>> Yes, propose a mechanism that keeps manual binding but allows the dm-mp
>>> user an exception. 
>> James, this is the current behavior. We wanted to make the binding
>> automatic, hence the patches.
> 
> OK, well then no ... I'm not breaking an unknown number of enterprise
> configurations by forcing a binding where none is wanted or needed.
> Find a way to do what you want while not breaking anyone else.

And what about the patch I sent you that makes the uevent modalias
change depend on a config option?  You've still not commented on it.

-- 
        Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[James Bottomley]

On Fri, 2009-07-17 at 10:14 -0400, Peter Jones wrote:
> On 07/17/2009 12:19 AM, James Bottomley wrote:
> > On Thu, 2009-07-16 at 18:01 -0700, Chandra Seetharaman wrote:
> >> On Thu, 2009-07-16 at 01:16 +0000, James Bottomley wrote:
> >>> On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
> >>>> James,
> >>>>
> >>>> Please let us know which way you want us to proceed ?
> >>> Yes, propose a mechanism that keeps manual binding but allows the dm-mp
> >>> user an exception. 
> >> James, this is the current behavior. We wanted to make the binding
> >> automatic, hence the patches.
> > 
> > OK, well then no ... I'm not breaking an unknown number of enterprise
> > configurations by forcing a binding where none is wanted or needed.
> > Find a way to do what you want while not breaking anyone else.
> 
> And what about the patch I sent you that makes the uevent modalias
> change depend on a config option?  You've still not commented on it.

A config option isn't right, but a runtime one might be if nothing else
comes along, I suppose.  The uevent modalias still needs to go via
multiple binding.

James

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Peter Jones]

On 07/17/2009 12:45 PM, James Bottomley wrote:
> On Fri, 2009-07-17 at 10:14 -0400, Peter Jones wrote:
>> On 07/17/2009 12:19 AM, James Bottomley wrote:
>>> On Thu, 2009-07-16 at 18:01 -0700, Chandra Seetharaman wrote:
>>>> On Thu, 2009-07-16 at 01:16 +0000, James Bottomley wrote:
>>>>> On Wed, 2009-07-15 at 13:33 -0700, Chandra Seetharaman wrote:
>>>>>> James,
>>>>>>
>>>>>> Please let us know which way you want us to proceed ?
>>>>> Yes, propose a mechanism that keeps manual binding but allows the dm-mp
>>>>> user an exception. 
>>>> James, this is the current behavior. We wanted to make the binding
>>>> automatic, hence the patches.
>>> OK, well then no ... I'm not breaking an unknown number of enterprise
>>> configurations by forcing a binding where none is wanted or needed.
>>> Find a way to do what you want while not breaking anyone else.
>> And what about the patch I sent you that makes the uevent modalias
>> change depend on a config option?  You've still not commented on it.
> 
> A config option isn't right, but a runtime one might be if nothing else
> comes along, I suppose.  The uevent modalias still needs to go via
> multiple binding.

Please read the patch.  It provides a runtime option, and the config
option is used to specify the default setting.

-- 
        Peter

Re: [PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Hannes Reinecke]

Hi Chandra,

Chandra Seetharaman wrote:
> Hello,
> 
> Currently, SCSI targets doesn't have modalias support. It wasn't an issue
> until SCSI device handler came along.
> 
> We want the SCSI device handler modules to be insmodded automatically
> when the specific SCSI targets are probed and found.
> 
> This set of patches adds the modalias support for SCSI targets and
> also makes the relevant changes to SCSI device handler modules to 
> make use of it.
> 
> Applies cleanly on 2.6.29-rc8 and is tested on the same.
> 
> Please review and consider this for 2.6.30.
> 
> Thanks & Regards,
> 
> chandra
> 
Well, agreed in principle. But do note that ALUA handling really
isn't supported well here.

Problem is that ALUA hooks off a different setting (namely the
'TGPS' bit in the inquiry data) and _not_ on the device name.
In fact, it's well possible to support _both_, old-style
proprietary failover and ALUA.

So to handle this properly we should be adding a 'tgps'
setting to the sysfs data and use this for ALUA keying.
With that we basically can get rid of the device table
in ALUA and live happily ever after.

I'll be posting a patch for this shortly.
We really should be integrating both of them, as
the proposed modalias support is a Good Thing (tm).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] scsi_dh: Make scsi device handler modules automatically inserted

[Chandra Seetharaman]

Hello,

Currently, SCSI targets doesn't have modalias support. It wasn't an issue
until SCSI device handler came along.

We want the SCSI device handler modules to be insmodded automatically
when the specific SCSI targets are probed and found.

This set of patches adds the modalias support for SCSI targets and
also makes the relevant changes to SCSI device handler modules to 
make use of it.

Applies cleanly on 2.6.29-rc8 and is tested on the same.

Please review and consider this for 2.6.30.

Thanks & Regards,

chandra