From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuCbi-0007Au-DF for qemu-devel@nongnu.org; Wed, 04 Nov 2015 23:57:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZuCbh-00032E-CA for qemu-devel@nongnu.org; Wed, 04 Nov 2015 23:57:06 -0500 Date: Thu, 5 Nov 2015 12:56:56 +0800 From: Fam Zheng Message-ID: <20151105045656.GD24893@ad.usersys.redhat.com> References: <1446600974-24676-1-git-send-email-famz@redhat.com> <563A66B6.3090500@kamp.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <563A66B6.3090500@kamp.de> Subject: Re: [Qemu-devel] [PATCH v2] iscsi: Translate scsi sense into error code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: Kevin Wolf , Paolo Bonzini , qemu-block@nongnu.org, qemu-devel@nongnu.org, Ronnie Sahlberg On Wed, 11/04 21:12, Peter Lieven wrote: > Am 04.11.2015 um 02:36 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 > > > > --- > > 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 | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 55 insertions(+), 8 deletions(-) > > > > diff --git a/block/iscsi.c b/block/iscsi.c > > index 9a628b7..b1bc0ef 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); > > @@ -536,14 +583,14 @@ retry: > > > > lbas = scsi_datain_unmarshall(iTask.task); > > if (lbas == NULL) { > > - ret = -EIO; > > + ret = iTask.err_code; > > goto out; > > } > > An unmarschalling error will not set iTask.err_code. It is likely > 0 here. Please leave it at -EIO. > > > > > lbasd = &lbas->descriptors[0]; > > > > if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) { > > - ret = -EIO; > > + ret = iTask.err_code; > > goto out; > > } > > This is an internal sanity check, iTask.err_code will also be 0 here. You're right in both cases, will fix. Thanks! Fam