All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11
@ 2013-09-12 11:17 Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 01/11] scsi: prefer UUID to VM name for the initiator name Paolo Bonzini
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel

Anthony,

The following changes since commit 2d1fe1873a984d1c2c89ffa3d12949cafc718551:

  Merge remote-tracking branch 'pmaydell/tags/pull-target-arm-20130910' into staging (2013-09-11 14:46:52 -0500)

are available in the git repository at:


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

for you to fetch changes up to f4ff3b7ba1bcb77d5b5cdbd6e695df793761170b:

  spapr-vscsi: Report error on unsupported MAD requests (2013-09-12 13:15:54 +0200)

----------------------------------------------------------------
Alexey Kardashevskiy (2):
      spapr-vscsi: add task management
      spapr-vscsi: Report error on unsupported MAD requests

Markus Armbruster (2):
      virtio-scsi: Make type virtio-scsi-common abstract
      scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial

Nikunj A. Dadhania (1):
      spapr-vscsi: Adding VSCSI capabilities

Paolo Bonzini (1):
      scsi: prefer UUID to VM name for the initiator name

Peter Lieven (3):
      iscsi: add logical block provisioning information to iscsilun
      iscsi: add .bdrv_get_block_status
      iscsi: split discard requests in multiple parts

Peter Maydell (2):
      hw/scsi/lsi53c895a: Use sextract32 for sign-extension
      hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask

 block/iscsi.c           | 392 +++++++++++++++++++++++++++++++++++++-----------
 hw/scsi/lsi53c895a.c    |  19 +--
 hw/scsi/scsi-bus.c      |   2 +-
 hw/scsi/spapr_vscsi.c   | 195 ++++++++++++++++++++----
 hw/scsi/srp.h           |   7 +
 hw/scsi/virtio-scsi.c   |   1 +
 include/sysemu/sysemu.h |   2 +
 stubs/Makefile.objs     |   1 +
 stubs/uuid.c            |  12 ++
 9 files changed, 496 insertions(+), 135 deletions(-)
 create mode 100644 stubs/uuid.c
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 01/11] scsi: prefer UUID to VM name for the initiator name
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 02/11] spapr-vscsi: add task management Paolo Bonzini
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel

The UUID is unique even across multiple hosts, thus it is
better than a VM name even if it is less user-friendly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c           | 23 ++++++++++++++++-------
 include/sysemu/sysemu.h |  2 ++
 stubs/Makefile.objs     |  1 +
 stubs/uuid.c            | 12 ++++++++++++
 4 files changed, 31 insertions(+), 7 deletions(-)
 create mode 100644 stubs/uuid.c

diff --git a/block/iscsi.c b/block/iscsi.c
index 813abd8..60b3967 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -33,6 +33,8 @@
 #include "trace.h"
 #include "block/scsi.h"
 #include "qemu/iov.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
 
 #include <iscsi/iscsi.h>
 #include <iscsi/scsi-lowlevel.h>
@@ -922,8 +924,9 @@ static char *parse_initiator_name(const char *target)
 {
     QemuOptsList *list;
     QemuOpts *opts;
-    const char *name = NULL;
-    const char *iscsi_name = qemu_get_vm_name();
+    const char *name;
+    char *iscsi_name;
+    UuidInfo *uuid_info;
 
     list = qemu_find_opts("iscsi");
     if (list) {
@@ -933,16 +936,22 @@ static char *parse_initiator_name(const char *target)
         }
         if (opts) {
             name = qemu_opt_get(opts, "initiator-name");
+            if (name) {
+                return g_strdup(name);
+            }
         }
     }
 
-    if (name) {
-        return g_strdup(name);
+    uuid_info = qmp_query_uuid(NULL);
+    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
+        name = qemu_get_vm_name();
     } else {
-        return g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
-                               iscsi_name ? ":" : "",
-                               iscsi_name ? iscsi_name : "");
+        name = uuid_info->UUID;
     }
+    iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
+                                 name ? ":" : "", name ? name : "");
+    qapi_free_UuidInfo(uuid_info);
+    return iscsi_name;
 }
 
 #if defined(LIBISCSI_FEATURE_NOP_COUNTER)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b1aa059..e2c6f58 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -17,7 +17,9 @@ extern const char *bios_name;
 extern const char *qemu_name;
 extern uint8_t qemu_uuid[];
 int qemu_uuid_parse(const char *str, uint8_t *uuid);
+
 #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
+#define UUID_NONE "00000000-0000-0000-0000-000000000000"
 
 bool runstate_check(RunState state);
 void runstate_set(RunState new_state);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f306cba..df92fe5 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -22,6 +22,7 @@ stub-obj-y += reset.o
 stub-obj-y += set-fd-handler.o
 stub-obj-y += slirp.o
 stub-obj-y += sysbus.o
+stub-obj-y += uuid.o
 stub-obj-y += vm-stop.o
 stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/uuid.c b/stubs/uuid.c
new file mode 100644
index 0000000..ffc0ed4
--- /dev/null
+++ b/stubs/uuid.c
@@ -0,0 +1,12 @@
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
+
+UuidInfo *qmp_query_uuid(Error **errp)
+{
+    UuidInfo *info = g_malloc0(sizeof(*info));
+
+    info->UUID = g_strdup(UUID_NONE);
+    return info;
+}
+
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 02/11] spapr-vscsi: add task management
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 01/11] scsi: prefer UUID to VM name for the initiator name Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 03/11] virtio-scsi: Make type virtio-scsi-common abstract Paolo Bonzini
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy

From: Alexey Kardashevskiy <aik@ozlabs.ru>

At the moment the guest kernel issues two types of task management
requests to the hypervisor - task about and lun reset. This adds
handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(),
free_request callback was implemented.

As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB
control byte does not seem to be used at all so NACA bit is not
set to the guest so the guest has no good reason to call CLEAR_ACA task.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
[Fix choice of UCSOLCNT vs. SCSOLCNT. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/spapr_vscsi.c | 119 ++++++++++++++++++++++++++++++++++++++------------
 hw/scsi/srp.h         |   7 +++
 2 files changed, 99 insertions(+), 27 deletions(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index b2fcd4b..c31a870 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -117,6 +117,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
     return NULL;
 }
 
+static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag)
+{
+    vscsi_req *req;
+    int i;
+
+    for (i = 0; i < VSCSI_REQ_LIMIT; i++) {
+        req = &s->reqs[i];
+        if (req->iu.srp.cmd.tag == srp_tag) {
+            return req;
+        }
+    }
+    return NULL;
+}
+
 static void vscsi_put_req(vscsi_req *req)
 {
     if (req->sreq != NULL) {
@@ -755,40 +769,91 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req)
 {
     union viosrp_iu *iu = &req->iu;
-    int fn;
+    vscsi_req *tmpreq;
+    int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE;
+    SCSIDevice *d;
+    uint64_t tag = iu->srp.rsp.tag;
+    uint8_t sol_not = iu->srp.cmd.sol_not;
 
     fprintf(stderr, "vscsi_process_tsk_mgmt %02x\n",
             iu->srp.tsk_mgmt.tsk_mgmt_func);
 
-    switch (iu->srp.tsk_mgmt.tsk_mgmt_func) {
-#if 0 /* We really don't deal with these for now */
-    case SRP_TSK_ABORT_TASK:
-        fn = ABORT_TASK;
-        break;
-    case SRP_TSK_ABORT_TASK_SET:
-        fn = ABORT_TASK_SET;
-        break;
-    case SRP_TSK_CLEAR_TASK_SET:
-        fn = CLEAR_TASK_SET;
-        break;
-    case SRP_TSK_LUN_RESET:
-        fn = LOGICAL_UNIT_RESET;
-        break;
-    case SRP_TSK_CLEAR_ACA:
-        fn = CLEAR_ACA;
-        break;
-#endif
-    default:
-        fn = 0;
+    d = vscsi_device_find(&s->bus, be64_to_cpu(req->iu.srp.tsk_mgmt.lun), &lun);
+    if (!d) {
+        resp = SRP_TSK_MGMT_FIELDS_INVALID;
+    } else {
+        switch (iu->srp.tsk_mgmt.tsk_mgmt_func) {
+        case SRP_TSK_ABORT_TASK:
+            if (d->lun != lun) {
+                resp = SRP_TSK_MGMT_FIELDS_INVALID;
+                break;
+            }
+
+            tmpreq = vscsi_find_req(s, req->iu.srp.tsk_mgmt.task_tag);
+            if (tmpreq && tmpreq->sreq) {
+                assert(tmpreq->sreq->hba_private);
+                scsi_req_cancel(tmpreq->sreq);
+            }
+            break;
+
+        case SRP_TSK_LUN_RESET:
+            if (d->lun != lun) {
+                resp = SRP_TSK_MGMT_FIELDS_INVALID;
+                break;
+            }
+
+            qdev_reset_all(&d->qdev);
+            break;
+
+        case SRP_TSK_ABORT_TASK_SET:
+        case SRP_TSK_CLEAR_TASK_SET:
+            if (d->lun != lun) {
+                resp = SRP_TSK_MGMT_FIELDS_INVALID;
+                break;
+            }
+
+            for (i = 0; i < VSCSI_REQ_LIMIT; i++) {
+                tmpreq = &s->reqs[i];
+                if (tmpreq->iu.srp.cmd.lun != req->iu.srp.tsk_mgmt.lun) {
+                    continue;
+                }
+                if (!tmpreq->active || !tmpreq->sreq) {
+                    continue;
+                }
+                assert(tmpreq->sreq->hba_private);
+                scsi_req_cancel(tmpreq->sreq);
+            }
+            break;
+
+        case SRP_TSK_CLEAR_ACA:
+            resp = SRP_TSK_MGMT_NOT_SUPPORTED;
+            break;
+
+        default:
+            resp = SRP_TSK_MGMT_FIELDS_INVALID;
+            break;
+        }
     }
-    if (fn) {
-        /* XXX Send/Handle target task management */
-        ;
+
+    /* Compose the response here as  */
+    memset(iu, 0, sizeof(struct srp_rsp) + 4);
+    iu->srp.rsp.opcode = SRP_RSP;
+    iu->srp.rsp.req_lim_delta = cpu_to_be32(1);
+    iu->srp.rsp.tag = tag;
+    iu->srp.rsp.flags |= SRP_RSP_FLAG_RSPVALID;
+    iu->srp.rsp.resp_data_len = cpu_to_be32(4);
+    if (resp) {
+        iu->srp.rsp.sol_not = (sol_not & 0x04) >> 2;
     } else {
-        vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
-        vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+        iu->srp.rsp.sol_not = (sol_not & 0x02) >> 1;
     }
-    return !fn;
+
+    iu->srp.rsp.status = GOOD;
+    iu->srp.rsp.data[3] = resp;
+
+    vscsi_send_iu(s, req, sizeof(iu->srp.rsp) + 4, VIOSRP_SRP_FORMAT);
+
+    return 1;
 }
 
 static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req)
diff --git a/hw/scsi/srp.h b/hw/scsi/srp.h
index 5e0cad5..d27f31d 100644
--- a/hw/scsi/srp.h
+++ b/hw/scsi/srp.h
@@ -90,6 +90,13 @@ enum {
     SRP_REV16A_IB_IO_CLASS = 0x0100
 };
 
+enum {
+    SRP_TSK_MGMT_COMPLETE       = 0x00,
+    SRP_TSK_MGMT_FIELDS_INVALID = 0x02,
+    SRP_TSK_MGMT_NOT_SUPPORTED  = 0x04,
+    SRP_TSK_MGMT_FAILED         = 0x05
+};
+
 struct srp_direct_buf {
     uint64_t    va;
     uint32_t    key;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 03/11] virtio-scsi: Make type virtio-scsi-common abstract
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 01/11] scsi: prefer UUID to VM name for the initiator name Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 02/11] spapr-vscsi: add task management Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 04/11] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial Paolo Bonzini
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

It's the abstract base of virtio-scsi-device and vhost-scsi.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3bd690d..26d95a1 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -693,6 +693,7 @@ static const TypeInfo virtio_scsi_common_info = {
     .name = TYPE_VIRTIO_SCSI_COMMON,
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VirtIOSCSICommon),
+    .abstract = true,
     .class_init = virtio_scsi_common_class_init,
 };
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 04/11] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 03/11] virtio-scsi: Make type virtio-scsi-common abstract Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 05/11] hw/scsi/lsi53c895a: Use sextract32 for sign-extension Paolo Bonzini
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, qemu-stable

From: Markus Armbruster <armbru@redhat.com>

scsi_bus_legacy_add_drive() creates either a scsi-disk or a
scsi-generic device.  It sets property "serial" to argument serial
unless null.  Crashes with scsi-generic, because it doesn't have such
the property.

Only usb_msd_initfn_storage() passes non-null serial.  Reproducer:

    $ qemu-system-x86_64 -nodefaults -display none -S -usb \
    -drive if=none,file=/dev/sg1,id=usb-drv0 \
    -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123
    qemu-system-x86_64: -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not found
    Aborted (core dumped)

Fix by handling exactly like "removable": set the property only when
it exists.

Cc: qemu-stable@nongnu.org
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 5cd6137..4d36841 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -224,7 +224,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
     if (object_property_find(OBJECT(dev), "removable", NULL)) {
         qdev_prop_set_bit(dev, "removable", removable);
     }
-    if (serial) {
+    if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
         qdev_prop_set_string(dev, "serial", serial);
     }
     if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 05/11] hw/scsi/lsi53c895a: Use sextract32 for sign-extension
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 04/11] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 06/11] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask Paolo Bonzini
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

From: Peter Maydell <peter.maydell@linaro.org>

Use sextract32() for doing sign-extension rather than rolling
our own implementation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/lsi53c895a.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 0c36842..a26b20b 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -998,12 +998,6 @@ bad:
     s->msg_action = 0;
 }
 
-/* Sign extend a 24-bit value.  */
-static inline int32_t sxt24(int32_t n)
-{
-    return (n << 8) >> 8;
-}
-
 #define LSI_BUF_SIZE 4096
 static void lsi_memcpy(LSIState *s, uint32_t dest, uint32_t src, int count)
 {
@@ -1083,7 +1077,7 @@ again:
             /* Table indirect addressing.  */
 
             /* 32-bit Table indirect */
-            offset = sxt24(addr);
+            offset = sextract32(addr, 0, 24);
             pci_dma_read(pci_dev, s->dsa + offset, buf, 8);
             /* byte count is stored in bits 0:23 only */
             s->dbc = cpu_to_le32(buf[0]) & 0xffffff;
@@ -1183,13 +1177,13 @@ again:
             uint32_t id;
 
             if (insn & (1 << 25)) {
-                id = read_dword(s, s->dsa + sxt24(insn));
+                id = read_dword(s, s->dsa + sextract32(insn, 0, 24));
             } else {
                 id = insn;
             }
             id = (id >> 16) & 0xf;
             if (insn & (1 << 26)) {
-                addr = s->dsp + sxt24(addr);
+                addr = s->dsp + sextract32(addr, 0, 24);
             }
             s->dnad = addr;
             switch (opcode) {
@@ -1385,7 +1379,7 @@ again:
             if (cond == jmp) {
                 if (insn & (1 << 23)) {
                     /* Relative address.  */
-                    addr = s->dsp + sxt24(addr);
+                    addr = s->dsp + sextract32(addr, 0, 24);
                 }
                 switch ((insn >> 27) & 7) {
                 case 0: /* Jump */
@@ -1438,7 +1432,7 @@ again:
             int i;
 
             if (insn & (1 << 28)) {
-                addr = s->dsa + sxt24(addr);
+                addr = s->dsa + sextract32(addr, 0, 24);
             }
             n = (insn & 7);
             reg = (insn >> 16) & 0xff;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 06/11] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 05/11] hw/scsi/lsi53c895a: Use sextract32 for sign-extension Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 07/11] iscsi: add logical block provisioning information to iscsilun Paolo Bonzini
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

From: Peter Maydell <peter.maydell@linaro.org>

Use deposit32() rather than handcoded shifts/masks to update the
scratch registers. This is cleaner and incidentally avoids a clang
sanitizer complaint ("runtime error: left shift of 255 by 24 places
cannot be represented in type 'int'").

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/lsi53c895a.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index a26b20b..5affc82 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1870,8 +1870,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             int shift;
             n = (offset - 0x58) >> 2;
             shift = (offset & 3) * 8;
-            s->scratch[n] &= ~(0xff << shift);
-            s->scratch[n] |= (val & 0xff) << shift;
+            s->scratch[n] = deposit32(s->scratch[n], shift, 8, val);
         } else {
             BADF("Unhandled writeb 0x%x = 0x%x\n", offset, val);
         }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 07/11] iscsi: add logical block provisioning information to iscsilun
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 06/11] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status Paolo Bonzini
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index 60b3967..bfd659a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -52,6 +52,10 @@ typedef struct IscsiLun {
     uint64_t num_blocks;
     int events;
     QEMUTimer *nop_timer;
+    uint8_t lbpme;
+    uint8_t lbprz;
+    struct scsi_inquiry_logical_block_provisioning lbp;
+    struct scsi_inquiry_block_limits bl;
 } IscsiLun;
 
 typedef struct IscsiAIOCB {
@@ -999,6 +1003,8 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
                 } else {
                     iscsilun->block_size = rc16->block_length;
                     iscsilun->num_blocks = rc16->returned_lba + 1;
+                    iscsilun->lbpme = rc16->lbpme;
+                    iscsilun->lbprz = rc16->lbprz;
                 }
             }
             break;
@@ -1051,6 +1057,37 @@ static QemuOptsList runtime_opts = {
     },
 };
 
+static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi,
+                                          int lun, int evpd, int pc) {
+        int full_size;
+        struct scsi_task *task = NULL;
+        task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64);
+        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
+            goto fail;
+        }
+        full_size = scsi_datain_getfullsize(task);
+        if (full_size > task->datain.size) {
+            scsi_free_scsi_task(task);
+
+            /* we need more data for the full list */
+            task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size);
+            if (task == NULL || task->status != SCSI_STATUS_GOOD) {
+                goto fail;
+            }
+        }
+
+        return task;
+
+fail:
+        error_report("iSCSI: Inquiry command failed : %s",
+                     iscsi_get_error(iscsi));
+        if (task) {
+            scsi_free_scsi_task(task);
+            return NULL;
+        }
+        return NULL;
+}
+
 /*
  * We support iscsi url's on the form
  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
@@ -1180,6 +1217,46 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
         bs->sg = 1;
     }
 
+    if (iscsilun->lbpme) {
+        struct scsi_inquiry_logical_block_provisioning *inq_lbp;
+        task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
+                                SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING);
+        if (task == NULL) {
+            ret = -EINVAL;
+            goto out;
+        }
+        inq_lbp = scsi_datain_unmarshall(task);
+        if (inq_lbp == NULL) {
+            error_report("iSCSI: failed to unmarshall inquiry datain blob");
+            ret = -EINVAL;
+            goto out;
+        }
+        memcpy(&iscsilun->lbp, inq_lbp,
+               sizeof(struct scsi_inquiry_logical_block_provisioning));
+        scsi_free_scsi_task(task);
+        task = NULL;
+    }
+
+    if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) {
+        struct scsi_inquiry_block_limits *inq_bl;
+        task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
+                                SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS);
+        if (task == NULL) {
+            ret = -EINVAL;
+            goto out;
+        }
+        inq_bl = scsi_datain_unmarshall(task);
+        if (inq_bl == NULL) {
+            error_report("iSCSI: failed to unmarshall inquiry datain blob");
+            ret = -EINVAL;
+            goto out;
+        }
+        memcpy(&iscsilun->bl, inq_bl,
+               sizeof(struct scsi_inquiry_block_limits));
+        scsi_free_scsi_task(task);
+        task = NULL;
+    }
+
 #if defined(LIBISCSI_FEATURE_NOP_COUNTER)
     /* Set up a timer for sending out iSCSI NOPs */
     iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, iscsi_nop_timed_event, iscsilun);
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 07/11] iscsi: add logical block provisioning information to iscsilun Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-17 17:18   ` Stefan Weil
  2013-09-12 11:17 ` [Qemu-devel] [PULL 09/11] iscsi: split discard requests in multiple parts Paolo Bonzini
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

this patch adds a coroutine for .bdrv_co_block_status as well as
a generic framework that can be used to build coroutines in block/iscsi.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index bfd659a..c377d21 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -58,6 +58,15 @@ typedef struct IscsiLun {
     struct scsi_inquiry_block_limits bl;
 } IscsiLun;
 
+typedef struct IscsiTask {
+    int status;
+    int complete;
+    int retries;
+    int do_retry;
+    struct scsi_task *task;
+    Coroutine *co;
+} IscsiTask;
+
 typedef struct IscsiAIOCB {
     BlockDriverAIOCB common;
     QEMUIOVector *qiov;
@@ -111,6 +120,41 @@ iscsi_schedule_bh(IscsiAIOCB *acb)
     qemu_bh_schedule(acb->bh);
 }
 
+static void
+iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
+                        void *command_data, void *opaque)
+{
+    struct IscsiTask *iTask = opaque;
+    struct scsi_task *task = command_data;
+
+    iTask->complete = 1;
+    iTask->status = status;
+    iTask->do_retry = 0;
+    iTask->task = task;
+
+    if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION
+        && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
+        iTask->do_retry = 1;
+        goto out;
+    }
+
+    if (status != SCSI_STATUS_GOOD) {
+        error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi));
+    }
+
+out:
+    if (iTask->co) {
+        qemu_coroutine_enter(iTask->co, NULL);
+    }
+}
+
+static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
+{
+    *iTask = (struct IscsiTask) {
+        .co         = qemu_coroutine_self(),
+        .retries    = ISCSI_CMD_RETRIES,
+    };
+}
 
 static void
 iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
@@ -848,6 +892,96 @@ iscsi_getlength(BlockDriverState *bs)
     return len;
 }
 
+static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
+                                                  int64_t sector_num,
+                                                  int nb_sectors, int *pnum)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct scsi_get_lba_status *lbas = NULL;
+    struct scsi_lba_status_descriptor *lbasd = NULL;
+    struct IscsiTask iTask;
+    int64_t ret;
+
+    iscsi_co_init_iscsitask(iscsilun, &iTask);
+
+    if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* default to all sectors allocated */
+    ret = BDRV_BLOCK_DATA;
+    ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
+    *pnum = nb_sectors;
+
+    /* LUN does not support logical block provisioning */
+    if (iscsilun->lbpme == 0) {
+        goto out;
+    }
+
+retry:
+    if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
+                                  sector_qemu2lun(sector_num, iscsilun),
+                                  8 + 16, iscsi_co_generic_cb,
+                                  &iTask) == NULL) {
+        ret = -EIO;
+        goto out;
+    }
+
+    while (!iTask.complete) {
+        iscsi_set_events(iscsilun);
+        qemu_coroutine_yield();
+    }
+
+    if (iTask.do_retry) {
+        if (iTask.task != NULL) {
+            scsi_free_scsi_task(iTask.task);
+            iTask.task = NULL;
+        }
+        goto retry;
+    }
+
+    if (iTask.status != SCSI_STATUS_GOOD) {
+        /* in case the get_lba_status_callout fails (i.e.
+         * because the device is busy or the cmd is not
+         * supported) we pretend all blocks are allocated
+         * for backwards compatiblity */
+        goto out;
+    }
+
+    lbas = scsi_datain_unmarshall(iTask.task);
+    if (lbas == NULL) {
+        ret = -EIO;
+        goto out;
+    }
+
+    lbasd = &lbas->descriptors[0];
+
+    if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
+        ret = -EIO;
+        goto out;
+    }
+
+    *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
+    if (*pnum > nb_sectors) {
+        *pnum = nb_sectors;
+    }
+
+    if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
+        lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
+        ret &= ~BDRV_BLOCK_DATA;
+        if (iscsilun->lbprz) {
+            ret |= BDRV_BLOCK_ZERO;
+        }
+    }
+
+out:
+    if (iTask.task != NULL) {
+        scsi_free_scsi_task(iTask.task);
+    }
+    return ret;
+}
+
 static int parse_chap(struct iscsi_context *iscsi, const char *target)
 {
     QemuOptsList *list;
@@ -1398,6 +1532,8 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_getlength  = iscsi_getlength,
     .bdrv_truncate   = iscsi_truncate,
 
+    .bdrv_co_get_block_status = iscsi_co_get_block_status,
+
     .bdrv_aio_readv  = iscsi_aio_readv,
     .bdrv_aio_writev = iscsi_aio_writev,
     .bdrv_aio_flush  = iscsi_aio_flush,
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 09/11] iscsi: split discard requests in multiple parts
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 10/11] spapr-vscsi: Adding VSCSI capabilities Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 11/11] spapr-vscsi: Report error on unsupported MAD requests Paolo Bonzini
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

Replace .bdrv_aio_discard with .bdrv_co_discard so that discard
requests can be split in multiple parts, each for a small amount
of sectors.

This is useful because we expose a generic API with no limit
on the amount of sectors that can be unmapped in one request.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 156 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 73 insertions(+), 83 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index c377d21..68f99d3 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -87,6 +87,7 @@ typedef struct IscsiAIOCB {
 #define NOP_INTERVAL 5000
 #define MAX_NOP_FAILURES 3
 #define ISCSI_CMD_RETRIES 5
+#define ISCSI_MAX_UNMAP 131072
 
 static void
 iscsi_bh_cb(void *p)
@@ -618,88 +619,6 @@ iscsi_aio_flush(BlockDriverState *bs,
     return &acb->common;
 }
 
-static int iscsi_aio_discard_acb(IscsiAIOCB *acb);
-
-static void
-iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
-                     void *command_data, void *opaque)
-{
-    IscsiAIOCB *acb = opaque;
-
-    if (acb->canceled != 0) {
-        return;
-    }
-
-    acb->status = 0;
-    if (status != 0) {
-        if (status == SCSI_STATUS_CHECK_CONDITION
-            && acb->task->sense.key == SCSI_SENSE_UNIT_ATTENTION
-            && acb->retries-- > 0) {
-            scsi_free_scsi_task(acb->task);
-            acb->task = NULL;
-            if (iscsi_aio_discard_acb(acb) == 0) {
-                iscsi_set_events(acb->iscsilun);
-                return;
-            }
-        }
-        error_report("Failed to unmap data on iSCSI lun. %s",
-                     iscsi_get_error(iscsi));
-        acb->status = -EIO;
-    }
-
-    iscsi_schedule_bh(acb);
-}
-
-static int iscsi_aio_discard_acb(IscsiAIOCB *acb) {
-    struct iscsi_context *iscsi = acb->iscsilun->iscsi;
-    struct unmap_list list[1];
-
-    acb->canceled   = 0;
-    acb->bh         = NULL;
-    acb->status     = -EINPROGRESS;
-    acb->buf        = NULL;
-
-    list[0].lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
-    list[0].num = acb->nb_sectors * BDRV_SECTOR_SIZE / acb->iscsilun->block_size;
-
-    acb->task = iscsi_unmap_task(iscsi, acb->iscsilun->lun,
-                                 0, 0, &list[0], 1,
-                                 iscsi_unmap_cb,
-                                 acb);
-    if (acb->task == NULL) {
-        error_report("iSCSI: Failed to send unmap command. %s",
-                     iscsi_get_error(iscsi));
-        return -1;
-    }
-
-    return 0;
-}
-
-static BlockDriverAIOCB *
-iscsi_aio_discard(BlockDriverState *bs,
-                  int64_t sector_num, int nb_sectors,
-                  BlockDriverCompletionFunc *cb, void *opaque)
-{
-    IscsiLun *iscsilun = bs->opaque;
-    IscsiAIOCB *acb;
-
-    acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
-
-    acb->iscsilun    = iscsilun;
-    acb->nb_sectors  = nb_sectors;
-    acb->sector_num  = sector_num;
-    acb->retries     = ISCSI_CMD_RETRIES;
-
-    if (iscsi_aio_discard_acb(acb) != 0) {
-        qemu_aio_release(acb);
-        return NULL;
-    }
-
-    iscsi_set_events(iscsilun);
-
-    return &acb->common;
-}
-
 #ifdef __linux__
 static void
 iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
@@ -982,6 +901,77 @@ out:
     return ret;
 }
 
+static int
+coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num,
+                                   int nb_sectors)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct IscsiTask iTask;
+    struct unmap_list list;
+    uint32_t nb_blocks;
+    uint32_t max_unmap;
+
+    if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+        return -EINVAL;
+    }
+
+    if (!iscsilun->lbp.lbpu) {
+        /* UNMAP is not supported by the target */
+        return 0;
+    }
+
+    list.lba = sector_qemu2lun(sector_num, iscsilun);
+    nb_blocks = sector_qemu2lun(nb_sectors, iscsilun);
+
+    max_unmap = iscsilun->bl.max_unmap;
+    if (max_unmap == 0xffffffff) {
+        max_unmap = ISCSI_MAX_UNMAP;
+    }
+
+    while (nb_blocks > 0) {
+        iscsi_co_init_iscsitask(iscsilun, &iTask);
+        list.num = nb_blocks;
+        if (list.num > max_unmap) {
+            list.num = max_unmap;
+        }
+retry:
+        if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1,
+                         iscsi_co_generic_cb, &iTask) == NULL) {
+            return -EIO;
+        }
+
+        while (!iTask.complete) {
+            iscsi_set_events(iscsilun);
+            qemu_coroutine_yield();
+        }
+
+        if (iTask.task != NULL) {
+            scsi_free_scsi_task(iTask.task);
+            iTask.task = NULL;
+        }
+
+        if (iTask.do_retry) {
+            goto retry;
+        }
+
+        if (iTask.status == SCSI_STATUS_CHECK_CONDITION) {
+            /* the target might fail with a check condition if it
+               is not happy with the alignment of the UNMAP request
+               we silently fail in this case */
+            return 0;
+        }
+
+        if (iTask.status != SCSI_STATUS_GOOD) {
+            return -EIO;
+        }
+
+        list.lba += list.num;
+        nb_blocks -= list.num;
+    }
+
+    return 0;
+}
+
 static int parse_chap(struct iscsi_context *iscsi, const char *target)
 {
     QemuOptsList *list;
@@ -1533,12 +1523,12 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_truncate   = iscsi_truncate,
 
     .bdrv_co_get_block_status = iscsi_co_get_block_status,
+    .bdrv_co_discard      = iscsi_co_discard,
 
     .bdrv_aio_readv  = iscsi_aio_readv,
     .bdrv_aio_writev = iscsi_aio_writev,
     .bdrv_aio_flush  = iscsi_aio_flush,
 
-    .bdrv_aio_discard = iscsi_aio_discard,
     .bdrv_has_zero_init = iscsi_has_zero_init,
 
 #ifdef __linux__
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 10/11] spapr-vscsi: Adding VSCSI capabilities
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 09/11] iscsi: split discard requests in multiple parts Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  2013-09-12 11:17 ` [Qemu-devel] [PULL 11/11] spapr-vscsi: Report error on unsupported MAD requests Paolo Bonzini
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Nikunj A. Dadhania

From: "Nikunj A. Dadhania" <nikunj@linux.vnet.ibm.com>

This implements capabilities exchange between vscsi host and client.  As
at the moment no capability is supported, put zero flags everywhere and
return.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/scsi/spapr_vscsi.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index c31a870..f206452 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -923,6 +923,57 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req *req)
     return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT);
 }
 
+static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
+{
+    struct viosrp_capabilities *vcap;
+    struct capabilities cap = { };
+    uint16_t len, req_len;
+    uint64_t buffer;
+    int rc;
+
+    vcap = &req->iu.mad.capabilities;
+    req_len = len = be16_to_cpu(vcap->common.length);
+    buffer = be64_to_cpu(vcap->buffer);
+    if (len > sizeof(cap)) {
+        fprintf(stderr, "vscsi_send_capabilities: capabilities size mismatch !\n");
+
+        /*
+         * Just read and populate the structure that is known.
+         * Zero rest of the structure.
+         */
+        len = sizeof(cap);
+    }
+    rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
+    if (rc)  {
+        fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
+    }
+
+    /*
+     * Current implementation does not suppport any migration or
+     * reservation capabilities. Construct the response telling the
+     * guest not to use them.
+     */
+    cap.flags = 0;
+    cap.migration.ecl = 0;
+    cap.reserve.type = 0;
+    cap.migration.common.server_support = 0;
+    cap.reserve.common.server_support = 0;
+
+    rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
+    if (rc)  {
+        fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
+    }
+    if (req_len > len) {
+        /*
+         * Being paranoid and lets not worry about the error code
+         * here. Actual write of the cap is done above.
+         */
+        spapr_vio_dma_set(&s->vdev, (buffer + len), 0, (req_len - len));
+    }
+    vcap->common.status = rc ? cpu_to_be32(1) : 0;
+    return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT);
+}
+
 static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
 {
     union mad_iu *mad = &req->iu.mad;
@@ -943,6 +994,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
         mad->host_config.common.status = cpu_to_be16(1);
         vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT);
         break;
+    case VIOSRP_CAPABILITIES_TYPE:
+        vscsi_send_capabilities(s, req);
+        break;
     default:
         fprintf(stderr, "VSCSI: Unknown MAD type %02x\n",
                 be32_to_cpu(mad->empty_iu.common.type));
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 11/11] spapr-vscsi: Report error on unsupported MAD requests
  2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2013-09-12 11:17 ` [Qemu-devel] [PULL 10/11] spapr-vscsi: Adding VSCSI capabilities Paolo Bonzini
@ 2013-09-12 11:17 ` Paolo Bonzini
  10 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-12 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy

From: Alexey Kardashevskiy <aik@ozlabs.ru>

The existing driver just dropped unsupported requests. This adds error
responses to those unhandled requests.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/spapr_vscsi.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index f206452..2a26042 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -977,29 +977,43 @@ static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
 static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
 {
     union mad_iu *mad = &req->iu.mad;
+    bool request_handled = false;
+    uint64_t retlen = 0;
 
     switch (be32_to_cpu(mad->empty_iu.common.type)) {
     case VIOSRP_EMPTY_IU_TYPE:
         fprintf(stderr, "Unsupported EMPTY MAD IU\n");
+        retlen = sizeof(mad->empty_iu);
         break;
     case VIOSRP_ERROR_LOG_TYPE:
         fprintf(stderr, "Unsupported ERROR LOG MAD IU\n");
-        mad->error_log.common.status = cpu_to_be16(1);
-        vscsi_send_iu(s, req, sizeof(mad->error_log), VIOSRP_MAD_FORMAT);
+        retlen = sizeof(mad->error_log);
         break;
     case VIOSRP_ADAPTER_INFO_TYPE:
         vscsi_send_adapter_info(s, req);
+        request_handled = true;
         break;
     case VIOSRP_HOST_CONFIG_TYPE:
-        mad->host_config.common.status = cpu_to_be16(1);
-        vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT);
+        retlen = sizeof(mad->host_config);
         break;
     case VIOSRP_CAPABILITIES_TYPE:
         vscsi_send_capabilities(s, req);
+        request_handled = true;
         break;
     default:
         fprintf(stderr, "VSCSI: Unknown MAD type %02x\n",
                 be32_to_cpu(mad->empty_iu.common.type));
+        /*
+         * PAPR+ says that "The length field is set to the length
+         * of the data structure(s) used in the command".
+         * As we did not recognize the request type, put zero there.
+         */
+        retlen = 0;
+    }
+
+    if (!request_handled) {
+        mad->empty_iu.common.status = cpu_to_be16(VIOSRP_MAD_NOT_SUPPORTED);
+        vscsi_send_iu(s, req, retlen, VIOSRP_MAD_FORMAT);
     }
 
     return 1;
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-12 11:17 ` [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status Paolo Bonzini
@ 2013-09-17 17:18   ` Stefan Weil
  2013-09-17 17:21     ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Weil @ 2013-09-17 17:18 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Peter Lieven, qemu-devel, Anthony Liguori

Am 12.09.2013 13:17, schrieb Paolo Bonzini:
> From: Peter Lieven <pl@kamp.de>
>
> this patch adds a coroutine for .bdrv_co_block_status as well as
> a generic framework that can be used to build coroutines in block/iscsi.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/iscsi.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 136 insertions(+)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index bfd659a..c377d21 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -58,6 +58,15 @@ typedef struct IscsiLun {
>      struct scsi_inquiry_block_limits bl;
>  } IscsiLun;
>  
> +typedef struct IscsiTask {
> +    int status;
> +    int complete;
> +    int retries;
> +    int do_retry;
> +    struct scsi_task *task;
> +    Coroutine *co;
> +} IscsiTask;
> +
>  typedef struct IscsiAIOCB {
>      BlockDriverAIOCB common;
>      QEMUIOVector *qiov;
> @@ -111,6 +120,41 @@ iscsi_schedule_bh(IscsiAIOCB *acb)
>      qemu_bh_schedule(acb->bh);
>  }
>  
> +static void
> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
> +                        void *command_data, void *opaque)
> +{
> +    struct IscsiTask *iTask = opaque;
> +    struct scsi_task *task = command_data;
> +
> +    iTask->complete = 1;
> +    iTask->status = status;
> +    iTask->do_retry = 0;
> +    iTask->task = task;
> +
> +    if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION
> +        && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
> +        iTask->do_retry = 1;
> +        goto out;
> +    }
> +
> +    if (status != SCSI_STATUS_GOOD) {
> +        error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi));
> +    }
> +
> +out:
> +    if (iTask->co) {
> +        qemu_coroutine_enter(iTask->co, NULL);
> +    }
> +}
> +
> +static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
> +{
> +    *iTask = (struct IscsiTask) {
> +        .co         = qemu_coroutine_self(),
> +        .retries    = ISCSI_CMD_RETRIES,
> +    };
> +}
>  
>  static void
>  iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
> @@ -848,6 +892,96 @@ iscsi_getlength(BlockDriverState *bs)
>      return len;
>  }
>  
> +static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
> +                                                  int64_t sector_num,
> +                                                  int nb_sectors, int *pnum)
> +{
> +    IscsiLun *iscsilun = bs->opaque;
> +    struct scsi_get_lba_status *lbas = NULL;
> +    struct scsi_lba_status_descriptor *lbasd = NULL;
> +    struct IscsiTask iTask;
> +    int64_t ret;
> +
> +    iscsi_co_init_iscsitask(iscsilun, &iTask);
> +
> +    if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    /* default to all sectors allocated */
> +    ret = BDRV_BLOCK_DATA;
> +    ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
> +    *pnum = nb_sectors;
> +
> +    /* LUN does not support logical block provisioning */
> +    if (iscsilun->lbpme == 0) {
> +        goto out;
> +    }
> +
> +retry:
> +    if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
> +                                  sector_qemu2lun(sector_num, iscsilun),
> +                                  8 + 16, iscsi_co_generic_cb,
> +                                  &iTask) == NULL) {
> +        ret = -EIO;
> +        goto out;
> +    }
> +
> +    while (!iTask.complete) {
> +        iscsi_set_events(iscsilun);
> +        qemu_coroutine_yield();
> +    }
> +
> +    if (iTask.do_retry) {
> +        if (iTask.task != NULL) {
> +            scsi_free_scsi_task(iTask.task);
> +            iTask.task = NULL;
> +        }
> +        goto retry;
> +    }
> +
> +    if (iTask.status != SCSI_STATUS_GOOD) {
> +        /* in case the get_lba_status_callout fails (i.e.
> +         * because the device is busy or the cmd is not
> +         * supported) we pretend all blocks are allocated
> +         * for backwards compatiblity */
> +        goto out;
> +    }
> +
> +    lbas = scsi_datain_unmarshall(iTask.task);
> +    if (lbas == NULL) {
> +        ret = -EIO;
> +        goto out;
> +    }
> +
> +    lbasd = &lbas->descriptors[0];
> +
> +    if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
> +        ret = -EIO;
> +        goto out;
> +    }
> +
> +    *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
> +    if (*pnum > nb_sectors) {
> +        *pnum = nb_sectors;
> +    }
> +
> +    if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
> +        lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
> +        ret &= ~BDRV_BLOCK_DATA;
> +        if (iscsilun->lbprz) {
> +            ret |= BDRV_BLOCK_ZERO;
> +        }
> +    }
> +
> +out:
> +    if (iTask.task != NULL) {
> +        scsi_free_scsi_task(iTask.task);
> +    }
> +    return ret;
> +}
> +
>  static int parse_chap(struct iscsi_context *iscsi, const char *target)
>  {
>      QemuOptsList *list;
> @@ -1398,6 +1532,8 @@ static BlockDriver bdrv_iscsi = {
>      .bdrv_getlength  = iscsi_getlength,
>      .bdrv_truncate   = iscsi_truncate,
>  
> +    .bdrv_co_get_block_status = iscsi_co_get_block_status,
> +
>      .bdrv_aio_readv  = iscsi_aio_readv,
>      .bdrv_aio_writev = iscsi_aio_writev,
>      .bdrv_aio_flush  = iscsi_aio_flush,


Latest QEMU git is broken on Debian wheezy:

block/iscsi.c: In function ‘iscsi_co_get_block_status’:
block/iscsi.c:842:5: error: implicit declaration of function
‘iscsi_get_lba_status_task’ [-Werror=implicit-function-declaration]
block/iscsi.c:842:5: error: nested extern declaration of
‘iscsi_get_lba_status_task’ [-Werror=nested-externs]
block/iscsi.c:845:43: error: comparison between pointer and integer
[-Werror]
block/iscsi.c:877:18: error: dereferencing pointer to incomplete type
block/iscsi.c:879:55: error: dereferencing pointer to incomplete type
block/iscsi.c:884:34: error: dereferencing pointer to incomplete type
block/iscsi.c:889:14: error: dereferencing pointer to incomplete type
block/iscsi.c:889:32: error: ‘SCSI_PROVISIONING_TYPE_DEALLOCATED’
undeclared (first use in this function)
block/iscsi.c:889:32: note: each undeclared identifier is reported only
once for each function it appears in
block/iscsi.c:890:14: error: dereferencing pointer to incomplete type
block/iscsi.c:890:32: error: ‘SCSI_PROVISIONING_TYPE_ANCHORED’
undeclared (first use in this function)
cc1: all warnings being treated as errors

Debian includes libiscsi-dev 1.4.0 without a libiscsi.pk, so it uses a
compile test
which thinks that libiscsi is good enough.

Did the latest changes raise the requirements for libscsi?

Regards,
Stefan

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

* Re: [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-17 17:18   ` Stefan Weil
@ 2013-09-17 17:21     ` Paolo Bonzini
  2013-09-19  7:24       ` Peter Lieven
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-17 17:21 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Peter Lieven, qemu-devel, Anthony Liguori

Il 17/09/2013 19:18, Stefan Weil ha scritto:
> Latest QEMU git is broken on Debian wheezy:
> 
> block/iscsi.c: In function ‘iscsi_co_get_block_status’:
> block/iscsi.c:842:5: error: implicit declaration of function
> ‘iscsi_get_lba_status_task’ [-Werror=implicit-function-declaration]
> block/iscsi.c:842:5: error: nested extern declaration of
> ‘iscsi_get_lba_status_task’ [-Werror=nested-externs]
> block/iscsi.c:845:43: error: comparison between pointer and integer
> [-Werror]
> block/iscsi.c:877:18: error: dereferencing pointer to incomplete type
> block/iscsi.c:879:55: error: dereferencing pointer to incomplete type
> block/iscsi.c:884:34: error: dereferencing pointer to incomplete type
> block/iscsi.c:889:14: error: dereferencing pointer to incomplete type
> block/iscsi.c:889:32: error: ‘SCSI_PROVISIONING_TYPE_DEALLOCATED’
> undeclared (first use in this function)
> block/iscsi.c:889:32: note: each undeclared identifier is reported only
> once for each function it appears in
> block/iscsi.c:890:14: error: dereferencing pointer to incomplete type
> block/iscsi.c:890:32: error: ‘SCSI_PROVISIONING_TYPE_ANCHORED’
> undeclared (first use in this function)
> cc1: all warnings being treated as errors
> 
> Debian includes libiscsi-dev 1.4.0 without a libiscsi.pk, so it uses a
> compile test
> which thinks that libiscsi is good enough.
> 
> Did the latest changes raise the requirements for libscsi?

Looks like.  Can you make a patch that drops the definition of
iscsi_co_get_block_status if SCSI_PROVISIONING_TYPE_DEALLOCATED is not
#defined?

Thanks,

Paolo

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

* Re: [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-17 17:21     ` Paolo Bonzini
@ 2013-09-19  7:24       ` Peter Lieven
  2013-09-19 13:46         ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Lieven @ 2013-09-19  7:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Stefan Weil, qemu-devel, Anthony Liguori

On 17.09.2013 19:21, Paolo Bonzini wrote:
> Il 17/09/2013 19:18, Stefan Weil ha scritto:
>> Latest QEMU git is broken on Debian wheezy:
>>
>> block/iscsi.c: In function ‘iscsi_co_get_block_status’:
>> block/iscsi.c:842:5: error: implicit declaration of function
>> ‘iscsi_get_lba_status_task’ [-Werror=implicit-function-declaration]
>> block/iscsi.c:842:5: error: nested extern declaration of
>> ‘iscsi_get_lba_status_task’ [-Werror=nested-externs]
>> block/iscsi.c:845:43: error: comparison between pointer and integer
>> [-Werror]
>> block/iscsi.c:877:18: error: dereferencing pointer to incomplete type
>> block/iscsi.c:879:55: error: dereferencing pointer to incomplete type
>> block/iscsi.c:884:34: error: dereferencing pointer to incomplete type
>> block/iscsi.c:889:14: error: dereferencing pointer to incomplete type
>> block/iscsi.c:889:32: error: ‘SCSI_PROVISIONING_TYPE_DEALLOCATED’
>> undeclared (first use in this function)
>> block/iscsi.c:889:32: note: each undeclared identifier is reported only
>> once for each function it appears in
>> block/iscsi.c:890:14: error: dereferencing pointer to incomplete type
>> block/iscsi.c:890:32: error: ‘SCSI_PROVISIONING_TYPE_ANCHORED’
>> undeclared (first use in this function)
>> cc1: all warnings being treated as errors
>>
>> Debian includes libiscsi-dev 1.4.0 without a libiscsi.pk, so it uses a
>> compile test
>> which thinks that libiscsi is good enough.
>>
>> Did the latest changes raise the requirements for libscsi?
> Looks like.  Can you make a patch that drops the definition of
> iscsi_co_get_block_status if SCSI_PROVISIONING_TYPE_DEALLOCATED is not
> #defined?
>
> Thanks,
>
> Paolo
do we still need this patch? if yes I can sent one shortly.

Peter

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

* Re: [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-19  7:24       ` Peter Lieven
@ 2013-09-19 13:46         ` Paolo Bonzini
  2013-10-02 11:37           ` Peter Lieven
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2013-09-19 13:46 UTC (permalink / raw)
  To: Peter Lieven; +Cc: Stefan Weil, qemu-devel, Anthony Liguori

Il 19/09/2013 09:24, Peter Lieven ha scritto:
>>
> do we still need this patch? if yes I can sent one shortly.

No, Stefan prepared it.  I'll send a pull request shortly.

Paolo

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

* Re: [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status
  2013-09-19 13:46         ` Paolo Bonzini
@ 2013-10-02 11:37           ` Peter Lieven
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Lieven @ 2013-10-02 11:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Stefan Weil, qemu-devel, Anthony Liguori

Am 19.09.2013 15:46, schrieb Paolo Bonzini:
> Il 19/09/2013 09:24, Peter Lieven ha scritto:
>> do we still need this patch? if yes I can sent one shortly.
> No, Stefan prepared it.  I'll send a pull request shortly.
The patch disables iscsi_get_block_status completely. The C preprocessor
can't check for enumeration constants. I would check for LIBISCSI_FEATURE_IOVECTOR
instead. At that point the enumeration constant already existed.

Peter

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

end of thread, other threads:[~2013-10-02 11:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-12 11:17 [Qemu-devel] [PULL 00/11] SCSI patches for 2013-09-11 Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 01/11] scsi: prefer UUID to VM name for the initiator name Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 02/11] spapr-vscsi: add task management Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 03/11] virtio-scsi: Make type virtio-scsi-common abstract Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 04/11] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 05/11] hw/scsi/lsi53c895a: Use sextract32 for sign-extension Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 06/11] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 07/11] iscsi: add logical block provisioning information to iscsilun Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 08/11] iscsi: add .bdrv_get_block_status Paolo Bonzini
2013-09-17 17:18   ` Stefan Weil
2013-09-17 17:21     ` Paolo Bonzini
2013-09-19  7:24       ` Peter Lieven
2013-09-19 13:46         ` Paolo Bonzini
2013-10-02 11:37           ` Peter Lieven
2013-09-12 11:17 ` [Qemu-devel] [PULL 09/11] iscsi: split discard requests in multiple parts Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 10/11] spapr-vscsi: Adding VSCSI capabilities Paolo Bonzini
2013-09-12 11:17 ` [Qemu-devel] [PULL 11/11] spapr-vscsi: Report error on unsupported MAD requests Paolo Bonzini

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.