replace dm hw handlers with scsi handlers

This is the same patch (as far as approach and design goes) as before with some modifications and updates to Jen's cmd_type code and I tried converting Ed's newer EMC code which is a lot more complicated (sends multiple commands for failover) to make sure it looks like it could work. Ed' does this look ok and will work for all that you need it to. I converted your code pretty closely. There were some bugs in the patch posted to dm-devel where you called dm_pg_init_compelete multiple times on the same pg_init which I do think is right (I did not copy that). And let me just make sure I saw the code, right, and that you could need to do a inquiry, mode select and then another inquiry.

This patch is a rough outline of what we could do. I do not think the code completely works today (I will try to get access to a clarriion box in a soon so I can try it out). There are some issues with how the cmd_type stuff should work that I am not sure about (See TODOs in patch for more details). But as I said the last go around, dm-multipath just sticks a activate device cmd_type request into
the lower level devices queue. The LLD (scsi in this case) then will call into some scsi_device_template that is specific to the hw and that module will do whatever commands it needs to do. When it is finished it notifies scsi-ml
that is is done and scsi-ml passes the activate device cmd_type request and its result upwards until in this case dm-mutlipath
gets it back and fails the path groups or tries another
path in the group or whatever it wants to do.

For the scsi_emc_clariion module, I still need to add in some interface so we can configure the device for different tresspass commands (Ed or Hannes what did you say was a safe default for these boxes). Again some of the emc check sense code is a little stupid in that it does the same thing as scsi-ml. I kept that in there as an example of how it would work for when we want different behavior.

Changes since v1:

- Bind per device instead of per target to allow for per LU failover.
- Merge Ed's updated dm-emc code into scsi_emc_clariion.
- Add basic code to hook into REQ_TYPE_LINUX_BLOCK cmd_type (must ask Jens if this is how he wanted it).
- Do a partial conversion of dm-mpath.c (hooked dm-mpath.c in, but all the hw handler code can now be removed (will do this in a seperate patch to reduce noise)).
- added device_id table to bind devices to modules by. Must ask Hannes if this is ok because he was set on compiling this into the kernel and binding off the devinfo list. The problem with this is that these scsi hw handlers look like they can get to be a good size and I do not want to compile it into the kernel when most people do not need it.

---
 block/ll_rw_blk.c                |    2 
 drivers/md/dm-mpath.c            |   55 +++++
 drivers/scsi/Kconfig             |   11 +
 drivers/scsi/Makefile            |    1 
 drivers/scsi/scsi.c              |   52 +++++
 drivers/scsi/scsi_emc_clariion.c |  421 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_error.c        |    9 +
 drivers/scsi/scsi_lib.c          |   45 ++++
 drivers/scsi/scsi_scan.c         |    1 
 drivers/scsi/scsi_sysfs.c        |    3 
 include/linux/blkdev.h           |   12 +
 include/scsi/scsi_cmnd.h         |    1 
 include/scsi/scsi_device.h       |   26 ++
 13 files changed, 634 insertions(+), 5 deletions(-)

index c847e17..af72cab 100644
@@ -3273,7 +3273,7 @@ static int __end_that_request_first(stru
 	 * for a REQ_BLOCK_PC request, we want to carry any eventual
 	 * sense key with us all the way through
 	 */
-	if (!blk_pc_request(req))
+	if (!blk_pc_request(req) && !blk_linux_request(req))
 		req->errors = 0;
 
 	if (!uptodate) {

index 9540eb8..65524f2 100644
@@ -254,6 +254,17 @@ source "drivers/scsi/libsas/Kconfig"
 
 endmenu
 
+menu "SCSI Target Drivers"
+	depends on SCSI
+
+config SCSI_CLARIION
+	tristate "EMC CLARiiON Target Driver"
+	depends on SCSI
+	help
+	  If you have a EMC CLARiiON selec y. Otherwise, say N.
+
+endmenu
+
 menu "SCSI low-level drivers"
 	depends on SCSI!=n
 

index a0a77fd..1526586 100644
@@ -128,6 +128,7 @@ obj-$(CONFIG_SCSI_IPR)		+= ipr.o
 obj-$(CONFIG_SCSI_IBMVSCSI)	+= ibmvscsi/
 obj-$(CONFIG_SCSI_HPTIOP)	+= hptiop.o
 obj-$(CONFIG_SCSI_STEX)		+= stex.o
+obj-$(CONFIG_SCSI_CLARIION)	+= scsi_emc_clariion.o
 
 obj-$(CONFIG_ARM)		+= arm/
 

index aff1b0c..11b5d68 100644
@@ -289,6 +289,7 @@ #endif
  **/
 static int scsi_check_sense(struct scsi_cmnd *scmd)
 {
+	struct scsi_device *sdev = scmd->device;
 	struct scsi_sense_hdr sshdr;
 
 	if (! scsi_command_normalize_sense(scmd, &sshdr))

@@ -297,6 +298,14 @@ static int scsi_check_sense(struct scsi_
 	if (scsi_sense_is_deferred(&sshdr))
 		return NEEDS_RETRY;
 
+	if (sdev->sdevt && sdev->sdevt->check_sense) {
+		int rc;
+
+		rc = sdev->sdevt->check_sense(&sshdr);
+		if (rc)
+			return rc;
+	}
+
 	/*
 	 * Previous logic looked for FILEMARK, EOM or ILI which are
 	 * mainly associated with tapes and returned SUCCESS.

index 148e24c..ee43c86 100644
@@ -709,6 +709,7 @@ static int scsi_add_lun(struct scsi_devi
 	sdev->vendor = (char *) (sdev->inquiry + 8);
 	sdev->model = (char *) (sdev->inquiry + 16);
 	sdev->rev = (char *) (sdev->inquiry + 32);
+	scsi_bind_device_template(sdev);
 
 	if (*bflags & BLIST_ISROM) {
 		/*

index e7fe565..619174e 100644
@@ -244,6 +244,9 @@ static void scsi_device_dev_release_user
 		sdev->request_queue = NULL;
 	}
 
+	if (sdev->sdevt && sdev->sdevt->destroy)
+		sdev->sdevt->destroy(sdev);
+
 	scsi_target_reap(scsi_target(sdev));
 
 	kfree(sdev->inquiry);

index 26f7856..6ea655f 100644
@@ -174,6 +174,15 @@ enum {
 	 */
 	REQ_LB_OP_EJECT	= 0x40,		/* eject request */
 	REQ_LB_OP_FLUSH = 0x41,		/* flush device */
+	REQ_LB_OP_ACTIVATE = 0x42,	/* initialize a device */
+};
+
+/* TODO: where should this go? And add some comments. *//
+enum {
+	BLK_ACTIVATE_SUCCESS,
+	BLK_ACTIVATE_FAILED,
+	BLK_ACTIVATE_BYPASS_DEV,
+	BLK_ACTIVATE_RES_TEMP_UNAVAIL,
 };
 
 /*

@@ -539,6 +548,9 @@ #define blk_fs_request(rq)	((rq)->cmd_ty
 #define blk_pc_request(rq)	((rq)->cmd_type == REQ_TYPE_BLOCK_PC)
 #define blk_special_request(rq)	((rq)->cmd_type == REQ_TYPE_SPECIAL)
 #define blk_sense_request(rq)	((rq)->cmd_type == REQ_TYPE_SENSE)
+#define blk_linux_request(rq)	((rq)->cmd_type == REQ_TYPE_LINUX_BLOCK)
+#define blk_activate_request(rq) (((rq)->cmd_type == REQ_TYPE_LINUX_BLOCK) && \
+				  ((rq)->cmd[0] == REQ_LB_OP_ACTIVATE))
 
 #define blk_noretry_request(rq)	((rq)->cmd_flags & REQ_FAILFAST)
 #define blk_rq_started(rq)	((rq)->cmd_flags & REQ_STARTED)

index be117f8..664efaa 100644
@@ -123,6 +123,7 @@ extern void scsi_put_command(struct scsi
 extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_finish_command(struct scsi_cmnd *cmd);
 extern void scsi_req_abort_cmd(struct scsi_cmnd *cmd);
+extern void scsi_msg_done(struct request *req, int err);
 
 extern void *scsi_kmap_atomic_sg(struct scatterlist *sg, int sg_count,
 				 size_t *offset, size_t *len);

index ebf31b1..56cd2c1 100644
@@ -8,9 +8,11 @@ #include <linux/workqueue.h>
 #include <asm/atomic.h>
 
 struct request_queue;
+struct request;
 struct scsi_cmnd;
 struct scsi_lun;
 struct scsi_sense_hdr;
+struct scsi_device_template;
 
 struct scsi_mode_data {
 	__u32	length;

@@ -140,6 +142,9 @@ #define SCSI_DEFAULT_DEVICE_BLOCKED	3
 
 	struct execute_work	ew; /* used to get process context on put */
 
+	struct scsi_device_template *sdevt;
+	void			*sdevt_data;
+
 	enum scsi_device_state sdev_state;
 	unsigned long		sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long))));

@@ -156,6 +161,24 @@ #define sdev_printk(prefix, sdev, fmt, a
 #define scmd_printk(prefix, scmd, fmt, a...)	\
 	dev_printk(prefix, &(scmd)->device->sdev_gendev, fmt, ##a)
 
+struct sdev_id {
+	char *vendor;
+	char *model;
+};
+
+struct scsi_device_template {
+	struct module *module;
+	const char *name;
+	struct sdev_id *id_table;
+
+	int (* check_sense)(struct scsi_sense_hdr *);
+	int (* activate)(struct request *);
+	int (* setup)(struct scsi_device *);
+	void (* destroy)(struct scsi_device *);
+
+	struct list_head list;
+};
+
 enum scsi_target_state {
 	STARGET_RUNNING = 1,
 	STARGET_DEL,

@@ -284,6 +307,9 @@ extern void int_to_scsilun(unsigned int,
 extern const char *scsi_device_state_name(enum scsi_device_state);
 extern int scsi_is_sdev_device(const struct device *);
 extern int scsi_is_target_device(const struct device *);
+extern int scsi_register_device_template(struct scsi_device_template *);
+extern void scsi_unregister_device_template(struct scsi_device_template *);
+extern void scsi_bind_device_template(struct scsi_device *);
 extern int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 			int data_direction, void *buffer, unsigned bufflen,
 			unsigned char *sense, int timeout, int retries,

1.4.1.1

I updated this patchset here

http://kernel.org/git/?p=linux/kernel/git/mnc/linux-2.6-iscsi.git;a=summary

in the multipath branch.

I added a rdac handler and hp storage works one that does START_STOP. I also integrated it into the devinfo table (cannot update from proc or module param just yet) and fixed some bugs and did some cleanup - still need to better integrate the REQ cmt type checking.

Christophe V, I was wondering what is the vendor/module info for your box? We have a MSA1000 VOLUME/COMPAQ here, but START_STOP does not work. I mean if I send the command it always executes successfully, but the device does not failover (READ/WRITEs fail). I tried just running sg_start and this runs ok but READ/WRITE still fail, so I hacked up scsi_debug to simulate my testing.

If you want to try it out, you just have to add a devinfo entry for your box. I did this for the MSA I thought used it by adding the "hp_sw" string to the end of an existing devinfo entry:

+ {"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN |
BLIST_NOSTARTONADD, "hp_sw"},
+ {"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN |
BLIST_NOSTARTONADD, "hp_sw"},

and you need to use a slightly different table format. I use this for a setup with two paths (one to each target controller) with each path in a different priority group.

0 16384 multipath 1 queue_if_no_path 1 1 2 1 round-robin 0 1 1 8:48 1000 round-robin 0 1 1 8:32 1000

0 16384 multipath 1 queue_if_no_path <the bits here "1 1" is what
changed > 2 1 round-robin 0 1 1 8:48 1000 round-robin 0 1 1 8:32
1000

If you have a device that needs manual failover, you pass "1 1". If it it does not require manual failover then just pass "0". There is no need to pass any hw handler type because the hw handler is no longer in dm. Later when we do more advanced configuration we will have to write to some sysfs interface at the scsi layer to configure values like the tresspass or page2c type (clariion and rdac settings).

It was a HSG80 based array, namely a HP EMA8000.
I do not have access to this hardware anymore.

Regards,
cvaroqui

-
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

Hi Mike,

Hmm. Finally I got a MSA1000 here, too (Thanks, HP!). dm_hp_sw seems to work properly there. Note that according to the HP qla2xxx sources the MSA1000 might take some time to actually do the switchover. Appearently they return NOT_READY / UNIT_ATTENTION and some weird status in byte 12 & 13 of the sense buffer (!). So please check the sense code if there is anything untoward.
Oh, and you can check whether the failover really occured by using the management console; 'show this_controller' and 'show other_controller' will tell you which one's active.
Drop me a mail if you got further questions.

Apart from this: good work!

Cheers,

Hannes

Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstra�e 5				+49 911 74053 688
90409 N�rnberg				http://www.suse.de
-
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

Will do. I wanted to see if the NOT_READY in scsi_error.c caught it but I guess it is not the same asc/asq. I will look at the qlogic driver for the values. Ditto for UNIT_ATTENTION.

Thanks for checking that out.

Hannes - did you run mike's code from his git tree or something else?

I'm confused by your reference to "dm_hp_sw".

Thanks.