From 837c390137193e715fee20b35c0ddb164b1c4fa4 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 18 Feb 2014 13:08:39 +0100 Subject: block/iscsi: fix deadlock on scsi check condition the retry logic was broken because the complete status of the task structure was not reset. this resulted in an infinite loop retrying the command over and over. CC: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index 41ec09709d..c77769c2c0 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -145,12 +145,13 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { + error_report("iSCSI CheckCondition: %s", iscsi_get_error(iscsi)); iTask->do_retry = 1; goto out; } if (status != SCSI_STATUS_GOOD) { - error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); + error_report("iSCSI Failure: %s", iscsi_get_error(iscsi)); } out: @@ -325,6 +326,7 @@ retry: } if (iTask.do_retry) { + iTask.complete = 0; goto retry; } @@ -399,6 +401,7 @@ retry: } if (iTask.do_retry) { + iTask.complete = 0; goto retry; } @@ -433,6 +436,7 @@ retry: } if (iTask.do_retry) { + iTask.complete = 0; goto retry; } @@ -683,6 +687,7 @@ retry: scsi_free_scsi_task(iTask.task); iTask.task = NULL; } + iTask.complete = 0; goto retry; } @@ -767,6 +772,7 @@ retry: } if (iTask.do_retry) { + iTask.complete = 0; goto retry; } @@ -836,6 +842,7 @@ retry: } if (iTask.do_retry) { + iTask.complete = 0; goto retry; } -- cgit v1.2.3-55-g7522 From 24d3bd67aca958c8ea103646d9d326de00056e4d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 17 Feb 2014 18:34:08 +0100 Subject: block/iscsi: query for supported VPD pages this patch ensures that we only query for block provisioning and block limits vpd pages if they are advertised. It also cleans up the inquiry code and eliminates some redundant code. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 107 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 54 insertions(+), 53 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index c77769c2c0..4360cf1f73 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1067,7 +1067,7 @@ static QemuOptsList runtime_opts = { }; static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, - int evpd, int pc, Error **errp) + int evpd, int pc, void **inq, Error **errp) { int full_size; struct scsi_task *task = NULL; @@ -1086,14 +1086,19 @@ static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, } } + *inq = scsi_datain_unmarshall(task); + if (*inq == NULL) { + error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); + goto fail; + } + return task; fail: error_setg(errp, "iSCSI: Inquiry command failed : %s", iscsi_get_error(iscsi)); - if (task) { + if (task != NULL) { scsi_free_scsi_task(task); - return NULL; } return NULL; } @@ -1114,11 +1119,12 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, struct iscsi_url *iscsi_url = NULL; struct scsi_task *task = NULL; struct scsi_inquiry_standard *inq = NULL; + struct scsi_inquiry_supported_pages *inq_vpd; char *initiator_name = NULL; QemuOpts *opts; Error *local_err = NULL; const char *filename; - int ret; + int i, ret; if ((BDRV_SECTOR_SIZE % 512) != 0) { error_setg(errp, "iSCSI: Invalid BDRV_SECTOR_SIZE. " @@ -1204,24 +1210,17 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsilun->iscsi = iscsi; iscsilun->lun = iscsi_url->lun; + iscsilun->has_write_same = true; - task = iscsi_inquiry_sync(iscsi, iscsilun->lun, 0, 0, 36); - - if (task == NULL || task->status != SCSI_STATUS_GOOD) { - error_setg(errp, "iSCSI: failed to send inquiry command."); - ret = -EINVAL; - goto out; - } - - inq = scsi_datain_unmarshall(task); - if (inq == NULL) { - error_setg(errp, "iSCSI: Failed to unmarshall inquiry data."); + task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 0, 0, + (void **) &inq, errp); + if (task == NULL) { ret = -EINVAL; goto out; } - iscsilun->type = inq->periperal_device_type; - iscsilun->has_write_same = true; + scsi_free_scsi_task(task); + task = NULL; iscsi_readcapacity_sync(iscsilun, &local_err); if (local_err != NULL) { @@ -1240,46 +1239,48 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, bs->sg = 1; } - if (iscsilun->lbpme) { - struct scsi_inquiry_logical_block_provisioning *inq_lbp; - task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, - SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING, - errp); - if (task == NULL) { - ret = -EINVAL; - goto out; - } - inq_lbp = scsi_datain_unmarshall(task); - if (inq_lbp == NULL) { - error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); - ret = -EINVAL; - goto out; - } - memcpy(&iscsilun->lbp, inq_lbp, - sizeof(struct scsi_inquiry_logical_block_provisioning)); - scsi_free_scsi_task(task); - task = NULL; + task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, + SCSI_INQUIRY_PAGECODE_SUPPORTED_VPD_PAGES, + (void **) &inq_vpd, errp); + if (task == NULL) { + ret = -EINVAL; + goto out; } - - if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { + for (i = 0; i < inq_vpd->num_pages; i++) { + struct scsi_task *inq_task; + struct scsi_inquiry_logical_block_provisioning *inq_lbp; struct scsi_inquiry_block_limits *inq_bl; - task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, - SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS, errp); - if (task == NULL) { - ret = -EINVAL; - goto out; - } - inq_bl = scsi_datain_unmarshall(task); - if (inq_bl == NULL) { - error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); - ret = -EINVAL; - goto out; + switch (inq_vpd->pages[i]) { + case SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING: + inq_task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, + SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING, + (void **) &inq_lbp, errp); + if (inq_task == NULL) { + ret = -EINVAL; + goto out; + } + memcpy(&iscsilun->lbp, inq_lbp, + sizeof(struct scsi_inquiry_logical_block_provisioning)); + scsi_free_scsi_task(inq_task); + break; + case SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS: + inq_task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, + SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS, + (void **) &inq_bl, errp); + if (inq_task == NULL) { + ret = -EINVAL; + goto out; + } + memcpy(&iscsilun->bl, inq_bl, + sizeof(struct scsi_inquiry_block_limits)); + scsi_free_scsi_task(inq_task); + break; + default: + break; } - memcpy(&iscsilun->bl, inq_bl, - sizeof(struct scsi_inquiry_block_limits)); - scsi_free_scsi_task(task); - task = NULL; } + scsi_free_scsi_task(task); + task = NULL; #if defined(LIBISCSI_FEATURE_NOP_COUNTER) /* Set up a timer for sending out iSCSI NOPs */ -- cgit v1.2.3-55-g7522 From d9738fd2463f71530d8d92fbb52ebdd1d78074fc Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sat, 22 Feb 2014 13:17:24 +0100 Subject: block/iscsi: fix segfault if writesame fails commit fa6252b0 introduced a segfault because it tries to read iTask.task->sense after iTask.task has been freed. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index 4360cf1f73..0a15f53f8c 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -836,6 +836,15 @@ retry: qemu_coroutine_yield(); } + if (iTask.status == SCSI_STATUS_CHECK_CONDITION && + iTask.task->sense.key == SCSI_SENSE_ILLEGAL_REQUEST && + iTask.task->sense.ascq == SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE) { + /* WRITE SAME is not supported by the target */ + iscsilun->has_write_same = false; + scsi_free_scsi_task(iTask.task); + return -ENOTSUP; + } + if (iTask.task != NULL) { scsi_free_scsi_task(iTask.task); iTask.task = NULL; @@ -847,14 +856,6 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - if (iTask.status == SCSI_STATUS_CHECK_CONDITION && - iTask.task->sense.key == SCSI_SENSE_ILLEGAL_REQUEST && - iTask.task->sense.ascq == SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE) { - /* WRITE SAME is not supported by the target */ - iscsilun->has_write_same = false; - return -ENOTSUP; - } - return -EIO; } -- cgit v1.2.3-55-g7522