Re: [Qemu-devel] [PATCH v3] iscsi: Translate scsi sense into error code

[Paolo Bonzini <pbonzini@redhat.com>]

On 05/11/2015 06:00, Fam Zheng wrote:
> Previously we return -EIO blindly when anything goes wrong. Add a helper
> function to parse sense fields and try to make the return code more
> meaningful.
> 
> This also fixes the default werror configuration (enospc) when we're
> using qcow2 on an iscsi lun. The old -EIO not being treated as out of
> space error failed to trigger vm stop.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> v3: Don't use err_code as return value when it's not set. [Peter]
> v2: Drop the qcow2 patch with ERANGE -> ENOSPC.
>     Drop dead break after return.
>     Return EIO for NO_SENSE.
>     Translate error code for other I/O paths too.
> ---
>  block/iscsi.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 6 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 9a628b7..26bd452 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -84,6 +84,7 @@ typedef struct IscsiTask {
>      IscsiLun *iscsilun;
>      QEMUTimer retry_timer;
>      bool force_next_flush;
> +    int err_code;
>  } IscsiTask;
>  
>  typedef struct IscsiAIOCB {
> @@ -182,6 +183,51 @@ static inline unsigned exp_random(double mean)
>  #define QEMU_SCSI_STATUS_TIMEOUT        SCSI_STATUS_TIMEOUT
>  #endif
>  
> +static int iscsi_translate_sense(struct scsi_sense *sense)
> +{
> +    int ret;
> +
> +    switch (sense->key) {
> +    case SCSI_SENSE_NOT_READY:
> +        return -EBUSY;
> +    case SCSI_SENSE_DATA_PROTECTION:
> +        return -EACCES;
> +    case SCSI_SENSE_COMMAND_ABORTED:
> +        return -ECANCELED;
> +    case SCSI_SENSE_ILLEGAL_REQUEST:
> +        /* Parse ASCQ */
> +        break;
> +    default:
> +        return -EIO;
> +    }
> +    switch (sense->ascq) {
> +    case SCSI_SENSE_ASCQ_PARAMETER_LIST_LENGTH_ERROR:
> +    case SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE:
> +    case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_CDB:
> +    case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST:
> +        ret = -EINVAL;
> +        break;
> +    case SCSI_SENSE_ASCQ_LBA_OUT_OF_RANGE:
> +        ret = -ENOSPC;
> +        break;
> +    case SCSI_SENSE_ASCQ_LOGICAL_UNIT_NOT_SUPPORTED:
> +        ret = -ENOTSUP;
> +        break;
> +    case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT:
> +    case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_CLOSED:
> +    case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_OPEN:
> +        ret = -ENOMEDIUM;
> +        break;
> +    case SCSI_SENSE_ASCQ_WRITE_PROTECTED:
> +        ret = -EACCES;
> +        break;
> +    default:
> +        ret = -EIO;
> +        break;
> +    }
> +    return ret;
> +}
> +
>  static void
>  iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
>                          void *command_data, void *opaque)
> @@ -226,6 +272,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
>                  return;
>              }
>          }
> +        iTask->err_code = iscsi_translate_sense(&task->sense);
>          error_report("iSCSI Failure: %s", iscsi_get_error(iscsi));
>      } else {
>          iTask->iscsilun->force_next_flush |= iTask->force_next_flush;
> @@ -455,7 +502,7 @@ retry:
>      }
>  
>      if (iTask.status != SCSI_STATUS_GOOD) {
> -        return -EIO;
> +        return iTask.err_code;
>      }
>  
>      iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
> @@ -644,7 +691,7 @@ retry:
>      }
>  
>      if (iTask.status != SCSI_STATUS_GOOD) {
> -        return -EIO;
> +        return iTask.err_code;
>      }
>  
>      return 0;
> @@ -683,7 +730,7 @@ retry:
>      }
>  
>      if (iTask.status != SCSI_STATUS_GOOD) {
> -        return -EIO;
> +        return iTask.err_code;
>      }
>  
>      return 0;
> @@ -703,7 +750,7 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
>      if (status < 0) {
>          error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s",
>                       iscsi_get_error(iscsi));
> -        acb->status = -EIO;
> +        acb->status = iscsi_translate_sense(&acb->task->sense);
>      }
>  
>      acb->ioh->driver_status = 0;
> @@ -905,7 +952,7 @@ retry:
>      }
>  
>      if (iTask.status != SCSI_STATUS_GOOD) {
> -        return -EIO;
> +        return iTask.err_code;
>      }
>  
>      iscsi_allocationmap_clear(iscsilun, sector_num, nb_sectors);
> @@ -999,7 +1046,7 @@ retry:
>      }
>  
>      if (iTask.status != SCSI_STATUS_GOOD) {
> -        return -EIO;
> +        return iTask.err_code;
>      }
>  
>      if (flags & BDRV_REQ_MAY_UNMAP) {
> 

Replaced, thanks.

Paolo