Merge remote-tracking branch 'bonzini/scsi-next' into staging

* bonzini/scsi-next:
  scsi-disk: add support for the UNMAP command
  scsi-disk: improve out-of-range LBA detection for WRITE SAME
  scsi-disk: more assertions and resets for aiocb
  virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags
  iscsi: Pick default initiator-name based on the name of the VM
  iscsi: reorganize code for parse_initiator_name
  iscsi: do not leak initiator_name

block/iscsi.c

@@ -896,26 +896,26 @@ static char *parse_initiator_name(const char *target)
     QemuOptsList *list;
     QemuOpts *opts;
     const char *name = NULL;
+    const char *iscsi_name = qemu_get_vm_name();
 
     list = qemu_find_opts("iscsi");
-    if (!list) {
+    if (list) {
-        return g_strdup("iqn.2008-11.org.linux-kvm");
+        opts = qemu_opts_find(list, target);
-    }
-
-    opts = qemu_opts_find(list, target);
-    if (opts == NULL) {
-        opts = QTAILQ_FIRST(&list->head);
         if (!opts) {
-            return g_strdup("iqn.2008-11.org.linux-kvm");
+            opts = QTAILQ_FIRST(&list->head);
+        }
+        if (opts) {
+            name = qemu_opt_get(opts, "initiator-name");
         }
     }
 
-    name = qemu_opt_get(opts, "initiator-name");
+    if (name) {
-    if (!name) {
+        return g_strdup(name);
-        return g_strdup("iqn.2008-11.org.linux-kvm");
+    } else {
+        return g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
+                               iscsi_name ? ":" : "",
+                               iscsi_name ? iscsi_name : "");
     }
-
-    return g_strdup(name);
 }
 
 /*
@@ -943,7 +943,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         error_report("Failed to parse URL : %s %s", filename,
                      iscsi_get_error(iscsi));
         ret = -EINVAL;
-        goto failed;
+        goto out;
     }
 
     memset(iscsilun, 0, sizeof(IscsiLun));
@@ -954,13 +954,13 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
     if (iscsi == NULL) {
         error_report("iSCSI: Failed to create iSCSI context.");
         ret = -ENOMEM;
-        goto failed;
+        goto out;
     }
 
     if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
         error_report("iSCSI: Failed to set target name.");
         ret = -EINVAL;
-        goto failed;
+        goto out;
     }
 
     if (iscsi_url->user != NULL) {
@@ -969,7 +969,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         if (ret != 0) {
             error_report("Failed to set initiator username and password");
             ret = -EINVAL;
-            goto failed;
+            goto out;
         }
     }
 
@@ -977,13 +977,13 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
     if (parse_chap(iscsi, iscsi_url->target) != 0) {
         error_report("iSCSI: Failed to set CHAP user/password");
         ret = -EINVAL;
-        goto failed;
+        goto out;
     }
 
     if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
         error_report("iSCSI: Failed to set session type to normal.");
         ret = -EINVAL;
-        goto failed;
+        goto out;
     }
 
     iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
@@ -1004,7 +1004,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         != 0) {
         error_report("iSCSI: Failed to start async connect.");
         ret = -EINVAL;
-        goto failed;
+        goto out;
     }
 
     while (!task.complete) {
@@ -1015,11 +1015,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         error_report("iSCSI: Failed to connect to LUN : %s",
                      iscsi_get_error(iscsi));
         ret = -EINVAL;
-        goto failed;
+        goto out;
-    }
-
-    if (iscsi_url != NULL) {
-        iscsi_destroy_url(iscsi_url);
     }
 
     /* Medium changer or tape. We dont have any emulation for this so this must
@@ -1031,19 +1027,22 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         bs->sg = 1;
     }
 
-    return 0;
+    ret = 0;
 
-failed:
+out:
     if (initiator_name != NULL) {
         g_free(initiator_name);
     }
     if (iscsi_url != NULL) {
         iscsi_destroy_url(iscsi_url);
     }
-    if (iscsi != NULL) {
+
-        iscsi_destroy_context(iscsi);
+    if (ret) {
+        if (iscsi != NULL) {
+            iscsi_destroy_context(iscsi);
+        }
+        memset(iscsilun, 0, sizeof(IscsiLun));
     }
-    memset(iscsilun, 0, sizeof(IscsiLun));
     return ret;
 }
 

hw/scsi-disk.c

@@ -175,6 +175,8 @@ static void scsi_aio_complete(void *opaque, int ret)
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
     bdrv_acct_done(s->qdev.conf.bs, &r->acct);
 
     if (ret < 0) {
@@ -238,10 +240,9 @@ static void scsi_dma_complete(void *opaque, int ret)
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
-    if (r->req.aiocb != NULL) {
+    assert(r->req.aiocb != NULL);
-        r->req.aiocb = NULL;
+    r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
+    bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
 
     if (ret < 0) {
         if (scsi_handle_rw_error(r, -ret)) {
@@ -270,10 +271,9 @@ static void scsi_read_complete(void * opaque, int ret)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     int n;
 
-    if (r->req.aiocb != NULL) {
+    assert(r->req.aiocb != NULL);
-        r->req.aiocb = NULL;
+    r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
+    bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
 
     if (ret < 0) {
         if (scsi_handle_rw_error(r, -ret)) {
@@ -637,7 +637,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
         {
             buflen = 8;
             outbuf[4] = 0;
-            outbuf[5] = 0x60; /* write_same 10/16 supported */
+            outbuf[5] = 0xe0; /* unmap & write_same 10/16 all supported */
             outbuf[6] = s->qdev.conf.discard_granularity ? 2 : 1;
             outbuf[7] = 0;
             break;
@@ -1449,6 +1449,89 @@ invalid_field:
     return;
 }
 
+typedef struct UnmapCBData {
+    SCSIDiskReq *r;
+    uint8_t *inbuf;
+    int count;
+} UnmapCBData;
+
+static void scsi_unmap_complete(void *opaque, int ret)
+{
+    UnmapCBData *data = opaque;
+    SCSIDiskReq *r = data->r;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    uint64_t sector_num;
+    uint32 nb_sectors;
+
+    r->req.aiocb = NULL;
+    if (ret < 0) {
+        if (scsi_handle_rw_error(r, -ret)) {
+            goto done;
+        }
+    }
+
+    if (data->count > 0 && !r->req.io_canceled) {
+        sector_num = ldq_be_p(&data->inbuf[0]);
+        nb_sectors = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL;
+        if (sector_num > sector_num + nb_sectors ||
+            sector_num + nb_sectors - 1 > s->qdev.max_lba) {
+            scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
+            goto done;
+        }
+
+        r->req.aiocb = bdrv_aio_discard(s->qdev.conf.bs,
+                                        sector_num * (s->qdev.blocksize / 512),
+                                        nb_sectors * (s->qdev.blocksize / 512),
+                                        scsi_unmap_complete, data);
+        data->count--;
+        data->inbuf += 16;
+        return;
+    }
+
+done:
+    if (data->count == 0) {
+        scsi_req_complete(&r->req, GOOD);
+    }
+    if (!r->req.io_canceled) {
+        scsi_req_unref(&r->req);
+    }
+    g_free(data);
+}
+
+static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
+{
+    uint8_t *p = inbuf;
+    int len = r->req.cmd.xfer;
+    UnmapCBData *data;
+
+    if (len < 8) {
+        goto invalid_param_len;
+    }
+    if (len < lduw_be_p(&p[0]) + 2) {
+        goto invalid_param_len;
+    }
+    if (len < lduw_be_p(&p[2]) + 8) {
+        goto invalid_param_len;
+    }
+    if (lduw_be_p(&p[2]) & 15) {
+        goto invalid_param_len;
+    }
+
+    data = g_new0(UnmapCBData, 1);
+    data->r = r;
+    data->inbuf = &p[8];
+    data->count = lduw_be_p(&p[2]) >> 4;
+
+    /* The matching unref is in scsi_unmap_complete, before data is freed.  */
+    scsi_req_ref(&r->req);
+    scsi_unmap_complete(data, 0);
+    return;
+
+invalid_param_len:
+    scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
+    return;
+}
+
 static void scsi_disk_emulate_write_data(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
@@ -1468,6 +1551,10 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
         scsi_disk_emulate_mode_select(r, r->iov.iov_base);
         break;
 
+    case UNMAP:
+        scsi_disk_emulate_unmap(r, r->iov.iov_base);
+        break;
+
     default:
         abort();
     }
@@ -1702,6 +1789,9 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     case MODE_SELECT_10:
         DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer);
         break;
+    case UNMAP:
+        DPRINTF("Unmap (len %lu)\n", (long)r->req.cmd.xfer);
+        break;
     case WRITE_SAME_10:
         nb_sectors = lduw_be_p(&req->cmd.buf[7]);
         goto write_same;
@@ -1712,7 +1802,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
             scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
             return 0;
         }
-        if (r->req.cmd.lba > s->qdev.max_lba) {
+        if (r->req.cmd.lba > r->req.cmd.lba + nb_sectors ||
+            r->req.cmd.lba + nb_sectors - 1 > s->qdev.max_lba) {
             goto illegal_lba;
         }
 
@@ -2067,6 +2158,7 @@ static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
     [SEEK_10]                         = &scsi_disk_emulate_reqops,
     [MODE_SELECT]                     = &scsi_disk_emulate_reqops,
     [MODE_SELECT_10]                  = &scsi_disk_emulate_reqops,
+    [UNMAP]                           = &scsi_disk_emulate_reqops,
     [WRITE_SAME_10]                   = &scsi_disk_emulate_reqops,
     [WRITE_SAME_16]                   = &scsi_disk_emulate_reqops,
 

hw/virtio-scsi.c

@@ -305,11 +305,17 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
             goto incorrect_lun;
         }
         QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
-            if (r->tag == req->req.tmf->tag) {
+            VirtIOSCSIReq *cmd_req = r->hba_private;
+            if (cmd_req && cmd_req->req.cmd->tag == req->req.tmf->tag) {
                 break;
             }
         }
-        if (r && r->hba_private) {
+        if (r) {
+            /*
+             * Assert that the request has not been completed yet, we
+             * check for it in the loop above.
+             */
+            assert(r->hba_private);
             if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
                 /* "If the specified command is present in the task set, then
                  * return a service response set to FUNCTION SUCCEEDED".

qemu-common.h

@@ -376,6 +376,7 @@ bool buffer_is_zero(const void *buf, size_t len);
 void qemu_progress_init(int enabled, float min_skip);
 void qemu_progress_end(void);
 void qemu_progress_print(float delta, int max);
+const char *qemu_get_vm_name(void);
 
 #define QEMU_FILE_TYPE_BIOS   0
 #define QEMU_FILE_TYPE_KEYMAP 1

qemu-doc.texi

@@ -734,6 +734,11 @@ Various session related parameters can be set via special options, either
 in a configuration file provided via '-readconfig' or directly on the
 command line.
 
+If the initiator-name is not specified qemu will use a default name
+of 'iqn.2008-11.org.linux-kvm[:<name>'] where <name> is the name of the
+virtual machine.
+
+
 @example
 Setting a specific initiator name to use when logging in to the target
 -iscsi initiator-name=iqn.qemu.test:my-initiator

qemu-options.hx

@@ -1897,6 +1897,11 @@ images for the guest storage. Both disk and cdrom images are supported.
 Syntax for specifying iSCSI LUNs is
 ``iscsi://<target-ip>[:<port>]/<target-iqn>/<lun>''
 
+By default qemu will use the iSCSI initiator-name
+'iqn.2008-11.org.linux-kvm[:<name>]' but this can also be set from the command
+line or a configuration file.
+
+
 Example (without authentication):
 @example
 qemu-system-i386 -iscsi initiator-name=iqn.2001-04.com.example:my-initiator \
@@ -1926,6 +1931,9 @@ DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
     "                iSCSI session parameters\n", QEMU_ARCH_ALL)
 STEXI
 
+iSCSI parameters such as username and password can also be specified via
+a configuration file. See qemu-doc for more information and examples.
+
 @item NBD
 QEMU supports NBD (Network Block Devices) both using TCP protocol as well
 as Unix Domain Sockets.

qemu-tool.c

@@ -30,6 +30,11 @@ struct QEMUBH
     void *opaque;
 };
 
+const char *qemu_get_vm_name(void)
+{
+    return NULL;
+}
+
 Monitor *cur_mon;
 
 int monitor_cur_is_qmp(void)

vl.c

@@ -293,6 +293,11 @@ static struct {
     { .driver = "qxl-vga",              .flag = &default_vga       },
 };
 
+const char *qemu_get_vm_name(void)
+{
+    return qemu_name;
+}
+
 static void res_free(void)
 {
     if (boot_splash_filedata != NULL) {