Re: [Qemu-devel] [PATCH 06/15] scsi: Update sense code handling

[Hannes Reinecke <hare@suse.de>]

On 11/25/2010 03:33 PM, Kevin Wolf wrote:
> Am 24.11.2010 12:16, schrieb Hannes Reinecke:
>> The SCSI spec has a quite detailed list of sense codes available.
>> It even mandates the use of specific ones for some failure cases.
>> The current implementation just has one type of 'generic' error
>> which is actually a violation of the spec in certain cases.
>> This patch introduces various predefined sense codes to have the
>> sense code reporting more in line with the spec.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Acked-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  hw/scsi-bus.c     |   92 ++++++++++++++++++++++++++++++++++++++++++++
>>  hw/scsi-disk.c    |  109 +++++++++++++++++++++++++++--------------------------
>>  hw/scsi-generic.c |   76 ++++++++++++++++++++++++++-----------
>>  hw/scsi.h         |   38 ++++++++++++++++++
>>  4 files changed, 239 insertions(+), 76 deletions(-)
>>
>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>> index 93f0e9a..afdf0ad 100644
>> --- a/hw/scsi-bus.c
>> +++ b/hw/scsi-bus.c
>> @@ -388,6 +388,98 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
>>      return 0;
>>  }
>>  
>> +/*
>> + * Predefined sense codes
>> + */
>> +
>> +/* No sense data available */
>> +const struct SCSISense sense_code_NO_SENSE = {
>> +    .key = NO_SENSE , .asc = 0x00 , .ascq = 0x00
>> +};
>> +
>> +/* LUN not ready, Manual intervention required */
>> +const struct SCSISense sense_code_LUN_NOT_READY = {
>> +    .key = NOT_READY, .asc = 0x04, .ascq = 0x03
>> +};
>> +
>> +/* LUN not ready, Medium not present */
>> +const struct SCSISense sense_code_NO_MEDIUM = {
>> +    .key = NOT_READY, .asc = 0x3a, .ascq = 0x00
>> +};
>> +
>> +/* Hardware error, internal target failure */
>> +const struct SCSISense sense_code_TARGET_FAILURE = {
>> +    .key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
>> +};
>> +
>> +/* Illegal request, invalid command operation code */
>> +const struct SCSISense sense_code_INVALID_OPCODE = {
>> +    .key = ILLEGAL_REQUEST, .asc = 0x20, .ascq = 0x00
>> +};
>> +
>> +/* Illegal request, LBA out of range */
>> +const struct SCSISense sense_code_LBA_OUT_OF_RANGE = {
>> +    .key = ILLEGAL_REQUEST, .asc = 0x21, .ascq = 0x00
>> +};
>> +
>> +/* Illegal request, Invalid field in CDB */
>> +const struct SCSISense sense_code_INVALID_FIELD = {
>> +    .key = ILLEGAL_REQUEST, .asc = 0x24, .ascq = 0x00
>> +};
>> +
>> +/* Illegal request, LUN not supported */
>> +const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
>> +    .key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
>> +};
>> +
>> +/* Command aborted, I/O process terminated */
>> +const struct SCSISense sense_code_IO_ERROR = {
>> +    .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
>> +};
>> +
>> +/* Command aborted, I_T Nexus loss occurred */
>> +const struct SCSISense sense_code_I_T_NEXUS_LOSS = {
>> +    .key = ABORTED_COMMAND, .asc = 0x29, .ascq = 0x07
>> +};
>> +
>> +/* Command aborted, Logical Unit failure */
>> +const struct SCSISense sense_code_LUN_FAILURE = {
>> +    .key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
>> +};
>> +
>> +/*
>> + * scsi_build_sense
>> + *
>> + * Build a sense buffer
>> + */
>> +int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
>> +{
>> +    if (len < 8)
>> +        return 0;
>> +    if (fixed && len < 14)
>> +        return 0;
>> +
>> +    memset(buf, 0, len);
>> +    if (fixed) {
>> +        /* Return fixed format sense buffer */
>> +        buf[0] = 0xf0;
>> +        buf[2] = sense.key;
>> +        buf[7] = 7;
>> +        buf[12] = sense.asc;
>> +        buf[13] = sense.ascq;
>> +        len = 14;
> 
> My spec says: "Device servers shall return at least 18 bytes of
> parameter data in response to a REQUEST SENSE command if the allocation
> length is 18 or greater and the DESC bit is set to zero."
> 
> So should this be MIN(len, 18) instead?
> 
Yes, you are correct.
And we should actually always return sense data, even if the length
is smaller than the minimum.

Fixed in my megasas git tree; will be included in the next round of
patches.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)