[Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Ronnie Sahlberg]

List,

Please find a patch that adds built-in iSCSI support to QEMU when built and linked against the multiplatform iscsi initiator library at  git://github.com/sahlberg/libiscsi.git


I have implemented all suggested changed from Stefans review, except
* Fail the build if qemu blocksize is not multiple of 512.
Maybe QEMU will never use such, but if it does I dont think a blockdriver that can not support such should fail the build. Hence it just refuses to use the driver at runtime instead.

* I did not implement a synchronous bdrv_flush since I understand that Stefans patch will be merged shortly since it is required for other backend drivers anyway.


In some situations, using a built-in iscsi inititator has benefits against mounting the LUNs on the host.

* Privacy: The iSCSI LUNs are private to the guest and are not visible either to the host, nor to any processes running on the host.
* Ease of managment : If you have very many guests and very many, thousands of, iSCSI LUNs. It is inconvenient to have to expose all LUNs to the underlying host.
* No root requirement. Since the iSCSI LUNs are not mounted as devices on the host, ordinary users can set up and use iSCSI resources without the need for root privilege on the host to map the devices to local scsi devices.
* Since the network I/O for the iSCSI LUN is now done by the QEMU process instead of the kernel, each VM's iSCSI I/O traffic can be insolated & controlled via the cgroups network contorller / tc network classifier.
* Portable across OS. Each OS has different tools for setting up iSCSI usage. A native driver lets QEMU users setup iSCSI in the same way regardless of the level of OS support for iSCSI.

Please merge this patch to master or explain how I should change the patch so that it becomes acceptable for inclusion into QEMU.


regards
ronnie sahlberg

[Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Ronnie Sahlberg]

This provides built-in support for iSCSI to QEMU.
This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.

This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
    git://github.com/sahlberg/libiscsi.git

The patch adds the driver to interface with the iscsi library.
It also updated the configure script to
* by default, probe is libiscsi is available and if so, build
  qemu against libiscsi.
* --enable-libiscsi
  Force a build against libiscsi. If libiscsi is not available
  the build will fail.
* --disable-libiscsi
  Do not link against libiscsi, even if it is available.

When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.

You can specify devices using a iscsi url of the form :
iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
When using authentication, the password can optionally be set with
LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 Makefile.objs |    1 +
 block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure     |   31 +++
 trace-events  |    7 +
 4 files changed, 635 insertions(+), 0 deletions(-)
 create mode 100644 block/iscsi.c

diff --git a/Makefile.objs b/Makefile.objs
index a529a11..8c8e420 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -36,6 +36,7 @@ block-nested-y += qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
+block-nested-$(CONFIG_LIBISCSI) += iscsi.o
 block-nested-$(CONFIG_CURL) += curl.o
 block-nested-$(CONFIG_RBD) += rbd.o
 
diff --git a/block/iscsi.c b/block/iscsi.c
new file mode 100644
index 0000000..6517576
--- /dev/null
+++ b/block/iscsi.c
@@ -0,0 +1,596 @@
+/*
+ * QEMU Block driver for iSCSI images
+ *
+ * Copyright (c) 2010-2011 Ronnie Sahlberg <ronniesahlberg@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include <poll.h>
+#include "sysemu.h"
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "block_int.h"
+#include "trace.h"
+
+#include <iscsi/iscsi.h>
+#include <iscsi/scsi-lowlevel.h>
+
+
+typedef struct IscsiLun {
+    struct iscsi_context *iscsi;
+    int lun;
+    int block_size;
+    unsigned long num_blocks;
+} IscsiLun;
+
+typedef struct IscsiAIOCB {
+    BlockDriverAIOCB common;
+    QEMUIOVector *qiov;
+    QEMUBH *bh;
+    IscsiLun *iscsilun;
+    struct scsi_task *task;
+    uint8_t *buf;
+    int canceled;
+    int status;
+    size_t read_size;
+    size_t read_offset;
+} IscsiAIOCB;
+
+struct IscsiTask {
+    IscsiLun *iscsilun;
+    int status;
+    int complete;
+};
+
+static void
+iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                    void *private_data)
+{
+}
+
+static void
+iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
+    IscsiLun *iscsilun = acb->iscsilun;
+
+    acb->status = -ECANCELED;
+    acb->common.cb(acb->common.opaque, acb->status);
+    acb->canceled = 1;
+
+    /* send a task mgmt call to the target to cancel the task on the target */
+    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
+                                     iscsi_abort_task_cb, NULL);
+
+    /* then also cancel the task locally in libiscsi */
+    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
+}
+
+static AIOPool iscsi_aio_pool = {
+    .aiocb_size         = sizeof(IscsiAIOCB),
+    .cancel             = iscsi_aio_cancel,
+};
+
+
+static void iscsi_process_read(void *arg);
+static void iscsi_process_write(void *arg);
+
+static int iscsi_process_flush(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+
+    return iscsi_queue_length(iscsilun->iscsi) > 0;
+}
+
+static void
+iscsi_set_events(IscsiLun *iscsilun)
+{
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
+                           (iscsi_which_events(iscsi) & POLLOUT)
+                           ? iscsi_process_write : NULL,
+                           iscsi_process_flush, NULL, iscsilun);
+}
+
+static void
+iscsi_process_read(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLIN);
+    iscsi_set_events(iscsilun);
+}
+
+static void
+iscsi_process_write(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLOUT);
+    iscsi_set_events(iscsilun);
+}
+
+
+static int
+iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
+{
+    acb->bh = qemu_bh_new(cb, acb);
+    if (!acb->bh) {
+        error_report("oom: could not create iscsi bh");
+        return -EIO;
+    }
+
+    qemu_bh_schedule(acb->bh);
+    return 0;
+}
+
+static void
+iscsi_readv_writev_bh_cb(void *p)
+{
+    IscsiAIOCB *acb = p;
+
+    qemu_bh_delete(acb->bh);
+
+    if (acb->status != -ECANCELED) {
+        acb->common.cb(acb->common.opaque, acb->status);
+    }
+
+    qemu_aio_release(acb);
+}
+
+
+static void
+iscsi_aio_write10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_write10_cb(iscsi, status, acb, acb->canceled);
+
+    g_free(acb->buf);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to write10 data to iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
+{
+    return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                 QEMUIOVector *qiov, int nb_sectors,
+                 BlockDriverCompletionFunc *cb,
+                 void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t size;
+    int fua = 0;
+
+    /* set FUA on writes when cache mode is write through */
+    if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
+        fua = 1;
+    }
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
+    if (!acb) {
+        return NULL;
+    }
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled   = 0;
+
+    /* XXX we should pass the iovec to write10 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_buffer(acb->qiov, acb->buf);
+    acb->task = iscsi_write10_task(iscsi, iscsilun->lun, acb->buf, size,
+                              sector_qemu2lun(sector_num, iscsilun),
+                              fua, 0, iscsilun->block_size,
+                              iscsi_aio_write10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send write10 command. %s",
+                     iscsi_get_error(iscsi));
+        g_free(acb->buf);
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static void
+iscsi_aio_read10_cb(struct iscsi_context *iscsi, int status,
+                    void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_read10_cb(iscsi, status, acb, acb->canceled);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status != 0) {
+        error_report("Failed to read10 data from iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                QEMUIOVector *qiov, int nb_sectors,
+                BlockDriverCompletionFunc *cb,
+                void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t qemu_read_size, lun_read_size;
+    int i;
+
+    qemu_read_size = BDRV_SECTOR_SIZE * (size_t)nb_sectors;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_readv(iscsi, sector_num, nb_sectors, opaque, acb);
+    if (!acb) {
+        return NULL;
+    }
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled    = 0;
+    acb->read_size   = qemu_read_size;
+    acb->buf         = NULL;
+
+    /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
+     * may be misaligned to the LUN, so we may need to read some extra
+     * data.
+     */
+    acb->read_offset = 0;
+    if (iscsilun->block_size > BDRV_SECTOR_SIZE) {
+        uint64_t bdrv_offset = BDRV_SECTOR_SIZE * sector_num;
+
+        acb->read_offset  = bdrv_offset % iscsilun->block_size;
+    }
+
+    lun_read_size  = (qemu_read_size + iscsilun->block_size
+                     + acb->read_offset - 1)
+                     / iscsilun->block_size * iscsilun->block_size;
+    acb->task = iscsi_read10_task(iscsi, iscsilun->lun,
+                             sector_qemu2lun(sector_num, iscsilun),
+                             lun_read_size, iscsilun->block_size,
+                             iscsi_aio_read10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send read10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    for (i = 0; i < acb->qiov->niov; i++) {
+        scsi_task_add_data_in_buffer(acb->task,
+                acb->qiov->iov[i].iov_len,
+                acb->qiov->iov[i].iov_base);
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+
+static void
+iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to sync10 data on iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_flush(BlockDriverState *bs,
+                BlockDriverCompletionFunc *cb, void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    if (!acb) {
+        return NULL;
+    }
+
+    acb->iscsilun = iscsilun;
+    acb->canceled   = 0;
+
+    acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
+                                         0, 0, 0, 0,
+                                         iscsi_synccache10_cb,
+                                         acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send synchronizecache10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static int64_t
+iscsi_getlength(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    int64_t len;
+
+    len  = iscsilun->num_blocks;
+    len *= iscsilun->block_size;
+
+    return len;
+}
+
+static void
+iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
+                        void *command_data, void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_readcapacity10 *rc10;
+    struct scsi_task *task = command_data;
+
+    if (status != 0) {
+        error_report("iSCSI: Failed to read capacity of iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    rc10 = scsi_datain_unmarshall(task);
+    if (rc10 == NULL) {
+        error_report("iSCSI: Failed to unmarshall readcapacity10 data.");
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    itask->iscsilun->block_size = rc10->block_size;
+    itask->iscsilun->num_blocks = rc10->lba;
+
+    itask->status   = 0;
+    itask->complete = 1;
+    scsi_free_scsi_task(task);
+}
+
+
+static void
+iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                 void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_task *task;
+
+    if (status != 0) {
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+
+    task = iscsi_readcapacity10_task(iscsi, itask->iscsilun->lun, 0, 0,
+                                   iscsi_readcapacity10_cb, opaque);
+    if (task == NULL) {
+        error_report("iSCSI: failed to send readcapacity command.");
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+}
+
+/*
+ * We support iscsi url's on the form
+ * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
+ */
+static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = NULL;
+    struct iscsi_url *iscsi_url = NULL;
+    struct IscsiTask task;
+    int ret;
+
+    if ((BDRV_SECTOR_SIZE % 512) != 0) {
+        error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. "
+                     "BDRV_SECTOR_SIZE(%lld) is not a multiple "
+                     "of 512", BDRV_SECTOR_SIZE);
+        return -EINVAL;
+    }
+
+    memset(iscsilun, 0, sizeof(IscsiLun));
+
+    /* Should really append the KVM name after the ':' here */
+    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
+    if (iscsi == NULL) {
+        error_report("iSCSI: Failed to create iSCSI context.");
+        ret = -ENOMEM;
+        goto failed;
+    }
+
+    iscsi_url = iscsi_parse_full_url(iscsi, filename);
+    if (iscsi_url == NULL) {
+        error_report("Failed to parse URL : %s %s", filename,
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
+        error_report("iSCSI: Failed to set target name.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    if (iscsi_url->user != NULL) {
+        ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
+                                              iscsi_url->passwd);
+        if (ret != 0) {
+            error_report("Failed to set initiator username and password");
+            ret = -EINVAL;
+            goto failed;
+        }
+    }
+    if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
+        error_report("iSCSI: Failed to set session type to normal.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+
+    task.iscsilun = iscsilun;
+    task.status = 0;
+    task.complete = 0;
+
+    iscsilun->iscsi = iscsi;
+    iscsilun->lun   = iscsi_url->lun;
+
+    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
+                                 iscsi_connect_cb, &task)
+        != 0) {
+        error_report("iSCSI: Failed to start async connect.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    while (!task.complete) {
+        iscsi_set_events(iscsilun);
+        qemu_aio_wait();
+    }
+    if (task.status != 0) {
+        error_report("iSCSI: Failed to connect to LUN : %s",
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    return 0;
+
+failed:
+    if (iscsi_url != NULL) {
+        iscsi_destroy_url(iscsi_url);
+    }
+    if (iscsi != NULL) {
+        iscsi_destroy_context(iscsi);
+    }
+    memset(iscsilun, 0, sizeof(IscsiLun));
+    return ret;
+}
+
+static void iscsi_close(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL, NULL);
+    iscsi_destroy_context(iscsi);
+    memset(iscsilun, 0, sizeof(IscsiLun));
+}
+
+static BlockDriver bdrv_iscsi = {
+    .format_name     = "iscsi",
+    .protocol_name   = "iscsi",
+
+    .instance_size   = sizeof(IscsiLun),
+    .bdrv_file_open  = iscsi_open,
+    .bdrv_close      = iscsi_close,
+
+    .bdrv_getlength  = iscsi_getlength,
+
+    .bdrv_aio_readv  = iscsi_aio_readv,
+    .bdrv_aio_writev = iscsi_aio_writev,
+    .bdrv_aio_flush  = iscsi_aio_flush,
+};
+
+static void iscsi_block_init(void)
+{
+    bdrv_register(&bdrv_iscsi);
+}
+
+block_init(iscsi_block_init);
+
diff --git a/configure b/configure
index 348f36a..a2c30e1 100755
--- a/configure
+++ b/configure
@@ -182,6 +182,7 @@ usb_redir=""
 opengl=""
 zlib="yes"
 guest_agent="yes"
+libiscsi=""
 
 # parse CC options first
 for opt do
@@ -651,6 +652,10 @@ for opt do
   ;;
   --enable-spice) spice="yes"
   ;;
+  --disable-libiscsi) libiscsi="no"
+  ;;
+  --enable-libiscsi) libiscsi="yes"
+  ;;
   --enable-profiler) profiler="yes"
   ;;
   --enable-cocoa)
@@ -1037,6 +1042,8 @@ echo "                           Default:trace-<pid>"
 echo "  --disable-spice          disable spice"
 echo "  --enable-spice           enable spice"
 echo "  --enable-rbd             enable building the rados block device (rbd)"
+echo "  --disable-libiscsi       disable iscsi support"
+echo "  --enable-libiscsi        enable iscsi support"
 echo "  --disable-smartcard      disable smartcard support"
 echo "  --enable-smartcard       enable smartcard support"
 echo "  --disable-smartcard-nss  disable smartcard nss support"
@@ -2326,6 +2333,25 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# Do we have libiscsi
+if test "$libiscsi" != "no" ; then
+  cat > $TMPC << EOF
+#include <iscsi/iscsi.h>
+int main(void) { iscsi_create_context(""); return 0; }
+EOF
+  if compile_prog "-Werror" "-liscsi" ; then
+    libiscsi="yes"
+    LIBS="$LIBS -liscsi"
+  else
+    if test "$libiscsi" = "yes" ; then
+      feature_not_found "libiscsi"
+    fi
+    libiscsi="no"
+  fi
+fi
+
+
+##########################################
 # Do we need librt
 cat > $TMPC <<EOF
 #include <signal.h>
@@ -2727,6 +2753,7 @@ echo "xfsctl support    $xfs"
 echo "nss used          $smartcard_nss"
 echo "usb net redir     $usb_redir"
 echo "OpenGL support    $opengl"
+echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 
 if test "$sdl_too_old" = "yes"; then
@@ -3028,6 +3055,10 @@ if test "$opengl" = "yes" ; then
   echo "CONFIG_OPENGL=y" >> $config_host_mak
 fi
 
+if test "$libiscsi" = "yes" ; then
+  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
diff --git a/trace-events b/trace-events
index 3fdd60f..ce68516 100644
--- a/trace-events
+++ b/trace-events
@@ -494,3 +494,10 @@ escc_sunkbd_event_in(int ch) "Untranslated keycode %2.2x"
 escc_sunkbd_event_out(int ch) "Translated keycode %2.2x"
 escc_kbd_command(int val) "Command %d"
 escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
+
+# block/iscsi.c
+iscsi_aio_write10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+iscsi_aio_read10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+
-- 
1.7.3.1

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Paolo Bonzini]

On 09/21/2011 11:37 AM, Ronnie Sahlberg wrote:
> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>      git://github.com/sahlberg/libiscsi.git

Do you plan to make a stable release, so that it can be packaged e.g. in 
Fedora?

Paolo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

On Wed, Sep 21, 2011 at 7:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 09/21/2011 11:37 AM, Ronnie Sahlberg wrote:
>>
>> This driver interfaces with the multiplatform posix library for iscsi
>> initiator/client access to iscsi devices hosted at
>>     git://github.com/sahlberg/libiscsi.git
>
> Do you plan to make a stable release, so that it can be packaged e.g. in
> Fedora?

Yes.

As soon as my primary consumer (==QEMU) has integrated the patch I
will make a stable release for libiscsi to match.
Until then I keep it "unstable" so I can implement what changes that
the QEMU iscsi patch may require.

regards
ronnie sahlberg

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

List,

What remains before this patch can be accepted?
Previous patch received good feedback and severa people indicated that
they would find the feature useful for several use cases.

regards
ronnie sahlberg

On Wed, Sep 21, 2011 at 7:52 PM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> On Wed, Sep 21, 2011 at 7:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 09/21/2011 11:37 AM, Ronnie Sahlberg wrote:
>>>
>>> This driver interfaces with the multiplatform posix library for iscsi
>>> initiator/client access to iscsi devices hosted at
>>>     git://github.com/sahlberg/libiscsi.git
>>
>> Do you plan to make a stable release, so that it can be packaged e.g. in
>> Fedora?
>
> Yes.
>
> As soon as my primary consumer (==QEMU) has integrated the patch I
> will make a stable release for libiscsi to match.
> Until then I keep it "unstable" so I can implement what changes that
> the QEMU iscsi patch may require.
>
> regards
> ronnie sahlberg
>

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Paolo Bonzini]

On 09/27/2011 10:08 PM, ronnie sahlberg wrote:
> List,
>
> What remains before this patch can be accepted?
> Previous patch received good feedback and severa people indicated that
> they would find the feature useful for several use cases.

Kevin is on vacation this week. :)

Paolo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Stefan Hajnoczi]

On Wed, Sep 21, 2011 at 07:37:55PM +1000, Ronnie Sahlberg wrote:
> This provides built-in support for iSCSI to QEMU.
> This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
> It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.
> 
> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>     git://github.com/sahlberg/libiscsi.git
> 
> The patch adds the driver to interface with the iscsi library.
> It also updated the configure script to
> * by default, probe is libiscsi is available and if so, build
>   qemu against libiscsi.
> * --enable-libiscsi
>   Force a build against libiscsi. If libiscsi is not available
>   the build will fail.
> * --disable-libiscsi
>   Do not link against libiscsi, even if it is available.
> 
> When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.
> 
> You can specify devices using a iscsi url of the form :
> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
> When using authentication, the password can optionally be set with
> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  Makefile.objs |    1 +
>  block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  configure     |   31 +++
>  trace-events  |    7 +
>  4 files changed, 635 insertions(+), 0 deletions(-)
>  create mode 100644 block/iscsi.c

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

ping?


On Thu, Sep 29, 2011 at 4:54 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Wed, Sep 21, 2011 at 07:37:55PM +1000, Ronnie Sahlberg wrote:
>> This provides built-in support for iSCSI to QEMU.
>> This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
>> It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.
>>
>> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>>     git://github.com/sahlberg/libiscsi.git
>>
>> The patch adds the driver to interface with the iscsi library.
>> It also updated the configure script to
>> * by default, probe is libiscsi is available and if so, build
>>   qemu against libiscsi.
>> * --enable-libiscsi
>>   Force a build against libiscsi. If libiscsi is not available
>>   the build will fail.
>> * --disable-libiscsi
>>   Do not link against libiscsi, even if it is available.
>>
>> When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.
>>
>> You can specify devices using a iscsi url of the form :
>> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
>> When using authentication, the password can optionally be set with
>> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> ---
>>  Makefile.objs |    1 +
>>  block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  configure     |   31 +++
>>  trace-events  |    7 +
>>  4 files changed, 635 insertions(+), 0 deletions(-)
>>  create mode 100644 block/iscsi.c
>
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

Previous version of the patch received very positive feedback and
several expressed seeing positive value of a built-in initiator.
I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.


Is there some other problem with the patch I am not aware of that I
should address?

I have been trying to push this patch in different versions since
December last year.
There is obviously a problem here I am not aware of.
Please advice what the problem is and I will try to rectify it.


Please advice on how I can move forward. I feel a bit at roads end
here. Please help.


regards
ronnie sahlberg



On Mon, Oct 10, 2011 at 7:46 AM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> ping?
>
>
> On Thu, Sep 29, 2011 at 4:54 PM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
>> On Wed, Sep 21, 2011 at 07:37:55PM +1000, Ronnie Sahlberg wrote:
>>> This provides built-in support for iSCSI to QEMU.
>>> This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
>>> It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.
>>>
>>> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>>>     git://github.com/sahlberg/libiscsi.git
>>>
>>> The patch adds the driver to interface with the iscsi library.
>>> It also updated the configure script to
>>> * by default, probe is libiscsi is available and if so, build
>>>   qemu against libiscsi.
>>> * --enable-libiscsi
>>>   Force a build against libiscsi. If libiscsi is not available
>>>   the build will fail.
>>> * --disable-libiscsi
>>>   Do not link against libiscsi, even if it is available.
>>>
>>> When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.
>>>
>>> You can specify devices using a iscsi url of the form :
>>> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
>>> When using authentication, the password can optionally be set with
>>> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
>>>
>>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>> ---
>>>  Makefile.objs |    1 +
>>>  block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  configure     |   31 +++
>>>  trace-events  |    7 +
>>>  4 files changed, 635 insertions(+), 0 deletions(-)
>>>  create mode 100644 block/iscsi.c
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>
>

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Paolo Bonzini]

On 10/13/2011 11:46 AM, ronnie sahlberg wrote:
> Previous version of the patch received very positive feedback and
> several expressed seeing positive value of a built-in initiator.
> I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.
>
>
> Is there some other problem with the patch I am not aware of that I
> should address?
>
> I have been trying to push this patch in different versions since
> December last year.
> There is obviously a problem here I am not aware of.
> Please advice what the problem is and I will try to rectify it.

The problem is just that everyone is busy. :)  The patch looks good to 
me too.

Paolo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Stefan Hajnoczi]

On Thu, Oct 13, 2011 at 10:46 AM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Previous version of the patch received very positive feedback and
> several expressed seeing positive value of a built-in initiator.
> I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.

I'm happy.

Stefan

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Daniel P. Berrange]

On Thu, Oct 13, 2011 at 08:46:54PM +1100, ronnie sahlberg wrote:
> Previous version of the patch received very positive feedback and
> several expressed seeing positive value of a built-in initiator.
> I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.
> 
> 
> Is there some other problem with the patch I am not aware of that I
> should address?
> 
> I have been trying to push this patch in different versions since
> December last year.
> There is obviously a problem here I am not aware of.
> Please advice what the problem is and I will try to rectify it.
> 
> 
> Please advice on how I can move forward. I feel a bit at roads end
> here. Please help.

I can't comment much on the code, but I'm supportive of QEMU gaining
this feature, because it addresses a number of use cases not satisfied
by using iSCSI via the host OS's block layer.


> >>> You can specify devices using a iscsi url of the form :
> >>> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
> >>> When using authentication, the password can optionally be set with
> >>> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list

I'm not a fan of sending passwords via command line args, or
environment variables.  Env variables *can* be exposed via
the process list, albeit not to unprivileged users. More
critically, env variables will end up in logfiles like
/var/log/libvirt/qemu/$GUESTNAME.log, and in data reported
to distro bug trackers, via tools like sosreport which
capture /proc/$PID/environ and aforementioned logfiles.

We have a similar requirement for specifying passwords with
the Ceph/RBD driver, and also for the curl/HTTP block drivers.
We have a monitor command for providing decryption passwords for
QCow2 disks. We could either reuse that for connection passwords,
or perhaps slightly better would be to have a separate command
for connection passwords.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Kevin Wolf]

Am 13.10.2011 11:46, schrieb ronnie sahlberg:
> Previous version of the patch received very positive feedback and
> several expressed seeing positive value of a built-in initiator.
> I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.
> 
> 
> Is there some other problem with the patch I am not aware of that I
> should address?

No problems that I know of, other than me being on vacation in the last
two weeks. I'm planning to have another quick look before I merge it,
but Stefan's ack suggests that it's ready to be merged and your part is
done.

Kevin

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Daniel P. Berrange]

On Thu, Oct 13, 2011 at 11:01:49AM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 13, 2011 at 08:46:54PM +1100, ronnie sahlberg wrote:
> > Previous version of the patch received very positive feedback and
> > several expressed seeing positive value of a built-in initiator.
> > I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it.
> > 
> > 
> > Is there some other problem with the patch I am not aware of that I
> > should address?
> > 
> > I have been trying to push this patch in different versions since
> > December last year.
> > There is obviously a problem here I am not aware of.
> > Please advice what the problem is and I will try to rectify it.
> > 
> > 
> > Please advice on how I can move forward. I feel a bit at roads end
> > here. Please help.
> 
> I can't comment much on the code, but I'm supportive of QEMU gaining
> this feature, because it addresses a number of use cases not satisfied
> by using iSCSI via the host OS's block layer.
> 
> 
> > >>> You can specify devices using a iscsi url of the form :
> > >>> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
> > >>> When using authentication, the password can optionally be set with
> > >>> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
> 
> I'm not a fan of sending passwords via command line args, or
> environment variables.  Env variables *can* be exposed via
> the process list, albeit not to unprivileged users. More
> critically, env variables will end up in logfiles like
> /var/log/libvirt/qemu/$GUESTNAME.log, and in data reported
> to distro bug trackers, via tools like sosreport which
> capture /proc/$PID/environ and aforementioned logfiles.
> 
> We have a similar requirement for specifying passwords with
> the Ceph/RBD driver, and also for the curl/HTTP block drivers.
> We have a monitor command for providing decryption passwords for
> QCow2 disks. We could either reuse that for connection passwords,
> or perhaps slightly better would be to have a separate command
> for connection passwords.

NB, I didn't mean to suggest that this issue should block merging
of this iSCSI driver. The problem with passwords already exists for
Ceph & Curl drivers, and I believe the Ceph developers are already
working on a patch for QEMU which should be able to apply to all
these network block devs

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Kevin Wolf]

Am 21.09.2011 11:37, schrieb Ronnie Sahlberg:
> This provides built-in support for iSCSI to QEMU.
> This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
> It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.
> 
> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>     git://github.com/sahlberg/libiscsi.git
> 
> The patch adds the driver to interface with the iscsi library.
> It also updated the configure script to
> * by default, probe is libiscsi is available and if so, build
>   qemu against libiscsi.
> * --enable-libiscsi
>   Force a build against libiscsi. If libiscsi is not available
>   the build will fail.
> * --disable-libiscsi
>   Do not link against libiscsi, even if it is available.
> 
> When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.
> 
> You can specify devices using a iscsi url of the form :
> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
> When using authentication, the password can optionally be set with
> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>

I have a few comments, but none of them are really critical and in
general the patch looks good to me.

So please let me know if you intend to send another version with the
comments addressed or if I should commit this version and you'll send
fixes/cleanups on top. I don't really mind at this point.

> ---
>  Makefile.objs |    1 +
>  block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  configure     |   31 +++
>  trace-events  |    7 +
>  4 files changed, 635 insertions(+), 0 deletions(-)
>  create mode 100644 block/iscsi.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index a529a11..8c8e420 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -36,6 +36,7 @@ block-nested-y += qed-check.o
>  block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
> +block-nested-$(CONFIG_LIBISCSI) += iscsi.o
>  block-nested-$(CONFIG_CURL) += curl.o
>  block-nested-$(CONFIG_RBD) += rbd.o
>  
> diff --git a/block/iscsi.c b/block/iscsi.c
> new file mode 100644
> index 0000000..6517576
> --- /dev/null
> +++ b/block/iscsi.c
> @@ -0,0 +1,596 @@
> +/*
> + * QEMU Block driver for iSCSI images
> + *
> + * Copyright (c) 2010-2011 Ronnie Sahlberg <ronniesahlberg@gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "config-host.h"
> +
> +#include <poll.h>
> +#include "sysemu.h"

I doubt that you really need sysemu.h. If you did, I think you couldn't
successfully build qemu-img and qemu-io.

> +#include "qemu-common.h"
> +#include "qemu-error.h"
> +#include "block_int.h"
> +#include "trace.h"
> +
> +#include <iscsi/iscsi.h>
> +#include <iscsi/scsi-lowlevel.h>
> +
> +
> +typedef struct IscsiLun {
> +    struct iscsi_context *iscsi;
> +    int lun;
> +    int block_size;
> +    unsigned long num_blocks;
> +} IscsiLun;
> +
> +typedef struct IscsiAIOCB {
> +    BlockDriverAIOCB common;
> +    QEMUIOVector *qiov;
> +    QEMUBH *bh;
> +    IscsiLun *iscsilun;
> +    struct scsi_task *task;
> +    uint8_t *buf;
> +    int canceled;
> +    int status;
> +    size_t read_size;
> +    size_t read_offset;
> +} IscsiAIOCB;
> +
> +struct IscsiTask {
> +    IscsiLun *iscsilun;
> +    int status;
> +    int complete;
> +};
> +
> +static void
> +iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
> +                    void *private_data)
> +{
> +}
> +
> +static void
> +iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
> +{
> +    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> +    IscsiLun *iscsilun = acb->iscsilun;
> +
> +    acb->status = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->status);
> +    acb->canceled = 1;

Is acb->status == -ECANCELED redundant with acb->canceled == 1?

In some places you check the former, in other place the latter, and I
don't understand the difference.

> +
> +    /* send a task mgmt call to the target to cancel the task on the target */
> +    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> +                                     iscsi_abort_task_cb, NULL);
> +
> +    /* then also cancel the task locally in libiscsi */
> +    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
> +}
> +
> +static AIOPool iscsi_aio_pool = {
> +    .aiocb_size         = sizeof(IscsiAIOCB),
> +    .cancel             = iscsi_aio_cancel,
> +};
> +
> +
> +static void iscsi_process_read(void *arg);
> +static void iscsi_process_write(void *arg);
> +
> +static int iscsi_process_flush(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +
> +    return iscsi_queue_length(iscsilun->iscsi) > 0;
> +}
> +
> +static void
> +iscsi_set_events(IscsiLun *iscsilun)
> +{
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
> +                           (iscsi_which_events(iscsi) & POLLOUT)
> +                           ? iscsi_process_write : NULL,
> +                           iscsi_process_flush, NULL, iscsilun);
> +}
> +
> +static void
> +iscsi_process_read(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    iscsi_service(iscsi, POLLIN);
> +    iscsi_set_events(iscsilun);
> +}
> +
> +static void
> +iscsi_process_write(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    iscsi_service(iscsi, POLLOUT);
> +    iscsi_set_events(iscsilun);
> +}
> +
> +
> +static int
> +iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
> +{
> +    acb->bh = qemu_bh_new(cb, acb);
> +    if (!acb->bh) {
> +        error_report("oom: could not create iscsi bh");
> +        return -EIO;
> +    }
> +
> +    qemu_bh_schedule(acb->bh);
> +    return 0;
> +}
> +
> +static void
> +iscsi_readv_writev_bh_cb(void *p)
> +{
> +    IscsiAIOCB *acb = p;
> +
> +    qemu_bh_delete(acb->bh);
> +
> +    if (acb->status != -ECANCELED) {
> +        acb->common.cb(acb->common.opaque, acb->status);
> +    }
> +
> +    qemu_aio_release(acb);
> +}
> +
> +
> +static void
> +iscsi_aio_write10_cb(struct iscsi_context *iscsi, int status,
> +                     void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    trace_iscsi_aio_write10_cb(iscsi, status, acb, acb->canceled);
> +
> +    g_free(acb->buf);
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status < 0) {
> +        error_report("Failed to write10 data to iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
> +{
> +    return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
> +                 QEMUIOVector *qiov, int nb_sectors,
> +                 BlockDriverCompletionFunc *cb,
> +                 void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +    size_t size;
> +    int fua = 0;
> +
> +    /* set FUA on writes when cache mode is write through */
> +    if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
> +        fua = 1;
> +    }
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
> +    if (!acb) {
> +        return NULL;
> +    }
> +
> +    acb->iscsilun = iscsilun;
> +    acb->qiov     = qiov;
> +
> +    acb->canceled   = 0;
> +
> +    /* XXX we should pass the iovec to write10 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_buffer(acb->qiov, acb->buf);
> +    acb->task = iscsi_write10_task(iscsi, iscsilun->lun, acb->buf, size,
> +                              sector_qemu2lun(sector_num, iscsilun),
> +                              fua, 0, iscsilun->block_size,
> +                              iscsi_aio_write10_cb, acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send write10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        g_free(acb->buf);
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +static void
> +iscsi_aio_read10_cb(struct iscsi_context *iscsi, int status,
> +                    void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    trace_iscsi_aio_read10_cb(iscsi, status, acb, acb->canceled);
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status != 0) {
> +        error_report("Failed to read10 data from iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
> +                QEMUIOVector *qiov, int nb_sectors,
> +                BlockDriverCompletionFunc *cb,
> +                void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +    size_t qemu_read_size, lun_read_size;
> +    int i;
> +
> +    qemu_read_size = BDRV_SECTOR_SIZE * (size_t)nb_sectors;
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    trace_iscsi_aio_readv(iscsi, sector_num, nb_sectors, opaque, acb);
> +    if (!acb) {
> +        return NULL;
> +    }
> +
> +    acb->iscsilun = iscsilun;
> +    acb->qiov     = qiov;
> +
> +    acb->canceled    = 0;
> +    acb->read_size   = qemu_read_size;
> +    acb->buf         = NULL;
> +
> +    /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
> +     * may be misaligned to the LUN, so we may need to read some extra
> +     * data.
> +     */
> +    acb->read_offset = 0;
> +    if (iscsilun->block_size > BDRV_SECTOR_SIZE) {
> +        uint64_t bdrv_offset = BDRV_SECTOR_SIZE * sector_num;
> +
> +        acb->read_offset  = bdrv_offset % iscsilun->block_size;
> +    }
> +
> +    lun_read_size  = (qemu_read_size + iscsilun->block_size
> +                     + acb->read_offset - 1)
> +                     / iscsilun->block_size * iscsilun->block_size;
> +    acb->task = iscsi_read10_task(iscsi, iscsilun->lun,
> +                             sector_qemu2lun(sector_num, iscsilun),
> +                             lun_read_size, iscsilun->block_size,
> +                             iscsi_aio_read10_cb, acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send read10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < acb->qiov->niov; i++) {
> +        scsi_task_add_data_in_buffer(acb->task,
> +                acb->qiov->iov[i].iov_len,
> +                acb->qiov->iov[i].iov_base);
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +
> +static void
> +iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
> +                     void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status < 0) {
> +        error_report("Failed to sync10 data on iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_flush(BlockDriverState *bs,
> +                BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    if (!acb) {
> +        return NULL;
> +    }

qemu_aio_get() never returns NULL, so this check is unnecessary.

> +
> +    acb->iscsilun = iscsilun;
> +    acb->canceled   = 0;
> +
> +    acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
> +                                         0, 0, 0, 0,
> +                                         iscsi_synccache10_cb,
> +                                         acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send synchronizecache10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +static int64_t
> +iscsi_getlength(BlockDriverState *bs)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    int64_t len;
> +
> +    len  = iscsilun->num_blocks;
> +    len *= iscsilun->block_size;
> +
> +    return len;
> +}
> +
> +static void
> +iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
> +                        void *command_data, void *opaque)
> +{
> +    struct IscsiTask *itask = opaque;
> +    struct scsi_readcapacity10 *rc10;
> +    struct scsi_task *task = command_data;
> +
> +    if (status != 0) {
> +        error_report("iSCSI: Failed to read capacity of iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        scsi_free_scsi_task(task);
> +        return;
> +    }
> +
> +    rc10 = scsi_datain_unmarshall(task);
> +    if (rc10 == NULL) {
> +        error_report("iSCSI: Failed to unmarshall readcapacity10 data.");
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        scsi_free_scsi_task(task);
> +        return;
> +    }
> +
> +    itask->iscsilun->block_size = rc10->block_size;
> +    itask->iscsilun->num_blocks = rc10->lba;

I think you should also set bs->total_sectors.

> +
> +    itask->status   = 0;
> +    itask->complete = 1;
> +    scsi_free_scsi_task(task);
> +}
> +
> +
> +static void
> +iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
> +                 void *opaque)
> +{
> +    struct IscsiTask *itask = opaque;
> +    struct scsi_task *task;
> +
> +    if (status != 0) {
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        return;
> +    }
> +
> +    task = iscsi_readcapacity10_task(iscsi, itask->iscsilun->lun, 0, 0,
> +                                   iscsi_readcapacity10_cb, opaque);
> +    if (task == NULL) {
> +        error_report("iSCSI: failed to send readcapacity command.");
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        return;
> +    }
> +}
> +
> +/*
> + * We support iscsi url's on the form
> + * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> + */
> +static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = NULL;
> +    struct iscsi_url *iscsi_url = NULL;
> +    struct IscsiTask task;
> +    int ret;
> +
> +    if ((BDRV_SECTOR_SIZE % 512) != 0) {
> +        error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. "
> +                     "BDRV_SECTOR_SIZE(%lld) is not a multiple "
> +                     "of 512", BDRV_SECTOR_SIZE);
> +        return -EINVAL;
> +    }
> +
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +
> +    /* Should really append the KVM name after the ':' here */
> +    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
> +    if (iscsi == NULL) {
> +        error_report("iSCSI: Failed to create iSCSI context.");
> +        ret = -ENOMEM;
> +        goto failed;
> +    }
> +
> +    iscsi_url = iscsi_parse_full_url(iscsi, filename);
> +    if (iscsi_url == NULL) {
> +        error_report("Failed to parse URL : %s %s", filename,
> +                     iscsi_get_error(iscsi));
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
> +        error_report("iSCSI: Failed to set target name.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    if (iscsi_url->user != NULL) {
> +        ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
> +                                              iscsi_url->passwd);
> +        if (ret != 0) {
> +            error_report("Failed to set initiator username and password");
> +            ret = -EINVAL;
> +            goto failed;
> +        }
> +    }
> +    if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
> +        error_report("iSCSI: Failed to set session type to normal.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> +
> +    task.iscsilun = iscsilun;
> +    task.status = 0;
> +    task.complete = 0;
> +
> +    iscsilun->iscsi = iscsi;
> +    iscsilun->lun   = iscsi_url->lun;
> +
> +    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
> +                                 iscsi_connect_cb, &task)
> +        != 0) {
> +        error_report("iSCSI: Failed to start async connect.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    while (!task.complete) {
> +        iscsi_set_events(iscsilun);
> +        qemu_aio_wait();
> +    }
> +    if (task.status != 0) {
> +        error_report("iSCSI: Failed to connect to LUN : %s",
> +                     iscsi_get_error(iscsi));
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    return 0;
> +
> +failed:
> +    if (iscsi_url != NULL) {
> +        iscsi_destroy_url(iscsi_url);
> +    }

iscsi_destroy_url() is only ever called here. Do we leak it in the
normal path?

> +    if (iscsi != NULL) {
> +        iscsi_destroy_context(iscsi);
> +    }
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +    return ret;
> +}
> +
> +static void iscsi_close(BlockDriverState *bs)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL, NULL);
> +    iscsi_destroy_context(iscsi);
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +
> +static BlockDriver bdrv_iscsi = {
> +    .format_name     = "iscsi",
> +    .protocol_name   = "iscsi",
> +
> +    .instance_size   = sizeof(IscsiLun),
> +    .bdrv_file_open  = iscsi_open,
> +    .bdrv_close      = iscsi_close,
> +
> +    .bdrv_getlength  = iscsi_getlength,
> +
> +    .bdrv_aio_readv  = iscsi_aio_readv,
> +    .bdrv_aio_writev = iscsi_aio_writev,
> +    .bdrv_aio_flush  = iscsi_aio_flush,
> +};
> +
> +static void iscsi_block_init(void)
> +{
> +    bdrv_register(&bdrv_iscsi);
> +}
> +
> +block_init(iscsi_block_init);
> +
> diff --git a/configure b/configure
> index 348f36a..a2c30e1 100755
> --- a/configure
> +++ b/configure
> @@ -182,6 +182,7 @@ usb_redir=""
>  opengl=""
>  zlib="yes"
>  guest_agent="yes"
> +libiscsi=""
>  
>  # parse CC options first
>  for opt do
> @@ -651,6 +652,10 @@ for opt do
>    ;;
>    --enable-spice) spice="yes"
>    ;;
> +  --disable-libiscsi) libiscsi="no"
> +  ;;
> +  --enable-libiscsi) libiscsi="yes"
> +  ;;
>    --enable-profiler) profiler="yes"
>    ;;
>    --enable-cocoa)
> @@ -1037,6 +1042,8 @@ echo "                           Default:trace-<pid>"
>  echo "  --disable-spice          disable spice"
>  echo "  --enable-spice           enable spice"
>  echo "  --enable-rbd             enable building the rados block device (rbd)"
> +echo "  --disable-libiscsi       disable iscsi support"
> +echo "  --enable-libiscsi        enable iscsi support"
>  echo "  --disable-smartcard      disable smartcard support"
>  echo "  --enable-smartcard       enable smartcard support"
>  echo "  --disable-smartcard-nss  disable smartcard nss support"
> @@ -2326,6 +2333,25 @@ if compile_prog "" "" ; then
>  fi
>  
>  ##########################################
> +# Do we have libiscsi
> +if test "$libiscsi" != "no" ; then
> +  cat > $TMPC << EOF
> +#include <iscsi/iscsi.h>
> +int main(void) { iscsi_create_context(""); return 0; }
> +EOF
> +  if compile_prog "-Werror" "-liscsi" ; then
> +    libiscsi="yes"
> +    LIBS="$LIBS -liscsi"
> +  else
> +    if test "$libiscsi" = "yes" ; then
> +      feature_not_found "libiscsi"
> +    fi
> +    libiscsi="no"
> +  fi
> +fi
> +
> +
> +##########################################
>  # Do we need librt
>  cat > $TMPC <<EOF
>  #include <signal.h>
> @@ -2727,6 +2753,7 @@ echo "xfsctl support    $xfs"
>  echo "nss used          $smartcard_nss"
>  echo "usb net redir     $usb_redir"
>  echo "OpenGL support    $opengl"
> +echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>  
>  if test "$sdl_too_old" = "yes"; then
> @@ -3028,6 +3055,10 @@ if test "$opengl" = "yes" ; then
>    echo "CONFIG_OPENGL=y" >> $config_host_mak
>  fi
>  
> +if test "$libiscsi" = "yes" ; then
> +  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
> +fi
> +
>  # XXX: suppress that
>  if [ "$bsd" = "yes" ] ; then
>    echo "CONFIG_BSD=y" >> $config_host_mak
> diff --git a/trace-events b/trace-events
> index 3fdd60f..ce68516 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -494,3 +494,10 @@ escc_sunkbd_event_in(int ch) "Untranslated keycode %2.2x"
>  escc_sunkbd_event_out(int ch) "Translated keycode %2.2x"
>  escc_kbd_command(int val) "Command %d"
>  escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
> +
> +# block/iscsi.c
> +iscsi_aio_write10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
> +iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
> +iscsi_aio_read10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
> +iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
> +

Kevin

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

Kevin,

Thanks for the review,
I have attached a modified patch that addresses all your comments.
Please review and/or apply.


> I doubt that you really need sysemu.h. If you did, I think you couldn't successfully build qemu-img and qemu-io.

You are right. I removed it.

> Is acb->status == -ECANCELED redundant with acb->canceled == 1?

Fixed.

> qemu_aio_get() never returns NULL, so this check is unnecessary.

Done.

> I think you should also set bs->total_sectors.

Done.

> iscsi_destroy_url() is only ever called here. Do we leak it in the normal path?

It didnt leak but the the label was confusing. I have changed it from
"failed" to "finished" since this part
is executed for both normal and failures.



Thanks
Ronnie Sahlberg

[-- Attachment #2: 0001-This-provides-built-in-support-for-iSCSI-to-QEMU.patch --]
[-- Type: text/x-diff, Size: 22253 bytes --]

From f5fc5880a3071188dbfec903bc1ea1247a1e375c Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date: Tue, 25 Oct 2011 19:03:18 +1100
Subject: [PATCH] This provides built-in support for iSCSI to QEMU.
 This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
 It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.

This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
    git://github.com/sahlberg/libiscsi.git

The patch adds the driver to interface with the iscsi library.
It also updated the configure script to
* by default, probe is libiscsi is available and if so, build
  qemu against libiscsi.
* --enable-libiscsi
  Force a build against libiscsi. If libiscsi is not available
  the build will fail.
* --disable-libiscsi
  Do not link against libiscsi, even if it is available.

When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.

You can specify devices using a iscsi url of the form :
iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
When using authentication, the password can optionally be set with
LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 Makefile.objs |    1 +
 block/iscsi.c |  589 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure     |   31 +++
 trace-events  |    6 +
 4 files changed, 627 insertions(+), 0 deletions(-)
 create mode 100644 block/iscsi.c

diff --git a/Makefile.objs b/Makefile.objs
index 01587c8..d18417c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -36,6 +36,7 @@ block-nested-y += qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
+block-nested-$(CONFIG_LIBISCSI) += iscsi.o
 block-nested-$(CONFIG_CURL) += curl.o
 block-nested-$(CONFIG_RBD) += rbd.o
 
diff --git a/block/iscsi.c b/block/iscsi.c
new file mode 100644
index 0000000..b105ab7
--- /dev/null
+++ b/block/iscsi.c
@@ -0,0 +1,589 @@
+/*
+ * QEMU Block driver for iSCSI images
+ *
+ * Copyright (c) 2010-2011 Ronnie Sahlberg <ronniesahlberg@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include <poll.h>
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "block_int.h"
+#include "trace.h"
+
+#include <iscsi/iscsi.h>
+#include <iscsi/scsi-lowlevel.h>
+
+
+typedef struct IscsiLun {
+    struct iscsi_context *iscsi;
+    int lun;
+    int block_size;
+    unsigned long num_blocks;
+} IscsiLun;
+
+typedef struct IscsiAIOCB {
+    BlockDriverAIOCB common;
+    QEMUIOVector *qiov;
+    QEMUBH *bh;
+    IscsiLun *iscsilun;
+    struct scsi_task *task;
+    uint8_t *buf;
+    int status;
+    int canceled;
+    size_t read_size;
+    size_t read_offset;
+} IscsiAIOCB;
+
+struct IscsiTask {
+    IscsiLun *iscsilun;
+    BlockDriverState *bs;
+    int status;
+    int complete;
+};
+
+static void
+iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                    void *private_data)
+{
+}
+
+static void
+iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
+    IscsiLun *iscsilun = acb->iscsilun;
+
+    acb->common.cb(acb->common.opaque, -ECANCELED);
+    acb->canceled = 1;
+
+    /* send a task mgmt call to the target to cancel the task on the target */
+    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
+                                     iscsi_abort_task_cb, NULL);
+
+    /* then also cancel the task locally in libiscsi */
+    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
+}
+
+static AIOPool iscsi_aio_pool = {
+    .aiocb_size         = sizeof(IscsiAIOCB),
+    .cancel             = iscsi_aio_cancel,
+};
+
+
+static void iscsi_process_read(void *arg);
+static void iscsi_process_write(void *arg);
+
+static int iscsi_process_flush(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+
+    return iscsi_queue_length(iscsilun->iscsi) > 0;
+}
+
+static void
+iscsi_set_events(IscsiLun *iscsilun)
+{
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
+                           (iscsi_which_events(iscsi) & POLLOUT)
+                           ? iscsi_process_write : NULL,
+                           iscsi_process_flush, NULL, iscsilun);
+}
+
+static void
+iscsi_process_read(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLIN);
+    iscsi_set_events(iscsilun);
+}
+
+static void
+iscsi_process_write(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLOUT);
+    iscsi_set_events(iscsilun);
+}
+
+
+static int
+iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
+{
+    acb->bh = qemu_bh_new(cb, acb);
+    if (!acb->bh) {
+        error_report("oom: could not create iscsi bh");
+        return -EIO;
+    }
+
+    qemu_bh_schedule(acb->bh);
+    return 0;
+}
+
+static void
+iscsi_readv_writev_bh_cb(void *p)
+{
+    IscsiAIOCB *acb = p;
+
+    qemu_bh_delete(acb->bh);
+
+    if (acb->canceled == 0) {
+        acb->common.cb(acb->common.opaque, acb->status);
+    }
+
+    qemu_aio_release(acb);
+}
+
+
+static void
+iscsi_aio_write10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_write10_cb(iscsi, status, acb, acb->canceled);
+
+    g_free(acb->buf);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to write10 data to iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
+{
+    return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                 QEMUIOVector *qiov, int nb_sectors,
+                 BlockDriverCompletionFunc *cb,
+                 void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t size;
+    int fua = 0;
+
+    /* set FUA on writes when cache mode is write through */
+    if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
+        fua = 1;
+    }
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled   = 0;
+
+    /* XXX we should pass the iovec to write10 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_buffer(acb->qiov, acb->buf);
+    acb->task = iscsi_write10_task(iscsi, iscsilun->lun, acb->buf, size,
+                              sector_qemu2lun(sector_num, iscsilun),
+                              fua, 0, iscsilun->block_size,
+                              iscsi_aio_write10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send write10 command. %s",
+                     iscsi_get_error(iscsi));
+        g_free(acb->buf);
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static void
+iscsi_aio_read10_cb(struct iscsi_context *iscsi, int status,
+                    void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_read10_cb(iscsi, status, acb, acb->canceled);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status != 0) {
+        error_report("Failed to read10 data from iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                QEMUIOVector *qiov, int nb_sectors,
+                BlockDriverCompletionFunc *cb,
+                void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t qemu_read_size, lun_read_size;
+    int i;
+
+    qemu_read_size = BDRV_SECTOR_SIZE * (size_t)nb_sectors;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_readv(iscsi, sector_num, nb_sectors, opaque, acb);
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled    = 0;
+    acb->read_size   = qemu_read_size;
+    acb->buf         = NULL;
+
+    /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
+     * may be misaligned to the LUN, so we may need to read some extra
+     * data.
+     */
+    acb->read_offset = 0;
+    if (iscsilun->block_size > BDRV_SECTOR_SIZE) {
+        uint64_t bdrv_offset = BDRV_SECTOR_SIZE * sector_num;
+
+        acb->read_offset  = bdrv_offset % iscsilun->block_size;
+    }
+
+    lun_read_size  = (qemu_read_size + iscsilun->block_size
+                     + acb->read_offset - 1)
+                     / iscsilun->block_size * iscsilun->block_size;
+    acb->task = iscsi_read10_task(iscsi, iscsilun->lun,
+                             sector_qemu2lun(sector_num, iscsilun),
+                             lun_read_size, iscsilun->block_size,
+                             iscsi_aio_read10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send read10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    for (i = 0; i < acb->qiov->niov; i++) {
+        scsi_task_add_data_in_buffer(acb->task,
+                acb->qiov->iov[i].iov_len,
+                acb->qiov->iov[i].iov_base);
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+
+static void
+iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to sync10 data on iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_flush(BlockDriverState *bs,
+                BlockDriverCompletionFunc *cb, void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+
+    acb->iscsilun = iscsilun;
+    acb->canceled   = 0;
+
+    acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
+                                         0, 0, 0, 0,
+                                         iscsi_synccache10_cb,
+                                         acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send synchronizecache10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static int64_t
+iscsi_getlength(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    int64_t len;
+
+    len  = iscsilun->num_blocks;
+    len *= iscsilun->block_size;
+
+    return len;
+}
+
+static void
+iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
+                        void *command_data, void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_readcapacity10 *rc10;
+    struct scsi_task *task = command_data;
+
+    if (status != 0) {
+        error_report("iSCSI: Failed to read capacity of iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    rc10 = scsi_datain_unmarshall(task);
+    if (rc10 == NULL) {
+        error_report("iSCSI: Failed to unmarshall readcapacity10 data.");
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    itask->iscsilun->block_size = rc10->block_size;
+    itask->iscsilun->num_blocks = rc10->lba;
+    itask->bs->total_sectors = (uint64_t)rc10->lba *
+                               rc10->block_size / BDRV_SECTOR_SIZE ;
+
+    itask->status   = 0;
+    itask->complete = 1;
+    scsi_free_scsi_task(task);
+}
+
+
+static void
+iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                 void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_task *task;
+
+    if (status != 0) {
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+
+    task = iscsi_readcapacity10_task(iscsi, itask->iscsilun->lun, 0, 0,
+                                   iscsi_readcapacity10_cb, opaque);
+    if (task == NULL) {
+        error_report("iSCSI: failed to send readcapacity command.");
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+}
+
+/*
+ * We support iscsi url's on the form
+ * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
+ */
+static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = NULL;
+    struct iscsi_url *iscsi_url = NULL;
+    struct IscsiTask task;
+    int ret;
+
+    if ((BDRV_SECTOR_SIZE % 512) != 0) {
+        error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. "
+                     "BDRV_SECTOR_SIZE(%lld) is not a multiple "
+                     "of 512", BDRV_SECTOR_SIZE);
+        return -EINVAL;
+    }
+
+    memset(iscsilun, 0, sizeof(IscsiLun));
+
+    /* Should really append the KVM name after the ':' here */
+    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
+    if (iscsi == NULL) {
+        error_report("iSCSI: Failed to create iSCSI context.");
+        ret = -ENOMEM;
+        goto finished;
+    }
+
+    iscsi_url = iscsi_parse_full_url(iscsi, filename);
+    if (iscsi_url == NULL) {
+        error_report("Failed to parse URL : %s %s", filename,
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto finished;
+    }
+
+    if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
+        error_report("iSCSI: Failed to set target name.");
+        ret = -EINVAL;
+        goto finished;
+    }
+
+    if (iscsi_url->user != NULL) {
+        ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
+                                              iscsi_url->passwd);
+        if (ret != 0) {
+            error_report("Failed to set initiator username and password");
+            ret = -EINVAL;
+            goto finished;
+        }
+    }
+    if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
+        error_report("iSCSI: Failed to set session type to normal.");
+        ret = -EINVAL;
+        goto finished;
+    }
+
+    iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+
+    task.iscsilun = iscsilun;
+    task.status = 0;
+    task.complete = 0;
+    task.bs = bs;
+
+    iscsilun->iscsi = iscsi;
+    iscsilun->lun   = iscsi_url->lun;
+
+    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
+                                 iscsi_connect_cb, &task)
+        != 0) {
+        error_report("iSCSI: Failed to start async connect.");
+        ret = -EINVAL;
+        goto finished;
+    }
+
+    while (!task.complete) {
+        iscsi_set_events(iscsilun);
+        qemu_aio_wait();
+    }
+    if (task.status != 0) {
+        error_report("iSCSI: Failed to connect to LUN : %s",
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto finished;
+    }
+
+    return 0;
+
+finished:
+    if (iscsi_url != NULL) {
+        iscsi_destroy_url(iscsi_url);
+    }
+    if (iscsi != NULL) {
+        iscsi_destroy_context(iscsi);
+    }
+    memset(iscsilun, 0, sizeof(IscsiLun));
+    return ret;
+}
+
+static void iscsi_close(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL, NULL);
+    iscsi_destroy_context(iscsi);
+    memset(iscsilun, 0, sizeof(IscsiLun));
+}
+
+static BlockDriver bdrv_iscsi = {
+    .format_name     = "iscsi",
+    .protocol_name   = "iscsi",
+
+    .instance_size   = sizeof(IscsiLun),
+    .bdrv_file_open  = iscsi_open,
+    .bdrv_close      = iscsi_close,
+
+    .bdrv_getlength  = iscsi_getlength,
+
+    .bdrv_aio_readv  = iscsi_aio_readv,
+    .bdrv_aio_writev = iscsi_aio_writev,
+    .bdrv_aio_flush  = iscsi_aio_flush,
+};
+
+static void iscsi_block_init(void)
+{
+    bdrv_register(&bdrv_iscsi);
+}
+
+block_init(iscsi_block_init);
+
diff --git a/configure b/configure
index 4f87e0a..3009bbc 100755
--- a/configure
+++ b/configure
@@ -182,6 +182,7 @@ usb_redir=""
 opengl=""
 zlib="yes"
 guest_agent="yes"
+libiscsi=""
 
 # parse CC options first
 for opt do
@@ -657,6 +658,10 @@ for opt do
   ;;
   --enable-spice) spice="yes"
   ;;
+  --disable-libiscsi) libiscsi="no"
+  ;;
+  --enable-libiscsi) libiscsi="yes"
+  ;;
   --enable-profiler) profiler="yes"
   ;;
   --enable-cocoa)
@@ -1046,6 +1051,8 @@ echo "                           Default:trace-<pid>"
 echo "  --disable-spice          disable spice"
 echo "  --enable-spice           enable spice"
 echo "  --enable-rbd             enable building the rados block device (rbd)"
+echo "  --disable-libiscsi       disable iscsi support"
+echo "  --enable-libiscsi        enable iscsi support"
 echo "  --disable-smartcard      disable smartcard support"
 echo "  --enable-smartcard       enable smartcard support"
 echo "  --disable-smartcard-nss  disable smartcard nss support"
@@ -2335,6 +2342,25 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# Do we have libiscsi
+if test "$libiscsi" != "no" ; then
+  cat > $TMPC << EOF
+#include <iscsi/iscsi.h>
+int main(void) { iscsi_create_context(""); return 0; }
+EOF
+  if compile_prog "-Werror" "-liscsi" ; then
+    libiscsi="yes"
+    LIBS="$LIBS -liscsi"
+  else
+    if test "$libiscsi" = "yes" ; then
+      feature_not_found "libiscsi"
+    fi
+    libiscsi="no"
+  fi
+fi
+
+
+##########################################
 # Do we need librt
 cat > $TMPC <<EOF
 #include <signal.h>
@@ -2744,6 +2770,7 @@ echo "xfsctl support    $xfs"
 echo "nss used          $smartcard_nss"
 echo "usb net redir     $usb_redir"
 echo "OpenGL support    $opengl"
+echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 
 if test "$sdl_too_old" = "yes"; then
@@ -3042,6 +3069,10 @@ if test "$opengl" = "yes" ; then
   echo "CONFIG_OPENGL=y" >> $config_host_mak
 fi
 
+if test "$libiscsi" = "yes" ; then
+  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
diff --git a/trace-events b/trace-events
index 7f9cec4..8890c83 100644
--- a/trace-events
+++ b/trace-events
@@ -504,6 +504,12 @@ escc_sunkbd_event_out(int ch) "Translated keycode %2.2x"
 escc_kbd_command(int val) "Command %d"
 escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
 
+# block/iscsi.c
+iscsi_aio_write10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+iscsi_aio_read10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+
 # hw/esp.c
 esp_raise_irq(void) "Raise IRQ"
 esp_lower_irq(void) "Lower IRQ"
-- 
1.7.3.1


[-- Attachment #3: 0001-This-provides-built-in-support-for-iSCSI-to-QEMU.patch.gz --]
[-- Type: application/x-gzip, Size: 6079 bytes --]

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Kevin Wolf]

Am 25.10.2011 10:04, schrieb ronnie sahlberg:
>> iscsi_destroy_url() is only ever called here. Do we leak it in the normal path?
> 
> It didnt leak but the the label was confusing. I have changed it from
> "failed" to "finished" since this part
> is executed for both normal and failures.

But there's a return 0; immediately before the label?

Kevin

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[ronnie sahlberg]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Hi,


On Tue, Oct 25, 2011 at 7:17 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 25.10.2011 10:04, schrieb ronnie sahlberg:
>>> iscsi_destroy_url() is only ever called here. Do we leak it in the normal path?
>>
>> It didnt leak but the the label was confusing. I have changed it from
>> "failed" to "finished" since this part
>> is executed for both normal and failures.
>
> But there's a return 0; immediately before the label?

Right. Its been a long day :-)

Please find attached a fixed version.


regards
ronnie sahlberg

>
> Kevin
>

[-- Attachment #2: 0001-This-provides-built-in-support-for-iSCSI-to-QEMU.patch --]
[-- Type: text/x-diff, Size: 22313 bytes --]

From 02c729b92d8fec01a04669a6dca815db2a5dc35b Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date: Tue, 25 Oct 2011 19:24:24 +1100
Subject: [PATCH] This provides built-in support for iSCSI to QEMU.
 This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
 It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.

This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
    git://github.com/sahlberg/libiscsi.git

The patch adds the driver to interface with the iscsi library.
It also updated the configure script to
* by default, probe is libiscsi is available and if so, build
  qemu against libiscsi.
* --enable-libiscsi
  Force a build against libiscsi. If libiscsi is not available
  the build will fail.
* --disable-libiscsi
  Do not link against libiscsi, even if it is available.

When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.

You can specify devices using a iscsi url of the form :
iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
When using authentication, the password can optionally be set with
LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 Makefile.objs |    1 +
 block/iscsi.c |  592 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure     |   31 +++
 trace-events  |    6 +
 4 files changed, 630 insertions(+), 0 deletions(-)
 create mode 100644 block/iscsi.c

diff --git a/Makefile.objs b/Makefile.objs
index 01587c8..d18417c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -36,6 +36,7 @@ block-nested-y += qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
+block-nested-$(CONFIG_LIBISCSI) += iscsi.o
 block-nested-$(CONFIG_CURL) += curl.o
 block-nested-$(CONFIG_RBD) += rbd.o
 
diff --git a/block/iscsi.c b/block/iscsi.c
new file mode 100644
index 0000000..010a9f6
--- /dev/null
+++ b/block/iscsi.c
@@ -0,0 +1,592 @@
+/*
+ * QEMU Block driver for iSCSI images
+ *
+ * Copyright (c) 2010-2011 Ronnie Sahlberg <ronniesahlberg@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include <poll.h>
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "block_int.h"
+#include "trace.h"
+
+#include <iscsi/iscsi.h>
+#include <iscsi/scsi-lowlevel.h>
+
+
+typedef struct IscsiLun {
+    struct iscsi_context *iscsi;
+    int lun;
+    int block_size;
+    unsigned long num_blocks;
+} IscsiLun;
+
+typedef struct IscsiAIOCB {
+    BlockDriverAIOCB common;
+    QEMUIOVector *qiov;
+    QEMUBH *bh;
+    IscsiLun *iscsilun;
+    struct scsi_task *task;
+    uint8_t *buf;
+    int status;
+    int canceled;
+    size_t read_size;
+    size_t read_offset;
+} IscsiAIOCB;
+
+struct IscsiTask {
+    IscsiLun *iscsilun;
+    BlockDriverState *bs;
+    int status;
+    int complete;
+};
+
+static void
+iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                    void *private_data)
+{
+}
+
+static void
+iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
+    IscsiLun *iscsilun = acb->iscsilun;
+
+    acb->common.cb(acb->common.opaque, -ECANCELED);
+    acb->canceled = 1;
+
+    /* send a task mgmt call to the target to cancel the task on the target */
+    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
+                                     iscsi_abort_task_cb, NULL);
+
+    /* then also cancel the task locally in libiscsi */
+    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
+}
+
+static AIOPool iscsi_aio_pool = {
+    .aiocb_size         = sizeof(IscsiAIOCB),
+    .cancel             = iscsi_aio_cancel,
+};
+
+
+static void iscsi_process_read(void *arg);
+static void iscsi_process_write(void *arg);
+
+static int iscsi_process_flush(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+
+    return iscsi_queue_length(iscsilun->iscsi) > 0;
+}
+
+static void
+iscsi_set_events(IscsiLun *iscsilun)
+{
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
+                           (iscsi_which_events(iscsi) & POLLOUT)
+                           ? iscsi_process_write : NULL,
+                           iscsi_process_flush, NULL, iscsilun);
+}
+
+static void
+iscsi_process_read(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLIN);
+    iscsi_set_events(iscsilun);
+}
+
+static void
+iscsi_process_write(void *arg)
+{
+    IscsiLun *iscsilun = arg;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    iscsi_service(iscsi, POLLOUT);
+    iscsi_set_events(iscsilun);
+}
+
+
+static int
+iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
+{
+    acb->bh = qemu_bh_new(cb, acb);
+    if (!acb->bh) {
+        error_report("oom: could not create iscsi bh");
+        return -EIO;
+    }
+
+    qemu_bh_schedule(acb->bh);
+    return 0;
+}
+
+static void
+iscsi_readv_writev_bh_cb(void *p)
+{
+    IscsiAIOCB *acb = p;
+
+    qemu_bh_delete(acb->bh);
+
+    if (acb->canceled == 0) {
+        acb->common.cb(acb->common.opaque, acb->status);
+    }
+
+    qemu_aio_release(acb);
+}
+
+
+static void
+iscsi_aio_write10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_write10_cb(iscsi, status, acb, acb->canceled);
+
+    g_free(acb->buf);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to write10 data to iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
+{
+    return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                 QEMUIOVector *qiov, int nb_sectors,
+                 BlockDriverCompletionFunc *cb,
+                 void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t size;
+    int fua = 0;
+
+    /* set FUA on writes when cache mode is write through */
+    if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
+        fua = 1;
+    }
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled   = 0;
+
+    /* XXX we should pass the iovec to write10 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_buffer(acb->qiov, acb->buf);
+    acb->task = iscsi_write10_task(iscsi, iscsilun->lun, acb->buf, size,
+                              sector_qemu2lun(sector_num, iscsilun),
+                              fua, 0, iscsilun->block_size,
+                              iscsi_aio_write10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send write10 command. %s",
+                     iscsi_get_error(iscsi));
+        g_free(acb->buf);
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static void
+iscsi_aio_read10_cb(struct iscsi_context *iscsi, int status,
+                    void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    trace_iscsi_aio_read10_cb(iscsi, status, acb, acb->canceled);
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status != 0) {
+        error_report("Failed to read10 data from iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                QEMUIOVector *qiov, int nb_sectors,
+                BlockDriverCompletionFunc *cb,
+                void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+    size_t qemu_read_size, lun_read_size;
+    int i;
+
+    qemu_read_size = BDRV_SECTOR_SIZE * (size_t)nb_sectors;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+    trace_iscsi_aio_readv(iscsi, sector_num, nb_sectors, opaque, acb);
+
+    acb->iscsilun = iscsilun;
+    acb->qiov     = qiov;
+
+    acb->canceled    = 0;
+    acb->read_size   = qemu_read_size;
+    acb->buf         = NULL;
+
+    /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
+     * may be misaligned to the LUN, so we may need to read some extra
+     * data.
+     */
+    acb->read_offset = 0;
+    if (iscsilun->block_size > BDRV_SECTOR_SIZE) {
+        uint64_t bdrv_offset = BDRV_SECTOR_SIZE * sector_num;
+
+        acb->read_offset  = bdrv_offset % iscsilun->block_size;
+    }
+
+    lun_read_size  = (qemu_read_size + iscsilun->block_size
+                     + acb->read_offset - 1)
+                     / iscsilun->block_size * iscsilun->block_size;
+    acb->task = iscsi_read10_task(iscsi, iscsilun->lun,
+                             sector_qemu2lun(sector_num, iscsilun),
+                             lun_read_size, iscsilun->block_size,
+                             iscsi_aio_read10_cb, acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send read10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    for (i = 0; i < acb->qiov->niov; i++) {
+        scsi_task_add_data_in_buffer(acb->task,
+                acb->qiov->iov[i].iov_len,
+                acb->qiov->iov[i].iov_base);
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+
+static void
+iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
+                     void *command_data, void *opaque)
+{
+    IscsiAIOCB *acb = opaque;
+
+    if (acb->canceled != 0) {
+        qemu_aio_release(acb);
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+        return;
+    }
+
+    acb->status = 0;
+    if (status < 0) {
+        error_report("Failed to sync10 data on iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        acb->status = -EIO;
+    }
+
+    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    scsi_free_scsi_task(acb->task);
+    acb->task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_flush(BlockDriverState *bs,
+                BlockDriverCompletionFunc *cb, void *opaque)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+    IscsiAIOCB *acb;
+
+    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
+
+    acb->iscsilun = iscsilun;
+    acb->canceled   = 0;
+
+    acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
+                                         0, 0, 0, 0,
+                                         iscsi_synccache10_cb,
+                                         acb);
+    if (acb->task == NULL) {
+        error_report("iSCSI: Failed to send synchronizecache10 command. %s",
+                     iscsi_get_error(iscsi));
+        qemu_aio_release(acb);
+        return NULL;
+    }
+
+    iscsi_set_events(iscsilun);
+
+    return &acb->common;
+}
+
+static int64_t
+iscsi_getlength(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    int64_t len;
+
+    len  = iscsilun->num_blocks;
+    len *= iscsilun->block_size;
+
+    return len;
+}
+
+static void
+iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
+                        void *command_data, void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_readcapacity10 *rc10;
+    struct scsi_task *task = command_data;
+
+    if (status != 0) {
+        error_report("iSCSI: Failed to read capacity of iSCSI lun. %s",
+                     iscsi_get_error(iscsi));
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    rc10 = scsi_datain_unmarshall(task);
+    if (rc10 == NULL) {
+        error_report("iSCSI: Failed to unmarshall readcapacity10 data.");
+        itask->status   = 1;
+        itask->complete = 1;
+        scsi_free_scsi_task(task);
+        return;
+    }
+
+    itask->iscsilun->block_size = rc10->block_size;
+    itask->iscsilun->num_blocks = rc10->lba;
+    itask->bs->total_sectors = (uint64_t)rc10->lba *
+                               rc10->block_size / BDRV_SECTOR_SIZE ;
+
+    itask->status   = 0;
+    itask->complete = 1;
+    scsi_free_scsi_task(task);
+}
+
+
+static void
+iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
+                 void *opaque)
+{
+    struct IscsiTask *itask = opaque;
+    struct scsi_task *task;
+
+    if (status != 0) {
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+
+    task = iscsi_readcapacity10_task(iscsi, itask->iscsilun->lun, 0, 0,
+                                   iscsi_readcapacity10_cb, opaque);
+    if (task == NULL) {
+        error_report("iSCSI: failed to send readcapacity command.");
+        itask->status   = 1;
+        itask->complete = 1;
+        return;
+    }
+}
+
+/*
+ * We support iscsi url's on the form
+ * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
+ */
+static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = NULL;
+    struct iscsi_url *iscsi_url = NULL;
+    struct IscsiTask task;
+    int ret;
+
+    if ((BDRV_SECTOR_SIZE % 512) != 0) {
+        error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. "
+                     "BDRV_SECTOR_SIZE(%lld) is not a multiple "
+                     "of 512", BDRV_SECTOR_SIZE);
+        return -EINVAL;
+    }
+
+    memset(iscsilun, 0, sizeof(IscsiLun));
+
+    /* Should really append the KVM name after the ':' here */
+    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
+    if (iscsi == NULL) {
+        error_report("iSCSI: Failed to create iSCSI context.");
+        ret = -ENOMEM;
+        goto failed;
+    }
+
+    iscsi_url = iscsi_parse_full_url(iscsi, filename);
+    if (iscsi_url == NULL) {
+        error_report("Failed to parse URL : %s %s", filename,
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
+        error_report("iSCSI: Failed to set target name.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    if (iscsi_url->user != NULL) {
+        ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
+                                              iscsi_url->passwd);
+        if (ret != 0) {
+            error_report("Failed to set initiator username and password");
+            ret = -EINVAL;
+            goto failed;
+        }
+    }
+    if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
+        error_report("iSCSI: Failed to set session type to normal.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+
+    task.iscsilun = iscsilun;
+    task.status = 0;
+    task.complete = 0;
+    task.bs = bs;
+
+    iscsilun->iscsi = iscsi;
+    iscsilun->lun   = iscsi_url->lun;
+
+    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
+                                 iscsi_connect_cb, &task)
+        != 0) {
+        error_report("iSCSI: Failed to start async connect.");
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    while (!task.complete) {
+        iscsi_set_events(iscsilun);
+        qemu_aio_wait();
+    }
+    if (task.status != 0) {
+        error_report("iSCSI: Failed to connect to LUN : %s",
+                     iscsi_get_error(iscsi));
+        ret = -EINVAL;
+        goto failed;
+    }
+
+    if (iscsi_url != NULL) {
+        iscsi_destroy_url(iscsi_url);
+    }
+    return 0;
+
+failed:
+    if (iscsi_url != NULL) {
+        iscsi_destroy_url(iscsi_url);
+    }
+    if (iscsi != NULL) {
+        iscsi_destroy_context(iscsi);
+    }
+    memset(iscsilun, 0, sizeof(IscsiLun));
+    return ret;
+}
+
+static void iscsi_close(BlockDriverState *bs)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct iscsi_context *iscsi = iscsilun->iscsi;
+
+    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL, NULL);
+    iscsi_destroy_context(iscsi);
+    memset(iscsilun, 0, sizeof(IscsiLun));
+}
+
+static BlockDriver bdrv_iscsi = {
+    .format_name     = "iscsi",
+    .protocol_name   = "iscsi",
+
+    .instance_size   = sizeof(IscsiLun),
+    .bdrv_file_open  = iscsi_open,
+    .bdrv_close      = iscsi_close,
+
+    .bdrv_getlength  = iscsi_getlength,
+
+    .bdrv_aio_readv  = iscsi_aio_readv,
+    .bdrv_aio_writev = iscsi_aio_writev,
+    .bdrv_aio_flush  = iscsi_aio_flush,
+};
+
+static void iscsi_block_init(void)
+{
+    bdrv_register(&bdrv_iscsi);
+}
+
+block_init(iscsi_block_init);
+
diff --git a/configure b/configure
index 4f87e0a..3009bbc 100755
--- a/configure
+++ b/configure
@@ -182,6 +182,7 @@ usb_redir=""
 opengl=""
 zlib="yes"
 guest_agent="yes"
+libiscsi=""
 
 # parse CC options first
 for opt do
@@ -657,6 +658,10 @@ for opt do
   ;;
   --enable-spice) spice="yes"
   ;;
+  --disable-libiscsi) libiscsi="no"
+  ;;
+  --enable-libiscsi) libiscsi="yes"
+  ;;
   --enable-profiler) profiler="yes"
   ;;
   --enable-cocoa)
@@ -1046,6 +1051,8 @@ echo "                           Default:trace-<pid>"
 echo "  --disable-spice          disable spice"
 echo "  --enable-spice           enable spice"
 echo "  --enable-rbd             enable building the rados block device (rbd)"
+echo "  --disable-libiscsi       disable iscsi support"
+echo "  --enable-libiscsi        enable iscsi support"
 echo "  --disable-smartcard      disable smartcard support"
 echo "  --enable-smartcard       enable smartcard support"
 echo "  --disable-smartcard-nss  disable smartcard nss support"
@@ -2335,6 +2342,25 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# Do we have libiscsi
+if test "$libiscsi" != "no" ; then
+  cat > $TMPC << EOF
+#include <iscsi/iscsi.h>
+int main(void) { iscsi_create_context(""); return 0; }
+EOF
+  if compile_prog "-Werror" "-liscsi" ; then
+    libiscsi="yes"
+    LIBS="$LIBS -liscsi"
+  else
+    if test "$libiscsi" = "yes" ; then
+      feature_not_found "libiscsi"
+    fi
+    libiscsi="no"
+  fi
+fi
+
+
+##########################################
 # Do we need librt
 cat > $TMPC <<EOF
 #include <signal.h>
@@ -2744,6 +2770,7 @@ echo "xfsctl support    $xfs"
 echo "nss used          $smartcard_nss"
 echo "usb net redir     $usb_redir"
 echo "OpenGL support    $opengl"
+echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 
 if test "$sdl_too_old" = "yes"; then
@@ -3042,6 +3069,10 @@ if test "$opengl" = "yes" ; then
   echo "CONFIG_OPENGL=y" >> $config_host_mak
 fi
 
+if test "$libiscsi" = "yes" ; then
+  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
diff --git a/trace-events b/trace-events
index 7f9cec4..8890c83 100644
--- a/trace-events
+++ b/trace-events
@@ -504,6 +504,12 @@ escc_sunkbd_event_out(int ch) "Translated keycode %2.2x"
 escc_kbd_command(int val) "Command %d"
 escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
 
+# block/iscsi.c
+iscsi_aio_write10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+iscsi_aio_read10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
+iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
+
 # hw/esp.c
 esp_raise_irq(void) "Raise IRQ"
 esp_lower_irq(void) "Lower IRQ"
-- 
1.7.3.1


[-- Attachment #3: 0001-This-provides-built-in-support-for-iSCSI-to-QEMU.patch.gz --]
[-- Type: application/x-gzip, Size: 6074 bytes --]

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Kevin Wolf]

Am 25.10.2011 10:23, schrieb ronnie sahlberg:
> Hi,
> 
> 
> On Tue, Oct 25, 2011 at 7:17 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 25.10.2011 10:04, schrieb ronnie sahlberg:
>>>> iscsi_destroy_url() is only ever called here. Do we leak it in the normal path?
>>>
>>> It didnt leak but the the label was confusing. I have changed it from
>>> "failed" to "finished" since this part
>>> is executed for both normal and failures.
>>
>> But there's a return 0; immediately before the label?
> 
> Right. Its been a long day :-)
> 
> Please find attached a fixed version.

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

[Zhi Yong Wu]

On Wed, Sep 21, 2011 at 5:37 PM, Ronnie Sahlberg
<ronniesahlberg@gmail.com> wrote:
> This provides built-in support for iSCSI to QEMU.
> This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices.
> It also has the benefit that non-root users of QEMU can access iSCSI devices across the network without requiring root privilege on the host.
>
> This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at
>    git://github.com/sahlberg/libiscsi.git
>
> The patch adds the driver to interface with the iscsi library.
> It also updated the configure script to
> * by default, probe is libiscsi is available and if so, build
>  qemu against libiscsi.
> * --enable-libiscsi
>  Force a build against libiscsi. If libiscsi is not available
>  the build will fail.
> * --disable-libiscsi
>  Do not link against libiscsi, even if it is available.
>
> When linked with libiscsi, qemu gains support to access iscsi resources such as disks and cdrom directly, without having to make the devices visible to the host.
>
> You can specify devices using a iscsi url of the form :
> iscsi://[<username>[:<password>@]]<host>[:<port]/<target-iqn-name>/<lun>
> When using authentication, the password can optionally be set with
> LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list
>
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  Makefile.objs |    1 +
>  block/iscsi.c |  596 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  configure     |   31 +++
>  trace-events  |    7 +
>  4 files changed, 635 insertions(+), 0 deletions(-)
>  create mode 100644 block/iscsi.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index a529a11..8c8e420 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -36,6 +36,7 @@ block-nested-y += qed-check.o
>  block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
> +block-nested-$(CONFIG_LIBISCSI) += iscsi.o
>  block-nested-$(CONFIG_CURL) += curl.o
>  block-nested-$(CONFIG_RBD) += rbd.o
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> new file mode 100644
> index 0000000..6517576
> --- /dev/null
> +++ b/block/iscsi.c
> @@ -0,0 +1,596 @@
> +/*
> + * QEMU Block driver for iSCSI images
> + *
> + * Copyright (c) 2010-2011 Ronnie Sahlberg <ronniesahlberg@gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "config-host.h"
> +
> +#include <poll.h>
> +#include "sysemu.h"
> +#include "qemu-common.h"
> +#include "qemu-error.h"
> +#include "block_int.h"
> +#include "trace.h"
> +
> +#include <iscsi/iscsi.h>
> +#include <iscsi/scsi-lowlevel.h>
> +
> +
> +typedef struct IscsiLun {
> +    struct iscsi_context *iscsi;
> +    int lun;
> +    int block_size;
> +    unsigned long num_blocks;
> +} IscsiLun;
> +
> +typedef struct IscsiAIOCB {
> +    BlockDriverAIOCB common;
> +    QEMUIOVector *qiov;
> +    QEMUBH *bh;
> +    IscsiLun *iscsilun;
> +    struct scsi_task *task;
> +    uint8_t *buf;
> +    int canceled;
> +    int status;
> +    size_t read_size;
> +    size_t read_offset;
> +} IscsiAIOCB;
> +
> +struct IscsiTask {
> +    IscsiLun *iscsilun;
> +    int status;
> +    int complete;
> +};
> +
> +static void
> +iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
> +                    void *private_data)
> +{
> +}
> +
> +static void
> +iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
> +{
> +    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> +    IscsiLun *iscsilun = acb->iscsilun;
> +
> +    acb->status = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->status);
> +    acb->canceled = 1;
> +
> +    /* send a task mgmt call to the target to cancel the task on the target */
> +    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> +                                     iscsi_abort_task_cb, NULL);
> +
> +    /* then also cancel the task locally in libiscsi */
> +    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
> +}
> +
> +static AIOPool iscsi_aio_pool = {
> +    .aiocb_size         = sizeof(IscsiAIOCB),
> +    .cancel             = iscsi_aio_cancel,
> +};
> +
> +
> +static void iscsi_process_read(void *arg);
> +static void iscsi_process_write(void *arg);
> +
> +static int iscsi_process_flush(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +
> +    return iscsi_queue_length(iscsilun->iscsi) > 0;
> +}
> +
> +static void
> +iscsi_set_events(IscsiLun *iscsilun)
> +{
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
> +                           (iscsi_which_events(iscsi) & POLLOUT)
> +                           ? iscsi_process_write : NULL,
> +                           iscsi_process_flush, NULL, iscsilun);
> +}
> +
> +static void
> +iscsi_process_read(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    iscsi_service(iscsi, POLLIN);
> +    iscsi_set_events(iscsilun);
> +}
> +
> +static void
> +iscsi_process_write(void *arg)
> +{
> +    IscsiLun *iscsilun = arg;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    iscsi_service(iscsi, POLLOUT);
> +    iscsi_set_events(iscsilun);
> +}
> +
> +
> +static int
> +iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
> +{
> +    acb->bh = qemu_bh_new(cb, acb);
> +    if (!acb->bh) {
> +        error_report("oom: could not create iscsi bh");
> +        return -EIO;
> +    }
> +
> +    qemu_bh_schedule(acb->bh);
> +    return 0;
> +}
> +
> +static void
> +iscsi_readv_writev_bh_cb(void *p)
> +{
> +    IscsiAIOCB *acb = p;
> +
> +    qemu_bh_delete(acb->bh);
> +
> +    if (acb->status != -ECANCELED) {
> +        acb->common.cb(acb->common.opaque, acb->status);
> +    }
> +
> +    qemu_aio_release(acb);
> +}
> +
> +
> +static void
> +iscsi_aio_write10_cb(struct iscsi_context *iscsi, int status,
> +                     void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    trace_iscsi_aio_write10_cb(iscsi, status, acb, acb->canceled);
> +
> +    g_free(acb->buf);
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status < 0) {
> +        error_report("Failed to write10 data to iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
> +{
> +    return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
> +                 QEMUIOVector *qiov, int nb_sectors,
> +                 BlockDriverCompletionFunc *cb,
> +                 void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +    size_t size;
> +    int fua = 0;
> +
> +    /* set FUA on writes when cache mode is write through */
> +    if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
> +        fua = 1;
> +    }
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    trace_iscsi_aio_writev(iscsi, sector_num, nb_sectors, opaque, acb);
> +    if (!acb) {
> +        return NULL;
> +    }
> +
> +    acb->iscsilun = iscsilun;
> +    acb->qiov     = qiov;
> +
> +    acb->canceled   = 0;
> +
> +    /* XXX we should pass the iovec to write10 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_buffer(acb->qiov, acb->buf);
> +    acb->task = iscsi_write10_task(iscsi, iscsilun->lun, acb->buf, size,
> +                              sector_qemu2lun(sector_num, iscsilun),
> +                              fua, 0, iscsilun->block_size,
> +                              iscsi_aio_write10_cb, acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send write10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        g_free(acb->buf);
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +static void
> +iscsi_aio_read10_cb(struct iscsi_context *iscsi, int status,
> +                    void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    trace_iscsi_aio_read10_cb(iscsi, status, acb, acb->canceled);
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status != 0) {
> +        error_report("Failed to read10 data from iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
> +                QEMUIOVector *qiov, int nb_sectors,
> +                BlockDriverCompletionFunc *cb,
> +                void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +    size_t qemu_read_size, lun_read_size;
> +    int i;
> +
> +    qemu_read_size = BDRV_SECTOR_SIZE * (size_t)nb_sectors;
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    trace_iscsi_aio_readv(iscsi, sector_num, nb_sectors, opaque, acb);
> +    if (!acb) {
> +        return NULL;
> +    }
> +
> +    acb->iscsilun = iscsilun;
> +    acb->qiov     = qiov;
> +
> +    acb->canceled    = 0;
> +    acb->read_size   = qemu_read_size;
> +    acb->buf         = NULL;
> +
> +    /* If LUN blocksize is bigger than BDRV_BLOCK_SIZE a read from QEMU
> +     * may be misaligned to the LUN, so we may need to read some extra
> +     * data.
> +     */
> +    acb->read_offset = 0;
> +    if (iscsilun->block_size > BDRV_SECTOR_SIZE) {
> +        uint64_t bdrv_offset = BDRV_SECTOR_SIZE * sector_num;
> +
> +        acb->read_offset  = bdrv_offset % iscsilun->block_size;
> +    }
> +
> +    lun_read_size  = (qemu_read_size + iscsilun->block_size
> +                     + acb->read_offset - 1)
> +                     / iscsilun->block_size * iscsilun->block_size;
> +    acb->task = iscsi_read10_task(iscsi, iscsilun->lun,
> +                             sector_qemu2lun(sector_num, iscsilun),
> +                             lun_read_size, iscsilun->block_size,
> +                             iscsi_aio_read10_cb, acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send read10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < acb->qiov->niov; i++) {
> +        scsi_task_add_data_in_buffer(acb->task,
> +                acb->qiov->iov[i].iov_len,
> +                acb->qiov->iov[i].iov_base);
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +
> +static void
> +iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
> +                     void *command_data, void *opaque)
> +{
> +    IscsiAIOCB *acb = opaque;
> +
> +    if (acb->canceled != 0) {
> +        qemu_aio_release(acb);
> +        scsi_free_scsi_task(acb->task);
> +        acb->task = NULL;
> +        return;
> +    }
> +
> +    acb->status = 0;
> +    if (status < 0) {
> +        error_report("Failed to sync10 data on iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        acb->status = -EIO;
> +    }
> +
> +    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
> +}
> +
> +static BlockDriverAIOCB *
> +iscsi_aio_flush(BlockDriverState *bs,
> +                BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +    IscsiAIOCB *acb;
> +
> +    acb = qemu_aio_get(&iscsi_aio_pool, bs, cb, opaque);
> +    if (!acb) {
> +        return NULL;
> +    }
> +
> +    acb->iscsilun = iscsilun;
> +    acb->canceled   = 0;
> +
> +    acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
> +                                         0, 0, 0, 0,
> +                                         iscsi_synccache10_cb,
> +                                         acb);
> +    if (acb->task == NULL) {
> +        error_report("iSCSI: Failed to send synchronizecache10 command. %s",
> +                     iscsi_get_error(iscsi));
> +        qemu_aio_release(acb);
> +        return NULL;
> +    }
> +
> +    iscsi_set_events(iscsilun);
> +
> +    return &acb->common;
> +}
> +
> +static int64_t
> +iscsi_getlength(BlockDriverState *bs)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    int64_t len;
> +
> +    len  = iscsilun->num_blocks;
> +    len *= iscsilun->block_size;
> +
> +    return len;
> +}
> +
> +static void
> +iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
> +                        void *command_data, void *opaque)
> +{
> +    struct IscsiTask *itask = opaque;
> +    struct scsi_readcapacity10 *rc10;
> +    struct scsi_task *task = command_data;
> +
> +    if (status != 0) {
> +        error_report("iSCSI: Failed to read capacity of iSCSI lun. %s",
> +                     iscsi_get_error(iscsi));
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        scsi_free_scsi_task(task);
> +        return;
> +    }
> +
> +    rc10 = scsi_datain_unmarshall(task);
> +    if (rc10 == NULL) {
> +        error_report("iSCSI: Failed to unmarshall readcapacity10 data.");
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        scsi_free_scsi_task(task);
> +        return;
> +    }
> +
> +    itask->iscsilun->block_size = rc10->block_size;
> +    itask->iscsilun->num_blocks = rc10->lba;
> +
> +    itask->status   = 0;
> +    itask->complete = 1;
> +    scsi_free_scsi_task(task);
> +}
> +
> +
> +static void
> +iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
> +                 void *opaque)
> +{
> +    struct IscsiTask *itask = opaque;
> +    struct scsi_task *task;
> +
> +    if (status != 0) {
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        return;
> +    }
> +
> +    task = iscsi_readcapacity10_task(iscsi, itask->iscsilun->lun, 0, 0,
> +                                   iscsi_readcapacity10_cb, opaque);
> +    if (task == NULL) {
> +        error_report("iSCSI: failed to send readcapacity command.");
> +        itask->status   = 1;
> +        itask->complete = 1;
> +        return;
> +    }
> +}
> +
> +/*
> + * We support iscsi url's on the form
> + * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> + */
> +static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = NULL;
> +    struct iscsi_url *iscsi_url = NULL;
> +    struct IscsiTask task;
> +    int ret;
> +
> +    if ((BDRV_SECTOR_SIZE % 512) != 0) {
> +        error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. "
> +                     "BDRV_SECTOR_SIZE(%lld) is not a multiple "
> +                     "of 512", BDRV_SECTOR_SIZE);
> +        return -EINVAL;
> +    }
> +
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +
> +    /* Should really append the KVM name after the ':' here */
> +    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
> +    if (iscsi == NULL) {
> +        error_report("iSCSI: Failed to create iSCSI context.");
> +        ret = -ENOMEM;
> +        goto failed;
> +    }
> +
> +    iscsi_url = iscsi_parse_full_url(iscsi, filename);
> +    if (iscsi_url == NULL) {
> +        error_report("Failed to parse URL : %s %s", filename,
> +                     iscsi_get_error(iscsi));
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
> +        error_report("iSCSI: Failed to set target name.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    if (iscsi_url->user != NULL) {
> +        ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
> +                                              iscsi_url->passwd);
> +        if (ret != 0) {
> +            error_report("Failed to set initiator username and password");
> +            ret = -EINVAL;
> +            goto failed;
> +        }
> +    }
> +    if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
> +        error_report("iSCSI: Failed to set session type to normal.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> +
> +    task.iscsilun = iscsilun;
> +    task.status = 0;
> +    task.complete = 0;
> +
> +    iscsilun->iscsi = iscsi;
> +    iscsilun->lun   = iscsi_url->lun;
> +
> +    if (iscsi_full_connect_async(iscsi, iscsi_url->portal, iscsi_url->lun,
> +                                 iscsi_connect_cb, &task)
> +        != 0) {
> +        error_report("iSCSI: Failed to start async connect.");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    while (!task.complete) {
> +        iscsi_set_events(iscsilun);
> +        qemu_aio_wait();
> +    }
Why need this use "while" ?  I thought that iscsi_set_events only need
to be called once, right?

> +    if (task.status != 0) {
> +        error_report("iSCSI: Failed to connect to LUN : %s",
> +                     iscsi_get_error(iscsi));
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
> +    return 0;
> +
> +failed:
> +    if (iscsi_url != NULL) {
> +        iscsi_destroy_url(iscsi_url);
> +    }
> +    if (iscsi != NULL) {
> +        iscsi_destroy_context(iscsi);
> +    }
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +    return ret;
> +}
> +
> +static void iscsi_close(BlockDriverState *bs)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL, NULL);
> +    iscsi_destroy_context(iscsi);
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +
> +static BlockDriver bdrv_iscsi = {
> +    .format_name     = "iscsi",
> +    .protocol_name   = "iscsi",
> +
> +    .instance_size   = sizeof(IscsiLun),
> +    .bdrv_file_open  = iscsi_open,
> +    .bdrv_close      = iscsi_close,
> +
> +    .bdrv_getlength  = iscsi_getlength,
> +
> +    .bdrv_aio_readv  = iscsi_aio_readv,
> +    .bdrv_aio_writev = iscsi_aio_writev,
> +    .bdrv_aio_flush  = iscsi_aio_flush,
> +};
> +
> +static void iscsi_block_init(void)
> +{
> +    bdrv_register(&bdrv_iscsi);
> +}
> +
> +block_init(iscsi_block_init);
> +
> diff --git a/configure b/configure
> index 348f36a..a2c30e1 100755
> --- a/configure
> +++ b/configure
> @@ -182,6 +182,7 @@ usb_redir=""
>  opengl=""
>  zlib="yes"
>  guest_agent="yes"
> +libiscsi=""
>
>  # parse CC options first
>  for opt do
> @@ -651,6 +652,10 @@ for opt do
>   ;;
>   --enable-spice) spice="yes"
>   ;;
> +  --disable-libiscsi) libiscsi="no"
> +  ;;
> +  --enable-libiscsi) libiscsi="yes"
> +  ;;
>   --enable-profiler) profiler="yes"
>   ;;
>   --enable-cocoa)
> @@ -1037,6 +1042,8 @@ echo "                           Default:trace-<pid>"
>  echo "  --disable-spice          disable spice"
>  echo "  --enable-spice           enable spice"
>  echo "  --enable-rbd             enable building the rados block device (rbd)"
> +echo "  --disable-libiscsi       disable iscsi support"
> +echo "  --enable-libiscsi        enable iscsi support"
>  echo "  --disable-smartcard      disable smartcard support"
>  echo "  --enable-smartcard       enable smartcard support"
>  echo "  --disable-smartcard-nss  disable smartcard nss support"
> @@ -2326,6 +2333,25 @@ if compile_prog "" "" ; then
>  fi
>
>  ##########################################
> +# Do we have libiscsi
> +if test "$libiscsi" != "no" ; then
> +  cat > $TMPC << EOF
> +#include <iscsi/iscsi.h>
> +int main(void) { iscsi_create_context(""); return 0; }
> +EOF
> +  if compile_prog "-Werror" "-liscsi" ; then
> +    libiscsi="yes"
> +    LIBS="$LIBS -liscsi"
> +  else
> +    if test "$libiscsi" = "yes" ; then
> +      feature_not_found "libiscsi"
> +    fi
> +    libiscsi="no"
> +  fi
> +fi
> +
> +
> +##########################################
>  # Do we need librt
>  cat > $TMPC <<EOF
>  #include <signal.h>
> @@ -2727,6 +2753,7 @@ echo "xfsctl support    $xfs"
>  echo "nss used          $smartcard_nss"
>  echo "usb net redir     $usb_redir"
>  echo "OpenGL support    $opengl"
> +echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>
>  if test "$sdl_too_old" = "yes"; then
> @@ -3028,6 +3055,10 @@ if test "$opengl" = "yes" ; then
>   echo "CONFIG_OPENGL=y" >> $config_host_mak
>  fi
>
> +if test "$libiscsi" = "yes" ; then
> +  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
> +fi
> +
>  # XXX: suppress that
>  if [ "$bsd" = "yes" ] ; then
>   echo "CONFIG_BSD=y" >> $config_host_mak
> diff --git a/trace-events b/trace-events
> index 3fdd60f..ce68516 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -494,3 +494,10 @@ escc_sunkbd_event_in(int ch) "Untranslated keycode %2.2x"
>  escc_sunkbd_event_out(int ch) "Translated keycode %2.2x"
>  escc_kbd_command(int val) "Command %d"
>  escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=%01x"
> +
> +# block/iscsi.c
> +iscsi_aio_write10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
> +iscsi_aio_writev(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
> +iscsi_aio_read10_cb(void *iscsi, int status, void *acb, int canceled) "iscsi %p status %d acb %p canceled %d"
> +iscsi_aio_readv(void *iscsi, int64_t sector_num, int nb_sectors, void *opaque, void *acb) "iscsi %p sector_num %"PRId64" nb_sectors %d opaque %p acb %p"
> +
> --
> 1.7.3.1
>
>
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Paolo Bonzini]

On 09/17/2011 09:08 PM, Laurent Vivier wrote:
>> >  The NBD protocol supports multiple named exports, just not QEMU's
>> >  implementation.
> Named exports are supported since commit
> 1d45f8b542f6b80b24c44533ef0dd9e1a3b17ea5

Yes, not in the qemu-nbd server though.

Paolo

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Laurent Vivier]

Le jeudi 15 septembre 2011 à 08:06 +0200, Paolo Bonzini a écrit :
> On 09/14/2011 06:36 PM, Orit Wasserman wrote:
> > >  I think NBD would be fine, especially with a flush command.
> >  I think NBD would be fine, especially with a flush command.
> > If I remember correctly , there is a problem with NBD with an image with
> > a backing file chain . NBD client only displays a single file image.
> > With ISCSI we can use different luns per image file.
> 
> The NBD protocol supports multiple named exports, just not QEMU's 
> implementation.

Named exports are supported since commit
1d45f8b542f6b80b24c44533ef0dd9e1a3b17ea5

Regards,
Laurent

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Kevin Wolf]

Am 15.09.2011 11:55, schrieb Paolo Bonzini:
> On 09/15/2011 11:52 AM, Orit Wasserman wrote:
>>>  The NBD protocol supports multiple named exports, just not QEMU's
>>>  implementation.
>> I guess we can fix that.
> 
> Yes, or just use Unix sockets and rely on libvirt to tunnel/multiplex 
> the NBD streams.

I don't think libvirt should have to deal with this when there is a good
solution that can be implemented in qemu.

Kevin

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Paolo Bonzini]

On 09/15/2011 11:52 AM, Orit Wasserman wrote:
> >  The NBD protocol supports multiple named exports, just not QEMU's
> >  implementation.
> I guess we can fix that.

Yes, or just use Unix sockets and rely on libvirt to tunnel/multiplex 
the NBD streams.

Paolo

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Orit Wasserman]

On Thu, 2011-09-15 at 08:06 +0200, Paolo Bonzini wrote:
> On 09/14/2011 06:36 PM, Orit Wasserman wrote:
> > >  I think NBD would be fine, especially with a flush command.
> >  I think NBD would be fine, especially with a flush command.
> > If I remember correctly , there is a problem with NBD with an image with
> > a backing file chain . NBD client only displays a single file image.
> > With ISCSI we can use different luns per image file.
> 
> The NBD protocol supports multiple named exports, just not QEMU's 
> implementation.
I guess we can fix that.
Orit
> 
> Paolo

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Paolo Bonzini]

On 09/14/2011 06:36 PM, Orit Wasserman wrote:
> >  I think NBD would be fine, especially with a flush command.
>  I think NBD would be fine, especially with a flush command.
> If I remember correctly , there is a problem with NBD with an image with
> a backing file chain . NBD client only displays a single file image.
> With ISCSI we can use different luns per image file.

The NBD protocol supports multiple named exports, just not QEMU's 
implementation.

Paolo

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[ronnie sahlberg]

On Wed, Sep 14, 2011 at 10:24 PM, Orit Wasserman <owasserm@redhat.com> wrote:
> Looks quite good, didn't have any more comments.
> We can use it for streaming with non shared storage with minor changes
> (http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage)
> Ronnie , are you looking into ISCSI target too?

No immediate plans. I mainly use real iSCSI targets.
Immediate plans are primarily getting iscsi initiator support into qemu and
second to maybe present it as a HBA to the guest, as a second step.

I do have iscsi target code from an old abandoned project of mine that
can be used, so yes,
I could build a small simple iscsi target at some stage for qemu for
usecases where TGTD or other solutions are inconvenient.


regards
ronnie sahlberg

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Paolo Bonzini]

On 09/14/2011 05:51 PM, Stefan Hajnoczi wrote:
> >  Why do we need to use the complex iSCSI protocol for that instead of
> >  simply using NBD?
>
> I think NBD would be fine, especially with a flush command.

Indeed: http://permalink.gmane.org/gmane.comp.emulators.qemu/113639

iSCSI is still very useful to easily deploy remote storage, and more 
long term for passthrough to an HBA provided by QEMU (as an alternative 
to scsi-generic).

Paolo

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Orit Wasserman]

On Wed, 2011-09-14 at 16:51 +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 14, 2011 at 4:40 PM, Christoph Hellwig <hch@lst.de> wrote:
> > On Wed, Sep 14, 2011 at 04:35:50PM +0100, Stefan Hajnoczi wrote:
> >> I think the motivation for iSCSI target support in QEMU is exactly
> >> what you mentioned - providing shared storage.  In migration scenarios
> >> it is necessary temporarily make shared storage possible with an image
> >> that is on a local disk.
> >
> > Why do we need to use the complex iSCSI protocol for that instead of
> > simply using NBD?
> 
> I think NBD would be fine, especially with a flush command.
If I remember correctly , there is a problem with NBD with an image with
a backing file chain . NBD client only displays a single file image.
With ISCSI we can use different luns per image file.
Orit
> 
> Stefan

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Stefan Hajnoczi]

On Wed, Sep 14, 2011 at 4:40 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Sep 14, 2011 at 04:35:50PM +0100, Stefan Hajnoczi wrote:
>> I think the motivation for iSCSI target support in QEMU is exactly
>> what you mentioned - providing shared storage.  In migration scenarios
>> it is necessary temporarily make shared storage possible with an image
>> that is on a local disk.
>
> Why do we need to use the complex iSCSI protocol for that instead of
> simply using NBD?

I think NBD would be fine, especially with a flush command.

Stefan

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Christoph Hellwig]

On Wed, Sep 14, 2011 at 04:35:50PM +0100, Stefan Hajnoczi wrote:
> I think the motivation for iSCSI target support in QEMU is exactly
> what you mentioned - providing shared storage.  In migration scenarios
> it is necessary temporarily make shared storage possible with an image
> that is on a local disk.

Why do we need to use the complex iSCSI protocol for that instead of
simply using NBD?

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Stefan Hajnoczi]

On Wed, Sep 14, 2011 at 3:37 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Sep 14, 2011 at 03:24:38PM +0300, Orit Wasserman wrote:
>> Looks quite good, didn't have any more comments.
>> We can use it for streaming with non shared storage with minor changes
>> (http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage)
>
> That whole section makes little sense to me.  An iSCSI target with multiple
> initiator talking to it is by defintion shared storage.  Having the target
> on the same physical box as on of the initiators doesn't really change that.

I think the motivation for iSCSI target support in QEMU is exactly
what you mentioned - providing shared storage.  In migration scenarios
it is necessary temporarily make shared storage possible with an image
that is on a local disk.

Stefan

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Christoph Hellwig]

On Wed, Sep 14, 2011 at 03:24:38PM +0300, Orit Wasserman wrote:
> Looks quite good, didn't have any more comments.
> We can use it for streaming with non shared storage with minor changes
> (http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage)

That whole section makes little sense to me.  An iSCSI target with multiple
initiator talking to it is by defintion shared storage.  Having the target
on the same physical box as on of the initiators doesn't really change that.

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Christoph Hellwig]

On Wed, Sep 14, 2011 at 03:24:38PM +0300, Orit Wasserman wrote:
> Looks quite good, didn't have any more comments.
> We can use it for streaming with non shared storage with minor changes
> (http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage)
> Ronnie , are you looking into ISCSI target too?

Why would you throw in a target into qemu?

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Orit Wasserman]

Looks quite good, didn't have any more comments.
We can use it for streaming with non shared storage with minor changes
(http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage)
Ronnie , are you looking into ISCSI target too?

Orit

On Mon, 2011-09-12 at 10:56 +0200, Kevin Wolf wrote:
> Am 10.09.2011 06:23, schrieb Ronnie Sahlberg:
> > List,
> > 
> > Please find a patch that adds built-in iSCSI support to QEMU when built and linked against the multiplatform iscsi initiator library at  git://github.com/sahlberg/libiscsi.git
> > 
> > All previous comments and suggestions have been addressed in this patch.
> > 
> > I and others have done extensive testing and used this patch extensively over the last ~6 months with great result.
> > 
> > 
> > In some situations, using a built-in iscsi inititator has benefits against mounting the LUNs on the host.
> > 
> > * Privacy: The iSCSI LUNs are private to the guest and are not visible either to the host, nor to any processes running on the host.
> > * Ease of managment : If you have very many guests and very many, thousands of, iSCSI LUNs. It is inconvenient to have to expose all LUNs to the underlying host.
> > * No root requirement. Since the iSCSI LUNs are not mounted as devices on the host, ordinary users can set up and use iSCSI resources without the need for root privilege on the host to map the devices to local scsi devices.
> > 
> > 
> > Please merge this patch to master or explain how I should change the patch so that it becomes acceptable for inclusion into QEMU.
> 
> Orit, I think you could be interested in reviewing this patch?
> 
> Kevin
> 

Re: [Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Kevin Wolf]

Am 10.09.2011 06:23, schrieb Ronnie Sahlberg:
> List,
> 
> Please find a patch that adds built-in iSCSI support to QEMU when built and linked against the multiplatform iscsi initiator library at  git://github.com/sahlberg/libiscsi.git
> 
> All previous comments and suggestions have been addressed in this patch.
> 
> I and others have done extensive testing and used this patch extensively over the last ~6 months with great result.
> 
> 
> In some situations, using a built-in iscsi inititator has benefits against mounting the LUNs on the host.
> 
> * Privacy: The iSCSI LUNs are private to the guest and are not visible either to the host, nor to any processes running on the host.
> * Ease of managment : If you have very many guests and very many, thousands of, iSCSI LUNs. It is inconvenient to have to expose all LUNs to the underlying host.
> * No root requirement. Since the iSCSI LUNs are not mounted as devices on the host, ordinary users can set up and use iSCSI resources without the need for root privilege on the host to map the devices to local scsi devices.
> 
> 
> Please merge this patch to master or explain how I should change the patch so that it becomes acceptable for inclusion into QEMU.

Orit, I think you could be interested in reviewing this patch?

Kevin

[Qemu-devel] [PATCH] Add iSCSI support for QEMU

[Ronnie Sahlberg]

List,

Please find a patch that adds built-in iSCSI support to QEMU when built and linked against the multiplatform iscsi initiator library at  git://github.com/sahlberg/libiscsi.git

All previous comments and suggestions have been addressed in this patch.

I and others have done extensive testing and used this patch extensively over the last ~6 months with great result.


In some situations, using a built-in iscsi inititator has benefits against mounting the LUNs on the host.

* Privacy: The iSCSI LUNs are private to the guest and are not visible either to the host, nor to any processes running on the host.
* Ease of managment : If you have very many guests and very many, thousands of, iSCSI LUNs. It is inconvenient to have to expose all LUNs to the underlying host.
* No root requirement. Since the iSCSI LUNs are not mounted as devices on the host, ordinary users can set up and use iSCSI resources without the need for root privilege on the host to map the devices to local scsi devices.


Please merge this patch to master or explain how I should change the patch so that it becomes acceptable for inclusion into QEMU.


regards
ronnie sahlberg