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

[Kevin Wolf]

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 <address@hidden>
> Acked-by: Christoph Hellwig <address@hidden>
> ---
>  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?

Kevin