From 5accc8408f2dac1e00d888e4fe75e6174ba2a940 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 2 Aug 2013 17:02:01 +0200 Subject: [PATCH 01/11] scsi: prefer UUID to VM name for the initiator name The UUID is unique even across multiple hosts, thus it is better than a VM name even if it is less user-friendly. Signed-off-by: Paolo Bonzini --- block/iscsi.c | 23 ++++++++++++++++------- include/sysemu/sysemu.h | 2 ++ stubs/Makefile.objs | 1 + stubs/uuid.c | 12 ++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 stubs/uuid.c diff --git a/block/iscsi.c b/block/iscsi.c index 813abd8fef..60b3967994 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -33,6 +33,8 @@ #include "trace.h" #include "block/scsi.h" #include "qemu/iov.h" +#include "sysemu/sysemu.h" +#include "qmp-commands.h" #include #include @@ -922,8 +924,9 @@ static char *parse_initiator_name(const char *target) { QemuOptsList *list; QemuOpts *opts; - const char *name = NULL; - const char *iscsi_name = qemu_get_vm_name(); + const char *name; + char *iscsi_name; + UuidInfo *uuid_info; list = qemu_find_opts("iscsi"); if (list) { @@ -933,16 +936,22 @@ static char *parse_initiator_name(const char *target) } if (opts) { name = qemu_opt_get(opts, "initiator-name"); + if (name) { + return g_strdup(name); + } } } - if (name) { - return g_strdup(name); + uuid_info = qmp_query_uuid(NULL); + if (strcmp(uuid_info->UUID, UUID_NONE) == 0) { + name = qemu_get_vm_name(); } else { - return g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s", - iscsi_name ? ":" : "", - iscsi_name ? iscsi_name : ""); + name = uuid_info->UUID; } + iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s", + name ? ":" : "", name ? name : ""); + qapi_free_UuidInfo(uuid_info); + return iscsi_name; } #if defined(LIBISCSI_FEATURE_NOP_COUNTER) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index b1aa059102..e2c6f58d9e 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -17,7 +17,9 @@ extern const char *bios_name; extern const char *qemu_name; extern uint8_t qemu_uuid[]; int qemu_uuid_parse(const char *str, uint8_t *uuid); + #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx" +#define UUID_NONE "00000000-0000-0000-0000-000000000000" bool runstate_check(RunState state); void runstate_set(RunState new_state); diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index f306cbada3..df92fe5b15 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -22,6 +22,7 @@ stub-obj-y += reset.o stub-obj-y += set-fd-handler.o stub-obj-y += slirp.o stub-obj-y += sysbus.o +stub-obj-y += uuid.o stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o diff --git a/stubs/uuid.c b/stubs/uuid.c new file mode 100644 index 0000000000..ffc0ed40ae --- /dev/null +++ b/stubs/uuid.c @@ -0,0 +1,12 @@ +#include "qemu-common.h" +#include "sysemu/sysemu.h" +#include "qmp-commands.h" + +UuidInfo *qmp_query_uuid(Error **errp) +{ + UuidInfo *info = g_malloc0(sizeof(*info)); + + info->UUID = g_strdup(UUID_NONE); + return info; +} + From eb37f14658df23c51f172bddb0c45eecf408927c Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Mon, 29 Jul 2013 14:49:29 +1000 Subject: [PATCH 02/11] spapr-vscsi: add task management At the moment the guest kernel issues two types of task management requests to the hypervisor - task about and lun reset. This adds handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(), free_request callback was implemented. As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB control byte does not seem to be used at all so NACA bit is not set to the guest so the guest has no good reason to call CLEAR_ACA task. Signed-off-by: Alexey Kardashevskiy [Fix choice of UCSOLCNT vs. SCSOLCNT. - Paolo] Signed-off-by: Paolo Bonzini --- hw/scsi/spapr_vscsi.c | 121 ++++++++++++++++++++++++++++++++---------- hw/scsi/srp.h | 7 +++ 2 files changed, 100 insertions(+), 28 deletions(-) diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index b2fcd4b3e3..c31a870486 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -117,6 +117,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s) return NULL; } +static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag) +{ + vscsi_req *req; + int i; + + for (i = 0; i < VSCSI_REQ_LIMIT; i++) { + req = &s->reqs[i]; + if (req->iu.srp.cmd.tag == srp_tag) { + return req; + } + } + return NULL; +} + static void vscsi_put_req(vscsi_req *req) { if (req->sreq != NULL) { @@ -755,40 +769,91 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req) static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req) { union viosrp_iu *iu = &req->iu; - int fn; + vscsi_req *tmpreq; + int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE; + SCSIDevice *d; + uint64_t tag = iu->srp.rsp.tag; + uint8_t sol_not = iu->srp.cmd.sol_not; fprintf(stderr, "vscsi_process_tsk_mgmt %02x\n", iu->srp.tsk_mgmt.tsk_mgmt_func); - switch (iu->srp.tsk_mgmt.tsk_mgmt_func) { -#if 0 /* We really don't deal with these for now */ - case SRP_TSK_ABORT_TASK: - fn = ABORT_TASK; - break; - case SRP_TSK_ABORT_TASK_SET: - fn = ABORT_TASK_SET; - break; - case SRP_TSK_CLEAR_TASK_SET: - fn = CLEAR_TASK_SET; - break; - case SRP_TSK_LUN_RESET: - fn = LOGICAL_UNIT_RESET; - break; - case SRP_TSK_CLEAR_ACA: - fn = CLEAR_ACA; - break; -#endif - default: - fn = 0; - } - if (fn) { - /* XXX Send/Handle target task management */ - ; + d = vscsi_device_find(&s->bus, be64_to_cpu(req->iu.srp.tsk_mgmt.lun), &lun); + if (!d) { + resp = SRP_TSK_MGMT_FIELDS_INVALID; } else { - vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0); - vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0); + switch (iu->srp.tsk_mgmt.tsk_mgmt_func) { + case SRP_TSK_ABORT_TASK: + if (d->lun != lun) { + resp = SRP_TSK_MGMT_FIELDS_INVALID; + break; + } + + tmpreq = vscsi_find_req(s, req->iu.srp.tsk_mgmt.task_tag); + if (tmpreq && tmpreq->sreq) { + assert(tmpreq->sreq->hba_private); + scsi_req_cancel(tmpreq->sreq); + } + break; + + case SRP_TSK_LUN_RESET: + if (d->lun != lun) { + resp = SRP_TSK_MGMT_FIELDS_INVALID; + break; + } + + qdev_reset_all(&d->qdev); + break; + + case SRP_TSK_ABORT_TASK_SET: + case SRP_TSK_CLEAR_TASK_SET: + if (d->lun != lun) { + resp = SRP_TSK_MGMT_FIELDS_INVALID; + break; + } + + for (i = 0; i < VSCSI_REQ_LIMIT; i++) { + tmpreq = &s->reqs[i]; + if (tmpreq->iu.srp.cmd.lun != req->iu.srp.tsk_mgmt.lun) { + continue; + } + if (!tmpreq->active || !tmpreq->sreq) { + continue; + } + assert(tmpreq->sreq->hba_private); + scsi_req_cancel(tmpreq->sreq); + } + break; + + case SRP_TSK_CLEAR_ACA: + resp = SRP_TSK_MGMT_NOT_SUPPORTED; + break; + + default: + resp = SRP_TSK_MGMT_FIELDS_INVALID; + break; + } } - return !fn; + + /* Compose the response here as */ + memset(iu, 0, sizeof(struct srp_rsp) + 4); + iu->srp.rsp.opcode = SRP_RSP; + iu->srp.rsp.req_lim_delta = cpu_to_be32(1); + iu->srp.rsp.tag = tag; + iu->srp.rsp.flags |= SRP_RSP_FLAG_RSPVALID; + iu->srp.rsp.resp_data_len = cpu_to_be32(4); + if (resp) { + iu->srp.rsp.sol_not = (sol_not & 0x04) >> 2; + } else { + iu->srp.rsp.sol_not = (sol_not & 0x02) >> 1; + } + + iu->srp.rsp.status = GOOD; + iu->srp.rsp.data[3] = resp; + + vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT); + + return 1; } static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req) diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h index 5e0cad5c19..d27f31d2d5 100644 --- a/hw/scsi/srp.h +++ b/hw/scsi/srp.h @@ -90,6 +90,13 @@ enum { SRP_REV16A_IB_IO_CLASS = 0x0100 }; +enum { + SRP_TSK_MGMT_COMPLETE = 0x00, + SRP_TSK_MGMT_FIELDS_INVALID = 0x02, + SRP_TSK_MGMT_NOT_SUPPORTED = 0x04, + SRP_TSK_MGMT_FAILED = 0x05 +}; + struct srp_direct_buf { uint64_t va; uint32_t key; From a27292b5d7545509bfa171922516d2033c570205 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 19 Aug 2013 17:53:15 +0200 Subject: [PATCH 03/11] virtio-scsi: Make type virtio-scsi-common abstract It's the abstract base of virtio-scsi-device and vhost-scsi. Signed-off-by: Markus Armbruster Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3bd690d128..26d95a14ec 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -693,6 +693,7 @@ static const TypeInfo virtio_scsi_common_info = { .name = TYPE_VIRTIO_SCSI_COMMON, .parent = TYPE_VIRTIO_DEVICE, .instance_size = sizeof(VirtIOSCSICommon), + .abstract = true, .class_init = virtio_scsi_common_class_init, }; From c24e7517ee4a98e90eee5f0f07708a1fa12326b3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 23 Aug 2013 18:01:58 +0200 Subject: [PATCH 04/11] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scsi_bus_legacy_add_drive() creates either a scsi-disk or a scsi-generic device. It sets property "serial" to argument serial unless null. Crashes with scsi-generic, because it doesn't have such the property. Only usb_msd_initfn_storage() passes non-null serial. Reproducer: $ qemu-system-x86_64 -nodefaults -display none -S -usb \ -drive if=none,file=/dev/sg1,id=usb-drv0 \ -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123 qemu-system-x86_64: -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not found Aborted (core dumped) Fix by handling exactly like "removable": set the property only when it exists. Cc: qemu-stable@nongnu.org Reviewed-by: Andreas Färber 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 5cd6137046..4d36841d40 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -224,7 +224,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, if (object_property_find(OBJECT(dev), "removable", NULL)) { qdev_prop_set_bit(dev, "removable", removable); } - if (serial) { + if (serial && object_property_find(OBJECT(dev), "serial", NULL)) { qdev_prop_set_string(dev, "serial", serial); } if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) { From 927941059b95c3cc6264241115a23b1d14f1beb0 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 23 Aug 2013 17:16:32 +0100 Subject: [PATCH 05/11] hw/scsi/lsi53c895a: Use sextract32 for sign-extension Use sextract32() for doing sign-extension rather than rolling our own implementation. Signed-off-by: Peter Maydell Signed-off-by: Paolo Bonzini --- hw/scsi/lsi53c895a.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index 0c36842729..a26b20b19e 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -998,12 +998,6 @@ bad: s->msg_action = 0; } -/* Sign extend a 24-bit value. */ -static inline int32_t sxt24(int32_t n) -{ - return (n << 8) >> 8; -} - #define LSI_BUF_SIZE 4096 static void lsi_memcpy(LSIState *s, uint32_t dest, uint32_t src, int count) { @@ -1083,7 +1077,7 @@ again: /* Table indirect addressing. */ /* 32-bit Table indirect */ - offset = sxt24(addr); + offset = sextract32(addr, 0, 24); pci_dma_read(pci_dev, s->dsa + offset, buf, 8); /* byte count is stored in bits 0:23 only */ s->dbc = cpu_to_le32(buf[0]) & 0xffffff; @@ -1183,13 +1177,13 @@ again: uint32_t id; if (insn & (1 << 25)) { - id = read_dword(s, s->dsa + sxt24(insn)); + id = read_dword(s, s->dsa + sextract32(insn, 0, 24)); } else { id = insn; } id = (id >> 16) & 0xf; if (insn & (1 << 26)) { - addr = s->dsp + sxt24(addr); + addr = s->dsp + sextract32(addr, 0, 24); } s->dnad = addr; switch (opcode) { @@ -1385,7 +1379,7 @@ again: if (cond == jmp) { if (insn & (1 << 23)) { /* Relative address. */ - addr = s->dsp + sxt24(addr); + addr = s->dsp + sextract32(addr, 0, 24); } switch ((insn >> 27) & 7) { case 0: /* Jump */ @@ -1438,7 +1432,7 @@ again: int i; if (insn & (1 << 28)) { - addr = s->dsa + sxt24(addr); + addr = s->dsa + sextract32(addr, 0, 24); } n = (insn & 7); reg = (insn >> 16) & 0xff; From 57ffcc4c83331cb4b2639a866b6ecc11b3e7092c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 23 Aug 2013 17:16:33 +0100 Subject: [PATCH 06/11] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask Use deposit32() rather than handcoded shifts/masks to update the scratch registers. This is cleaner and incidentally avoids a clang sanitizer complaint ("runtime error: left shift of 255 by 24 places cannot be represented in type 'int'"). Signed-off-by: Peter Maydell Signed-off-by: Paolo Bonzini --- hw/scsi/lsi53c895a.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index a26b20b19e..5affc82d2b 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -1870,8 +1870,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val) int shift; n = (offset - 0x58) >> 2; shift = (offset & 3) * 8; - s->scratch[n] &= ~(0xff << shift); - s->scratch[n] |= (val & 0xff) << shift; + s->scratch[n] = deposit32(s->scratch[n], shift, 8, val); } else { BADF("Unhandled writeb 0x%x = 0x%x\n", offset, val); } From f18a7cbb0992a02225d26afd336aaf47de75e11c Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 19 Jul 2013 09:19:39 +0200 Subject: [PATCH 07/11] iscsi: add logical block provisioning information to iscsilun Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index 60b3967994..bfd659a0ca 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -52,6 +52,10 @@ typedef struct IscsiLun { uint64_t num_blocks; int events; QEMUTimer *nop_timer; + uint8_t lbpme; + uint8_t lbprz; + struct scsi_inquiry_logical_block_provisioning lbp; + struct scsi_inquiry_block_limits bl; } IscsiLun; typedef struct IscsiAIOCB { @@ -999,6 +1003,8 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun) } else { iscsilun->block_size = rc16->block_length; iscsilun->num_blocks = rc16->returned_lba + 1; + iscsilun->lbpme = rc16->lbpme; + iscsilun->lbprz = rc16->lbprz; } } break; @@ -1051,6 +1057,37 @@ static QemuOptsList runtime_opts = { }, }; +static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, + int lun, int evpd, int pc) { + int full_size; + struct scsi_task *task = NULL; + task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64); + if (task == NULL || task->status != SCSI_STATUS_GOOD) { + goto fail; + } + full_size = scsi_datain_getfullsize(task); + if (full_size > task->datain.size) { + scsi_free_scsi_task(task); + + /* we need more data for the full list */ + task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size); + if (task == NULL || task->status != SCSI_STATUS_GOOD) { + goto fail; + } + } + + return task; + +fail: + error_report("iSCSI: Inquiry command failed : %s", + iscsi_get_error(iscsi)); + if (task) { + scsi_free_scsi_task(task); + return NULL; + } + return NULL; +} + /* * We support iscsi url's on the form * iscsi://[%@][:]// @@ -1180,6 +1217,46 @@ 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); + if (task == NULL) { + ret = -EINVAL; + goto out; + } + inq_lbp = scsi_datain_unmarshall(task); + if (inq_lbp == NULL) { + error_report("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; + } + + if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { + struct scsi_inquiry_block_limits *inq_bl; + task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, + SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS); + if (task == NULL) { + ret = -EINVAL; + goto out; + } + inq_bl = scsi_datain_unmarshall(task); + if (inq_bl == NULL) { + error_report("iSCSI: failed to unmarshall inquiry datain blob"); + ret = -EINVAL; + goto out; + } + memcpy(&iscsilun->bl, inq_bl, + sizeof(struct scsi_inquiry_block_limits)); + scsi_free_scsi_task(task); + task = NULL; + } + #if defined(LIBISCSI_FEATURE_NOP_COUNTER) /* Set up a timer for sending out iSCSI NOPs */ iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, iscsi_nop_timed_event, iscsilun); From 54a5c1d5db47b4146490937ed73e3f56022aaba6 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 19 Jul 2013 09:19:40 +0200 Subject: [PATCH 08/11] iscsi: add .bdrv_get_block_status this patch adds a coroutine for .bdrv_co_block_status as well as a generic framework that can be used to build coroutines in block/iscsi. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index bfd659a0ca..c377d21589 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -58,6 +58,15 @@ typedef struct IscsiLun { struct scsi_inquiry_block_limits bl; } IscsiLun; +typedef struct IscsiTask { + int status; + int complete; + int retries; + int do_retry; + struct scsi_task *task; + Coroutine *co; +} IscsiTask; + typedef struct IscsiAIOCB { BlockDriverAIOCB common; QEMUIOVector *qiov; @@ -111,6 +120,41 @@ iscsi_schedule_bh(IscsiAIOCB *acb) qemu_bh_schedule(acb->bh); } +static void +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, + void *command_data, void *opaque) +{ + struct IscsiTask *iTask = opaque; + struct scsi_task *task = command_data; + + iTask->complete = 1; + iTask->status = status; + iTask->do_retry = 0; + iTask->task = task; + + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { + iTask->do_retry = 1; + goto out; + } + + if (status != SCSI_STATUS_GOOD) { + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); + } + +out: + if (iTask->co) { + qemu_coroutine_enter(iTask->co, NULL); + } +} + +static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) +{ + *iTask = (struct IscsiTask) { + .co = qemu_coroutine_self(), + .retries = ISCSI_CMD_RETRIES, + }; +} static void iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, @@ -848,6 +892,96 @@ iscsi_getlength(BlockDriverState *bs) return len; } +static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, + int64_t sector_num, + int nb_sectors, int *pnum) +{ + IscsiLun *iscsilun = bs->opaque; + struct scsi_get_lba_status *lbas = NULL; + struct scsi_lba_status_descriptor *lbasd = NULL; + struct IscsiTask iTask; + int64_t ret; + + iscsi_co_init_iscsitask(iscsilun, &iTask); + + if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { + ret = -EINVAL; + goto out; + } + + /* default to all sectors allocated */ + ret = BDRV_BLOCK_DATA; + ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID; + *pnum = nb_sectors; + + /* LUN does not support logical block provisioning */ + if (iscsilun->lbpme == 0) { + goto out; + } + +retry: + if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, + sector_qemu2lun(sector_num, iscsilun), + 8 + 16, iscsi_co_generic_cb, + &iTask) == NULL) { + ret = -EIO; + goto out; + } + + while (!iTask.complete) { + iscsi_set_events(iscsilun); + qemu_coroutine_yield(); + } + + if (iTask.do_retry) { + if (iTask.task != NULL) { + scsi_free_scsi_task(iTask.task); + iTask.task = NULL; + } + goto retry; + } + + if (iTask.status != SCSI_STATUS_GOOD) { + /* in case the get_lba_status_callout fails (i.e. + * because the device is busy or the cmd is not + * supported) we pretend all blocks are allocated + * for backwards compatiblity */ + goto out; + } + + lbas = scsi_datain_unmarshall(iTask.task); + if (lbas == NULL) { + ret = -EIO; + goto out; + } + + lbasd = &lbas->descriptors[0]; + + if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) { + ret = -EIO; + goto out; + } + + *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun); + if (*pnum > nb_sectors) { + *pnum = nb_sectors; + } + + if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED || + lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) { + ret &= ~BDRV_BLOCK_DATA; + if (iscsilun->lbprz) { + ret |= BDRV_BLOCK_ZERO; + } + } + +out: + if (iTask.task != NULL) { + scsi_free_scsi_task(iTask.task); + } + return ret; +} + static int parse_chap(struct iscsi_context *iscsi, const char *target) { QemuOptsList *list; @@ -1398,6 +1532,8 @@ static BlockDriver bdrv_iscsi = { .bdrv_getlength = iscsi_getlength, .bdrv_truncate = iscsi_truncate, + .bdrv_co_get_block_status = iscsi_co_get_block_status, + .bdrv_aio_readv = iscsi_aio_readv, .bdrv_aio_writev = iscsi_aio_writev, .bdrv_aio_flush = iscsi_aio_flush, From 65f3e33964bc4bb634d61463814a4ccca794e3c0 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 19 Jul 2013 09:19:41 +0200 Subject: [PATCH 09/11] iscsi: split discard requests in multiple parts Replace .bdrv_aio_discard with .bdrv_co_discard so that discard requests can be split in multiple parts, each for a small amount of sectors. This is useful because we expose a generic API with no limit on the amount of sectors that can be unmapped in one request. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 156 +++++++++++++++++++++++--------------------------- 1 file changed, 73 insertions(+), 83 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index c377d21589..68f99d3195 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -87,6 +87,7 @@ typedef struct IscsiAIOCB { #define NOP_INTERVAL 5000 #define MAX_NOP_FAILURES 3 #define ISCSI_CMD_RETRIES 5 +#define ISCSI_MAX_UNMAP 131072 static void iscsi_bh_cb(void *p) @@ -618,88 +619,6 @@ iscsi_aio_flush(BlockDriverState *bs, return &acb->common; } -static int iscsi_aio_discard_acb(IscsiAIOCB *acb); - -static void -iscsi_unmap_cb(struct iscsi_context *iscsi, int status, - void *command_data, void *opaque) -{ - IscsiAIOCB *acb = opaque; - - if (acb->canceled != 0) { - return; - } - - acb->status = 0; - if (status != 0) { - if (status == SCSI_STATUS_CHECK_CONDITION - && acb->task->sense.key == SCSI_SENSE_UNIT_ATTENTION - && acb->retries-- > 0) { - scsi_free_scsi_task(acb->task); - acb->task = NULL; - if (iscsi_aio_discard_acb(acb) == 0) { - iscsi_set_events(acb->iscsilun); - return; - } - } - error_report("Failed to unmap data on iSCSI lun. %s", - iscsi_get_error(iscsi)); - acb->status = -EIO; - } - - iscsi_schedule_bh(acb); -} - -static int iscsi_aio_discard_acb(IscsiAIOCB *acb) { - struct iscsi_context *iscsi = acb->iscsilun->iscsi; - struct unmap_list list[1]; - - acb->canceled = 0; - acb->bh = NULL; - acb->status = -EINPROGRESS; - acb->buf = NULL; - - list[0].lba = sector_qemu2lun(acb->sector_num, acb->iscsilun); - list[0].num = acb->nb_sectors * BDRV_SECTOR_SIZE / acb->iscsilun->block_size; - - acb->task = iscsi_unmap_task(iscsi, acb->iscsilun->lun, - 0, 0, &list[0], 1, - iscsi_unmap_cb, - acb); - if (acb->task == NULL) { - error_report("iSCSI: Failed to send unmap command. %s", - iscsi_get_error(iscsi)); - return -1; - } - - return 0; -} - -static BlockDriverAIOCB * -iscsi_aio_discard(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque) -{ - IscsiLun *iscsilun = bs->opaque; - IscsiAIOCB *acb; - - acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque); - - acb->iscsilun = iscsilun; - acb->nb_sectors = nb_sectors; - acb->sector_num = sector_num; - acb->retries = ISCSI_CMD_RETRIES; - - if (iscsi_aio_discard_acb(acb) != 0) { - qemu_aio_release(acb); - return NULL; - } - - iscsi_set_events(iscsilun); - - return &acb->common; -} - #ifdef __linux__ static void iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, @@ -982,6 +901,77 @@ out: return ret; } +static int +coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num, + int nb_sectors) +{ + IscsiLun *iscsilun = bs->opaque; + struct IscsiTask iTask; + struct unmap_list list; + uint32_t nb_blocks; + uint32_t max_unmap; + + if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { + return -EINVAL; + } + + if (!iscsilun->lbp.lbpu) { + /* UNMAP is not supported by the target */ + return 0; + } + + list.lba = sector_qemu2lun(sector_num, iscsilun); + nb_blocks = sector_qemu2lun(nb_sectors, iscsilun); + + max_unmap = iscsilun->bl.max_unmap; + if (max_unmap == 0xffffffff) { + max_unmap = ISCSI_MAX_UNMAP; + } + + while (nb_blocks > 0) { + iscsi_co_init_iscsitask(iscsilun, &iTask); + list.num = nb_blocks; + if (list.num > max_unmap) { + list.num = max_unmap; + } +retry: + if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1, + iscsi_co_generic_cb, &iTask) == NULL) { + return -EIO; + } + + while (!iTask.complete) { + iscsi_set_events(iscsilun); + qemu_coroutine_yield(); + } + + if (iTask.task != NULL) { + scsi_free_scsi_task(iTask.task); + iTask.task = NULL; + } + + if (iTask.do_retry) { + goto retry; + } + + if (iTask.status == SCSI_STATUS_CHECK_CONDITION) { + /* the target might fail with a check condition if it + is not happy with the alignment of the UNMAP request + we silently fail in this case */ + return 0; + } + + if (iTask.status != SCSI_STATUS_GOOD) { + return -EIO; + } + + list.lba += list.num; + nb_blocks -= list.num; + } + + return 0; +} + static int parse_chap(struct iscsi_context *iscsi, const char *target) { QemuOptsList *list; @@ -1533,12 +1523,12 @@ static BlockDriver bdrv_iscsi = { .bdrv_truncate = iscsi_truncate, .bdrv_co_get_block_status = iscsi_co_get_block_status, + .bdrv_co_discard = iscsi_co_discard, .bdrv_aio_readv = iscsi_aio_readv, .bdrv_aio_writev = iscsi_aio_writev, .bdrv_aio_flush = iscsi_aio_flush, - .bdrv_aio_discard = iscsi_aio_discard, .bdrv_has_zero_init = iscsi_has_zero_init, #ifdef __linux__ From 26573a0c1f0427be168a537abccbcc942e07aa72 Mon Sep 17 00:00:00 2001 From: "Nikunj A. Dadhania" Date: Mon, 2 Sep 2013 10:33:38 +0530 Subject: [PATCH 10/11] spapr-vscsi: Adding VSCSI capabilities This implements capabilities exchange between vscsi host and client. As at the moment no capability is supported, put zero flags everywhere and return. Signed-off-by: Nikunj A Dadhania Signed-off-by: Paolo Bonzini Signed-off-by: Nikunj A. Dadhania --- hw/scsi/spapr_vscsi.c | 54 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index c31a870486..f206452c74 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -923,6 +923,57 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req *req) return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT); } +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req) +{ + struct viosrp_capabilities *vcap; + struct capabilities cap = { }; + uint16_t len, req_len; + uint64_t buffer; + int rc; + + vcap = &req->iu.mad.capabilities; + req_len = len = be16_to_cpu(vcap->common.length); + buffer = be64_to_cpu(vcap->buffer); + if (len > sizeof(cap)) { + fprintf(stderr, "vscsi_send_capabilities: capabilities size mismatch !\n"); + + /* + * Just read and populate the structure that is known. + * Zero rest of the structure. + */ + len = sizeof(cap); + } + rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); + } + + /* + * Current implementation does not suppport any migration or + * reservation capabilities. Construct the response telling the + * guest not to use them. + */ + cap.flags = 0; + cap.migration.ecl = 0; + cap.reserve.type = 0; + cap.migration.common.server_support = 0; + cap.reserve.common.server_support = 0; + + rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n"); + } + if (req_len > len) { + /* + * Being paranoid and lets not worry about the error code + * here. Actual write of the cap is done above. + */ + spapr_vio_dma_set(&s->vdev, (buffer + len), 0, (req_len - len)); + } + vcap->common.status = rc ? cpu_to_be32(1) : 0; + return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT); +} + static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) { union mad_iu *mad = &req->iu.mad; @@ -943,6 +994,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) mad->host_config.common.status = cpu_to_be16(1); vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT); break; + case VIOSRP_CAPABILITIES_TYPE: + vscsi_send_capabilities(s, req); + break; default: fprintf(stderr, "VSCSI: Unknown MAD type %02x\n", be32_to_cpu(mad->empty_iu.common.type)); From f4ff3b7ba1bcb77d5b5cdbd6e695df793761170b Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Thu, 29 Aug 2013 18:13:25 +1000 Subject: [PATCH 11/11] spapr-vscsi: Report error on unsupported MAD requests The existing driver just dropped unsupported requests. This adds error responses to those unhandled requests. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Paolo Bonzini --- hw/scsi/spapr_vscsi.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index f206452c74..2a26042701 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -977,29 +977,43 @@ static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req) static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) { union mad_iu *mad = &req->iu.mad; + bool request_handled = false; + uint64_t retlen = 0; switch (be32_to_cpu(mad->empty_iu.common.type)) { case VIOSRP_EMPTY_IU_TYPE: fprintf(stderr, "Unsupported EMPTY MAD IU\n"); + retlen = sizeof(mad->empty_iu); break; case VIOSRP_ERROR_LOG_TYPE: fprintf(stderr, "Unsupported ERROR LOG MAD IU\n"); - mad->error_log.common.status = cpu_to_be16(1); - vscsi_send_iu(s, req, sizeof(mad->error_log), VIOSRP_MAD_FORMAT); + retlen = sizeof(mad->error_log); break; case VIOSRP_ADAPTER_INFO_TYPE: vscsi_send_adapter_info(s, req); + request_handled = true; break; case VIOSRP_HOST_CONFIG_TYPE: - mad->host_config.common.status = cpu_to_be16(1); - vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT); + retlen = sizeof(mad->host_config); break; case VIOSRP_CAPABILITIES_TYPE: vscsi_send_capabilities(s, req); + request_handled = true; break; default: fprintf(stderr, "VSCSI: Unknown MAD type %02x\n", be32_to_cpu(mad->empty_iu.common.type)); + /* + * PAPR+ says that "The length field is set to the length + * of the data structure(s) used in the command". + * As we did not recognize the request type, put zero there. + */ + retlen = 0; + } + + if (!request_handled) { + mad->empty_iu.common.status = cpu_to_be16(VIOSRP_MAD_NOT_SUPPORTED); + vscsi_send_iu(s, req, retlen, VIOSRP_MAD_FORMAT); } return 1;