[Stgt-devel] [Patch 1/3] Rework of mnemonic ASC/ASCQ values

[FUJITA Tomonori]

From: "Mark Harvey" <markh794 at gmail.com>
Subject: Re: [Stgt-devel] [Patch 1/3] Rework of mnemonic ASC/ASCQ values
Date: Thu, 28 Jun 2007 07:58:43 +1000

> On 6/27/07, FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp> wrote:
> > From: Mark Harvey <markh794 at gmail.com>
> > Subject: [Stgt-devel] [Patch 1/3] Rework of mnemonic ASC/ASCQ values
> > Date: Sat, 23 Jun 2007 18:11:52 +1000
> >
> > > >From d163aa03f036609ac5343c2d8983aba38ecec679 Mon Sep 17 00:00:00 2001
> > > From: Mark Harvey <markh794 at gmail.com>
> > > Date: Fri, 22 Jun 2007 18:20:54 +1000
> > > Subject: Use nmemonic codes for SENSE codes instead of numeric value.
> > >
> > > Using nmemonic representation for ASC/ASCQ as a 16bit value.
> > >  - Values defined in sense_codes.h
> > >  - Updated sense_data_build() to accept a single 16bit
> > >    combined asc/ascq value.
> > >
> > > Signed-off-by: Mark Harvey <markh794 at gmail.com>
> > > ---
> > >  usr/bs_sync.c       |    3 +-
> > >  usr/ibmvio/ibmvio.c |   15 ++++---
> > >  usr/mmc.c           |    7 ++-
> > >  usr/osd.c           |    5 +-
> > >  usr/sbc.c           |   33 +++++++++-------
> > >  usr/scsi.c          |   13 +++---
> > >  usr/sense_codes.h   |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  usr/spc.c           |   19 +++++----
> > >  usr/spt.c           |    5 +-
> > >  usr/tgtd.h          |    3 +-
> > >  10 files changed, 161 insertions(+), 45 deletions(-)
> > >  create mode 100644 usr/sense_codes.h
> > >
> > > diff --git a/usr/bs_sync.c b/usr/bs_sync.c
> > > index 6789b5b..4986c59 100644
> > > --- a/usr/bs_sync.c
> > > +++ b/usr/bs_sync.c
> > > @@ -37,6 +37,7 @@
> > >  #include "util.h"
> > >  #include "tgtd.h"
> > >  #include "scsi.h"
> > > +#include "sense_codes.h"
> > >
> > >  #define NR_WORKER_THREADS    4
> > >
> > > @@ -161,7 +162,7 @@ static void *bs_sync_worker_fn(void *arg)
> > >                       eprintf("io error %p %x %d %d %" PRIu64 ", %m\n",
> > >                               cmd, cmd->scb[0], ret, cmd->len, cmd->offset);
> > >                       cmd->result = SAM_STAT_CHECK_CONDITION;
> > > -                     sense_data_build(cmd, MEDIUM_ERROR, 0x11, 0x0);
> > > +                     sense_data_build(cmd, MEDIUM_ERROR, ASC_READ_ERROR);
> > >               }
> > >
> > >               pthread_mutex_lock(&info->finished_lock);
> > > diff --git a/usr/ibmvio/ibmvio.c b/usr/ibmvio/ibmvio.c
> > > index 5a2f8fe..8cfc8d6 100644
> > > --- a/usr/ibmvio/ibmvio.c
> > > +++ b/usr/ibmvio/ibmvio.c
> > > @@ -44,6 +44,7 @@
> > >  #include "target.h"
> > >  #include "driver.h"
> > >  #include "spc.h"
> > > +#include "sense_codes.h"
> > >
> > >  #define GETTARGET(x) ((int)((((uint64_t)(x)) >> 56) & 0x003f))
> > >  #define GETBUS(x) ((int)((((uint64_t)(x)) >> 53) & 0x0007))
> > > @@ -140,7 +141,8 @@ static int ibmvio_inquiry(int host_no, struct scsi_cmd *cmd)
> > >  {
> > >       int ret = SAM_STAT_CHECK_CONDITION;
> > >       uint8_t *data, *scb = cmd->scb;
> > > -     unsigned char key = ILLEGAL_REQUEST, asc = 0x24;
> > > +     unsigned char key = ILLEGAL_REQUEST;
> > > +     uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
> > >
> > >       if (((scb[1] & 0x3) == 0x3) || (!(scb[1] & 0x3) && scb[2]))
> > >               goto sense;
> > > @@ -148,7 +150,7 @@ static int ibmvio_inquiry(int host_no, struct scsi_cmd *cmd)
> > >       data = valloc(pagesize);
> > >       if (!data) {
> > >               key = HARDWARE_ERROR;
> > > -             asc = 0;
> > > +             asc = ASC_INTERNAL_TGT_FAILURE;
> >
> > Just ASC_INTERNAL_FAILURE would look better?
> 
> Took the Values & names from the t10 SPC doco
> This was INTERNAL TARGET FAILURE, however I shortened it from TARGET to TGT
> I can shorten this again to ASC_INTERNAL_FAILURE if you like..

ASC_INTERNAL_TGT_FAILURE or ASC_INTERNAL_TARGET_FAILURE is
fine. Sorry, I remember the wrong name.


> > >  #define        TGT_INVALID_DEV_ID      ~0ULL
> > > diff --git a/usr/sense_codes.h b/usr/sense_codes.h
> > > new file mode 100644
> > > index 0000000..ea8eebc
> > > --- /dev/null
> > > +++ b/usr/sense_codes.h
> > > @@ -0,0 +1,103 @@
> > > +/*
> > > + * The SCSI sense key Additional Sense Code / Additional Sense Code Qualifier
> > > + *
> > > + * Copyright (C) 2007 Mark Harvey markh794 at gmail dot com
> >
> > How about:
> >
> > + * Copyright (C) 2007      Mark Harvey <markh794 at gmail.com>
> >
> > Looks better.
> >
> > I don't think camouflaging your address is useful since spammers can
> > find your address in the git history.
> 
> Actually, I'm not even sure I should have a "Copyright" here as the
> details have been taken from information at www.t10.org
> 
> Should I remove my "Copyright" message altogether or leave it there
> and update as per your suggestion ?

I'm not sure. Either is ok for me though as you said, putting the
copyright in this file might be meaningless.