[patch 0/3] Add HP hardware handler support to dm-mp

[patch 0/3] Add HP hardware handler support to dm-mp

[dwysocha]

The following 3 patches add HP hardware handler support to dm-multipath.
The first patch is very basic and provides a baseline of support but it is not
complete (has no retries, error code handling, etc).  Second and third patches
add retries and some error code handling.

-- 

[patch 1/3] Extremely basic hp hardware handler (no retries, no error handling, etc).

[dwysocha]

[-- Attachment #1: dm-hp-sw-v0.0.2.patch --]
[-- Type: text/plain, Size: 6919 bytes --]

Signed-off-by: Dave Wysochanski <dave.wysochanski@redhat.com>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>

--
Index: linux-2.6.22-rc1/drivers/md/Makefile
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/Makefile
+++ linux-2.6.22-rc1/drivers/md/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_DM_CRYPT)		+= dm-crypt.o
 obj-$(CONFIG_DM_DELAY)		+= dm-delay.o
 obj-$(CONFIG_DM_MULTIPATH)	+= dm-multipath.o dm-round-robin.o
 obj-$(CONFIG_DM_MULTIPATH_EMC)	+= dm-emc.o
+obj-$(CONFIG_DM_MULTIPATH_HP)	+= dm-hp-sw.o
 obj-$(CONFIG_DM_SNAPSHOT)	+= dm-snapshot.o
 obj-$(CONFIG_DM_MIRROR)		+= dm-mirror.o
 obj-$(CONFIG_DM_ZERO)		+= dm-zero.o
Index: linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
===================================================================
--- /dev/null
+++ linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright (C) 2005 Mike Christie, All rights reserved.
+ * Copyright (C) 2007 Red Hat, Inc. All rights reserved.
+ * Authors: Mike Christie
+ *          Dave Wysochanski
+ *
+ * This file is released under the GPL.
+ *
+ * This module implements the specific path activation code for
+ * HP StorageWorks and FSC FibreCat Asymmetric (Active/Passive)
+ * storage arrays.
+ * These storage arrays have controller-based failover, not
+ * LUN-based failover.  However, LUN-based failover is the design
+ * of dm-multipath. Thus, this module is written for LUN-based failover.
+ */
+#include <linux/list.h>
+#include <linux/types.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+
+#include "dm.h"
+#include "dm-hw-handler.h"
+
+#define DM_MSG_PREFIX "multipath hp"
+
+struct hp_sw_context {
+	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+};
+
+
+/**
+ * hp_sw_end_io - Completion handler for HP path activation.
+ * @req: failover request
+ * @error: scsi-ml error
+ *
+ *  Check sense data, free request structure, and notify dm that
+ *  pg initialization has completed.
+ *
+ * Context: scsi-ml softirq
+ *
+ * Possible optimizations
+ * 1. Actually check sense data for retryable error (e.g. NOT_READY)
+ */
+static void hp_sw_end_io(struct request *req, int error)
+{
+	struct dm_path *path = req->end_io_data;
+	unsigned err_flags;
+
+	if (!error) {
+		err_flags = 0;
+		DMDEBUG("hp_sw: path activation command on %s - success",
+		       	path->dev->name);
+	} else {
+		DMWARN("hp_sw: path activation command on %s - error=0x%x",
+		       path->dev->name, error);
+		err_flags = MP_FAIL_PATH;
+	}
+
+	req->end_io_data = NULL;
+	__blk_put_request(req->q, req);
+	dm_pg_init_complete(path, err_flags);
+}
+
+/**
+ * hp_sw_get_request - Allocate an HP specific path activation request
+ * @path: path on which request will be sent (needed for request queue)
+ *
+ * The START command is used for path activation request.
+ * These arrays are controller-based failover, not LUN based.
+ * One START command issued to a single path will fail over all
+ * LUNs for the same controller.
+ *
+ * Possible optimizations
+ * 1. Make timeout configurable
+ * 2. Preallocate request
+ */
+static struct request *hp_sw_get_request(struct dm_path *path)
+{
+	struct request *req=NULL;
+	struct block_device *bdev = path->dev->bdev;
+	struct request_queue *q = bdev_get_queue(bdev);
+	struct hp_sw_context *h = path->hwhcontext;
+
+	req = blk_get_request(q, WRITE, GFP_ATOMIC);
+	if (req == NULL)
+		goto exit;
+
+	req->timeout = 60*HZ;
+
+	req->errors = 0;
+	req->cmd_type = REQ_TYPE_BLOCK_PC;
+	req->cmd_flags |= REQ_FAILFAST | REQ_NOMERGE;
+	req->end_io = hp_sw_end_io;
+	req->end_io_data = path;
+	req->sense = h->sense;
+	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
+
+	memset(&req->cmd, 0, BLK_MAX_CDB);
+	req->cmd[0] = START_STOP;
+	req->cmd[4] = 1;
+	req->cmd_len = COMMAND_SIZE(req->cmd[0]);
+exit:
+	return req;
+}
+
+/**
+ * hp_sw_pg_init - HP path activation implementation.
+ * @hwh: hardware handler specific data
+ * @bypassed: unused; is the path group bypassed? (see dm-mpath.c)
+ * @path: path to send initialization command
+ *
+ * Send an HP-specific path activation command on 'path'.
+ * Do not try to optimize in any way, just send the activation command.
+ * More than one path activation command may be sent to the same controller.
+ * This seems to work fine for basic failover support.
+ *
+ * Possible optimizations
+ * 1. Detect an in-progress activation request and avoid submitting another one
+ * 2. Model the controller and only send a single activation request at a time
+ * 3. Determine the state of a path before sending an activation request
+ *
+ * Context: kmpathd (see process_queued_ios() in dm-mpath.c)
+ */
+static void hp_sw_pg_init(struct hw_handler *hwh, unsigned bypassed,
+			  struct dm_path *path)
+{
+	struct request *req;
+	struct hp_sw_context *h;
+
+	path->hwhcontext = hwh->context;
+	h = (struct hp_sw_context *) hwh->context;
+
+	req = hp_sw_get_request(path);
+	if (!req) {
+		DMERR("hp_sw: path activation command allocation fail on %s ",
+		      path->dev->name);
+		goto fail;
+	}
+
+	DMDEBUG("hp_sw: path activation command sent on %s",
+		path->dev->name);
+
+	elv_add_request(req->q, req, ELEVATOR_INSERT_FRONT, 1);
+	return;
+
+ fail:
+	dm_pg_init_complete(path, MP_FAIL_PATH);
+}
+
+static int hp_sw_create(struct hw_handler *hwh, unsigned argc, char **argv)
+{
+	struct hp_sw_context *h;
+
+	h = kmalloc(sizeof(*h), GFP_KERNEL);
+	if (!h)
+		return -ENOMEM;
+	hwh->context = h;
+	return 0;
+}
+
+static void hp_sw_destroy(struct hw_handler *hwh)
+{
+	struct hp_sw_context *h = hwh->context;
+
+	kfree(h);
+	hwh->context = NULL;
+}
+
+static struct hw_handler_type hp_sw_hwh = {
+	.name = "hp_sw",
+	.module = THIS_MODULE,
+	.create = hp_sw_create,
+	.destroy = hp_sw_destroy,
+	.pg_init = hp_sw_pg_init,
+};
+
+static int __init hp_sw_init(void)
+{
+	int r;
+
+	r = dm_register_hw_handler(&hp_sw_hwh);
+	if (r < 0)
+		DMERR("hp_sw: register failed %d", r);
+
+	DMINFO("hp_sw version 0.0.2 loaded");
+
+	return r;
+}
+
+static void __exit hp_sw_exit(void)
+{
+	int r;
+
+	r = dm_unregister_hw_handler(&hp_sw_hwh);
+	if (r < 0)
+		DMERR("hp_sw: unregister failed %d", r);
+}
+
+module_init(hp_sw_init);
+module_exit(hp_sw_exit);
+
+MODULE_DESCRIPTION("HP StorageWorks and FSC FibreCat (A/P) support for dm-multipath");
+MODULE_AUTHOR("Mike Christie <michaelc@cs.wisc.edu>");
+MODULE_LICENSE("GPL");
Index: linux-2.6.22-rc1/drivers/md/Kconfig
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/Kconfig
+++ linux-2.6.22-rc1/drivers/md/Kconfig
@@ -271,6 +271,12 @@ config DM_DELAY
 
 	If unsure, say N.
 
+config DM_MULTIPATH_HP
+        tristate "HP MSA multipath support (EXPERIMENTAL)"
+        depends on DM_MULTIPATH && BLK_DEV_DM && EXPERIMENTAL
+        ---help---
+          Multipath support for HP MSA (Active/Passive) series hardware.
+
 endmenu
 
 endif

-- 

[patch 2/3] Add MP_RETRY_PG_INIT flag for hw handlers to tell dm-mpath to retry pg_init.

[dwysocha]

[-- Attachment #1: dm-mpath-add-retry-pg-init.patch --]
[-- Type: text/plain, Size: 1231 bytes --]

Useful for cases where a hw handler sends a path initialization command to
the storage and it completes with an error code indicating the command should
be retried.

Signed-off-by: Dave Wysochanski <dave.wysochanski@redhat.com>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

--
Index: linux-2.6.22-rc1/drivers/md/dm-hw-handler.h
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/dm-hw-handler.h
+++ linux-2.6.22-rc1/drivers/md/dm-hw-handler.h
@@ -58,5 +58,6 @@ unsigned dm_scsi_err_handler(struct hw_h
 #define MP_FAIL_PATH 1
 #define MP_BYPASS_PG 2
 #define MP_ERROR_IO  4	/* Don't retry this I/O */
+#define MP_RETRY_PG_INIT 8
 
 #endif
Index: linux-2.6.22-rc1/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/dm-mpath.c
+++ linux-2.6.22-rc1/drivers/md/dm-mpath.c
@@ -999,7 +999,9 @@ void dm_pg_init_complete(struct dm_path 
 		bypass_pg(m, pg, 1);
 
 	spin_lock_irqsave(&m->lock, flags);
-	if (err_flags) {
+	if (err_flags & MP_RETRY_PG_INIT)
+		m->pg_init_required = 1;
+	else if (err_flags) {
 		m->current_pgpath = NULL;
 		m->current_pg = NULL;
 	} else if (!m->pg_init_required)

-- 

[patch 3/3] Add retries to hp hardware handler when path activation command completes w/err

[dwysocha]

[-- Attachment #1: dm-hp-sw-add-retries-handle-not-ready.patch --]
[-- Type: text/plain, Size: 5276 bytes --]

This patch depends on the following patch:
dm-mpath: Add MP_RETRY_PG_INIT flag for hw handlers to tell dm-mpath to retry

Add retries to hp hardware handler if path initialization command completes
with a check condition.  For now we just assume we can retry the command
because we only have partial information on the check conditions of the 
HP hardware.  Testing has shown that sending additional path initialization
commands do no extra harm so we just be conservative and retry 5 times.

Signed-off-by: Dave Wysochanski <dave.wysochanski@redhat.com>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

--
Index: linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/dm-hp-sw.c
+++ linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
@@ -17,20 +17,58 @@
 #include <linux/types.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
 
 #include "dm.h"
 #include "dm-hw-handler.h"
 
 #define DM_MSG_PREFIX "multipath hp"
 
+#define HP_SW_PG_INIT_RETRIES 5
+
 struct hp_sw_context {
 	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+	unsigned pg_init_count;
 };
 
+/**
+ * hp_sw_error_is_retryable - Is an HP-specific check condition retryable?
+ * @req: path activation request
+ *
+ * Examine error codes of request and determine whether the error is retryable.
+ * Some error codes are already retried by scsi-ml (see
+ * scsi_decide_disposition), but some HP specific codes are not.
+ * The intent of this routine is to supply the logic for the HP specific
+ * check conditions.
+ *
+ * Returns:
+ *  1 - command completed with retryable error
+ *  0 - command completed with non-retryable error
+ *
+ * Possible optimizations
+ * 1. More hardware-specific error codes
+ */
+static int hp_sw_error_is_retryable(struct request *req)
+{
+	/*
+	 * NOT_READY is known to be retryable
+	 * For now we just dump out the sense data and call it retryable
+	 */
+	if ((status_byte(req->errors) == CHECK_CONDITION) &&
+	    (driver_byte(req->errors) & DRIVER_SENSE))
+		__scsi_print_sense("hp_sw", req->sense, req->sense_len);
+
+	/*
+	 * At this point we don't have complete information about all the error
+	 * codes from this hardware, so we are just conservative and retry
+	 * when in doubt.
+	 */
+	return 1;
+}
 
 /**
  * hp_sw_end_io - Completion handler for HP path activation.
- * @req: failover request
+ * @req: path activation request
  * @error: scsi-ml error
  *
  *  Check sense data, free request structure, and notify dm that
@@ -38,24 +76,38 @@ struct hp_sw_context {
  *
  * Context: scsi-ml softirq
  *
- * Possible optimizations
- * 1. Actually check sense data for retryable error (e.g. NOT_READY)
  */
 static void hp_sw_end_io(struct request *req, int error)
 {
 	struct dm_path *path = req->end_io_data;
+	struct hp_sw_context *h = path->hwhcontext;
 	unsigned err_flags;
 
 	if (!error) {
+		h->pg_init_count = 0;
 		err_flags = 0;
-		DMDEBUG("hp_sw: path activation command on %s - success",
+		DMDEBUG("hp_sw: %s path activation command - success",
 		       	path->dev->name);
 	} else {
-		DMWARN("hp_sw: path activation command on %s - error=0x%x",
+		DMWARN("hp_sw: %s path activation command - error=0x%x",
 		       path->dev->name, error);
+		if (hp_sw_error_is_retryable(req)) {
+			if (h->pg_init_count <= HP_SW_PG_INIT_RETRIES) {
+				DMWARN("hp_sw: %s path activation command "
+				       "count=%d",
+				       path->dev->name, h->pg_init_count);
+				err_flags = MP_RETRY_PG_INIT;
+				goto exit;
+			} else
+				DMWARN("hp_sw: %s path activation command "
+				       "out of retries",
+				       path->dev->name);
+		}
+		DMWARN("hp_sw: %s path activation fail", path->dev->name);
+		h->pg_init_count = 0;
 		err_flags = MP_FAIL_PATH;
 	}
-
+ exit:
 	req->end_io_data = NULL;
 	__blk_put_request(req->q, req);
 	dm_pg_init_complete(path, err_flags);
@@ -126,25 +178,31 @@ static void hp_sw_pg_init(struct hw_hand
 {
 	struct request *req;
 	struct hp_sw_context *h;
+	unsigned err_flags;
 
 	path->hwhcontext = hwh->context;
 	h = (struct hp_sw_context *) hwh->context;
+	h->pg_init_count++;
 
 	req = hp_sw_get_request(path);
 	if (!req) {
-		DMERR("hp_sw: path activation command allocation fail on %s ",
+		DMERR("hp_sw: %s path activation command allocation fail ",
 		      path->dev->name);
 		goto fail;
 	}
 
-	DMDEBUG("hp_sw: path activation command sent on %s",
-		path->dev->name);
+	DMDEBUG("hp_sw: %s path activation command sent, pg_init_count=%d",
+		path->dev->name, h->pg_init_count);
 
 	elv_add_request(req->q, req, ELEVATOR_INSERT_FRONT, 1);
 	return;
 
  fail:
-	dm_pg_init_complete(path, MP_FAIL_PATH);
+	if (h->pg_init_count <= HP_SW_PG_INIT_RETRIES)
+		err_flags = MP_RETRY_PG_INIT;
+	else
+		err_flags = MP_FAIL_PATH;
+	dm_pg_init_complete(path, err_flags);
 }
 
 static int hp_sw_create(struct hw_handler *hwh, unsigned argc, char **argv)
@@ -155,6 +213,7 @@ static int hp_sw_create(struct hw_handle
 	if (!h)
 		return -ENOMEM;
 	hwh->context = h;
+	h->pg_init_count = 0;
 	return 0;
 }
 
@@ -182,7 +241,7 @@ static int __init hp_sw_init(void)
 	if (r < 0)
 		DMERR("hp_sw: register failed %d", r);
 
-	DMINFO("hp_sw version 0.0.2 loaded");
+	DMINFO("hp_sw version 0.0.3 loaded");
 
 	return r;
 }

-- 

[patch 3/3] Add retries to hp hardware handler when path activation command completes w/err

[dwysocha]

[-- Attachment #1: dm-hp-sw-add-retries-handle-not-ready.patch --]
[-- Type: text/plain, Size: 5158 bytes --]

This patch depends on the following patch:
dm-mpath: Add MP_RETRY_PG_INIT flag for hw handlers to tell dm-mpath to retry

Add retries to hp hardware handler if path initialization command completes
with a check condition.  For now we just assume we can retry the command
because we only have partial information on the check conditions of the 
HP hardware.  Testing has shown that sending additional path initialization
commands do no extra harm so we just be conservative and retry 5 times.

Index: linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
===================================================================
--- linux-2.6.22-rc1.orig/drivers/md/dm-hp-sw.c
+++ linux-2.6.22-rc1/drivers/md/dm-hp-sw.c
@@ -17,20 +17,58 @@
 #include <linux/types.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
 
 #include "dm.h"
 #include "dm-hw-handler.h"
 
 #define DM_MSG_PREFIX "multipath hp"
 
+#define HP_SW_PG_INIT_RETRIES 5
+
 struct hp_sw_context {
 	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+	unsigned pg_init_count;
 };
 
+/**
+ * hp_sw_error_is_retryable - Is an HP-specific check condition retryable?
+ * @req: path activation request
+ *
+ * Examine error codes of request and determine whether the error is retryable.
+ * Some error codes are already retried by scsi-ml (see
+ * scsi_decide_disposition), but some HP specific codes are not.
+ * The intent of this routine is to supply the logic for the HP specific
+ * check conditions.
+ *
+ * Returns:
+ *  1 - command completed with retryable error
+ *  0 - command completed with non-retryable error
+ *
+ * Possible optimizations
+ * 1. More hardware-specific error codes
+ */
+static int hp_sw_error_is_retryable(struct request *req)
+{
+	/*
+	 * NOT_READY is known to be retryable
+	 * For now we just dump out the sense data and call it retryable
+	 */
+	if ((status_byte(req->errors) == CHECK_CONDITION) &&
+	    (driver_byte(req->errors) & DRIVER_SENSE))
+		__scsi_print_sense("hp_sw", req->sense, req->sense_len);
+
+	/*
+	 * At this point we don't have complete information about all the error
+	 * codes from this hardware, so we are just conservative and retry
+	 * when in doubt.
+	 */
+	return 1;
+}
 
 /**
  * hp_sw_end_io - Completion handler for HP path activation.
- * @req: failover request
+ * @req: path activation request
  * @error: scsi-ml error
  *
  *  Check sense data, free request structure, and notify dm that
@@ -38,24 +76,38 @@ struct hp_sw_context {
  *
  * Context: scsi-ml softirq
  *
- * Possible optimizations
- * 1. Actually check sense data for retryable error (e.g. NOT_READY)
  */
 static void hp_sw_end_io(struct request *req, int error)
 {
 	struct dm_path *path = req->end_io_data;
+	struct hp_sw_context *h = path->hwhcontext;
 	unsigned err_flags;
 
 	if (!error) {
+		h->pg_init_count = 0;
 		err_flags = 0;
-		DMDEBUG("hp_sw: path activation command on %s - success",
+		DMDEBUG("hp_sw: %s path activation command - success",
 		       	path->dev->name);
 	} else {
-		DMWARN("hp_sw: path activation command on %s - error=0x%x",
+		DMWARN("hp_sw: %s path activation command - error=0x%x",
 		       path->dev->name, error);
+		if (hp_sw_error_is_retryable(req)) {
+			if (h->pg_init_count <= HP_SW_PG_INIT_RETRIES) {
+				DMWARN("hp_sw: %s path activation command "
+				       "count=%d",
+				       path->dev->name, h->pg_init_count);
+				err_flags = MP_RETRY_PG_INIT;
+				goto exit;
+			} else
+				DMWARN("hp_sw: %s path activation command "
+				       "out of retries",
+				       path->dev->name);
+		}
+		DMWARN("hp_sw: %s path activation fail", path->dev->name);
+		h->pg_init_count = 0;
 		err_flags = MP_FAIL_PATH;
 	}
-
+ exit:
 	req->end_io_data = NULL;
 	__blk_put_request(req->q, req);
 	dm_pg_init_complete(path, err_flags);
@@ -126,25 +178,31 @@ static void hp_sw_pg_init(struct hw_hand
 {
 	struct request *req;
 	struct hp_sw_context *h;
+	unsigned err_flags;
 
 	path->hwhcontext = hwh->context;
 	h = (struct hp_sw_context *) hwh->context;
+	h->pg_init_count++;
 
 	req = hp_sw_get_request(path);
 	if (!req) {
-		DMERR("hp_sw: path activation command allocation fail on %s ",
+		DMERR("hp_sw: %s path activation command allocation fail ",
 		      path->dev->name);
 		goto fail;
 	}
 
-	DMDEBUG("hp_sw: path activation command sent on %s",
-		path->dev->name);
+	DMDEBUG("hp_sw: %s path activation command sent, pg_init_count=%d",
+		path->dev->name, h->pg_init_count);
 
 	elv_add_request(req->q, req, ELEVATOR_INSERT_FRONT, 1);
 	return;
 
  fail:
-	dm_pg_init_complete(path, MP_FAIL_PATH);
+	if (h->pg_init_count <= HP_SW_PG_INIT_RETRIES)
+		err_flags = MP_RETRY_PG_INIT;
+	else
+		err_flags = MP_FAIL_PATH;
+	dm_pg_init_complete(path, err_flags);
 }
 
 static int hp_sw_create(struct hw_handler *hwh, unsigned argc, char **argv)
@@ -155,6 +213,7 @@ static int hp_sw_create(struct hw_handle
 	if (!h)
 		return -ENOMEM;
 	hwh->context = h;
+	h->pg_init_count = 0;
 	return 0;
 }
 
@@ -182,7 +241,7 @@ static int __init hp_sw_init(void)
 	if (r < 0)
 		DMERR("hp_sw: register failed %d", r);
 
-	DMINFO("hp_sw version 0.0.2 loaded");
+	DMINFO("hp_sw version 0.0.3 loaded");
 
 	return r;
 }

--