From efc6de0d0eb21bbd8fbc90da1faf7dd8ed9f5321 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 30 May 2014 23:36:47 +0200 Subject: [PATCH 01/18] block/iscsi: handle BUSY condition this patch adds handling of BUSY status reponse from an iSCSI target. Currently, we fail with -EIO in case of SCSI_STATUS_BUSY while the obvious reaction would be to retry the operation after some time. The retry time is randomly choosen from a range with exponential growth increasing with each retry. This patch includes most of the changes by a an upcoming patch from Stefan Hajnoczi: iscsi: implement .bdrv_detach/attach_aio_context() because I also need the reference to the aio_context for the retry timer to work. I included the changes to maintain better mergeability. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 55 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 6f87605e72..e64659ffda 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -26,6 +26,7 @@ #include "config-host.h" #include +#include #include #include "qemu-common.h" #include "qemu/config-file.h" @@ -75,6 +76,7 @@ typedef struct IscsiTask { Coroutine *co; QEMUBH *bh; IscsiLun *iscsilun; + QEMUTimer retry_timer; } IscsiTask; typedef struct IscsiAIOCB { @@ -86,7 +88,6 @@ typedef struct IscsiAIOCB { uint8_t *buf; int status; int canceled; - int retries; int64_t sector_num; int nb_sectors; #ifdef __linux__ @@ -96,7 +97,8 @@ typedef struct IscsiAIOCB { #define NOP_INTERVAL 5000 #define MAX_NOP_FAILURES 3 -#define ISCSI_CMD_RETRIES 5 +#define ISCSI_CMD_RETRIES ARRAY_SIZE(iscsi_retry_times) +static const unsigned iscsi_retry_times[] = {8, 32, 128, 512, 2048}; /* this threshhold is a trade-off knob to choose between * the potential additional overhead of an extra GET_LBA_STATUS request @@ -146,6 +148,19 @@ static void iscsi_co_generic_bh_cb(void *opaque) qemu_coroutine_enter(iTask->co, NULL); } +static void iscsi_retry_timer_expired(void *opaque) +{ + struct IscsiTask *iTask = opaque; + if (iTask->co) { + qemu_coroutine_enter(iTask->co, NULL); + } +} + +static inline unsigned exp_random(double mean) +{ + return -mean * log((double)rand() / RAND_MAX); +} + static void iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, void *command_data, void *opaque) @@ -158,14 +173,30 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, iTask->do_retry = 0; iTask->task = task; - 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) { + if (iTask->retries++ < ISCSI_CMD_RETRIES) { + if (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_BUSY) { + unsigned retry_time = + exp_random(iscsi_retry_times[iTask->retries - 1]); + error_report("iSCSI Busy (retry #%u in %u ms): %s", + iTask->retries, retry_time, + iscsi_get_error(iscsi)); + aio_timer_init(iTask->iscsilun->aio_context, + &iTask->retry_timer, QEMU_CLOCK_REALTIME, + SCALE_MS, iscsi_retry_timer_expired, iTask); + timer_mod(&iTask->retry_timer, + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + retry_time); + iTask->do_retry = 1; + return; + } + } error_report("iSCSI Failure: %s", iscsi_get_error(iscsi)); } @@ -180,9 +211,9 @@ out: static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) { *iTask = (struct IscsiTask) { - .co = qemu_coroutine_self(), - .retries = ISCSI_CMD_RETRIES, - .iscsilun = iscsilun, + .co = qemu_coroutine_self(), + .retries = ISCSI_CMD_RETRIES, + .iscsilun = iscsilun, }; } From fcd470d857a02ad76cd5a351a80bc0bb46588314 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 10 Jun 2014 09:52:16 +0200 Subject: [PATCH 02/18] block/iscsi: fix potential segfault on early callback it might happen in the future that a function directly invokes its callback. In this case we end up in a segfault because the iTask is gone when the BH is scheduled. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index e64659ffda..09485fe6b2 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -144,6 +144,7 @@ iscsi_schedule_bh(IscsiAIOCB *acb) static void iscsi_co_generic_bh_cb(void *opaque) { struct IscsiTask *iTask = opaque; + iTask->complete = 1; qemu_bh_delete(iTask->bh); qemu_coroutine_enter(iTask->co, NULL); } @@ -151,6 +152,7 @@ static void iscsi_co_generic_bh_cb(void *opaque) static void iscsi_retry_timer_expired(void *opaque) { struct IscsiTask *iTask = opaque; + iTask->complete = 1; if (iTask->co) { qemu_coroutine_enter(iTask->co, NULL); } @@ -168,7 +170,6 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, struct IscsiTask *iTask = opaque; struct scsi_task *task = command_data; - iTask->complete = 1; iTask->status = status; iTask->do_retry = 0; iTask->task = task; @@ -205,6 +206,8 @@ out: iTask->bh = aio_bh_new(iTask->iscsilun->aio_context, iscsi_co_generic_bh_cb, iTask); qemu_bh_schedule(iTask->bh); + } else { + iTask->complete = 1; } } From 9281fe9eea5c9e5037fbf4fb75c5dc8f6e1a24a3 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 4 Jun 2014 15:47:39 +0200 Subject: [PATCH 03/18] block/iscsi: use 16 byte CDBs only when necessary this patch changes the driver to uses 16 Byte CDBs for READ/WRITE only if the target requires 64bit lba addressing. On one hand this saves 6 bytes in each PDU on the other hand it seems that 10 Byte CDBs seems to be much better supported and tested as a recent issue I had with a major storage supplier lined out. For WRITESAME the logic is a bit more tricky as WRITESAME10 with UNMAP was added really late. Thus a fallback to WRITESAME16 is possible if it supports UNMAP and WRITESAME10 not. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 58 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 09485fe6b2..38754872fa 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -65,6 +65,7 @@ typedef struct IscsiLun { unsigned char *zeroblock; unsigned long *allocationmap; int cluster_sectors; + bool use_16_for_rw; } IscsiLun; typedef struct IscsiTask { @@ -381,10 +382,17 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, #endif iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, - data, num_sectors * iscsilun->block_size, - iscsilun->block_size, 0, 0, 0, 0, 0, - iscsi_co_generic_cb, &iTask); + if (iscsilun->use_16_for_rw) { + iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, + data, num_sectors * iscsilun->block_size, + iscsilun->block_size, 0, 0, 0, 0, 0, + iscsi_co_generic_cb, &iTask); + } else { + iTask.task = iscsi_write10_task(iscsilun->iscsi, iscsilun->lun, lba, + data, num_sectors * iscsilun->block_size, + iscsilun->block_size, 0, 0, 0, 0, 0, + iscsi_co_generic_cb, &iTask); + } if (iTask.task == NULL) { g_free(buf); return -ENOMEM; @@ -570,14 +578,12 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - switch (iscsilun->type) { - case TYPE_DISK: + if (iscsilun->use_16_for_rw) { iTask.task = iscsi_read16_task(iscsilun->iscsi, iscsilun->lun, lba, num_sectors * iscsilun->block_size, iscsilun->block_size, 0, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask); - break; - default: + } else { iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba, num_sectors * iscsilun->block_size, iscsilun->block_size, @@ -585,7 +591,6 @@ retry: 0, 0, 0, 0, 0, #endif iscsi_co_generic_cb, &iTask); - break; } if (iTask.task == NULL) { return -ENOMEM; @@ -921,19 +926,27 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, struct IscsiTask iTask; uint64_t lba; uint32_t nb_blocks; + bool use_16_for_ws = iscsilun->use_16_for_rw; if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { return -EINVAL; } - if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) { - /* WRITE SAME with UNMAP is not supported by the target, - * fall back and try WRITE SAME without UNMAP */ - flags &= ~BDRV_REQ_MAY_UNMAP; + if (flags & BDRV_REQ_MAY_UNMAP) { + if (!use_16_for_ws && !iscsilun->lbp.lbpws10) { + /* WRITESAME10 with UNMAP is unsupported try WRITESAME16 */ + use_16_for_ws = true; + } + if (use_16_for_ws && !iscsilun->lbp.lbpws) { + /* WRITESAME16 with UNMAP is not supported by the target, + * fall back and try WRITESAME10/16 without UNMAP */ + flags &= ~BDRV_REQ_MAY_UNMAP; + use_16_for_ws = iscsilun->use_16_for_rw; + } } if (!(flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->has_write_same) { - /* WRITE SAME without UNMAP is not supported by the target */ + /* WRITESAME without UNMAP is not supported by the target */ return -ENOTSUP; } @@ -946,10 +959,18 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - if (iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba, - iscsilun->zeroblock, iscsilun->block_size, - nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), - 0, 0, iscsi_co_generic_cb, &iTask) == NULL) { + if (use_16_for_ws) { + iTask.task = iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba, + iscsilun->zeroblock, iscsilun->block_size, + nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), + 0, 0, iscsi_co_generic_cb, &iTask); + } else { + iTask.task = iscsi_writesame10_task(iscsilun->iscsi, iscsilun->lun, lba, + iscsilun->zeroblock, iscsilun->block_size, + nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP), + 0, 0, iscsi_co_generic_cb, &iTask); + } + if (iTask.task == NULL) { return -ENOMEM; } @@ -1147,6 +1168,7 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) iscsilun->num_blocks = rc16->returned_lba + 1; iscsilun->lbpme = rc16->lbpme; iscsilun->lbprz = rc16->lbprz; + iscsilun->use_16_for_rw = (rc16->returned_lba > 0xffffffff); } } break; From 4525c1337f2a5fc99570ce392e552b8905e36c9a Mon Sep 17 00:00:00 2001 From: Paul Janzen Date: Wed, 4 Jun 2014 22:33:40 -0700 Subject: [PATCH 04/18] scsi-disk.c: Fix compilation with -DDEBUG_SCSI In scsi-disk.c, if you #define DEBUG_SCSI=1, you get: hw/scsi/scsi-disk.c: In function 'scsi_disk_emulate_command': hw/scsi/scsi-disk.c:2018: error: 'SCSIRequest' has no member named 'buf' Change the debugging statement to match the actual value tested. Signed-off-by: Paul Janzen Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index fc6e32ada5..9afbb8abf5 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2015,7 +2015,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) case VERIFY_10: case VERIFY_12: case VERIFY_16: - DPRINTF("Verify (bytchk %lu)\n", (r->req.buf[1] >> 1) & 3); + DPRINTF("Verify (bytchk %d)\n", (req->cmd.buf[1] >> 1) & 3); if (req->cmd.buf[1] & 6) { goto illegal_request; } From 2fe5a9f73b3446690db2cae8a58473b0b4beaa32 Mon Sep 17 00:00:00 2001 From: Ulrich Obergfell Date: Sun, 8 Jun 2014 19:22:33 +0200 Subject: [PATCH 05/18] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c This patch fixes a bug in scsi_block_new_request() that was introduced by commit 137745c5c60f083ec982fe9e861e8c16ebca1ba8. If the host cache is used - i.e. if BDRV_O_NOCACHE is _not_ set - the 'break' statement needs to be executed to 'fall back' to SG_IO. Cc: qemu-stable@nongnu.org Signed-off-by: Ulrich Obergfell Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 9afbb8abf5..fd82a410b5 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2526,7 +2526,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, * ones (such as WRITE SAME or EXTENDED COPY, etc.). So, without * O_DIRECT everything must go through SG_IO. */ - if (bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE) { + if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) { break; } From b9e77bc718db3593ffbb56f3a51488cdce4d5af3 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Thu, 12 Jun 2014 15:41:37 +1000 Subject: [PATCH 06/18] scsi: Print command name in debug This makes scsi_command_name() public. This makes use of scsi_command_name() in debug output for scsi-disk and spapr-vscsi host bus adapter. Before this, SCSI used to print hex numbers instead of human-friendly strings. This adds GET_EVENT_STATUS_NOTIFICATION and READ_DISC_INFORMATION to the list of SCSI commands supported by scsi_command_name(). Signed-off-by: Alexey Kardashevskiy Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 4 +++- hw/scsi/scsi-disk.c | 3 ++- hw/scsi/spapr_vscsi.c | 5 +++-- include/block/scsi.h | 2 ++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 003d2843df..ea1ac09c8a 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1429,7 +1429,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len, } } -static const char *scsi_command_name(uint8_t cmd) +const char *scsi_command_name(uint8_t cmd) { static const char *names[] = { [ TEST_UNIT_READY ] = "TEST_UNIT_READY", @@ -1545,6 +1545,8 @@ static const char *scsi_command_name(uint8_t cmd) [ SET_READ_AHEAD ] = "SET_READ_AHEAD", [ ALLOW_OVERWRITE ] = "ALLOW_OVERWRITE", [ MECHANISM_STATUS ] = "MECHANISM_STATUS", + [ GET_EVENT_STATUS_NOTIFICATION ] = "GET_EVENT_STATUS_NOTIFICATION", + [ READ_DISC_INFORMATION ] = "READ_DISC_INFORMATION", }; if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index fd82a410b5..a529ad24c7 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2027,7 +2027,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) (long)r->req.cmd.xfer); break; default: - DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]); + DPRINTF("Unknown SCSI command (%2.2x=%s)\n", buf[0], + scsi_command_name(buf[0])); scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE)); return 0; } diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index f96b7af791..048cfc7b05 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -799,8 +799,9 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req) req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req); n = scsi_req_enqueue(req->sreq); - DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x LUN %d ret: %d\n", - req->qtag, srp->cmd.cdb[0], lun, n); + DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x=%s LUN %d ret: %d\n", + req->qtag, srp->cmd.cdb[0], scsi_command_name(srp->cmd.cdb[0]), + lun, n); if (n) { /* Transfer direction must be set before preprocessing the diff --git a/include/block/scsi.h b/include/block/scsi.h index 9ab045b613..edde960d18 100644 --- a/include/block/scsi.h +++ b/include/block/scsi.h @@ -143,6 +143,8 @@ #define READ_CD 0xbe #define SEND_DVD_STRUCTURE 0xbf +const char *scsi_command_name(uint8_t cmd); + /* * SERVICE ACTION IN subcodes */ From 1016b239c5c7b54a02fbd3121f68fb264a42d0a1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jun 2014 17:26:13 +0200 Subject: [PATCH 07/18] megasas: use PCI DMA API MegaSAS emulation is not IOMMU-friendly. Fix this by switching to pci_dma_* functions. Signed-off-by: Paolo Bonzini --- hw/scsi/megasas.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index b05c47abdc..c68a873f18 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -294,6 +294,7 @@ static void megasas_unmap_sgl(MegasasCmd *cmd) static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr, uint8_t sense_len) { + PCIDevice *pcid = PCI_DEVICE(cmd->state); uint32_t pa_hi = 0, pa_lo; hwaddr pa; @@ -306,7 +307,7 @@ static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr, pa_hi = le32_to_cpu(cmd->frame->pass.sense_addr_hi); } pa = ((uint64_t) pa_hi << 32) | pa_lo; - cpu_physical_memory_write(pa, sense_ptr, sense_len); + pci_dma_write(pcid, pa, sense_ptr, sense_len); cmd->frame->header.sense_len = sense_len; } return sense_len; @@ -472,6 +473,7 @@ static MegasasCmd *megasas_next_frame(MegasasState *s, static MegasasCmd *megasas_enqueue_frame(MegasasState *s, hwaddr frame, uint64_t context, int count) { + PCIDevice *pcid = PCI_DEVICE(s); MegasasCmd *cmd = NULL; int frame_size = MFI_FRAME_SIZE * 16; hwaddr frame_size_p = frame_size; @@ -484,11 +486,11 @@ static MegasasCmd *megasas_enqueue_frame(MegasasState *s, if (!cmd->pa) { cmd->pa = frame; /* Map all possible frames */ - cmd->frame = cpu_physical_memory_map(frame, &frame_size_p, 0); + cmd->frame = pci_dma_map(pcid, frame, &frame_size_p, 0); if (frame_size_p != frame_size) { trace_megasas_qf_map_failed(cmd->index, (unsigned long)frame); if (cmd->frame) { - cpu_physical_memory_unmap(cmd->frame, frame_size_p, 0, 0); + pci_dma_unmap(pcid, cmd->frame, frame_size_p, 0, 0); cmd->frame = NULL; cmd->pa = 0; } @@ -561,13 +563,14 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context) static void megasas_reset_frames(MegasasState *s) { + PCIDevice *pcid = PCI_DEVICE(s); int i; MegasasCmd *cmd; for (i = 0; i < s->fw_cmds; i++) { cmd = &s->frames[i]; if (cmd->pa) { - cpu_physical_memory_unmap(cmd->frame, cmd->pa_size, 0, 0); + pci_dma_unmap(pcid, cmd->frame, cmd->pa_size, 0, 0); cmd->frame = NULL; cmd->pa = 0; } @@ -584,6 +587,7 @@ static void megasas_abort_command(MegasasCmd *cmd) static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd) { + PCIDevice *pcid = PCI_DEVICE(s); uint32_t pa_hi, pa_lo; hwaddr iq_pa, initq_size; struct mfi_init_qinfo *initq; @@ -595,7 +599,7 @@ static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd) iq_pa = (((uint64_t) pa_hi << 32) | pa_lo); trace_megasas_init_firmware((uint64_t)iq_pa); initq_size = sizeof(*initq); - initq = cpu_physical_memory_map(iq_pa, &initq_size, 0); + initq = pci_dma_map(pcid, iq_pa, &initq_size, 0); if (!initq || initq_size != sizeof(*initq)) { trace_megasas_initq_map_failed(cmd->index); s->event_count++; @@ -631,7 +635,7 @@ static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd) s->fw_state = MFI_FWSTATE_OPERATIONAL; out: if (initq) { - cpu_physical_memory_unmap(initq, initq_size, 0, 0); + pci_dma_unmap(pcid, initq, initq_size, 0, 0); } return ret; } From 519661ee6514f22b1e289b1c4c8c076378bc4202 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:21:28 +0200 Subject: [PATCH 08/18] util: add return value to qemu_iovec_concat_iov This will be necessary later to recognize the case where a request has both dataout and datain. Signed-off-by: Paolo Bonzini --- include/qemu-common.h | 6 +++--- util/iov.c | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/qemu-common.h b/include/qemu-common.h index 66ceceb2ad..ae76197532 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -315,9 +315,9 @@ void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov); void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len); void qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t soffset, size_t sbytes); -void qemu_iovec_concat_iov(QEMUIOVector *dst, - struct iovec *src_iov, unsigned int src_cnt, - size_t soffset, size_t sbytes); +size_t qemu_iovec_concat_iov(QEMUIOVector *dst, + struct iovec *src_iov, unsigned int src_cnt, + size_t soffset, size_t sbytes); bool qemu_iovec_is_zero(QEMUIOVector *qiov); void qemu_iovec_destroy(QEMUIOVector *qiov); void qemu_iovec_reset(QEMUIOVector *qiov); diff --git a/util/iov.c b/util/iov.c index 49f88388f8..2b4f46da75 100644 --- a/util/iov.c +++ b/util/iov.c @@ -295,15 +295,15 @@ void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len) * of src". * Only vector pointers are processed, not the actual data buffers. */ -void qemu_iovec_concat_iov(QEMUIOVector *dst, - struct iovec *src_iov, unsigned int src_cnt, - size_t soffset, size_t sbytes) +size_t qemu_iovec_concat_iov(QEMUIOVector *dst, + struct iovec *src_iov, unsigned int src_cnt, + size_t soffset, size_t sbytes) { int i; size_t done; if (!sbytes) { - return; + return 0; } assert(dst->nalloc != -1); for (i = 0, done = 0; done < sbytes && i < src_cnt; i++) { @@ -317,6 +317,8 @@ void qemu_iovec_concat_iov(QEMUIOVector *dst, } } assert(soffset == 0); /* offset beyond end of src */ + + return done; } /* From 36b15c79aa1bef5fe7543f9f2629b6413720bbfb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:21:18 +0200 Subject: [PATCH 09/18] virtio-scsi: start preparing for any_layout - Introduce virtio_scsi_init_req and virtio_scsi_free_req - rename qemu_sgl_init_external to qemu_sgl_concat - move virtio_scsi_parse_req from virtio_scsi_pop_req to callers and add header length checks to virtio_scsi_parse_req. Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 123 +++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 50 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index b39880a9cd..f013e35d69 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -15,6 +15,7 @@ #include "hw/virtio/virtio-scsi.h" #include "qemu/error-report.h" +#include "qemu/iov.h" #include #include #include @@ -56,18 +57,35 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); } +static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq) +{ + VirtIOSCSIReq *req; + req = g_malloc(sizeof(*req)); + + req->vq = vq; + req->dev = s; + req->sreq = NULL; + qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); + return req; +} + +static void virtio_scsi_free_req(VirtIOSCSIReq *req) +{ + qemu_sglist_destroy(&req->qsgl); + g_free(req); +} + static void virtio_scsi_complete_req(VirtIOSCSIReq *req) { VirtIOSCSI *s = req->dev; VirtQueue *vq = req->vq; VirtIODevice *vdev = VIRTIO_DEVICE(s); virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len); - qemu_sglist_destroy(&req->qsgl); if (req->sreq) { req->sreq->hba_private = NULL; scsi_req_unref(req->sreq); } - g_free(req); + virtio_scsi_free_req(req); virtio_notify(vdev, vq); } @@ -77,50 +95,55 @@ static void virtio_scsi_bad_req(void) exit(1); } -static void qemu_sgl_init_external(VirtIOSCSIReq *req, struct iovec *sg, +static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg, hwaddr *addr, int num) { QEMUSGList *qsgl = &req->qsgl; - qemu_sglist_init(qsgl, DEVICE(req->dev), num, &address_space_memory); while (num--) { qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len); } } -static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq, - VirtIOSCSIReq *req) +static int virtio_scsi_parse_req(VirtIOSCSIReq *req, + unsigned req_size, unsigned resp_size) { - assert(req->elem.in_num); - req->vq = vq; - req->dev = s; - req->sreq = NULL; + if (req->elem.in_num == 0) { + return -EINVAL; + } + + if (req->elem.out_sg[0].iov_len < req_size) { + return -EINVAL; + } if (req->elem.out_num) { req->req.buf = req->elem.out_sg[0].iov_base; } + + if (req->elem.in_sg[0].iov_len < resp_size) { + return -EINVAL; + } req->resp.buf = req->elem.in_sg[0].iov_base; if (req->elem.out_num > 1) { - qemu_sgl_init_external(req, &req->elem.out_sg[1], - &req->elem.out_addr[1], - req->elem.out_num - 1); + qemu_sgl_concat(req, &req->elem.out_sg[1], + &req->elem.out_addr[1], + req->elem.out_num - 1); } else { - qemu_sgl_init_external(req, &req->elem.in_sg[1], - &req->elem.in_addr[1], - req->elem.in_num - 1); + qemu_sgl_concat(req, &req->elem.in_sg[1], + &req->elem.in_addr[1], + req->elem.in_num - 1); } + + return 0; } static VirtIOSCSIReq *virtio_scsi_pop_req(VirtIOSCSI *s, VirtQueue *vq) { - VirtIOSCSIReq *req; - req = g_malloc(sizeof(*req)); + VirtIOSCSIReq *req = virtio_scsi_init_req(s, vq); if (!virtqueue_pop(vq, &req->elem)) { - g_free(req); + virtio_scsi_free_req(req); return NULL; } - - virtio_scsi_parse_req(s, vq, req); return req; } @@ -143,9 +166,9 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) VirtIOSCSIReq *req; uint32_t n; - req = g_malloc(sizeof(*req)); qemu_get_be32s(f, &n); assert(n < vs->conf.num_queues); + req = virtio_scsi_init_req(s, vs->cmd_vqs[n]); qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem)); /* TODO: add a way for SCSIBusInfo's load_request to fail, * and fail migration instead of asserting here. @@ -156,7 +179,12 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) #endif assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg)); assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg)); - virtio_scsi_parse_req(s, vs->cmd_vqs[n], req); + + if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size, + sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) { + error_report("invalid SCSI request migration data"); + exit(1); + } scsi_req_ref(sreq); req->sreq = sreq; @@ -281,29 +309,29 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) VirtIOSCSIReq *req; while ((req = virtio_scsi_pop_req(s, vq))) { - int out_size, in_size; - if (req->elem.out_num < 1 || req->elem.in_num < 1) { - virtio_scsi_bad_req(); - continue; - } + int type; - out_size = req->elem.out_sg[0].iov_len; - in_size = req->elem.in_sg[0].iov_len; - if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) { - if (out_size < sizeof(VirtIOSCSICtrlTMFReq) || - in_size < sizeof(VirtIOSCSICtrlTMFResp)) { + if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0, + &type, sizeof(type)) < sizeof(type)) { + virtio_scsi_bad_req(); + + } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) { + if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq), + sizeof(VirtIOSCSICtrlTMFResp)) < 0) { virtio_scsi_bad_req(); + } else { + virtio_scsi_do_tmf(s, req); } - virtio_scsi_do_tmf(s, req); } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY || req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) { - if (out_size < sizeof(VirtIOSCSICtrlANReq) || - in_size < sizeof(VirtIOSCSICtrlANResp)) { + if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq), + sizeof(VirtIOSCSICtrlANResp)) < 0) { virtio_scsi_bad_req(); + } else { + req->resp.an->event_actual = 0; + req->resp.an->response = VIRTIO_SCSI_S_OK; } - req->resp.an->event_actual = 0; - req->resp.an->response = VIRTIO_SCSI_S_OK; } virtio_scsi_complete_req(req); } @@ -373,23 +401,18 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) while ((req = virtio_scsi_pop_req(s, vq))) { SCSIDevice *d; - int out_size, in_size; - if (req->elem.out_num < 1 || req->elem.in_num < 1) { - virtio_scsi_bad_req(); - } - - out_size = req->elem.out_sg[0].iov_len; - in_size = req->elem.in_sg[0].iov_len; - if (out_size < sizeof(VirtIOSCSICmdReq) + vs->cdb_size || - in_size < sizeof(VirtIOSCSICmdResp) + vs->sense_size) { - virtio_scsi_bad_req(); - } - + int rc; if (req->elem.out_num > 1 && req->elem.in_num > 1) { virtio_scsi_fail_cmd_req(req); continue; } + rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size, + sizeof(VirtIOSCSICmdResp) + vs->sense_size); + if (rc < 0) { + virtio_scsi_bad_req(); + } + d = virtio_scsi_device_find(s, req->req.cmd->lun); if (!d) { req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET; From b0b4ea17dc7572ca79b2bb54447de5333dada5b2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:53:39 +0200 Subject: [PATCH 10/18] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index f013e35d69..ec9a5362f7 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -207,6 +207,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */ req->resp.tmf->response = VIRTIO_SCSI_S_OK; + tswap32s(&req->req.tmf->subtype); switch (req->req.tmf->subtype) { case VIRTIO_SCSI_T_TMF_ABORT_TASK: case VIRTIO_SCSI_T_TMF_QUERY_TASK: @@ -314,8 +315,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0, &type, sizeof(type)) < sizeof(type)) { virtio_scsi_bad_req(); + continue; + } - } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) { + tswap32s(&req->req.tmf->type); + if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) { if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq), sizeof(VirtIOSCSICtrlTMFResp)) < 0) { virtio_scsi_bad_req(); From 57fbae6e2cbbea037196a9ad67487c3fe78217f0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:39:24 +0200 Subject: [PATCH 11/18] virtio-scsi: add extra argument and return type to qemu_sgl_concat Will be used for anylayout support. Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index ec9a5362f7..0718626e1c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -95,14 +95,27 @@ static void virtio_scsi_bad_req(void) exit(1); } -static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg, - hwaddr *addr, int num) +static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov, + hwaddr *addr, int num, size_t skip) { QEMUSGList *qsgl = &req->qsgl; + size_t copied = 0; - while (num--) { - qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len); + while (num) { + if (skip >= iov->iov_len) { + skip -= iov->iov_len; + } else { + qemu_sglist_add(qsgl, *addr + skip, iov->iov_len - skip); + copied += iov->iov_len - skip; + skip = 0; + } + iov++; + addr++; + num--; } + + assert(skip == 0); + return copied; } static int virtio_scsi_parse_req(VirtIOSCSIReq *req, @@ -127,11 +140,11 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req, if (req->elem.out_num > 1) { qemu_sgl_concat(req, &req->elem.out_sg[1], &req->elem.out_addr[1], - req->elem.out_num - 1); + req->elem.out_num - 1, 0); } else { qemu_sgl_concat(req, &req->elem.in_sg[1], &req->elem.in_addr[1], - req->elem.in_num - 1); + req->elem.in_num - 1, 0); } return 0; From 489c7901a6e92d65afab333d498b3790b73f7153 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:58:19 +0200 Subject: [PATCH 12/18] virtio-scsi: prepare sense data handling for any_layout Retrieve sense and copy it to guest memory, to prepare for when we will use qemu_iovec_from_buf. Swap response and request, since we'll use the tail of VirtIOSCSIReq for the CDB. Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 0718626e1c..fbc7db776c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -26,12 +26,7 @@ typedef struct VirtIOSCSIReq { VirtQueueElement elem; QEMUSGList qsgl; SCSIRequest *sreq; - union { - char *buf; - VirtIOSCSICmdReq *cmd; - VirtIOSCSICtrlTMFReq *tmf; - VirtIOSCSICtrlANReq *an; - } req; + size_t resp_size; union { char *buf; VirtIOSCSICmdResp *cmd; @@ -39,6 +34,12 @@ typedef struct VirtIOSCSIReq { VirtIOSCSICtrlANResp *an; VirtIOSCSIEvent *event; } resp; + union { + char *buf; + VirtIOSCSICmdReq *cmd; + VirtIOSCSICtrlTMFReq *tmf; + VirtIOSCSICtrlANReq *an; + } req; } VirtIOSCSIReq; static inline int virtio_scsi_get_lun(uint8_t *lun) @@ -136,6 +137,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req, return -EINVAL; } req->resp.buf = req->elem.in_sg[0].iov_base; + req->resp_size = resp_size; if (req->elem.out_num > 1) { qemu_sgl_concat(req, &req->elem.out_sg[1], @@ -358,8 +360,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, size_t resid) { VirtIOSCSIReq *req = r->hba_private; - VirtIOSCSI *s = req->dev; - VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); + uint8_t sense[SCSI_SENSE_BUF_SIZE]; uint32_t sense_len; if (r->io_canceled) { @@ -372,8 +373,9 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, req->resp.cmd->resid = tswap32(resid); } else { req->resp.cmd->resid = 0; - sense_len = scsi_req_get_sense(r, req->resp.cmd->sense, - vs->sense_size); + sense_len = scsi_req_get_sense(r, sense, sizeof(sense)); + sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd)); + memcpy(req->resp.cmd->sense, sense, sense_len); req->resp.cmd->sense_len = tswap32(sense_len); } virtio_scsi_complete_req(req); From a3de269ccc6f724e40702d8a9adcc3d41d0db369 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 20:16:20 +0200 Subject: [PATCH 13/18] virtio-scsi: introduce virtio_scsi_complete_cmd_req This is also related to sense handling, and will be used by anylayout. Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index fbc7db776c..06fda896ec 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -356,6 +356,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) } } +static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req) +{ + virtio_scsi_complete_req(req); +} + static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, size_t resid) { @@ -378,7 +383,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, memcpy(req->resp.cmd->sense, sense, sense_len); req->resp.cmd->sense_len = tswap32(sense_len); } - virtio_scsi_complete_req(req); + virtio_scsi_complete_cmd_req(req); } static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r) @@ -400,13 +405,13 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r) } else { req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED; } - virtio_scsi_complete_req(req); + virtio_scsi_complete_cmd_req(req); } static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req) { req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE; - virtio_scsi_complete_req(req); + virtio_scsi_complete_cmd_req(req); } static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) @@ -435,7 +440,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) d = virtio_scsi_device_find(s, req->req.cmd->lun); if (!d) { req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET; - virtio_scsi_complete_req(req); + virtio_scsi_complete_cmd_req(req); continue; } req->sreq = scsi_req_new(d, req->req.cmd->tag, @@ -449,7 +454,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) if (req->sreq->cmd.mode != req_mode || req->sreq->cmd.xfer > req->qsgl.size) { req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN; - virtio_scsi_complete_req(req); + virtio_scsi_complete_cmd_req(req); continue; } } From 3eff1f46f08a360a4ae9f834ce9fef4c45bf6f0f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 16:40:31 +0200 Subject: [PATCH 14/18] virtio-scsi: add support for the any_layout feature Store the request and response headers by value, and let virtio_scsi_parse_req check that there is only one of datain and dataout. Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 193 +++++++++++++++++--------------- include/hw/i386/pc.h | 4 + include/hw/virtio/virtio-scsi.h | 4 +- 3 files changed, 109 insertions(+), 92 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 06fda896ec..3870c4781c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -27,21 +27,27 @@ typedef struct VirtIOSCSIReq { QEMUSGList qsgl; SCSIRequest *sreq; size_t resp_size; + enum SCSIXferMode mode; + QEMUIOVector resp_iov; union { - char *buf; - VirtIOSCSICmdResp *cmd; - VirtIOSCSICtrlTMFResp *tmf; - VirtIOSCSICtrlANResp *an; - VirtIOSCSIEvent *event; + VirtIOSCSICmdResp cmd; + VirtIOSCSICtrlTMFResp tmf; + VirtIOSCSICtrlANResp an; + VirtIOSCSIEvent event; } resp; union { - char *buf; - VirtIOSCSICmdReq *cmd; - VirtIOSCSICtrlTMFReq *tmf; - VirtIOSCSICtrlANReq *an; + struct { + VirtIOSCSICmdReq cmd; + uint8_t cdb[]; + } QEMU_PACKED; + VirtIOSCSICtrlTMFReq tmf; + VirtIOSCSICtrlANReq an; } req; } VirtIOSCSIReq; +QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) != + offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq)); + static inline int virtio_scsi_get_lun(uint8_t *lun) { return ((lun[2] << 8) | lun[3]) & 0x3FFF; @@ -61,17 +67,21 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq) { VirtIOSCSIReq *req; - req = g_malloc(sizeof(*req)); + VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); + + req = g_malloc0(sizeof(*req) + vs->cdb_size); req->vq = vq; req->dev = s; req->sreq = NULL; qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); + qemu_iovec_init(&req->resp_iov, 1); return req; } static void virtio_scsi_free_req(VirtIOSCSIReq *req) { + qemu_iovec_destroy(&req->resp_iov); qemu_sglist_destroy(&req->qsgl); g_free(req); } @@ -81,7 +91,9 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req) VirtIOSCSI *s = req->dev; VirtQueue *vq = req->vq; VirtIODevice *vdev = VIRTIO_DEVICE(s); - virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len); + + qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size); + virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size); if (req->sreq) { req->sreq->hba_private = NULL; scsi_req_unref(req->sreq); @@ -122,31 +134,35 @@ static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov, static int virtio_scsi_parse_req(VirtIOSCSIReq *req, unsigned req_size, unsigned resp_size) { - if (req->elem.in_num == 0) { + size_t in_size, out_size; + + if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0, + &req->req, req_size) < req_size) { return -EINVAL; } - if (req->elem.out_sg[0].iov_len < req_size) { + if (qemu_iovec_concat_iov(&req->resp_iov, + req->elem.in_sg, req->elem.in_num, 0, + resp_size) < resp_size) { return -EINVAL; } - if (req->elem.out_num) { - req->req.buf = req->elem.out_sg[0].iov_base; - } - - if (req->elem.in_sg[0].iov_len < resp_size) { - return -EINVAL; - } - req->resp.buf = req->elem.in_sg[0].iov_base; req->resp_size = resp_size; - if (req->elem.out_num > 1) { - qemu_sgl_concat(req, &req->elem.out_sg[1], - &req->elem.out_addr[1], - req->elem.out_num - 1, 0); - } else { - qemu_sgl_concat(req, &req->elem.in_sg[1], - &req->elem.in_addr[1], - req->elem.in_num - 1, 0); + out_size = qemu_sgl_concat(req, req->elem.out_sg, + &req->elem.out_addr[0], req->elem.out_num, + req_size); + in_size = qemu_sgl_concat(req, req->elem.in_sg, + &req->elem.in_addr[0], req->elem.in_num, + resp_size); + + if (out_size && in_size) { + return -ENOTSUP; + } + + if (out_size) { + req->mode = SCSI_XFER_TO_DEV; + } else if (in_size) { + req->mode = SCSI_XFER_FROM_DEV; } return 0; @@ -204,37 +220,34 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) scsi_req_ref(sreq); req->sreq = sreq; if (req->sreq->cmd.mode != SCSI_XFER_NONE) { - int req_mode = - (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV); - - assert(req->sreq->cmd.mode == req_mode); + assert(req->sreq->cmd.mode == req->mode); } return req; } static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) { - SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf->lun); + SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun); SCSIRequest *r, *next; BusChild *kid; int target; /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */ - req->resp.tmf->response = VIRTIO_SCSI_S_OK; + req->resp.tmf.response = VIRTIO_SCSI_S_OK; - tswap32s(&req->req.tmf->subtype); - switch (req->req.tmf->subtype) { + tswap32s(&req->req.tmf.subtype); + switch (req->req.tmf.subtype) { case VIRTIO_SCSI_T_TMF_ABORT_TASK: case VIRTIO_SCSI_T_TMF_QUERY_TASK: if (!d) { goto fail; } - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) { + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) { goto incorrect_lun; } QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) { VirtIOSCSIReq *cmd_req = r->hba_private; - if (cmd_req && cmd_req->req.cmd->tag == req->req.tmf->tag) { + if (cmd_req && cmd_req->req.cmd.tag == req->req.tmf.tag) { break; } } @@ -244,11 +257,11 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) * check for it in the loop above. */ assert(r->hba_private); - if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) { + if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) { /* "If the specified command is present in the task set, then * return a service response set to FUNCTION SUCCEEDED". */ - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; } else { scsi_req_cancel(r); } @@ -259,7 +272,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) if (!d) { goto fail; } - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) { + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) { goto incorrect_lun; } s->resetting++; @@ -273,16 +286,16 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) if (!d) { goto fail; } - if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) { + if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) { goto incorrect_lun; } QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) { if (r->hba_private) { - if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) { + if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) { /* "If there is any command present in the task set, then * return a service response set to FUNCTION SUCCEEDED". */ - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; break; } else { scsi_req_cancel(r); @@ -292,7 +305,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) break; case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET: - target = req->req.tmf->lun[1]; + target = req->req.tmf.lun[1]; s->resetting++; QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { d = DO_UPCAST(SCSIDevice, qdev, kid->child); @@ -305,18 +318,18 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) case VIRTIO_SCSI_T_TMF_CLEAR_ACA: default: - req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_REJECTED; + req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_REJECTED; break; } return; incorrect_lun: - req->resp.tmf->response = VIRTIO_SCSI_S_INCORRECT_LUN; + req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN; return; fail: - req->resp.tmf->response = VIRTIO_SCSI_S_BAD_TARGET; + req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET; } static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) @@ -333,8 +346,8 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) continue; } - tswap32s(&req->req.tmf->type); - if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) { + tswap32s(&req->req.tmf.type); + if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) { if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq), sizeof(VirtIOSCSICtrlTMFResp)) < 0) { virtio_scsi_bad_req(); @@ -342,14 +355,14 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) virtio_scsi_do_tmf(s, req); } - } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY || - req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) { + } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY || + req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) { if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq), sizeof(VirtIOSCSICtrlANResp)) < 0) { virtio_scsi_bad_req(); } else { - req->resp.an->event_actual = 0; - req->resp.an->response = VIRTIO_SCSI_S_OK; + req->resp.an.event_actual = 0; + req->resp.an.response = VIRTIO_SCSI_S_OK; } } virtio_scsi_complete_req(req); @@ -358,6 +371,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req) { + /* Sense data is not in req->resp and is copied separately + * in virtio_scsi_command_complete. + */ + req->resp_size = sizeof(VirtIOSCSICmdResp); virtio_scsi_complete_req(req); } @@ -372,16 +389,17 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, return; } - req->resp.cmd->response = VIRTIO_SCSI_S_OK; - req->resp.cmd->status = status; - if (req->resp.cmd->status == GOOD) { - req->resp.cmd->resid = tswap32(resid); + req->resp.cmd.response = VIRTIO_SCSI_S_OK; + req->resp.cmd.status = status; + if (req->resp.cmd.status == GOOD) { + req->resp.cmd.resid = tswap32(resid); } else { - req->resp.cmd->resid = 0; + req->resp.cmd.resid = 0; sense_len = scsi_req_get_sense(r, sense, sizeof(sense)); - sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd)); - memcpy(req->resp.cmd->sense, sense, sense_len); - req->resp.cmd->sense_len = tswap32(sense_len); + sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd)); + qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd), + &req->resp, sense_len); + req->resp.cmd.sense_len = tswap32(sense_len); } virtio_scsi_complete_cmd_req(req); } @@ -401,16 +419,16 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r) return; } if (req->dev->resetting) { - req->resp.cmd->response = VIRTIO_SCSI_S_RESET; + req->resp.cmd.response = VIRTIO_SCSI_S_RESET; } else { - req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED; + req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED; } virtio_scsi_complete_cmd_req(req); } static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req) { - req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE; + req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE; virtio_scsi_complete_cmd_req(req); } @@ -426,37 +444,34 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) while ((req = virtio_scsi_pop_req(s, vq))) { SCSIDevice *d; int rc; - if (req->elem.out_num > 1 && req->elem.in_num > 1) { - virtio_scsi_fail_cmd_req(req); - continue; - } rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size, sizeof(VirtIOSCSICmdResp) + vs->sense_size); if (rc < 0) { - virtio_scsi_bad_req(); + if (rc == -ENOTSUP) { + virtio_scsi_fail_cmd_req(req); + } else { + virtio_scsi_bad_req(); + } + continue; } - d = virtio_scsi_device_find(s, req->req.cmd->lun); + d = virtio_scsi_device_find(s, req->req.cmd.lun); if (!d) { - req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET; + req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET; virtio_scsi_complete_cmd_req(req); continue; } - req->sreq = scsi_req_new(d, req->req.cmd->tag, - virtio_scsi_get_lun(req->req.cmd->lun), - req->req.cmd->cdb, req); + req->sreq = scsi_req_new(d, req->req.cmd.tag, + virtio_scsi_get_lun(req->req.cmd.lun), + req->req.cdb, req); - if (req->sreq->cmd.mode != SCSI_XFER_NONE) { - int req_mode = - (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV); - - if (req->sreq->cmd.mode != req_mode || - req->sreq->cmd.xfer > req->qsgl.size) { - req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN; - virtio_scsi_complete_cmd_req(req); - continue; - } + if (req->sreq->cmd.mode != SCSI_XFER_NONE + && (req->sreq->cmd.mode != req->mode || + req->sreq->cmd.xfer > req->qsgl.size)) { + req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN; + virtio_scsi_complete_cmd_req(req); + continue; } n = scsi_req_enqueue(req->sreq); @@ -560,7 +575,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, return; } - if (req->elem.out_num || req->elem.in_num != 1) { + if (req->elem.out_num) { virtio_scsi_bad_req(); } @@ -569,12 +584,12 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, s->events_dropped = false; } - in_size = req->elem.in_sg[0].iov_len; + in_size = iov_size(req->elem.in_sg, req->elem.in_num); if (in_size < sizeof(VirtIOSCSIEvent)) { virtio_scsi_bad_req(); } - evt = req->resp.event; + evt = &req->resp.event; memset(evt, 0, sizeof(VirtIOSCSIEvent)); evt->event = event; evt->reason = reason; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index fa9d99792a..ca7a0bdd1a 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -268,6 +268,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); #define PC_COMPAT_2_0 \ {\ + .driver = "virtio-scsi-pci",\ + .property = "any_layout",\ + .value = "off",\ + },{\ .driver = "apic",\ .property = "version",\ .value = stringify(0x11),\ diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 42b102487a..de0615bf13 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -84,14 +84,13 @@ #define VIRTIO_SCSI_EVT_RESET_RESCAN 1 #define VIRTIO_SCSI_EVT_RESET_REMOVED 2 -/* SCSI command request, followed by data-out */ +/* SCSI command request, followed by CDB and data-out */ typedef struct { uint8_t lun[8]; /* Logical Unit Number */ uint64_t tag; /* Command identifier */ uint8_t task_attr; /* Task attribute */ uint8_t prio; uint8_t crn; - uint8_t cdb[]; } QEMU_PACKED VirtIOSCSICmdReq; /* Response, followed by sense data and data-in */ @@ -101,7 +100,6 @@ typedef struct { uint16_t status_qualifier; /* Status qualifier */ uint8_t status; /* Command completion status */ uint8_t response; /* Response values */ - uint8_t sense[]; } QEMU_PACKED VirtIOSCSICmdResp; /* Task Management Request */ From e49ab19fcaa617ad6cdfe1ac401327326b6a2552 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 4 Jun 2014 14:33:26 +0200 Subject: [PATCH 15/18] block/iscsi: bump libiscsi requirement to 1.9.0 This patch lifts the minimum supported libiscsi version from 1.4.0 to 1.9.0 since the BUSY patch required that change. On one this allows us to remove all #ifdefs from the code which makes the code easier to maintain and read. On the other hand I would not recommend libiscsi prior to 1.8.0 for production use because the following important libiscsi fixes for deadlocks and protocol errors are missing prior to 1.8.0: dbe9a1e SOCKET queue cmd PDUs directly in waitpdu queue 30df192 DATA-OUT set pdu->cmdsn appropriately 548bd22 ISCSI fix broken send logic in iscsi_scsi_async_command 14bee10 RECONNECT do not increase CmdSN for immediate PDUs 1f4a66a PDU queue out PDUs in order of itt. 562dd46 PDU avoid incrementing itt to 0xffffffff cd09c0f PDU use serial32 arithmetic for cmdsn, maxcmdsn and expcmdsn. 89e918e SOCKET validate data_size in in_pdu header 91267f5 Limit immediate and unsolicited data to FirstBurstLength Note that libiscsi 1.9.0 was released on Feb 24th, 2013, about one month after 1.8.0. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 67 --------------------------------------------------- configure | 39 +++--------------------------- 2 files changed, 3 insertions(+), 103 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 38754872fa..83c7992fc7 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -369,17 +369,6 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, lba = sector_qemu2lun(sector_num, iscsilun); num_sectors = sector_qemu2lun(nb_sectors, iscsilun); -#if !defined(LIBISCSI_FEATURE_IOVECTOR) - /* if the iovec only contains one buffer we can pass it directly */ - if (iov->niov == 1) { - data = iov->iov[0].iov_base; - } else { - size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size); - buf = g_malloc(size); - qemu_iovec_to_buf(iov, 0, buf, size); - data = buf; - } -#endif iscsi_co_init_iscsitask(iscsilun, &iTask); retry: if (iscsilun->use_16_for_rw) { @@ -397,10 +386,8 @@ retry: g_free(buf); return -ENOMEM; } -#if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov, iov->niov); -#endif while (!iTask.complete) { iscsi_set_events(iscsilun); qemu_coroutine_yield(); @@ -428,7 +415,6 @@ retry: } -#if defined(LIBISCSI_FEATURE_IOVECTOR) static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, int64_t sector_num, int nb_sectors) { @@ -538,9 +524,6 @@ out: return ret; } -#endif /* LIBISCSI_FEATURE_IOVECTOR */ - - static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *iov) @@ -549,15 +532,11 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, struct IscsiTask iTask; uint64_t lba; uint32_t num_sectors; -#if !defined(LIBISCSI_FEATURE_IOVECTOR) - int i; -#endif if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { return -EINVAL; } -#if defined(LIBISCSI_FEATURE_IOVECTOR) if (iscsilun->lbprz && nb_sectors >= ISCSI_CHECKALLOC_THRES && !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) { int64_t ret; @@ -571,7 +550,6 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, return 0; } } -#endif lba = sector_qemu2lun(sector_num, iscsilun); num_sectors = sector_qemu2lun(nb_sectors, iscsilun); @@ -587,23 +565,13 @@ retry: iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba, num_sectors * iscsilun->block_size, iscsilun->block_size, -#if !defined(CONFIG_LIBISCSI_1_4) /* API change from 1.4.0 to 1.5.0 */ 0, 0, 0, 0, 0, -#endif iscsi_co_generic_cb, &iTask); } if (iTask.task == NULL) { return -ENOMEM; } -#if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *) iov->iov, iov->niov); -#else - for (i = 0; i < iov->niov; i++) { - scsi_task_add_data_in_buffer(iTask.task, - iov->iov[i].iov_len, - iov->iov[i].iov_base); - } -#endif while (!iTask.complete) { iscsi_set_events(iscsilun); @@ -758,18 +726,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, data.data = acb->ioh->dxferp; data.size = acb->ioh->dxfer_len; } else { -#if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_out(acb->task, (struct scsi_iovec *) acb->ioh->dxferp, acb->ioh->iovec_count); -#else - struct iovec *iov = (struct iovec *)acb->ioh->dxferp; - - acb->buf = g_malloc(acb->ioh->dxfer_len); - data.data = acb->buf; - data.size = iov_to_buf(iov, acb->ioh->iovec_count, 0, - acb->buf, acb->ioh->dxfer_len); -#endif } } @@ -789,20 +748,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, acb->ioh->dxfer_len, acb->ioh->dxferp); } else { -#if defined(LIBISCSI_FEATURE_IOVECTOR) scsi_task_set_iov_in(acb->task, (struct scsi_iovec *) acb->ioh->dxferp, acb->ioh->iovec_count); -#else - int i; - for (i = 0; i < acb->ioh->iovec_count; i++) { - struct iovec *iov = (struct iovec *)acb->ioh->dxferp; - - scsi_task_add_data_in_buffer(acb->task, - iov[i].iov_len, - iov[i].iov_base); - } -#endif } } @@ -811,7 +759,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, return &acb->common; } - static void ioctl_cb(void *opaque, int status) { int *p_status = opaque; @@ -916,8 +863,6 @@ retry: return 0; } -#if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED) - static int coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) @@ -1012,8 +957,6 @@ retry: return 0; } -#endif /* SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED */ - static void parse_chap(struct iscsi_context *iscsi, const char *target, Error **errp) { @@ -1123,7 +1066,6 @@ static char *parse_initiator_name(const char *target) return iscsi_name; } -#if defined(LIBISCSI_FEATURE_NOP_COUNTER) static void iscsi_nop_timed_event(void *opaque) { IscsiLun *iscsilun = opaque; @@ -1141,7 +1083,6 @@ static void iscsi_nop_timed_event(void *opaque) timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL); iscsi_set_events(iscsilun); } -#endif static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) { @@ -1280,14 +1221,12 @@ static void iscsi_attach_aio_context(BlockDriverState *bs, iscsilun->aio_context = new_context; iscsi_set_events(iscsilun); -#if defined(LIBISCSI_FEATURE_NOP_COUNTER) /* Set up a timer for sending out iSCSI NOPs */ iscsilun->nop_timer = aio_timer_new(iscsilun->aio_context, QEMU_CLOCK_REALTIME, SCALE_MS, iscsi_nop_timed_event, iscsilun); timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL); -#endif } /* @@ -1479,13 +1418,11 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) { iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran * iscsilun->block_size) >> BDRV_SECTOR_BITS; -#if defined(LIBISCSI_FEATURE_IOVECTOR) if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) { iscsilun->allocationmap = bitmap_new(DIV_ROUND_UP(bs->total_sectors, iscsilun->cluster_sectors)); } -#endif } out: @@ -1670,13 +1607,9 @@ static BlockDriver bdrv_iscsi = { .bdrv_truncate = iscsi_truncate, .bdrv_refresh_limits = iscsi_refresh_limits, -#if defined(LIBISCSI_FEATURE_IOVECTOR) .bdrv_co_get_block_status = iscsi_co_get_block_status, -#endif .bdrv_co_discard = iscsi_co_discard, -#if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED) .bdrv_co_write_zeroes = iscsi_co_write_zeroes, -#endif .bdrv_co_readv = iscsi_co_readv, .bdrv_co_writev = iscsi_co_writev, .bdrv_co_flush_to_disk = iscsi_co_flush, diff --git a/configure b/configure index 27d84d9fdb..a69e90b570 100755 --- a/configure +++ b/configure @@ -3405,46 +3405,20 @@ if compile_prog "" "" ; then fi ########################################## -# Do we have libiscsi -# We check for iscsi_write16_sync() to make sure we have a -# at least version 1.4.0 of libiscsi. +# Do we have libiscsi >= 1.9.0 if test "$libiscsi" != "no" ; then - cat > $TMPC << EOF -#include -#include -int main(void) { iscsi_write16_sync(NULL,0,0,NULL,0,0,0,0,0,0,0); return 0; } -EOF - if $pkg_config --atleast-version=1.7.0 libiscsi; then + if $pkg_config --atleast-version=1.9.0 libiscsi; then libiscsi="yes" libiscsi_cflags=$($pkg_config --cflags libiscsi) libiscsi_libs=$($pkg_config --libs libiscsi) - elif compile_prog "" "-liscsi" ; then - libiscsi="yes" - libiscsi_libs="-liscsi" else if test "$libiscsi" = "yes" ; then - feature_not_found "libiscsi" "Install libiscsi devel" + feature_not_found "libiscsi" "Install libiscsi >= 1.9.0" fi libiscsi="no" fi fi -# We also need to know the API version because there was an -# API change from 1.4.0 to 1.5.0. -if test "$libiscsi" = "yes"; then - cat >$TMPC < -int main(void) -{ - iscsi_read10_task(0, 0, 0, 0, 0, 0, 0); - return 0; -} -EOF - if compile_prog "" "-liscsi"; then - libiscsi_version="1.4.0" - fi -fi - ########################################## # Do we need libm cat > $TMPC << EOF @@ -4218,11 +4192,7 @@ echo "nss used $smartcard_nss" echo "libusb $libusb" echo "usb net redir $usb_redir" echo "GLX support $glx" -if test "$libiscsi_version" = "1.4.0"; then -echo "libiscsi support $libiscsi (1.4.0)" -else echo "libiscsi support $libiscsi" -fi echo "libnfs support $libnfs" echo "build guest agent $guest_agent" echo "QGA VSS support $guest_agent_with_vss" @@ -4579,9 +4549,6 @@ fi if test "$libiscsi" = "yes" ; then echo "CONFIG_LIBISCSI=m" >> $config_host_mak - if test "$libiscsi_version" = "1.4.0"; then - echo "CONFIG_LIBISCSI_1_4=y" >> $config_host_mak - fi echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak fi From 9d2e256e62738f17d067258012f9a32e74bb9a8e Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 18 Jun 2014 18:40:11 +0200 Subject: [PATCH 16/18] block/iscsi: fix init value for iTask->retries during rebasing the changed init value for the retry counter was missed. This resulted in no retries being performed at all. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index 83c7992fc7..327353c3b6 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -216,7 +216,6 @@ static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask) { *iTask = (struct IscsiTask) { .co = qemu_coroutine_self(), - .retries = ISCSI_CMD_RETRIES, .iscsilun = iscsilun, }; } From 8c215a9fbdfe9476dd8b907681fc38d0bae78854 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 18 Jun 2014 18:40:22 +0200 Subject: [PATCH 17/18] block/iscsi: drop obsolete pointers from iscsi_co_writev Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 327353c3b6..84aa22a62e 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -359,8 +359,6 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, struct IscsiTask iTask; uint64_t lba; uint32_t num_sectors; - uint8_t *data = NULL; - uint8_t *buf = NULL; if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { return -EINVAL; @@ -372,17 +370,16 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, retry: if (iscsilun->use_16_for_rw) { iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, - data, num_sectors * iscsilun->block_size, + NULL, num_sectors * iscsilun->block_size, iscsilun->block_size, 0, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask); } else { iTask.task = iscsi_write10_task(iscsilun->iscsi, iscsilun->lun, lba, - data, num_sectors * iscsilun->block_size, + NULL, num_sectors * iscsilun->block_size, iscsilun->block_size, 0, 0, 0, 0, 0, iscsi_co_generic_cb, &iTask); } if (iTask.task == NULL) { - g_free(buf); return -ENOMEM; } scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov, @@ -402,8 +399,6 @@ retry: goto retry; } - g_free(buf); - if (iTask.status != SCSI_STATUS_GOOD) { return -EIO; } From 91d670fbf9945ca4ecbd123affb36889e7fe8a5d Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 19 Jun 2014 16:12:00 +0800 Subject: [PATCH 18/18] virtio-scsi: define dummy handle_output for vhost-scsi vqs vhost userspace needn't to handle vq's notification from guest, so define dummy handle_output callback for all vqs of vhost-scsi. In some corner cases(such as when handling vq's reset from VM), virtio-pci still trys to handle pending virtio-scsi events, then object check failure inside virtio_scsi_handle_event() for vhost-scsi can be triggered. The issue can be reproduced by 'rmmod virtio-scsi', 'system sleep' or reboot inside VM. Cc: qemu-stable@nongnu.org Cc: Anthony Liguori Cc: "Michael S. Tsirkin" Cc: Paolo Bonzini Signed-off-by: Ming Lei Signed-off-by: Paolo Bonzini --- hw/scsi/vhost-scsi.c | 8 +++++++- hw/scsi/virtio-scsi.c | 14 +++++++++----- include/hw/virtio/virtio-scsi.h | 7 ++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 668bafa72a..cc8df4ef03 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -196,6 +196,10 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) } } +static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) +{ +} + static void vhost_scsi_realize(DeviceState *dev, Error **errp) { VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); @@ -217,7 +221,9 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) } } - virtio_scsi_common_realize(dev, &err); + virtio_scsi_common_realize(dev, &err, vhost_dummy_handle_output, + vhost_dummy_handle_output, + vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); return; diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3870c4781c..8c8c9d1f61 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -667,7 +667,9 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .load_request = virtio_scsi_load_request, }; -void virtio_scsi_common_realize(DeviceState *dev, Error **errp) +void virtio_scsi_common_realize(DeviceState *dev, Error **errp, + HandleOutput ctrl, HandleOutput evt, + HandleOutput cmd) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(dev); @@ -681,12 +683,12 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp) s->cdb_size = VIRTIO_SCSI_CDB_SIZE; s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, - virtio_scsi_handle_ctrl); + ctrl); s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, - virtio_scsi_handle_event); + evt); for (i = 0; i < s->conf.num_queues; i++) { s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, - virtio_scsi_handle_cmd); + cmd); } } @@ -697,7 +699,9 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp) static int virtio_scsi_id; Error *err = NULL; - virtio_scsi_common_realize(dev, &err); + virtio_scsi_common_realize(dev, &err, virtio_scsi_handle_ctrl, + virtio_scsi_handle_event, + virtio_scsi_handle_cmd); if (err != NULL) { error_propagate(errp, err); return; diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index de0615bf13..a8f618578b 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -184,7 +184,12 @@ typedef struct { DEFINE_PROP_BIT("param_change", _state, _feature_field, \ VIRTIO_SCSI_F_CHANGE, true) -void virtio_scsi_common_realize(DeviceState *dev, Error **errp); +typedef void (*HandleOutput)(VirtIODevice *, VirtQueue *); + +void virtio_scsi_common_realize(DeviceState *dev, Error **errp, + HandleOutput ctrl, HandleOutput evt, + HandleOutput cmd); + void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp); #endif /* _QEMU_VIRTIO_SCSI_H */