Re: [PATCH] scsi_io_completion: extend description of default host-byte handling

[Rob Evers <revers@xxxxxxxxxx>]

 On 10/13/2011 06:08 PM, Ankit Jain wrote:
> On 10/14/2011 03:01 AM, Rob Evers wrote:
> > replace "unhandled error code" message in scsi_io_completion default
> > host-byte condition with a descriptive message of what the host-byte
> > indicates.  The descriptive messages for the host bytes were derived
> > from the corresponding comments for the DID_HOST_ definitions in scsi.h.
> >
> > Also softened the corresponding default sense code message.
> >
> > Signed-off-by: Rob Evers<revers@xxxxxxxxxx>
> > ---
> >   drivers/scsi/constants.c |   45 ++++++++++++++++++++++++++++++++++++++++++++-
> >   drivers/scsi/scsi_lib.c  |    4 ++--
> >   include/scsi/scsi_dbg.h  |    1 +
> >   3 files changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> > index 450e011..900333c 100644
> > --- a/drivers/scsi/constants.c
> > +++ b/drivers/scsi/constants.c
> > @@ -1422,9 +1422,52 @@ static const char * const hostbyte_table[]={
> >   "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET",
> >   "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR",
> >   "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE",
> > -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST" };
> > +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", "DID_TARGET_FAILURE",
> > +"DID_NEXUS_FAILURE" };
> >   #define NUM_HOSTBYTE_STRS ARRAY_SIZE(hostbyte_table)
> >
> > +/*
> > + * derived from comments following 'Host byte codes' in scsi.h
> > + */
> > +static char *hostbyte_table_ext_msg[] = {
> > +	"No Error",
> > +	"Couldn't connect before timeout period",
> > +	"Bus stayed busy through timeout period",
> > +	"Timed out for other reason",
> > +	"Bad target",
> > +	"Abort for some other reason",
> > +	"Parity Error",
> > +	"Internal Error",
> > +	"Reset",
> > +	"Interrupt that is not expected",
> > +	"Force command past mid-layer",
> > +	"Low level driver requests a retry",
> > +	"Retry immediately, don't decrement retry count",
> > +	"Requeue command, not immediate, don't decrement retry count",
> > +	"Transport disrupted, driver blocked port to recover link, transport class will retry or fail the IO",
> > +	"Transport class fastfailed the IO",
> > +	"Permanent target failure, don't retry other paths",
> > +	"Permanent nexus failure, retry on other paths may yield different results"
> > +};
> > +#define NUM_HOSTBYTE_EXT_MSG_STRS ARRAY_SIZE(hostbyte_table_ext_msg)
> > +
> > +#endif
> > +
> > +char *
> > +scsi_ext_host_byte_msg(unsigned char index)
> > +{
> > +#ifdef CONFIG_SCSI_CONSTANTS
> > +	if (index<  NUM_HOSTBYTE_EXT_MSG_STRS)
> > +		return hostbyte_table_ext_msg[index];
> > +	else
> > +		return NULL;
> Some message, rather than nothing(NULL) would be more useful, IMHO.
>
> > +#else
> > +	return NULL;
> > +#endif
> Wouldn't it be better to fallback to "Unhandled error code" in case
> CONFIG_SCSI_CONSTANTS is not defined? Maybe same for the earlier comment?

Thankyou for your review in both cases, though "Unhandled error code" 
was the impetus for this to begin with.

Any other feedback from anyone else?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html