From de8864e5ae645fc22aa4ecf1999705c2dd5cf93c Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sat, 17 Nov 2012 16:13:24 +0100 Subject: [PATCH 1/6] iscsi: add iscsi_create support This patch adds support for bdrv_create. This allows e.g. to use qemu-img to convert from any supported device to an iscsi backed storage as destination. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index 041ee07de3..d8382fdd5d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -957,6 +957,56 @@ static int iscsi_has_zero_init(BlockDriverState *bs) return 0; } +static int iscsi_create(const char *filename, QEMUOptionParameter *options) +{ + int ret = 0; + int64_t total_size = 0; + BlockDriverState bs; + IscsiLun *iscsilun = NULL; + + memset(&bs, 0, sizeof(BlockDriverState)); + + /* Read out options */ + while (options && options->name) { + if (!strcmp(options->name, "size")) { + total_size = options->value.n / BDRV_SECTOR_SIZE; + } + options++; + } + + bs.opaque = g_malloc0(sizeof(struct IscsiLun)); + iscsilun = bs.opaque; + + ret = iscsi_open(&bs, filename, 0); + if (ret != 0) { + goto out; + } + if (iscsilun->type != TYPE_DISK) { + ret = -ENODEV; + goto out; + } + if (bs.total_sectors < total_size) { + ret = -ENOSPC; + } + + ret = 0; +out: + if (iscsilun->iscsi != NULL) { + iscsi_destroy_context(iscsilun->iscsi); + } + g_free(bs.opaque); + return ret; +} + +static QEMUOptionParameter iscsi_create_options[] = { + { + .name = BLOCK_OPT_SIZE, + .type = OPT_SIZE, + .help = "Virtual disk size" + }, + { NULL } +}; + static BlockDriver bdrv_iscsi = { .format_name = "iscsi", .protocol_name = "iscsi", @@ -964,6 +1014,8 @@ static BlockDriver bdrv_iscsi = { .instance_size = sizeof(IscsiLun), .bdrv_file_open = iscsi_open, .bdrv_close = iscsi_close, + .bdrv_create = iscsi_create, + .create_options = iscsi_create_options, .bdrv_getlength = iscsi_getlength, From 4cc841b57c1dc91d71bafc25b53ffab4eff7959b Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 19 Nov 2012 15:58:31 +0100 Subject: [PATCH 2/6] iscsi: partly avoid iovec linearization in iscsi_aio_writev libiscsi expects all write16 data in a linear buffer. If the iovec only contains one buffer we can skip the linearization step as well as the additional malloc/free and pass the buffer directly. Reported-by: Ronnie Sahlberg Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index d8382fdd5d..259192f535 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -241,8 +241,17 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num, /* XXX we should pass the iovec to write16 to avoid the extra copy */ /* this will allow us to get rid of 'buf' completely */ size = nb_sectors * BDRV_SECTOR_SIZE; - acb->buf = g_malloc(size); - qemu_iovec_to_buf(acb->qiov, 0, acb->buf, size); + data.size = MIN(size, acb->qiov->size); + + /* if the iovec only contains one buffer we can pass it directly */ + if (acb->qiov->niov == 1) { + acb->buf = NULL; + data.data = acb->qiov->iov[0].iov_base; + } else { + acb->buf = g_malloc(data.size); + qemu_iovec_to_buf(acb->qiov, 0, acb->buf, data.size); + data.data = acb->buf; + } acb->task = malloc(sizeof(struct scsi_task)); if (acb->task == NULL) { @@ -263,9 +272,6 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num, *(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors); acb->task->expxferlen = size; - data.data = acb->buf; - data.size = size; - if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task, iscsi_aio_write16_cb, &data, From 5b5d34ec9882b29b757f6808693308e52a8e8ba7 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 6 Dec 2012 10:46:47 +0100 Subject: [PATCH 3/6] iscsi: add support for iSCSI NOPs [v2] This patch will send NOP-Out PDUs every 5 seconds to the iSCSI target. If a consecutive number of NOP-In replies fail a reconnect is initiated. iSCSI NOPs help to ensure that the connection to the target is still operational. This should not, but in reality may be the case even if the TCP connection is still alive if there are bugs in either the target or the initiator implementation. v2: - track the NOPs inside libiscsi so libiscsi can reset the counter in case it initiates a reconnect. Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index 259192f535..249778986d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -48,6 +48,7 @@ typedef struct IscsiLun { int block_size; uint64_t num_blocks; int events; + QEMUTimer *nop_timer; } IscsiLun; typedef struct IscsiAIOCB { @@ -66,6 +67,9 @@ typedef struct IscsiAIOCB { #endif } IscsiAIOCB; +#define NOP_INTERVAL 5000 +#define MAX_NOP_FAILURES 3 + static void iscsi_bh_cb(void *p) { @@ -768,6 +772,26 @@ static char *parse_initiator_name(const char *target) } } +#if defined(LIBISCSI_FEATURE_NOP_COUNTER) +static void iscsi_nop_timed_event(void *opaque) +{ + IscsiLun *iscsilun = opaque; + + if (iscsi_get_nops_in_flight(iscsilun->iscsi) > MAX_NOP_FAILURES) { + error_report("iSCSI: NOP timeout. Reconnecting..."); + iscsi_reconnect(iscsilun->iscsi); + } + + if (iscsi_nop_out_async(iscsilun->iscsi, NULL, NULL, 0, NULL) != 0) { + error_report("iSCSI: failed to sent NOP-Out. Disabling NOP messages."); + return; + } + + qemu_mod_timer(iscsilun->nop_timer, qemu_get_clock_ms(rt_clock) + NOP_INTERVAL); + iscsi_set_events(iscsilun); +} +#endif + /* * We support iscsi url's on the form * iscsi://[%@][:]// @@ -928,6 +952,12 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) ret = 0; +#if defined(LIBISCSI_FEATURE_NOP_COUNTER) + /* Set up a timer for sending out iSCSI NOPs */ + iscsilun->nop_timer = qemu_new_timer_ms(rt_clock, iscsi_nop_timed_event, iscsilun); + qemu_mod_timer(iscsilun->nop_timer, qemu_get_clock_ms(rt_clock) + NOP_INTERVAL); +#endif + out: if (initiator_name != NULL) { g_free(initiator_name); @@ -953,6 +983,10 @@ static void iscsi_close(BlockDriverState *bs) IscsiLun *iscsilun = bs->opaque; struct iscsi_context *iscsi = iscsilun->iscsi; + if (iscsilun->nop_timer) { + qemu_del_timer(iscsilun->nop_timer); + qemu_free_timer(iscsilun->nop_timer); + } qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL); iscsi_destroy_context(iscsi); memset(iscsilun, 0, sizeof(IscsiLun)); @@ -987,6 +1021,10 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) if (ret != 0) { goto out; } + if (iscsilun->nop_timer) { + qemu_del_timer(iscsilun->nop_timer); + qemu_free_timer(iscsilun->nop_timer); + } if (iscsilun->type != TYPE_DISK) { ret = -ENODEV; goto out; From 0369f06f7464e7fb023f103aff889d28e99c43c4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 10 Jan 2013 15:08:05 +0100 Subject: [PATCH 4/6] scsi: fix segfault with 0-byte disk When a 0-sized disk is found, READ CAPACITY will return a LUN NOT READY error. However, because it returns -1 instead of zero, the HBA will call scsi_req_continue. This will typically cause a segmentation fault or an assertion failure. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index f8d7ef3374..658e315660 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1682,7 +1682,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) bdrv_get_geometry(s->qdev.conf.bs, &nb_sectors); if (!nb_sectors) { scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY)); - return -1; + return 0; } if ((req->cmd.buf[8] & 1) == 0 && req->cmd.lba) { goto illegal_request; @@ -1751,7 +1751,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) bdrv_get_geometry(s->qdev.conf.bs, &nb_sectors); if (!nb_sectors) { scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY)); - return -1; + return 0; } if ((req->cmd.buf[14] & 1) == 0 && req->cmd.lba) { goto illegal_request; From 032f0101aa6e009efda3a419379837ebceaeade1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Dec 2012 11:23:25 +0100 Subject: [PATCH 5/6] lsi: use qbus_reset_all to reset SCSI bus Signed-off-by: Paolo Bonzini --- hw/lsi53c895a.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c index 89c657fb00..860df328e5 100644 --- a/hw/lsi53c895a.c +++ b/hw/lsi53c895a.c @@ -1670,12 +1670,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val) } if (val & LSI_SCNTL1_RST) { if (!(s->sstat0 & LSI_SSTAT0_RST)) { - BusChild *kid; - - QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - DeviceState *dev = kid->child; - device_reset(dev); - } + qbus_reset_all(&s->bus.qbus); s->sstat0 |= LSI_SSTAT0_RST; lsi_script_scsi_interrupt(s, LSI_SIST0_RST, 0); } From 0bf8264e2d2bd19c1eecf9bde0e59284ef47eabb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Jan 2013 13:07:47 +0100 Subject: [PATCH 6/6] scsi: Drop useless null test in scsi_unit_attention() req was created by scsi_req_alloc(), which initializes req->dev to a value it dereferences. req->dev isn't changed anywhere else. Therefore, req->dev can't be null. Drop the useless null test; it spooks Coverity. Signed-off-by: Markus Armbruster --- hw/scsi-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 267a942f76..a97f1cdc1c 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -282,7 +282,7 @@ static const struct SCSIReqOps reqops_invalid_opcode = { static int32_t scsi_unit_attention(SCSIRequest *req, uint8_t *buf) { - if (req->dev && req->dev->unit_attention.key == UNIT_ATTENTION) { + if (req->dev->unit_attention.key == UNIT_ATTENTION) { scsi_req_build_sense(req, req->dev->unit_attention); } else if (req->bus->unit_attention.key == UNIT_ATTENTION) { scsi_req_build_sense(req, req->bus->unit_attention);