From c5f52875b980e54e6bebad6121c76863356e1d7f Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 24 Jan 2014 15:02:24 +0800 Subject: scsi: Change scsi sense buf size to 252 Current buffer size fails the assersion check in like hw/scsi/scsi-bus.c:1655: assert(req->sense_len <= sizeof(req->sense)); when backend (block/iscsi.c) returns more data then 96. Exercise the core dump path by booting an Gentoo ISO with scsi-generic device backed with iscsi (built with libiscsi 1.7.0): x86_64-softmmu/qemu-system-x86_64 \ -drive file=iscsi://localhost:3260/iqn.foobar/0,if=none,id=drive-disk \ -device virtio-scsi-pci,id=scsi1,bus=pci.0,addr=0x6 \ -device scsi-generic,drive=drive-disk,bus=scsi1.0,id=iscsi-disk \ -boot d \ -cdrom gentoo.iso qemu-system-x86_64: hw/scsi/scsi-bus.c:1655: scsi_req_complete: Assertion `req->sense_len <= sizeof(req->sense)' failed. According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So increase the value to fix it. Also remove duplicated define for the macro. Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-generic.c | 2 -- hw/scsi/spapr_vscsi.c | 1 - include/hw/scsi/scsi.h | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index f08b64e177..8d92e0da15 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -37,8 +37,6 @@ do { fprintf(stderr, "scsi-generic: " fmt , ## __VA_ARGS__); } while (0) #include #include "block/scsi.h" -#define SCSI_SENSE_BUF_SIZE 96 - #define SG_ERR_DRIVER_TIMEOUT 0x06 #define SG_ERR_DRIVER_SENSE 0x08 diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index c0c46d7f7c..e8bca390dd 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -60,7 +60,6 @@ #define VSCSI_MAX_SECTORS 4096 #define VSCSI_REQ_LIMIT 24 -#define SCSI_SENSE_BUF_SIZE 96 #define SRP_RSP_SENSE_DATA_LEN 18 typedef union vscsi_crq { diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index bf6da3d632..ca66454c4c 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -31,7 +31,7 @@ typedef struct SCSISense { uint8_t ascq; } SCSISense; -#define SCSI_SENSE_BUF_SIZE 96 +#define SCSI_SENSE_BUF_SIZE 252 struct SCSICommand { uint8_t buf[SCSI_CMD_BUF_SIZE]; -- cgit v1.2.1 From 703dd81aca15ef1d91dba013b6b66c6e3ff88628 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 23 Jan 2014 13:57:21 +0100 Subject: scsi: report thin provisioning errors with werror=report SCSI defines a status code for when a thin-provisioned LUNs would exceed the allocated space, map ENOSPC to it. Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 5 +++++ hw/scsi/scsi-disk.c | 3 +++ include/hw/scsi/scsi.h | 2 ++ 3 files changed, 10 insertions(+) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 50b89ad4aa..054a7d407a 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1367,6 +1367,11 @@ const struct SCSISense sense_code_WRITE_PROTECTED = { .key = DATA_PROTECT, .asc = 0x27, .ascq = 0x00 }; +/* Data Protection, Space Allocation Failed Write Protect */ +const struct SCSISense sense_code_SPACE_ALLOC_FAILED = { + .key = DATA_PROTECT, .asc = 0x27, .ascq = 0x07 +}; + /* * scsi_build_sense * diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index b4fadd2f24..d6141482aa 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -428,6 +428,9 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error) case EINVAL: scsi_check_condition(r, SENSE_CODE(INVALID_FIELD)); break; + case ENOSPC: + scsi_check_condition(r, SENSE_CODE(SPACE_ALLOC_FAILED)); + break; default: scsi_check_condition(r, SENSE_CODE(IO_ERROR)); break; diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index ca66454c4c..e5fc39d504 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -223,6 +223,8 @@ extern const struct SCSISense sense_code_REPORTED_LUNS_CHANGED; extern const struct SCSISense sense_code_DEVICE_INTERNAL_RESET; /* Data Protection, Write Protected */ extern const struct SCSISense sense_code_WRITE_PROTECTED; +/* Data Protection, Space Allocation Failed Write Protect */ +extern const struct SCSISense sense_code_SPACE_ALLOC_FAILED; #define SENSE_CODE(x) sense_code_ ## x -- cgit v1.2.1 From 7ef8cf9a0861b6f67f5e57428478c31bfd811651 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 29 Jan 2014 18:47:39 +0100 Subject: scsi-bus: Fix transfer length for VERIFY with BYTCHK=11b The transfer length depends on field BYTCHK, which is encoded in byte 1, bits 1..2. However, the guard for for case BYTCHK=11b doesn't work, and we get case 01b instead. Fix it. Note that since emulated scsi-hd fails the command outright, it takes SCSI passthrough of a device that actually implements VERIFY with BYTCHK=11b to make the bug bite. Screwed up in commit d12ad44. Spotted by Coverity. Cc: qemu-stable@nongnu.org Signed-off-by: Markus Armbruster Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 054a7d407a..50a0acf1fe 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -909,7 +909,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) case VERIFY_16: if ((buf[1] & 2) == 0) { cmd->xfer = 0; - } else if ((buf[1] & 4) == 1) { + } else if ((buf[1] & 4) != 0) { cmd->xfer = 1; } cmd->xfer *= dev->blocksize; -- cgit v1.2.1 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(-) 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.1 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(-) 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.1 From 64cc22841e72d37d577416f5836155ecd0a9bfb6 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 19 Feb 2014 08:28:41 -0800 Subject: scsi-disk: Add support for port WWN and index descriptors in VPD page 83h To make a VM more convincing to my application, it's useful to be able to add a port WWN and relative target port index to the descriptors returned for VPD page 83h. Add device properties to allow setting these, and return them from INQUIRY commands. Signed-off-by: Roland Dreier Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index d6141482aa..48a28ae199 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -75,6 +75,8 @@ struct SCSIDiskState bool media_event; bool eject_request; uint64_t wwn; + uint64_t port_wwn; + uint16_t port_index; uint64_t max_unmap_size; QEMUBH *bh; char *version; @@ -620,6 +622,24 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) stq_be_p(&outbuf[buflen], s->wwn); buflen += 8; } + + if (s->port_wwn) { + outbuf[buflen++] = 0x61; // SAS / Binary + outbuf[buflen++] = 0x93; // PIV / Target port / NAA + outbuf[buflen++] = 0; // reserved + outbuf[buflen++] = 8; + stq_be_p(&outbuf[buflen], s->port_wwn); + buflen += 8; + } + + if (s->port_index) { + outbuf[buflen++] = 0x61; // SAS / Binary + outbuf[buflen++] = 0x94; // PIV / Target port / relative target port + outbuf[buflen++] = 0; // reserved + outbuf[buflen++] = 4; + stw_be_p(&outbuf[buflen + 2], s->port_index); + buflen += 4; + } break; } case 0xb0: /* block limits */ @@ -2539,6 +2559,8 @@ static Property scsi_hd_properties[] = { DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, SCSI_DISK_F_DPOFUA, false), DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0), + DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, port_wwn, 0), + DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0), DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size, DEFAULT_MAX_UNMAP_SIZE), DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf), @@ -2587,6 +2609,8 @@ static const TypeInfo scsi_hd_info = { static Property scsi_cd_properties[] = { DEFINE_SCSI_DISK_PROPERTIES(), DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0), + DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, port_wwn, 0), + DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -2650,6 +2674,8 @@ static Property scsi_disk_properties[] = { DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, SCSI_DISK_F_DPOFUA, false), DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0), + DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, port_wwn, 0), + DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0), DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size, DEFAULT_MAX_UNMAP_SIZE), DEFINE_PROP_END_OF_LIST(), -- cgit v1.2.1 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(-) 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.1