Looks good. Acked by: Chandra Seetharaman <sekharan@xxxxxxxxxx> On Mon, 2007-07-30 at 15:10 -0400, Dave Wysochanski wrote: > On Thu, 2007-07-26 at 12:19 -0700, Chandra Seetharaman wrote: > > On Thu, 2007-07-26 at 00:44 -0400, dwysocha@xxxxxxxxxx wrote: > > > plain text document attachment (dm-hp-sw-add-retries-handle-not- > > > ready.patch) > > > This patch adds retries to the hp hardware handler, and utilizes the > > > MP_RETRY flag of dm-multipath. For now in the hp handler, if we get a > > > pg_init completed with a check condition we just assume we can retry the > > > pg_init command. We make this assumption because of incomplete data on > > > specific check condition code of the HP hardware, and because testing > > > has shown the HP path initialization command to be idempotent. > > > The number of times we retry is settable via the "pg_init_retries" > > > multipath map feature. > > > > > > > > > Index: linux-2.6.23-rc1/drivers/md/dm-hp-sw.c > > > =================================================================== > > > --- linux-2.6.23-rc1.orig/drivers/md/dm-hp-sw.c > > > +++ linux-2.6.23-rc1/drivers/md/dm-hp-sw.c > > > @@ -18,6 +18,7 @@ > > > #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" > > > @@ -26,8 +27,42 @@ > > > > > > struct hp_sw_context { > > > unsigned char sense[SCSI_SENSE_BUFFERSIZE]; > > > + unsigned argc; > > > }; > > > > > > +/** > > > + * 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) > > > + __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. > > > @@ -39,8 +74,6 @@ 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) > > > { > > > @@ -52,11 +85,17 @@ static void hp_sw_end_io(struct request > > > DMDEBUG("%s path activation command - success", > > > path->dev->name); > > > } else { > > > - DMWARN("path activation command on %s - error=0x%x", > > > - path->dev->name, error); > > > + if (hp_sw_error_is_retryable(req)) { > > > + DMDEBUG("%s path activation command retry", > > > + path->dev->name); > > > > mixed use of space and tabs > > > + err_flags = MP_RETRY; > > > + goto out; > > > + } > > > + DMWARN("%s path activation fail - error=0x%x", > > > + path->dev->name, error); > > > err_flags = MP_FAIL_PATH; > > > } > > > - > > > + out: > > > > space in front of label > > > > > req->end_io_data = NULL; > > > __blk_put_request(req->q, req); > > > dm_pg_init_complete(path, err_flags); > > > @@ -134,17 +173,16 @@ static void hp_sw_pg_init(struct hw_hand > > > if (!req) { > > > DMERR("%s path activation command allocation fail ", > > > path->dev->name); > > > - goto fail; > > > + goto retry; > > > } > > > > > > - DMDEBUG("path activation command sent on %s", > > > - path->dev->name); > > > + DMDEBUG("%s path activation command sent", path->dev->name); > > > > > > blk_execute_rq_nowait(req->q, NULL, req, 1, hp_sw_end_io); > > > return; > > > > > > - fail: > > > - dm_pg_init_complete(path, MP_FAIL_PATH); > > > + retry: > > > + dm_pg_init_complete(path, MP_RETRY); > > > } > > > > > > static int hp_sw_create(struct hw_handler *hwh, unsigned argc, char **argv) > > > @@ -181,7 +219,7 @@ static int __init hp_sw_init(void) > > > if (r < 0) > > > DMERR("register failed %d", r); > > > else > > > - DMINFO("version 0.0.2 loaded"); > > > + DMINFO("version 0.0.3 loaded"); > > > > > > return r; > > > } > > > > > Attached should fix the whitespace issues mentioned. > > -- ---------------------------------------------------------------------- Chandra Seetharaman | Be careful what you choose.... - sekharan@xxxxxxxxxx | .......you may get it. ---------------------------------------------------------------------- -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel