[PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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

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

[Chandra Seetharaman]

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 <sekharan@us.ibm.com>
---
 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(+)

Index: linux-2.6.31-rc1/include/scsi/scsi_device.h
===================================================================
--- linux-2.6.31-rc1.orig/include/scsi/scsi_device.h
+++ 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 {
Index: linux-2.6.31-rc1/include/scsi/scsi_dh.h
===================================================================
--- linux-2.6.31-rc1.orig/include/scsi/scsi_dh.h
+++ 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
Index: linux-2.6.31-rc1/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh.c
+++ 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.

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

[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 <sekharan@us.ibm.com>
---
 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
===================================================================
--- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh_emc.c
+++ 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,
 };
 
 /*

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

[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 <sekharan@us.ibm.com>
---
 drivers/md/dm-mpath.c |   42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

Index: linux-2.6.31-rc1/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.31-rc1.orig/drivers/md/dm-mpath.c
+++ 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)

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Eddie Williams]

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
On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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.  

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 ]---

On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Eddie Williams]

On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> 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.

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Chandra Seetharaman]

On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > 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.
> 
Which version of the kernel were you originally testing with ? (when you
found the feature was gone).

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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
On Thu, 2009-07-02 at 13:47 -0700, Chandra Seetharaman wrote:
> On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> > On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > > 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.
> > 
> Which version of the kernel were you originally testing with ? (when you
> found the feature was gone).
> 

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Chandra Seetharaman]

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

On Tue, 2009-07-07 at 08:34 -0400, Eddie Williams wrote:
> 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
> On Thu, 2009-07-02 at 13:47 -0700, Chandra Seetharaman wrote:
> > On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> > > On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > > > 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.
> > > 
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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

On Thu, 2009-07-09 at 12:55 -0700, Chandra Seetharaman wrote:
> Did you try to port/apply my patches to the SLES11 tree and see if it
> works ?
> 
> On Tue, 2009-07-07 at 08:34 -0400, Eddie Williams wrote:
> > 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
> > On Thu, 2009-07-02 at 13:47 -0700, Chandra Seetharaman wrote:
> > > On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> > > > On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > > > > 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.
> > > > 
> > > 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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [dm-devel] Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Chandra Seetharaman]

[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

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
On Thu, 2009-07-09 at 17:16 -0400, Eddie Williams wrote:
> 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
> 
> On Thu, 2009-07-09 at 12:55 -0700, Chandra Seetharaman wrote:
> > Did you try to port/apply my patches to the SLES11 tree and see if it
> > works ?
> > 
> > On Tue, 2009-07-07 at 08:34 -0400, Eddie Williams wrote:
> > > 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
> > > On Thu, 2009-07-02 at 13:47 -0700, Chandra Seetharaman wrote:
> > > > On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> > > > > On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > > > > > 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.
> > > > > 
> > > > 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 majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

[-- Attachment #2: sles11_emc_parameters --]
[-- Type: text/plain, Size: 2408 bytes --]

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 <sekharan@us.ibm.com>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |   56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

Index: linux/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
--- linux.orig/drivers/scsi/device_handler/scsi_dh_emc.c
+++ 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,
 };
 
 /*

[-- Attachment #3: sles11_parameters_fix_for_dmmpath --]
[-- Type: text/plain, Size: 2503 bytes --]

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 <sekharan@us.ibm.com>
---
 drivers/md/dm-mpath.c |   40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

Index: linux/drivers/md/dm-mpath.c
===================================================================
--- linux.orig/drivers/md/dm-mpath.c
+++ 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)

[-- Attachment #4: sles11_scsi_dh_params --]
[-- Type: text/plain, Size: 3482 bytes --]

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 <sekharan@us.ibm.com>
---
 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(+)

Index: linux/include/scsi/scsi_device.h
===================================================================
--- linux.orig/include/scsi/scsi_device.h
+++ 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 {
Index: linux/include/scsi/scsi_dh.h
===================================================================
--- linux.orig/include/scsi/scsi_dh.h
+++ 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
Index: linux/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux.orig/drivers/scsi/device_handler/scsi_dh.c
+++ 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.

Re: [dm-devel] Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Eddie Williams]

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

On Thu, 2009-07-09 at 18:59 -0700, Chandra Seetharaman wrote:
> 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
> On Thu, 2009-07-09 at 17:16 -0400, Eddie Williams wrote:
> > 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
> > 
> > On Thu, 2009-07-09 at 12:55 -0700, Chandra Seetharaman wrote:
> > > Did you try to port/apply my patches to the SLES11 tree and see if it
> > > works ?
> > > 
> > > On Tue, 2009-07-07 at 08:34 -0400, Eddie Williams wrote:
> > > > 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
> > > > On Thu, 2009-07-02 at 13:47 -0700, Chandra Seetharaman wrote:
> > > > > On Thu, 2009-07-02 at 16:29 -0400, Eddie Williams wrote:
> > > > > > On Thu, 2009-07-02 at 15:34 -0400, Eddie Williams wrote:
> > > > > > > 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.
> > > > > > 
> > > > > 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 majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel

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

[Eddie Williams]

On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> 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 <sekharan@us.ibm.com>
> ---
>  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
> ===================================================================
> --- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh_emc.c
> +++ 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;

This throws a compiler warning resolved with a type cast:
	char *p = (char *)params;

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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;
+ 		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 ----

On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[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
On Mon, 2009-07-20 at 09:05 -0400, Eddie Williams wrote:
> 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 ----
> 
> On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> > 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 majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chandra Seetharaman]

I will reroll the patch with this fix and ported to the latest RC.
On Mon, 2009-07-20 at 09:00 -0400, Eddie Williams wrote:
> On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> > 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 <sekharan@us.ibm.com>
> > ---
> >  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
> > ===================================================================
> > --- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh_emc.c
> > +++ 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;
> 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Asim Zia]

[-- Attachment #1.1: Type: text/plain, Size: 2095 bytes --]

unsubscribe linux-scsi





On Tue, Jul 21, 2009 at 6:02 AM, Chandra Seetharaman <sekharan@us.ibm.com>wrote:

>
> I will reroll the patch with this fix and ported to the latest RC.
>  On Mon, 2009-07-20 at 09:00 -0400, Eddie Williams wrote:
> > On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> > > 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 <sekharan@us.ibm.com>
> > > ---
> > >  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
> > > ===================================================================
> > > --- linux-2.6.31-rc1.orig/drivers/scsi/device_handler/scsi_dh_emc.c
> > > +++ 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;
> >
> > 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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 3088 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/3] scsi_dh: Add ability to set parameters for scsi device handler

[Eddie Williams]

Tested-by: Eddie Williams <Eddie.Williams@steeleye.com>


On Mon, 2009-07-20 at 18:01 -0700, Chandra Seetharaman wrote:
> 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
> On Mon, 2009-07-20 at 09:05 -0400, Eddie Williams wrote:
> > 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 ----
> > 
> > On Wed, 2009-07-01 at 20:13 -0700, Chandra Seetharaman wrote:
> > > 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 majordomo@vger.kernel.org
> > > 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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>