[v3] iscsi: Translate scsi sense into error code

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(-)

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

Am 05.11.2015 um 06:00 schrieb Fam Zheng:
> 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) {

Reviewed-by: Peter Lieven <pl@kamp.de>