Skip to contentSkip to conversation

scsi_dh: Add ability to set parameters for scsi device handler

PATCH Superseded Chandra Seetharaman 3 participants 19 messages Jul 2, 2009 – Jul 21, 2009

Subsystems (2)
SCSI Device mapper
lore
Share & tools
lore

Chandra Seetharaman

author
#0 lore raw

cover scsi_dh: Add ability to set parameters for scsi device handler

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Thanks,

chandra

author
#1 lore raw

PATCH 1/3 scsi_dh: add the interface scsi_dh_set_params()

When we moved the device handler functionality from dm layer to SCSI layer we dropped the paramaeter functionality.

This path adds an interface to scsi dh layer to set device handler parameters.

Basically, multipath layer need to create a string with all the parameters and call scsi_dh_set_params() after it called scsi_dh_attach() on a device.

If a device handler provides such an interface it will handle the parameters as it expects them.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh.c |   33 +++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h            |    1 +
 include/scsi/scsi_dh.h                |    5 +++++
 3 files changed, 39 insertions(+)
include/scsi/scsi_device.h +1 -0
Index: linux-2.6.31-rc1/include/scsi/scsi_device.h
===================================================================
@@ -187,6 +187,7 @@ struct scsi_device_handler {
 	void (*detach)(struct scsi_device *);
 	int (*activate)(struct scsi_device *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
+	int (*set_params)(struct scsi_device *, const char *);
 };
 
 struct scsi_dh_data {
include/scsi/scsi_dh.h +5 -0
Index: linux-2.6.31-rc1/include/scsi/scsi_dh.h
===================================================================
@@ -60,6 +60,7 @@ extern int scsi_dh_activate(struct reque
 extern int scsi_dh_handler_exist(const char *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
 extern void scsi_dh_detach(struct request_queue *);
+extern int scsi_dh_set_params(struct request_queue *, const char *);
 #else
 static inline int scsi_dh_activate(struct request_queue *req)
 {

@@ -77,4 +78,8 @@ static inline void scsi_dh_detach(struct
 {
 	return;
 }
+static inline int scsi_dh_set_params(struct request_queue *req, const char *params)
+{
+	return -SCSI_DH_NOSYS;
+}
 #endif
drivers/scsi/device_handler/scsi_dh.c +33 -0
Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh.c
===================================================================
@@ -449,6 +449,39 @@ int scsi_dh_activate(struct request_queu
 EXPORT_SYMBOL_GPL(scsi_dh_activate);
 
 /*
+ * scsi_dh_set_params - set the parameters for the device as per the
+ *      string specified in params.
+ * @q - Request queue that is associated with the scsi_device for
+ *      which the parameters to be set.
+ * @params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+int scsi_dh_set_params(struct request_queue *q, const char *params)
+{
+	int err = -SCSI_DH_NOSYS;
+	unsigned long flags;
+	struct scsi_device *sdev;
+	struct scsi_device_handler *scsi_dh = NULL;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	sdev = q->queuedata;
+	if (sdev && sdev->scsi_dh_data)
+		scsi_dh = sdev->scsi_dh_data->scsi_dh;
+	if (scsi_dh && scsi_dh->set_params && get_device(&sdev->sdev_gendev))
+		err = 0;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	if (err)
+		return err;
+	err = scsi_dh->set_params(sdev, params);
+	put_device(&sdev->sdev_gendev);
+	return err;
+}
+EXPORT_SYMBOL_GPL(scsi_dh_set_params);
+
+/*
  * scsi_dh_handler_exist - Return TRUE(1) if a device handler exists for
  *	the given name. FALSE(0) otherwise.
  * @name - name of the device handler.
author
#2 lore raw

PATCH 2/3 scsi_dh: Provide set_params interface in emc device handler

In reply to Chandra Seetharaman

Handle the parameters provided by user thru multipath.

This handler expects only 2 parameters and their value can either be 0 or 1.

This code originates from the old dm-emc.c file. Appropriate changes have been made to make it work in the new design.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
drivers/scsi/device_handler/scsi_dh_emc.c +56 -0
Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
@@ -561,6 +561,61 @@ done:
 
 	return result;
 }
+/*
+ * params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+static int clariion_set_params(struct scsi_device *sdev, const char *params)
+{
+	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	unsigned int hr = 0, st = 0, argc;
+	char *p = params;
+	int result = SCSI_DH_OK;
+
+	if ((sscanf(params, "%u", &argc) != 1) || (argc != 2))
+		return -EINVAL;
+
+	while (*p++)
+		;
+	if ((sscanf(p, "%u", &st) != 1) || (st > 1))
+		return -EINVAL;
+
+	while (*p++)
+		;
+	if ((sscanf(p, "%u", &hr) != 1) || (hr > 1))
+		return -EINVAL;
+
+	if (st)
+		csdev->flags |= CLARIION_SHORT_TRESPASS;
+	else
+		csdev->flags &= ~CLARIION_SHORT_TRESPASS;
+
+	if (hr)
+		csdev->flags |= CLARIION_HONOR_RESERVATIONS;
+	else
+		csdev->flags &= ~CLARIION_HONOR_RESERVATIONS;
+
+	/*
+	 * If this path is owned, we have to send a trespass command
+	 * with the new parameters. If not, simply return. Next trespass
+	 * command would use the parameters.
+	 */
+	if (csdev->lun_state != CLARIION_LUN_OWNED)
+		goto done;
+
+	csdev->lun_state = CLARIION_LUN_UNINITIALIZED;
+	result = send_trespass_cmd(sdev, csdev);
+	if (result != SCSI_DH_OK)
+		goto done;
+
+	/* Update status */
+	result = clariion_send_inquiry(sdev, csdev);
+
+done:
+	return result;
+}
 
 static const struct scsi_dh_devlist clariion_dev_list[] = {
 	{"DGC", "RAID"},

@@ -581,6 +636,7 @@ static struct scsi_device_handler clarii
 	.check_sense	= clariion_check_sense,
 	.activate	= clariion_activate,
 	.prep_fn	= clariion_prep_fn,
+	.set_params	= clariion_set_params,
 };
 
 /*
#3 lore raw

Re: [PATCH 2/3] scsi_dh: Provide set_params interface in emc device handler

Quoted 21 lines · Chandra Seetharaman

Handle the parameters provided by user thru multipath.

This handler expects only 2 parameters and their value can either be 0 or 1.

This code originates from the old dm-emc.c file. Appropriate changes have been made to make it work in the new design.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================

drivers/scsi/device_handler/scsi_dh_emc.c +1 -0
@@ -561,6 +561,61 @@ done:
 
 	return result;
 }
+/*

drivers/scsi/device_handler/scsi_dh_emc.c done: · line 574

Quoted 10 lines
drivers/scsi/device_handler/scsi_dh_emc.c +10 -0
+ * params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+static int clariion_set_params(struct scsi_device *sdev, const char *params)
+{
+	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	unsigned int hr = 0, st = 0, argc;
+	char *p = params;

This throws a compiler warning resolved with a type cast:

	char *p = (char *)params;
author
#4 lore raw

Re: [PATCH 2/3] scsi_dh: Provide set_params interface in emc device handler

I will reroll the patch with this fix and ported to the latest RC.

Quoted 42 lines · Eddie Williams
Quoted 21 lines · Chandra Seetharaman

Handle the parameters provided by user thru multipath.

This handler expects only 2 parameters and their value can either be 0 or 1.

This code originates from the old dm-emc.c file. Appropriate changes have been made to make it work in the new design.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================

drivers/scsi/device_handler/scsi_dh_emc.c +1 -0
@@ -561,6 +561,61 @@ done:
 
 	return result;
 }
+/*

drivers/scsi/device_handler/scsi_dh_emc.c done: · line 574

Quoted 10 lines
drivers/scsi/device_handler/scsi_dh_emc.c +10 -0
+ * params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+static int clariion_set_params(struct scsi_device *sdev, const char *params)
+{
+	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	unsigned int hr = 0, st = 0, argc;
+	char *p = params;

This throws a compiler warning resolved with a type cast:

	char *p = (char *)params;

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

#5 lore raw

Re: Re: [PATCH 2/3] scsi_dh: Provide set_params interface in emc device handler

unsubscribe linux-scsi

Quoted 55 lines · Chandra Seetharaman

I will reroll the patch with this fix and ported to the latest RC.

Quoted 4 lines · Eddie Williams
Quoted 3 lines · Chandra Seetharaman

Handle the parameters provided by user thru multipath.

This handler expects only 2 parameters and their value can either be 0

or 1.

Quoted 2 lines
Quoted 2 lines

This code originates from the old dm-emc.c file. Appropriate changes

have

Quoted 5 lines
Quoted 5 lines

been made to make it work in the new design.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56

++++++++++++++++++++++++++++++

Quoted 8 lines
Quoted 8 lines
 1 file changed, 56 insertions(+)

Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================

drivers/scsi/device_handler/scsi_dh_emc.c +0 -0
@@ -561,6 +561,61 @@ done:
Quoted 9 lines

drivers/scsi/device_handler/scsi_dh_emc.c done:

Quoted 9 lines
drivers/scsi/device_handler/scsi_dh_emc.c +7 -0
    return result;
 }
+/*
+ * params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+static int clariion_set_params(struct scsi_device *sdev, const char

*params)

Quoted 6 lines

drivers/scsi/device_handler/scsi_dh_emc.c done: · line 574

Quoted 4 lines
drivers/scsi/device_handler/scsi_dh_emc.c +4 -0
+{
+   struct clariion_dh_data *csdev = get_clariion_data(sdev);
+   unsigned int hr = 0, st = 0, argc;
+   char *p = params;

This throws a compiler warning resolved with a type cast:

Quoted 8 lines
      char *p = (char *)params;

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

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

author
#6 lore raw

PATCH 3/3 scsi_dh: Use scsi_dh_set_params() in multipath.

In reply to Chandra Seetharaman

Use scsi_dh_set_params() set parameters provided. Save the parameters in parse_hw_handler() and use it in parse_path().

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/md/dm-mpath.c |   42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)
drivers/md/dm-mpath.c +36 -6
Index: linux-2.6.31-rc1/drivers/md/dm-mpath.c
===================================================================
@@ -64,6 +64,7 @@ struct multipath {
 	spinlock_t lock;
 
 	const char *hw_handler_name;
+	char *hw_handler_params;
 	unsigned nr_priority_groups;
 	struct list_head priority_groups;
 	unsigned pg_init_required;	/* pg_init needs calling? */

@@ -219,6 +220,7 @@ static void free_multipath(struct multip
 	}
 
 	kfree(m->hw_handler_name);
+	kfree(m->hw_handler_params);
 	mempool_destroy(m->mpio_pool);
 	kfree(m);
 }

@@ -615,6 +617,17 @@ static struct pgpath *parse_path(struct
 			dm_put_device(ti, p->path.dev);
 			goto bad;
 		}
+
+		if (m->hw_handler_params) {
+			r = scsi_dh_set_params(q, m->hw_handler_params);
+			if (r < 0) {
+				ti->error = "unable to set hardware "
+							"handler parameters";
+				scsi_dh_detach(q);
+				dm_put_device(ti, p->path.dev);
+				goto bad;
+			}
+		}
 	}
 
 	r = ps->type->add_path(ps, &p->path, as->argc, as->argv, &ti->error);

@@ -705,6 +718,7 @@ static struct priority_group *parse_prio
 static int parse_hw_handler(struct arg_set *as, struct multipath *m)
 {
 	unsigned hw_argc;
+	int ret;
 	struct dm_target *ti = m->ti;
 
 	static struct param _params[] = {

@@ -726,17 +740,33 @@ static int parse_hw_handler(struct arg_s
 	request_module("scsi_dh_%s", m->hw_handler_name);
 	if (scsi_dh_handler_exist(m->hw_handler_name) == 0) {
 		ti->error = "unknown hardware handler type";
-		kfree(m->hw_handler_name);
-		m->hw_handler_name = NULL;
-		return -EINVAL;
+		ret = -EINVAL;
+		goto fail;
 	}
 
-	if (hw_argc > 1)
-		DMWARN("Ignoring user-specified arguments for "
-		       "hardware handler \"%s\"", m->hw_handler_name);
+	if (hw_argc > 1) {
+		char *p;
+		int i, j, len = 4;
+
+		for (i = 0; i <= hw_argc - 2; i++)
+			len += strlen(as->argv[i]) + 1;
+		p = m->hw_handler_params = kzalloc(len, GFP_KERNEL);
+		if (!p) {
+			ti->error = "memory allocation failed";
+			ret = -ENOMEM;
+			goto fail;
+		}
+		j = sprintf(p, "%d", hw_argc - 1);
+		for (i = 0, p+=j+1; i <= hw_argc - 2; i++, p+=j+1)
+			j = sprintf(p, "%s", as->argv[i]);
+	}
 	consume(as, hw_argc - 1);
 
 	return 0;
+fail:
+	kfree(m->hw_handler_name);
+	m->hw_handler_name = NULL;
+	return ret;
 }
 
 static int parse_features(struct arg_set *as, struct multipath *m)
#7 lore raw
In reply to Chandra Seetharaman

Yes, hopefully this afternoon (I head to the doctor in a few minutes - back pain). If I can not do it today then I wont be able to do it until Tuesday (July 7th) as I will be on vacation for a long weekend.

Eddie

Quoted 28 lines · Chandra Seetharaman

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Thanks,

chandra

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

#8 lore raw
In reply to Chandra Seetharaman

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

Here is the data:

Jul  2 14:57:57 bullwinkle kernel: kernel BUG at
block/cfq-iosched.c:2273!
Jul  2 14:57:57 bullwinkle kernel: invalid opcode: 0000 [#1] SMP 
Jul  2 14:57:57 bullwinkle kernel: last sysfs
file: /sys/kernel/uevent_seqnum
Jul  2 14:57:57 bullwinkle kernel: CPU 1 
Jul  2 14:57:57 bullwinkle kernel: Modules linked in: scsi_dh_emc

dm_round_robin dm_multipath scsi_dh microcode fuse loop dm_mod i5k_amb iTCO_wdt iTCO_vendor_support i2c_i801 ibmpex i5000_edac ses rtc_cmos ibmaem bnx2 ipmi_msghandler sr_mod rtc_core enclosure i2c_core pcspkr edac_core cdrom serio_raw rtc_lib joydev shpchp pci_hotplug button sg usbhid hid uhci_hcd ehci_hcd usbcore sd_mod crc_t10dif edd ext3 mbcache jbd fan ide_pci_generic ide_core ata_generic ata_piix libata lpfc scsi_transport_fc scsi_tgt aacraid scsi_mod thermal processor thermal_sys hwmon

Jul  2 14:57:57 bullwinkle kernel: Pid: 4509, comm: kmpath_handlerd
Tainted: G        W  2.6.31-rc1-0.1-default #1 IBM System x3650
-[7979A2U]-
Jul  2 14:57:57 bullwinkle kernel: RIP: 0010:[<ffffffff81174b36>]
[<ffffffff81174b36>] cfq_put_request+0x29/0x67
Jul  2 14:57:57 bullwinkle kernel: RSP: 0018:ffff8804a5ce1c70  EFLAGS:
00010046
Jul  2 14:57:57 bullwinkle kernel: RAX: 0000000000000000 RBX:
ffff88049ec891f0 RCX: 0000000000000579
Jul  2 14:57:57 bullwinkle kernel: RDX: 0000000000000019 RSI:
ffff88049ec891f0 RDI: ffff88049ec891f0
Jul  2 14:57:57 bullwinkle kernel: RBP: ffff8804a5ce1c80 R08:
ffff8804a5ce0000 R09: 0000000000000000
Jul  2 14:57:57 bullwinkle kernel: R10: ffffffffa002bb0d R11:
0000000000000000 R12: ffff8804a356a5d0
Jul  2 14:57:57 bullwinkle kernel: R13: ffff8804a54d6b08 R14:
ffff8804a5052400 R15: ffff8804a5052400
Jul  2 14:57:57 bullwinkle kernel: FS:  0000000000000000(0000)
GS:ffff880028050000(0000) knlGS:0000000000000000
Jul  2 14:57:57 bullwinkle kernel: CS:  0010 DS: 0018 ES: 0018 CR0:
000000008005003b
Jul  2 14:57:57 bullwinkle kernel: CR2: 0000000000617b48 CR3:
0000000492c72000 CR4: 00000000000006e0
Jul  2 14:57:57 bullwinkle kernel: DR0: 0000000000000000 DR1:
0000000000000000 DR2: 0000000000000000
Jul  2 14:57:57 bullwinkle kernel: DR3: 0000000000000000 DR6:
00000000ffff0ff0 DR7: 0000000000000400
Jul  2 14:57:57 bullwinkle kernel: Process kmpath_handlerd (pid: 4509,
threadinfo ffff8804a5ce0000, task ffff8804a4d2a100)
Jul  2 14:57:57 bullwinkle kernel: Stack:
Jul  2 14:57:57 bullwinkle kernel:  ffff88049ec891f0 0000000001282c4f
ffff8804a5ce1c90 ffffffff811615f7
Jul  2 14:57:57 bullwinkle kernel: <0> ffff8804a5ce1cc0 ffffffff81166d45
0000000000000055 ffff8804a54d6b08
Jul  2 14:57:57 bullwinkle kernel: <0> ffff88049ec891f0 0000000000000282
ffff8804a5ce1cf0 ffffffff81167272
Jul  2 14:57:57 bullwinkle kernel: Call Trace:
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff811615f7>] elv_put_request
+0x19/0x1b
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff81166d45>]
__blk_put_request+0x87/0xbd
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff81167272>] blk_put_request
+0x2e/0x45
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02dd52a>]
send_trespass_cmd+0x250/0x262 [scsi_dh_emc]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02dd573>] ?
clariion_send_inquiry+0x37/0x200 [scsi_dh_emc]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02dd88e>]
clariion_activate+0x4d/0x131 [scsi_dh_emc]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02b37e4>]
scsi_dh_activate+0x87/0xa4 [scsi_dh]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02d81d3>] activate_path
+0x32/0x151 [dm_multipath]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff81054424>] worker_thread
+0x172/0x20c
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffffa02d81a1>] ? activate_path
+0x0/0x151 [dm_multipath]
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff81058774>] ?
autoremove_wake_function+0x0/0x38
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff810542b2>] ? worker_thread
+0x0/0x20c
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff81058406>] kthread
+0x88/0x90
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff8100ca3a>] child_rip
+0xa/0x20
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff8105837e>] ? kthread
+0x0/0x90
Jul  2 14:57:57 bullwinkle kernel:  [<ffffffff8100ca30>] ? child_rip
+0x0/0x20
Jul  2 14:57:57 bullwinkle kernel: Code: c9 c3 55 48 89 e5 41 54 53 4c
8b a7 a8 00 00 00 48 89 fb 4d 85 e4 74 4c 8b 47 48 83 e0 01 48 8d 50 18
41 8b 44 94 08 85 c0 75 04 <0f> 0b eb fe ff c8 41 89 44 94 08 48 8b 87
a0 00 00 00 48 8b 78 
Jul  2 14:57:57 bullwinkle kernel: RIP  [<ffffffff81174b36>]
cfq_put_request+0x29/0x67
Jul  2 14:57:57 bullwinkle kernel:  RSP <ffff8804a5ce1c70>
Jul  2 14:57:57 bullwinkle kernel: ---[ end trace 4eaa2a86a8e2da24 ]---
Quoted 28 lines · Chandra Seetharaman

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Thanks,

chandra

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

#9 lore raw
Quoted 8 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

I backed out the 3 patches with the same panic.

author
#10 lore raw
Quoted 4 lines · Eddie Williams
Quoted 3 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

Quoted 8 lines
Quoted 5 lines

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

#11 lore raw

I was testing with the SLES 11 kernel. Initially I was using 2.6.27.19-5 and also verified the same issue with their errata kernel 2.6.27.23-0.1.

Eddie

Quoted 16 lines · Chandra Seetharaman
Quoted 4 lines · Eddie Williams
Quoted 3 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

Quoted 8 lines
Quoted 5 lines

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

author
#12 lore raw

Did you try to port/apply my patches to the SLES11 tree and see if it works ?

Quoted 27 lines · Eddie Williams

I was testing with the SLES 11 kernel. Initially I was using 2.6.27.19-5 and also verified the same issue with their errata kernel 2.6.27.23-0.1.

Eddie

Quoted 11 lines · Chandra Seetharaman
Quoted 10 lines · Eddie Williams
Quoted 8 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

Quoted 5 lines
Quoted 2 lines

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

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

#13 lore raw

I have not tried to patch the 2.6.27-23 kernel. Given that the patches were against 31-RC1 I assumed that there would be other changes I would need to pull in. I have been looking at why the 31-RC1 was panicing as well as working with my EMC contacts to see if they already had a working 31-RC1 environment they could verify the patch in. Unfortunately I have been mostly in meetings...

I will give it a try. I should have results in the morning.

Eddie

Quoted 32 lines · Chandra Seetharaman

Did you try to port/apply my patches to the SLES11 tree and see if it works ?

Quoted 27 lines · Eddie Williams

I was testing with the SLES 11 kernel. Initially I was using 2.6.27.19-5 and also verified the same issue with their errata kernel 2.6.27.23-0.1.

Eddie

Quoted 11 lines · Chandra Seetharaman
Quoted 10 lines · Eddie Williams
Quoted 8 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

Quoted 5 lines
Quoted 2 lines

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

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

author
#14 lore raw

Eddie,

Here is the set of patches ported to sles11.

It applies cleanly on linux-2.6.27.23-0.1

Anyways, you have to test the mainline version for it to be pushed upstream. (this patch is just for verification purposes).

series:
sles11_scsi_dh_params
sles11_emc_parameters
sles11_parameters_fix_for_dmmpath

chandra

Quoted 45 lines · Eddie Williams

I have not tried to patch the 2.6.27-23 kernel. Given that the patches were against 31-RC1 I assumed that there would be other changes I would need to pull in. I have been looking at why the 31-RC1 was panicing as well as working with my EMC contacts to see if they already had a working 31-RC1 environment they could verify the patch in. Unfortunately I have been mostly in meetings...

I will give it a try. I should have results in the morning.

Eddie

Quoted 32 lines · Chandra Seetharaman

Did you try to port/apply my patches to the SLES11 tree and see if it works ?

Quoted 27 lines · Eddie Williams

I was testing with the SLES 11 kernel. Initially I was using 2.6.27.19-5 and also verified the same issue with their errata kernel 2.6.27.23-0.1.

Eddie

Quoted 11 lines · Chandra Seetharaman
Quoted 10 lines · Eddie Williams
Quoted 8 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

Quoted 5 lines
Quoted 2 lines

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

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

Quoted 4 lines

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Handle the parameters provided by user thru multipath.

This handler expects only 2 parameters and their value can either be 0 or 1.

This code originates from the old dm-emc.c file. Appropriate changes have been made to make it work in the new design.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
drivers/scsi/device_handler/scsi_dh_emc.c +56 -0
Index: linux/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
@@ -563,6 +563,61 @@ done:
 
 	return result;
 }
+/*
+ * params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+static int clariion_set_params(struct scsi_device *sdev, const char *params)
+{
+	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	unsigned int hr = 0, st = 0, argc;
+	char *p = params;
+	int result = SCSI_DH_OK;
+
+	if ((sscanf(params, "%u", &argc) != 1) || (argc != 2))
+		return -EINVAL;
+
+	while (*p++)
+		;
+	if ((sscanf(p, "%u", &st) != 1) || (st > 1))
+		return -EINVAL;
+
+	while (*p++)
+		;
+	if ((sscanf(p, "%u", &hr) != 1) || (hr > 1))
+		return -EINVAL;
+
+	if (st)
+		csdev->flags |= CLARIION_SHORT_TRESPASS;
+	else
+		csdev->flags &= ~CLARIION_SHORT_TRESPASS;
+
+	if (hr)
+		csdev->flags |= CLARIION_HONOR_RESERVATIONS;
+	else
+		csdev->flags &= ~CLARIION_HONOR_RESERVATIONS;
+
+	/*
+	 * If this path is owned, we have to send a trespass command
+	 * with the new parameters. If not, simply return. Next trespass
+	 * command would use the parameters.
+	 */
+	if (csdev->lun_state != CLARIION_LUN_OWNED)
+		goto done;
+
+	csdev->lun_state = CLARIION_LUN_UNINITIALIZED;
+	result = send_trespass_cmd(sdev, csdev);
+	if (result != SCSI_DH_OK)
+		goto done;
+
+	/* Update status */
+	result = clariion_send_inquiry(sdev, csdev);
+
+done:
+	return result;
+}
 
 static const struct scsi_dh_devlist clariion_dev_list[] = {
 	{"DGC", "RAID", 0},

@@ -583,6 +638,7 @@ static struct scsi_device_handler clarii
 	.check_sense	= clariion_check_sense,
 	.activate	= clariion_activate,
 	.prep_fn	= clariion_prep_fn,
+	.set_params	= clariion_set_params,
 };
 
 /*

Use scsi_dh_set_params() set parameters provided. Save the parameters in parse_hw_handler() and use it in parse_path().

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/md/dm-mpath.c |   40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)
drivers/md/dm-mpath.c +37 -3
Index: linux/drivers/md/dm-mpath.c
===================================================================
@@ -63,6 +63,7 @@ struct multipath {
 	spinlock_t lock;
 
 	const char *hw_handler_name;
+	char *hw_handler_params;
 	unsigned nr_priority_groups;
 	struct list_head priority_groups;
 	unsigned pg_init_required;	/* pg_init needs calling? */

@@ -216,6 +217,7 @@ static void free_multipath(struct multip
 	}
 
 	kfree(m->hw_handler_name);
+	kfree(m->hw_handler_params);
 	mempool_destroy(m->mpio_pool);
 	kfree(m);
 }

@@ -645,6 +647,17 @@ static struct pgpath *parse_path(struct
 				goto bad;
 			}
 		}
+
+		if (m->hw_handler_params) {
+			r = scsi_dh_set_params(q, m->hw_handler_params);
+			if (r < 0) {
+				ti->error = "unable to set hardware "
+							"handler parameters";
+				scsi_dh_detach(q);
+				dm_put_device(ti, p->path.dev);
+				goto bad;
+			}
+		}
 	}
 
 	r = ps->type->add_path(ps, &p->path, as->argc, as->argv, &ti->error);

@@ -738,6 +751,7 @@ static struct priority_group *parse_prio
 static int parse_hw_handler(struct arg_set *as, struct multipath *m)
 {
 	unsigned hw_argc;
+	int ret;
 	struct dm_target *ti = m->ti;
 
 	static struct param _params[] = {

@@ -754,13 +768,33 @@ static int parse_hw_handler(struct arg_s
 	request_module("scsi_dh_%s", m->hw_handler_name);
 	if (scsi_dh_handler_exist(m->hw_handler_name) == 0) {
 		ti->error = "unknown hardware handler type";
-		kfree(m->hw_handler_name);
-		m->hw_handler_name = NULL;
-		return -EINVAL;
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	if (hw_argc > 1) {
+		char *p;
+		int i, j, len = 4;
+
+		for (i = 0; i <= hw_argc - 2; i++)
+			len += strlen(as->argv[i]) + 1;
+		p = m->hw_handler_params = kzalloc(len, GFP_KERNEL);
+		if (!p) {
+			ti->error = "memory allocation failed";
+			ret = -ENOMEM;
+			goto fail;
+		}
+		j = sprintf(p, "%d", hw_argc - 1);
+		for (i = 0, p+=j+1; i <= hw_argc - 2; i++, p+=j+1)
+			j = sprintf(p, "%s", as->argv[i]);
 	}
 	consume(as, hw_argc - 1);
 
 	return 0;
+fail:
+	kfree(m->hw_handler_name);
+	m->hw_handler_name = NULL;
+	return ret;
 }
 
 static int parse_features(struct arg_set *as, struct multipath *m)

When we moved the device handler functionality from dm layer to SCSI layer we dropped the paramaeter functionality.

This path adds an interface to scsi dh layer to set device handler parameters.

Basically, multipath layer need to create a string with all the parameters and call scsi_dh_set_params() after it called scsi_dh_attach() on a device.

If a device handler provides such an interface it will handle the parameters as it expects them.

Signed-off-by Chandra Seetharaman <[email protected]>
---
 drivers/scsi/device_handler/scsi_dh.c |   33 +++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h            |    1 +
 include/scsi/scsi_dh.h                |    5 +++++
 3 files changed, 39 insertions(+)
include/scsi/scsi_device.h +1 -0
Index: linux/include/scsi/scsi_device.h
===================================================================
@@ -189,6 +189,7 @@ struct scsi_device_handler {
 	void (*detach)(struct scsi_device *);
 	int (*activate)(struct scsi_device *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
+	int (*set_params)(struct scsi_device *, const char *);
 };
 
 struct scsi_dh_data {
include/scsi/scsi_dh.h +5 -0
Index: linux/include/scsi/scsi_dh.h
===================================================================
@@ -60,6 +60,7 @@ extern int scsi_dh_activate(struct reque
 extern int scsi_dh_handler_exist(const char *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
 extern void scsi_dh_detach(struct request_queue *);
+extern int scsi_dh_set_params(struct request_queue *, const char *);
 #else
 static inline int scsi_dh_activate(struct request_queue *req)
 {

@@ -77,4 +78,8 @@ static inline void scsi_dh_detach(struct
 {
 	return;
 }
+static inline int scsi_dh_set_params(struct request_queue *req, const char *params)
+{
+	return -SCSI_DH_NOSYS;
+}
 #endif
drivers/scsi/device_handler/scsi_dh.c +33 -0
Index: linux/drivers/scsi/device_handler/scsi_dh.c
===================================================================
@@ -445,6 +445,39 @@ int scsi_dh_activate(struct request_queu
 EXPORT_SYMBOL_GPL(scsi_dh_activate);
 
 /*
+ * scsi_dh_set_params - set the parameters for the device as per the
+ *      string specified in params.
+ * @q - Request queue that is associated with the scsi_device for
+ *      which the parameters to be set.
+ * @params - parameters in the following format
+ *      "no_of_params\0param1\0param2\0param3\0...\0"
+ *      for example, string for 2 parameters with value 10 and 21
+ *      is specified as "2\010\021\0".
+ */
+int scsi_dh_set_params(struct request_queue *q, const char *params)
+{
+	int err = -SCSI_DH_NOSYS;
+	unsigned long flags;
+	struct scsi_device *sdev;
+	struct scsi_device_handler *scsi_dh = NULL;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	sdev = q->queuedata;
+	if (sdev && sdev->scsi_dh_data)
+		scsi_dh = sdev->scsi_dh_data->scsi_dh;
+	if (scsi_dh && scsi_dh->set_params && get_device(&sdev->sdev_gendev))
+		err = 0;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	if (err)
+		return err;
+	err = scsi_dh->set_params(sdev, params);
+	put_device(&sdev->sdev_gendev);
+	return err;
+}
+EXPORT_SYMBOL_GPL(scsi_dh_set_params);
+
+/*
  * scsi_dh_handler_exist - Return TRUE(1) if a device handler exists for
  *	the given name. FALSE(0) otherwise.
  * @name - name of the device handler.
#15 lore raw

I had taken the patches you sent earlier and tried them on the SLES 11. There were a couple of collisions but not much. I am able to set the parameters in hardware handler with the patches. This also now allows multipath to handle path failures correctly where reservations are involved, aka honor reservations set on trespass command. I will do more testing but so far so good.

I made one type cast change in scsi_dh_emc.c to avoid a warning

	char *p = (char *)params;

I will look at your patches to make sure I reconciled the conflicts correctly.

Eddie

Quoted 65 lines · Chandra Seetharaman

Eddie,

Here is the set of patches ported to sles11.

It applies cleanly on linux-2.6.27.23-0.1

Anyways, you have to test the mainline version for it to be pushed upstream. (this patch is just for verification purposes).

series:
sles11_scsi_dh_params
sles11_emc_parameters
sles11_parameters_fix_for_dmmpath

chandra

Quoted 49 lines · Eddie Williams

I have not tried to patch the 2.6.27-23 kernel. Given that the patches were against 31-RC1 I assumed that there would be other changes I would need to pull in. I have been looking at why the 31-RC1 was panicing as well as working with my EMC contacts to see if they already had a working 31-RC1 environment they could verify the patch in. Unfortunately I have been mostly in meetings...

I will give it a try. I should have results in the morning.

Eddie

Quoted 32 lines · Chandra Seetharaman

Did you try to port/apply my patches to the SLES11 tree and see if it works ?

Quoted 27 lines · Eddie Williams

I was testing with the SLES 11 kernel. Initially I was using 2.6.27.19-5 and also verified the same issue with their errata kernel 2.6.27.23-0.1.

Eddie

Quoted 11 lines · Chandra Seetharaman
Quoted 10 lines · Eddie Williams
Quoted 8 lines · Eddie Williams

I have tried to set this up but have run into a problem probably due to my error. It has been a while since I have had to build kernels...

I pulled 2.6.31-rc1, applied the two patches below and then applied the 3 patches for the interface. The new kernel loads fine but when a trespass command is sent I get an panic. I am looking though how I built the kernel and perhaps build with your 3 patches to see if it happened before.

Quoted 5 lines
Quoted 2 lines

I backed out the 3 patches with the same panic.

Which version of the kernel were you originally testing with ? (when you found the feature was gone).

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

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

#16 lore raw
In reply to Chandra Seetharaman

Your patch set works well but the scsi_dh_emc.c in rc1 does not. I applied your patches to the SLES 11 (2.6.27.23) and I am able to set the parameter with reservations being handled correctly. As noted in an earlier email when I tested the 2.6.31-rc1 kernel with and without your patches I would see a panic. I compared the scsi_dh_emc.c in rc1 with SLES 11. There are a couple of differences. The panic is resolved with the changes from SLES 11. I am not sure if these are changes made to SLES 11 and not made to the upstream kernel or whether there was a patch to the upstream kernel after 2.6.27 that broke this. Here are the changes when applied to rc1 that resolves the panic and allowed me to test your patches.

*** linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c

2009-07-20 08:37:14.000000000 -0400
--- linux-2.6.31-rc1.0.3/drivers/scsi/device_handler/scsi_dh_emc.c
2009-07-20 08:42:36.000000000 -0400

***************
*** 176,182 ****
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
--- 176,182 ----
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] < 0 || csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
***************
*** 272,278 ****
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
--- 272,278 ----
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd != INQUIRY) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
***************
*** 286,299 ****
--- 286,302 ----
len = sizeof(short_trespass);
rq->cmd_flags |= REQ_RW;
rq->cmd[1] = 0x10;
+ rq->cmd[4] = len;
break;
case MODE_SELECT_10:
len = sizeof(long_trespass);
rq->cmd_flags |= REQ_RW;
rq->cmd[1] = 0x10;
+ rq->cmd[8] = len;
break;
case INQUIRY:
len = CLARIION_BUFFER_SIZE;
+ rq->cmd[4] = len;
memset(buffer, 0, len);
break;
default:
***************
*** 301,307 ****
break;
}

- rq->cmd[4] = len;
rq->cmd_type = REQ_TYPE_BLOCK_PC;
rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
REQ_FAILFAST_DRIVER;
--- 304,309 ----

Quoted 28 lines · Chandra Seetharaman

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Thanks,

chandra

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

author
#17 lore raw

Hannes would know about the origin of this.

If the patches are working as expected, can you send a "Tested-by" so that James can accept this patch.

Thanks.

chandra

Quoted 115 lines · Eddie Williams

Your patch set works well but the scsi_dh_emc.c in rc1 does not. I applied your patches to the SLES 11 (2.6.27.23) and I am able to set the parameter with reservations being handled correctly. As noted in an earlier email when I tested the 2.6.31-rc1 kernel with and without your patches I would see a panic. I compared the scsi_dh_emc.c in rc1 with SLES 11. There are a couple of differences. The panic is resolved with the changes from SLES 11. I am not sure if these are changes made to SLES 11 and not made to the upstream kernel or whether there was a patch to the upstream kernel after 2.6.27 that broke this. Here are the changes when applied to rc1 that resolves the panic and allowed me to test your patches.

*** linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c

2009-07-20 08:37:14.000000000 -0400
--- linux-2.6.31-rc1.0.3/drivers/scsi/device_handler/scsi_dh_emc.c
2009-07-20 08:42:36.000000000 -0400

***************
*** 176,182 ****
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
--- 176,182 ----
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] < 0 || csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
***************
*** 272,278 ****
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
--- 272,278 ----
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd != INQUIRY) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
***************
*** 286,299 ****
--- 286,302 ----
len = sizeof(short_trespass);
rq->cmd_flags |= REQ_RW;
rq->cmd[1] = 0x10;

(quoted diff) +3 -0
+ 		rq->cmd[4] = len;
  		break;
  	case MODE_SELECT_10:
  		len = sizeof(long_trespass);
  		rq->cmd_flags |= REQ_RW;
  		rq->cmd[1] = 0x10;
+ 		rq->cmd[8] = len;
  		break;
  	case INQUIRY:
  		len = CLARIION_BUFFER_SIZE;
+ 		rq->cmd[4] = len;
  		memset(buffer, 0, len);
  		break;
  	default:

***************
*** 301,307 ****
break;
}

(quoted diff) +0 -2
- 	rq->cmd[4] = len;
  	rq->cmd_type = REQ_TYPE_BLOCK_PC;
  	rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
  			 REQ_FAILFAST_DRIVER;
--- 304,309 ----
Quoted 20 lines · Chandra Seetharaman

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Quoted 8 lines

Thanks,

chandra

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

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

Tested-by
#18 lore raw
Tested-by Eddie Williams <[email protected]>
Quoted 125 lines · Chandra Seetharaman

Hannes would know about the origin of this.

If the patches are working as expected, can you send a "Tested-by" so that James can accept this patch.

Thanks.

chandra

Quoted 115 lines · Eddie Williams

Your patch set works well but the scsi_dh_emc.c in rc1 does not. I applied your patches to the SLES 11 (2.6.27.23) and I am able to set the parameter with reservations being handled correctly. As noted in an earlier email when I tested the 2.6.31-rc1 kernel with and without your patches I would see a panic. I compared the scsi_dh_emc.c in rc1 with SLES 11. There are a couple of differences. The panic is resolved with the changes from SLES 11. I am not sure if these are changes made to SLES 11 and not made to the upstream kernel or whether there was a patch to the upstream kernel after 2.6.27 that broke this. Here are the changes when applied to rc1 that resolves the panic and allowed me to test your patches.

*** linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh_emc.c

2009-07-20 08:37:14.000000000 -0400
--- linux-2.6.31-rc1.0.3/drivers/scsi/device_handler/scsi_dh_emc.c
2009-07-20 08:42:36.000000000 -0400

***************
*** 176,182 ****
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
--- 176,182 ----
err = SCSI_DH_DEV_TEMP_BUSY;
goto out;
}
! if (csdev->buffer[4] < 0 || csdev->buffer[4] > 2) {
/* Invalid buffer format */ sdev_printk(KERN_NOTICE, sdev,
"%s: invalid VPD page 0xC0 format\n",
***************
*** 272,278 ****
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
--- 272,278 ----
int len = 0;

rq = blk_get_request(sdev->request_queue,
! (cmd != INQUIRY) ? WRITE : READ, GFP_NOIO);
if (!rq) {
sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed"); return NULL;
***************
*** 286,299 ****
--- 286,302 ----
len = sizeof(short_trespass);
rq->cmd_flags |= REQ_RW;
rq->cmd[1] = 0x10;

(quoted diff) +3 -0
+ 		rq->cmd[4] = len;
  		break;
  	case MODE_SELECT_10:
  		len = sizeof(long_trespass);
  		rq->cmd_flags |= REQ_RW;
  		rq->cmd[1] = 0x10;
+ 		rq->cmd[8] = len;
  		break;
  	case INQUIRY:
  		len = CLARIION_BUFFER_SIZE;
+ 		rq->cmd[4] = len;
  		memset(buffer, 0, len);
  		break;
  	default:

***************
*** 301,307 ****
break;
}

(quoted diff) +0 -2
- 	rq->cmd[4] = len;
  	rq->cmd_type = REQ_TYPE_BLOCK_PC;
  	rq->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
  			 REQ_FAILFAST_DRIVER;
--- 304,309 ----
Quoted 20 lines · Chandra Seetharaman

Hello,

Last week, Eddie Williams reported a regression w.r.t scsi_dh
(http://www.redhat.com/archives/dm-devel/2009-June/msg00285.html).
Parameter setting part of dm multipath handler has been removed when we moved to scsi device handler.

This set of patches adds an new interface to scsi_dh and uses it.

This patch set applies on 2.6.31-rc1 and the following 2 patches:

	(1) http://patchwork.kernel.org/patch/32646/raw/
	(2) http://patchwork.kernel.org/patch/32647/raw/

This interface part of the patch has been tested.

Eddie,

If you can test it on an EMC storage and respond with your findings it will be of great help.

Quoted 8 lines

Thanks,

chandra

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

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