All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2
@ 2012-08-09 13:38 Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 1/7] iscsi: do not leak initiator_name Paolo Bonzini
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori

Anthony,

The following changes since commit 0b8db8fe15d17a529a5ea90614c11e9f031dfee8:

  slirp: fix build on mingw32 (2012-08-06 19:31:55 -0500)

are available in the git repository at:

  git://github.com/bonzini/qemu.git scsi-next

for you to fetch changes up to 5222aaf223e52961cabeb7cabc579892ccd8bc59:

  scsi-disk: add support for the UNMAP command (2012-08-09 15:04:09 +0200)

----------------------------------------------------------------
Paolo Bonzini (6):
      iscsi: do not leak initiator_name
      iscsi: reorganize code for parse_initiator_name
      virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags
      scsi-disk: more assertions and resets for aiocb
      scsi-disk: improve out-of-range LBA detection for WRITE SAME
      scsi-disk: add support for the UNMAP command

Ronnie Sahlberg (1):
      iscsi: Pick default initiator-name based on the name of the VM

 block/iscsi.c    |  59 ++++++++++++++---------------
 hw/scsi-disk.c   | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/virtio-scsi.c |  10 ++++-
 qemu-common.h    |   1 +
 qemu-doc.texi    |   5 +++
 qemu-options.hx  |   8 ++++
 qemu-tool.c      |   5 +++
 vl.c             |   5 +++
 8 file modificati, 163 inserzioni(+), 42 rimozioni(-)
-- 
1.7.11.2

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/7] iscsi: do not leak initiator_name
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 2/7] iscsi: reorganize code for parse_initiator_name Paolo Bonzini
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

The argument of iscsi_create_context is never freed by libiscsi,
which in fact calls strdup on it.  Avoid a leak.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 33 ++++++++++++++++-----------------
 1 file modificato, 16 inserzioni(+), 17 rimozioni(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 993a86d..94063ab 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -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;
-    }
-
-    if (iscsi_url != NULL) {
-        iscsi_destroy_url(iscsi_url);
+        goto out;
     }
 
     /* 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;
 }
 
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/7] iscsi: reorganize code for parse_initiator_name
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 1/7] iscsi: do not leak initiator_name Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 3/7] iscsi: Pick default initiator-name based on the name of the VM Paolo Bonzini
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

Merge the occurrences of the "iqn.2008-11.org.linux-kvm" string
to avoid duplication.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 21 +++++++++------------
 1 file modificato, 9 inserzioni(+), 12 rimozioni(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 94063ab..fd954d4 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -898,24 +898,21 @@ static char *parse_initiator_name(const char *target)
     const char *name = NULL;
 
     list = qemu_find_opts("iscsi");
-    if (!list) {
-        return g_strdup("iqn.2008-11.org.linux-kvm");
-    }
-
-    opts = qemu_opts_find(list, target);
-    if (opts == NULL) {
-        opts = QTAILQ_FIRST(&list->head);
+    if (list) {
+        opts = qemu_opts_find(list, target);
         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);
+    } else {
         return g_strdup("iqn.2008-11.org.linux-kvm");
     }
-
-    return g_strdup(name);
 }
 
 /*
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/7] iscsi: Pick default initiator-name based on the name of the VM
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 1/7] iscsi: do not leak initiator_name Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 2/7] iscsi: reorganize code for parse_initiator_name Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 4/7] virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags Paolo Bonzini
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ronnie Sahlberg

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

This patch updates the iscsi layer to automatically pick a 'unique'
initiator-name based on the name of the vm in case the user has not set
an explicit iqn-name to use.

Create a new function qemu_get_vm_name() that returns the name of the VM,
if specified.

This way we can thus create default names to use as the initiator name
based on the guest session.

If the VM is not named via the '-name' command line argument, the iscsi
initiator-name used wiull simply be

    iqn.2008-11.org.linux-kvm

If a name for the VM was specified with the '-name' option, iscsi will
use a default initiatorname of

    iqn.2008-11.org.linux-kvm:<name>

These names are just the default iscsi initiator name that qemu will
generate/use only when the user has not set an explicit initiator name
via the commandlines or config files.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c   | 5 ++++-
 qemu-common.h   | 1 +
 qemu-doc.texi   | 5 +++++
 qemu-options.hx | 8 ++++++++
 qemu-tool.c     | 5 +++++
 vl.c            | 5 +++++
 6 file modificati, 28 inserzioni(+). 1 rimozione(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index fd954d4..219f927 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -896,6 +896,7 @@ 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) {
@@ -911,7 +912,9 @@ static char *parse_initiator_name(const char *target)
     if (name) {
         return g_strdup(name);
     } else {
-        return g_strdup("iqn.2008-11.org.linux-kvm");
+        return g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
+                               iscsi_name ? ":" : "",
+                               iscsi_name ? iscsi_name : "");
     }
 }
 
diff --git a/qemu-common.h b/qemu-common.h
index f16079f..f9deca6 100644
--- a/qemu-common.h
+++ b/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
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f32e9e2..35cabbc 100644
--- a/qemu-doc.texi
+++ b/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
diff --git a/qemu-options.hx b/qemu-options.hx
index 5e7d0dc..47cb5bd 100644
--- a/qemu-options.hx
+++ b/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.
diff --git a/qemu-tool.c b/qemu-tool.c
index 318c5fc..64b5e88 100644
--- a/qemu-tool.c
+++ b/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)
diff --git a/vl.c b/vl.c
index e71cb30..065aec2 100644
--- a/vl.c
+++ b/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) {
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/7] virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 3/7] iscsi: Pick default initiator-name based on the name of the VM Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb Paolo Bonzini
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

This patch fixes a problem in handling task management functions
in virtio-scsi.  The cause of the problem is a mismatch between
the size of the tag in QEMU (32-bit) and virtio-scsi (64-bit).
Changing the QEMU size is hard because the migration format
uses 32 bits to store the tag; so just don't use the QEMU tag
(virtio-scsi only uses the tag for task management functions
anyway) and look up the full 64-bit tag in the hba_private field.

The reproducer is a bit obscure.  If you cause an I/O timeout
(for example with rerror=stop and doing 'cont' on the monitor
continuously without fixing the error), sooner or later the
guest will try to abort the command and reissue it.  At this
point, QEMU will report _two_ errors instead of one when you
hit 'c', because the first error has not been canceled correctly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c | 10 ++++++++--
 1 file modificato, 8 inserzioni(+), 2 rimozioni(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index c4a5b22..5f737ac 100644
--- a/hw/virtio-scsi.c
+++ b/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".
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 4/7] virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 17:16   ` Michael Tokarev
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 6/7] scsi-disk: improve out-of-range LBA detection for WRITE SAME Paolo Bonzini
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

Leaving the aiocb to a non-NULL value leads to an assertion failure when
rerror/werror are set to stop or enospc, and the operation is retried.
scsi-disk checks that the aiocb member is NULL before filling it.

This patch correctly resets the aiocb to NULL values everywhere,
and adds the dual assertion that the aiocb was non-NULL before
calling bdrv_acct_done.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 16 ++++++++--------
 1 file modificato, 8 inserzioni(+), 8 rimozioni(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a9c7279..3baa238 100644
--- a/hw/scsi-disk.c
+++ b/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) {
-        r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+    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) {
-        r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+    bdrv_acct_done(s->qdev.conf.bs, &r->acct);
 
     if (ret < 0) {
         if (scsi_handle_rw_error(r, -ret)) {
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 6/7] scsi-disk: improve out-of-range LBA detection for WRITE SAME
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 7/7] scsi-disk: add support for the UNMAP command Paolo Bonzini
  2012-08-11 22:33 ` [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Anthony Liguori
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 3 ++-
 1 file modificato, 2 inserzioni(+). 1 rimozione(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 3baa238..dd7ae6d 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1712,7 +1712,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;
         }
 
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 7/7] scsi-disk: add support for the UNMAP command
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 6/7] scsi-disk: improve out-of-range LBA detection for WRITE SAME Paolo Bonzini
@ 2012-08-09 13:38 ` Paolo Bonzini
  2012-08-11 22:33 ` [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Anthony Liguori
  7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 13:38 UTC (permalink / raw)
  To: qemu-devel

The unmap command can reuse the same infrastructure as MODE SELECT
for reading the descriptor list into memory.  The descriptors are
processed sequentially.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file modificato, 92 inserzioni(+). 1 rimozione(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index dd7ae6d..25dbefe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -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;
@@ -2067,6 +2157,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,
 
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb Paolo Bonzini
@ 2012-08-09 17:16   ` Michael Tokarev
  2012-08-09 18:07     ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2012-08-09 17:16 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 09.08.2012 17:38, Paolo Bonzini wrote:
> Leaving the aiocb to a non-NULL value leads to an assertion failure when
> rerror/werror are set to stop or enospc, and the operation is retried.
> scsi-disk checks that the aiocb member is NULL before filling it.
> 
> This patch correctly resets the aiocb to NULL values everywhere,
> and adds the dual assertion that the aiocb was non-NULL before
> calling bdrv_acct_done.

Stable matherial?

Thanks,

/mjt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb
  2012-08-09 17:16   ` Michael Tokarev
@ 2012-08-09 18:07     ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-08-09 18:07 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-devel

Il 09/08/2012 19:16, Michael Tokarev ha scritto:
>> > Leaving the aiocb to a non-NULL value leads to an assertion failure when
>> > rerror/werror are set to stop or enospc, and the operation is retried.
>> > scsi-disk checks that the aiocb member is NULL before filling it.
>> > 
>> > This patch correctly resets the aiocb to NULL values everywhere,
>> > and adds the dual assertion that the aiocb was non-NULL before
>> > calling bdrv_acct_done.
> Stable matherial?

Possibly, but quite unlikely to trigger (since this would happen only on
flushes, basically).

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2
  2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2012-08-09 13:38 ` [Qemu-devel] [PATCH 7/7] scsi-disk: add support for the UNMAP command Paolo Bonzini
@ 2012-08-11 22:33 ` Anthony Liguori
  7 siblings, 0 replies; 11+ messages in thread
From: Anthony Liguori @ 2012-08-11 22:33 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

> Anthony,
>
> The following changes since commit 0b8db8fe15d17a529a5ea90614c11e9f031dfee8:
>
>   slirp: fix build on mingw32 (2012-08-06 19:31:55 -0500)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git scsi-next
>
> for you to fetch changes up to 5222aaf223e52961cabeb7cabc579892ccd8bc59:
>
>   scsi-disk: add support for the UNMAP command (2012-08-09 15:04:09 +0200)

Pulled. Thanks.

Regards,

Anthony Liguori

>
> ----------------------------------------------------------------
> Paolo Bonzini (6):
>       iscsi: do not leak initiator_name
>       iscsi: reorganize code for parse_initiator_name
>       virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags
>       scsi-disk: more assertions and resets for aiocb
>       scsi-disk: improve out-of-range LBA detection for WRITE SAME
>       scsi-disk: add support for the UNMAP command
>
> Ronnie Sahlberg (1):
>       iscsi: Pick default initiator-name based on the name of the VM
>
>  block/iscsi.c    |  59 ++++++++++++++---------------
>  hw/scsi-disk.c   | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  hw/virtio-scsi.c |  10 ++++-
>  qemu-common.h    |   1 +
>  qemu-doc.texi    |   5 +++
>  qemu-options.hx  |   8 ++++
>  qemu-tool.c      |   5 +++
>  vl.c             |   5 +++
>  8 file modificati, 163 inserzioni(+), 42 rimozioni(-)
> -- 
> 1.7.11.2

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-08-11 22:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-09 13:38 [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 1/7] iscsi: do not leak initiator_name Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 2/7] iscsi: reorganize code for parse_initiator_name Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 3/7] iscsi: Pick default initiator-name based on the name of the VM Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 4/7] virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi tags Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb Paolo Bonzini
2012-08-09 17:16   ` Michael Tokarev
2012-08-09 18:07     ` Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 6/7] scsi-disk: improve out-of-range LBA detection for WRITE SAME Paolo Bonzini
2012-08-09 13:38 ` [Qemu-devel] [PATCH 7/7] scsi-disk: add support for the UNMAP command Paolo Bonzini
2012-08-11 22:33 ` [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2 Anthony Liguori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.