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

[FUJITA Tomonori]

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?


> diff --git a/usr/scsi.c b/usr/scsi.c
> index 36722e1..5ddc94c 100644
> --- a/usr/scsi.c
> +++ b/usr/scsi.c
> @@ -38,27 +38,28 @@
>  #include "scsi.h"
>  #include "spc.h"
>  
> -void sense_data_build(struct scsi_cmd *cmd, uint8_t key, uint8_t asc, uint8_t asq)
> +void sense_data_build(struct scsi_cmd *cmd, uint8_t key, uint16_t asc)
>  {
> +	uint16_t *sense_code;
> +
>  	if (cmd->dev->attrs.sense_format) {
>  		/* descriptor format */
> +		sense_code = (uint16_t *)&cmd->sense_buffer[2];
>  
>  		cmd->sense_buffer[0] = 0x72;  /* current, not deferred */
>  		cmd->sense_buffer[1] = key;
> -		cmd->sense_buffer[2] = asc;
> -		cmd->sense_buffer[3] = asq;
>  		cmd->sense_len = 8;
>  	} else {
>  		/* fixed format */
> +		sense_code = (uint16_t *)&cmd->sense_buffer[12];
>  
>  		int len = 0xa;
> -		cmd->sense_buffer[0] = 0x70;
> +		cmd->sense_buffer[0] = 0x70;  /* current, not deferred */
>  		cmd->sense_buffer[2] = key;
>  		cmd->sense_buffer[7] = len;
> -		cmd->sense_buffer[12] = asc;
> -		cmd->sense_buffer[13] = asq;
>  		cmd->sense_len = len + 8;
>  	}
> +	*sense_code = __cpu_to_be16(asc);
>  }

This doesn't work on big-endian boxes?

I prefer more simple code like:

void sense_data_build(struct scsi_cmd *cmd, uint8_t key, uint16_t code)
{
	uint8_t asc, asq;

	asc = code >> 8;
	asq = code & 0xff;

	if (cmd->dev->attrs.sense_format) {
		cmd->sense_buffer[0] = 0x72;  /* current, not deferred */
		cmd->sense_buffer[1] = key;
		cmd->sense_buffer[2] = asc;
		cmd->sense_buffer[3] = asq;
		cmd->sense_len = 8;


>  #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.