All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18
@ 2014-06-18 16:03 Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition Paolo Bonzini
                   ` (15 more replies)
  0 siblings, 16 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit af44da87e926ff64260b95f4350d338c4fc113ca:

  Merge remote-tracking branch 'remotes/agraf/tags/signed-ppc-for-upstream' into staging (2014-06-16 18:26:21 +0100)

are available in the git repository at:


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

for you to fetch changes up to e49ab19fcaa617ad6cdfe1ac401327326b6a2552:

  block/iscsi: bump libiscsi requirement to 1.9.0 (2014-06-18 18:03:33 +0200)

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      scsi: Print command name in debug

Paolo Bonzini (8):
      megasas: use PCI DMA API
      util: add return value to qemu_iovec_concat_iov
      virtio-scsi: start preparing for any_layout
      virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields
      virtio-scsi: add extra argument and return type to qemu_sgl_concat
      virtio-scsi: prepare sense data handling for any_layout
      virtio-scsi: introduce virtio_scsi_complete_cmd_req
      virtio-scsi: add support for the any_layout feature

Paul Janzen (1):
      scsi-disk.c: Fix compilation with -DDEBUG_SCSI

Peter Lieven (4):
      block/iscsi: handle BUSY condition
      block/iscsi: fix potential segfault on early callback
      block/iscsi: use 16 byte CDBs only when necessary
      block/iscsi: bump libiscsi requirement to 1.9.0

Ulrich Obergfell (1):
      scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c

 block/iscsi.c                   | 185 +++++++++++------------
 configure                       |  39 +----
 hw/scsi/megasas.c               |  16 +-
 hw/scsi/scsi-bus.c              |   4 +-
 hw/scsi/scsi-disk.c             |   7 +-
 hw/scsi/spapr_vscsi.c           |   5 +-
 hw/scsi/virtio-scsi.c           | 314 ++++++++++++++++++++++++----------------
 include/block/scsi.h            |   2 +
 include/hw/i386/pc.h            |   4 +
 include/hw/virtio/virtio-scsi.h |   4 +-
 include/qemu-common.h           |   6 +-
 util/iov.c                      |  10 +-
 12 files changed, 314 insertions(+), 282 deletions(-)
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
@ 2014-06-18 16:03 ` Paolo Bonzini
  2014-06-30  9:39   ` Alexey Kardashevskiy
  2014-06-18 16:03 ` [Qemu-devel] [PULL 02/15] block/iscsi: fix potential segfault on early callback Paolo Bonzini
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

this patch adds handling of BUSY status reponse from an iSCSI target.
Currently, we fail with -EIO in case of SCSI_STATUS_BUSY while the
obvious reaction would be to retry the operation after some time.
The retry time is randomly choosen from a range with exponential
growth increasing with each retry.

This patch includes most of the changes by a an upcoming patch
from Stefan Hajnoczi:

 iscsi: implement .bdrv_detach/attach_aio_context()

because I also need the reference to the aio_context for
the retry timer to work. I included the changes to maintain
better mergeability.

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

diff --git a/block/iscsi.c b/block/iscsi.c
index 6f87605..e64659f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -26,6 +26,7 @@
 #include "config-host.h"
 
 #include <poll.h>
+#include <math.h>
 #include <arpa/inet.h>
 #include "qemu-common.h"
 #include "qemu/config-file.h"
@@ -75,6 +76,7 @@ typedef struct IscsiTask {
     Coroutine *co;
     QEMUBH *bh;
     IscsiLun *iscsilun;
+    QEMUTimer retry_timer;
 } IscsiTask;
 
 typedef struct IscsiAIOCB {
@@ -86,7 +88,6 @@ typedef struct IscsiAIOCB {
     uint8_t *buf;
     int status;
     int canceled;
-    int retries;
     int64_t sector_num;
     int nb_sectors;
 #ifdef __linux__
@@ -96,7 +97,8 @@ typedef struct IscsiAIOCB {
 
 #define NOP_INTERVAL 5000
 #define MAX_NOP_FAILURES 3
-#define ISCSI_CMD_RETRIES 5
+#define ISCSI_CMD_RETRIES ARRAY_SIZE(iscsi_retry_times)
+static const unsigned iscsi_retry_times[] = {8, 32, 128, 512, 2048};
 
 /* this threshhold is a trade-off knob to choose between
  * the potential additional overhead of an extra GET_LBA_STATUS request
@@ -146,6 +148,19 @@ static void iscsi_co_generic_bh_cb(void *opaque)
     qemu_coroutine_enter(iTask->co, NULL);
 }
 
+static void iscsi_retry_timer_expired(void *opaque)
+{
+    struct IscsiTask *iTask = opaque;
+    if (iTask->co) {
+        qemu_coroutine_enter(iTask->co, NULL);
+    }
+}
+
+static inline unsigned exp_random(double mean)
+{
+    return -mean * log((double)rand() / RAND_MAX);
+}
+
 static void
 iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
                         void *command_data, void *opaque)
@@ -158,14 +173,30 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
     iTask->do_retry = 0;
     iTask->task = task;
 
-    if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION
-        && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
-        error_report("iSCSI CheckCondition: %s", iscsi_get_error(iscsi));
-        iTask->do_retry = 1;
-        goto out;
-    }
-
     if (status != SCSI_STATUS_GOOD) {
+        if (iTask->retries++ < ISCSI_CMD_RETRIES) {
+            if (status == SCSI_STATUS_CHECK_CONDITION
+                && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
+                error_report("iSCSI CheckCondition: %s",
+                             iscsi_get_error(iscsi));
+                iTask->do_retry = 1;
+                goto out;
+            }
+            if (status == SCSI_STATUS_BUSY) {
+                unsigned retry_time =
+                    exp_random(iscsi_retry_times[iTask->retries - 1]);
+                error_report("iSCSI Busy (retry #%u in %u ms): %s",
+                             iTask->retries, retry_time,
+                             iscsi_get_error(iscsi));
+                aio_timer_init(iTask->iscsilun->aio_context,
+                               &iTask->retry_timer, QEMU_CLOCK_REALTIME,
+                               SCALE_MS, iscsi_retry_timer_expired, iTask);
+                timer_mod(&iTask->retry_timer,
+                          qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + retry_time);
+                iTask->do_retry = 1;
+                return;
+            }
+        }
         error_report("iSCSI Failure: %s", iscsi_get_error(iscsi));
     }
 
@@ -180,9 +211,9 @@ out:
 static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
 {
     *iTask = (struct IscsiTask) {
-        .co             = qemu_coroutine_self(),
-        .retries        = ISCSI_CMD_RETRIES,
-        .iscsilun       = iscsilun,
+        .co         = qemu_coroutine_self(),
+        .retries    = ISCSI_CMD_RETRIES,
+        .iscsilun   = iscsilun,
     };
 }
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 02/15] block/iscsi: fix potential segfault on early callback
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition Paolo Bonzini
@ 2014-06-18 16:03 ` Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 03/15] block/iscsi: use 16 byte CDBs only when necessary Paolo Bonzini
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

it might happen in the future that a function directly invokes its callback.
In this case we end up in a segfault because the iTask is gone when the BH
is scheduled.

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

diff --git a/block/iscsi.c b/block/iscsi.c
index e64659f..09485fe 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -144,6 +144,7 @@ iscsi_schedule_bh(IscsiAIOCB *acb)
 static void iscsi_co_generic_bh_cb(void *opaque)
 {
     struct IscsiTask *iTask = opaque;
+    iTask->complete = 1;
     qemu_bh_delete(iTask->bh);
     qemu_coroutine_enter(iTask->co, NULL);
 }
@@ -151,6 +152,7 @@ static void iscsi_co_generic_bh_cb(void *opaque)
 static void iscsi_retry_timer_expired(void *opaque)
 {
     struct IscsiTask *iTask = opaque;
+    iTask->complete = 1;
     if (iTask->co) {
         qemu_coroutine_enter(iTask->co, NULL);
     }
@@ -168,7 +170,6 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
     struct IscsiTask *iTask = opaque;
     struct scsi_task *task = command_data;
 
-    iTask->complete = 1;
     iTask->status = status;
     iTask->do_retry = 0;
     iTask->task = task;
@@ -205,6 +206,8 @@ out:
         iTask->bh = aio_bh_new(iTask->iscsilun->aio_context,
                                iscsi_co_generic_bh_cb, iTask);
         qemu_bh_schedule(iTask->bh);
+    } else {
+        iTask->complete = 1;
     }
 }
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 03/15] block/iscsi: use 16 byte CDBs only when necessary
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 02/15] block/iscsi: fix potential segfault on early callback Paolo Bonzini
@ 2014-06-18 16:03 ` Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 04/15] scsi-disk.c: Fix compilation with -DDEBUG_SCSI Paolo Bonzini
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

this patch changes the driver to uses 16 Byte CDBs for
READ/WRITE only if the target requires 64bit lba addressing.

On one hand this saves 6 bytes in each PDU on the other
hand it seems that 10 Byte CDBs seems to be much better
supported and tested as a recent issue I had with a
major storage supplier lined out.

For WRITESAME the logic is a bit more tricky as WRITESAME10
with UNMAP was added really late. Thus a fallback to WRITESAME16
is possible if it supports UNMAP and WRITESAME10 not.

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

diff --git a/block/iscsi.c b/block/iscsi.c
index 09485fe..3875487 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -65,6 +65,7 @@ typedef struct IscsiLun {
     unsigned char *zeroblock;
     unsigned long *allocationmap;
     int cluster_sectors;
+    bool use_16_for_rw;
 } IscsiLun;
 
 typedef struct IscsiTask {
@@ -381,10 +382,17 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
 #endif
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
-    iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
-                                    data, num_sectors * iscsilun->block_size,
-                                    iscsilun->block_size, 0, 0, 0, 0, 0,
-                                    iscsi_co_generic_cb, &iTask);
+    if (iscsilun->use_16_for_rw) {
+        iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
+                                        data, num_sectors * iscsilun->block_size,
+                                        iscsilun->block_size, 0, 0, 0, 0, 0,
+                                        iscsi_co_generic_cb, &iTask);
+    } else {
+        iTask.task = iscsi_write10_task(iscsilun->iscsi, iscsilun->lun, lba,
+                                        data, num_sectors * iscsilun->block_size,
+                                        iscsilun->block_size, 0, 0, 0, 0, 0,
+                                        iscsi_co_generic_cb, &iTask);
+    }
     if (iTask.task == NULL) {
         g_free(buf);
         return -ENOMEM;
@@ -570,14 +578,12 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
-    switch (iscsilun->type) {
-    case TYPE_DISK:
+    if (iscsilun->use_16_for_rw) {
         iTask.task = iscsi_read16_task(iscsilun->iscsi, iscsilun->lun, lba,
                                        num_sectors * iscsilun->block_size,
                                        iscsilun->block_size, 0, 0, 0, 0, 0,
                                        iscsi_co_generic_cb, &iTask);
-        break;
-    default:
+    } else {
         iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba,
                                        num_sectors * iscsilun->block_size,
                                        iscsilun->block_size,
@@ -585,7 +591,6 @@ retry:
                                        0, 0, 0, 0, 0,
 #endif
                                        iscsi_co_generic_cb, &iTask);
-        break;
     }
     if (iTask.task == NULL) {
         return -ENOMEM;
@@ -921,19 +926,27 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
     struct IscsiTask iTask;
     uint64_t lba;
     uint32_t nb_blocks;
+    bool use_16_for_ws = iscsilun->use_16_for_rw;
 
     if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
         return -EINVAL;
     }
 
-    if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) {
-        /* WRITE SAME with UNMAP is not supported by the target,
-         * fall back and try WRITE SAME without UNMAP */
-        flags &= ~BDRV_REQ_MAY_UNMAP;
+    if (flags & BDRV_REQ_MAY_UNMAP) {
+        if (!use_16_for_ws && !iscsilun->lbp.lbpws10) {
+            /* WRITESAME10 with UNMAP is unsupported try WRITESAME16 */
+            use_16_for_ws = true;
+        }
+        if (use_16_for_ws && !iscsilun->lbp.lbpws) {
+            /* WRITESAME16 with UNMAP is not supported by the target,
+             * fall back and try WRITESAME10/16 without UNMAP */
+            flags &= ~BDRV_REQ_MAY_UNMAP;
+            use_16_for_ws = iscsilun->use_16_for_rw;
+        }
     }
 
     if (!(flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->has_write_same) {
-        /* WRITE SAME without UNMAP is not supported by the target */
+        /* WRITESAME without UNMAP is not supported by the target */
         return -ENOTSUP;
     }
 
@@ -946,10 +959,18 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
-    if (iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba,
-                               iscsilun->zeroblock, iscsilun->block_size,
-                               nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP),
-                               0, 0, iscsi_co_generic_cb, &iTask) == NULL) {
+    if (use_16_for_ws) {
+        iTask.task = iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba,
+                                            iscsilun->zeroblock, iscsilun->block_size,
+                                            nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP),
+                                            0, 0, iscsi_co_generic_cb, &iTask);
+    } else {
+        iTask.task = iscsi_writesame10_task(iscsilun->iscsi, iscsilun->lun, lba,
+                                            iscsilun->zeroblock, iscsilun->block_size,
+                                            nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP),
+                                            0, 0, iscsi_co_generic_cb, &iTask);
+    }
+    if (iTask.task == NULL) {
         return -ENOMEM;
     }
 
@@ -1147,6 +1168,7 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
                     iscsilun->num_blocks = rc16->returned_lba + 1;
                     iscsilun->lbpme = rc16->lbpme;
                     iscsilun->lbprz = rc16->lbprz;
+                    iscsilun->use_16_for_rw = (rc16->returned_lba > 0xffffffff);
                 }
             }
             break;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 04/15] scsi-disk.c: Fix compilation with -DDEBUG_SCSI
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2014-06-18 16:03 ` [Qemu-devel] [PULL 03/15] block/iscsi: use 16 byte CDBs only when necessary Paolo Bonzini
@ 2014-06-18 16:03 ` Paolo Bonzini
  2014-06-18 16:03 ` [Qemu-devel] [PULL 05/15] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c Paolo Bonzini
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Janzen

From: Paul Janzen <pcj@pauljanzen.org>

In scsi-disk.c, if you #define DEBUG_SCSI=1, you get:
hw/scsi/scsi-disk.c: In function 'scsi_disk_emulate_command':
hw/scsi/scsi-disk.c:2018: error: 'SCSIRequest' has no member named 'buf'

Change the debugging statement to match the actual value tested.

Signed-off-by: Paul Janzen <pcj@pauljanzen.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index fc6e32a..9afbb8a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2015,7 +2015,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     case VERIFY_10:
     case VERIFY_12:
     case VERIFY_16:
-        DPRINTF("Verify (bytchk %lu)\n", (r->req.buf[1] >> 1) & 3);
+        DPRINTF("Verify (bytchk %d)\n", (req->cmd.buf[1] >> 1) & 3);
         if (req->cmd.buf[1] & 6) {
             goto illegal_request;
         }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 05/15] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2014-06-18 16:03 ` [Qemu-devel] [PULL 04/15] scsi-disk.c: Fix compilation with -DDEBUG_SCSI Paolo Bonzini
@ 2014-06-18 16:03 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 06/15] scsi: Print command name in debug Paolo Bonzini
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ulrich Obergfell, qemu-stable

From: Ulrich Obergfell <uobergfe@redhat.com>

This patch fixes a bug in scsi_block_new_request() that was introduced
by commit 137745c5c60f083ec982fe9e861e8c16ebca1ba8. If the host cache
is used - i.e. if BDRV_O_NOCACHE is _not_ set - the 'break' statement
needs to be executed to 'fall back' to SG_IO.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 9afbb8a..fd82a41 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2526,7 +2526,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
 	 * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
 	 * O_DIRECT everything must go through SG_IO.
          */
-        if (bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE) {
+        if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) {
             break;
         }
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 06/15] scsi: Print command name in debug
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2014-06-18 16:03 ` [Qemu-devel] [PULL 05/15] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 07/15] megasas: use PCI DMA API Paolo Bonzini
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This makes scsi_command_name() public.

This makes use of scsi_command_name() in debug output for scsi-disk and
spapr-vscsi host bus adapter. Before this, SCSI used to print hex numbers
instead of human-friendly strings.

This adds GET_EVENT_STATUS_NOTIFICATION and READ_DISC_INFORMATION to
the list of SCSI commands supported by scsi_command_name().

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c    | 4 +++-
 hw/scsi/scsi-disk.c   | 3 ++-
 hw/scsi/spapr_vscsi.c | 5 +++--
 include/block/scsi.h  | 2 ++
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 003d284..ea1ac09 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1429,7 +1429,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
     }
 }
 
-static const char *scsi_command_name(uint8_t cmd)
+const char *scsi_command_name(uint8_t cmd)
 {
     static const char *names[] = {
         [ TEST_UNIT_READY          ] = "TEST_UNIT_READY",
@@ -1545,6 +1545,8 @@ static const char *scsi_command_name(uint8_t cmd)
         [ SET_READ_AHEAD           ] = "SET_READ_AHEAD",
         [ ALLOW_OVERWRITE          ] = "ALLOW_OVERWRITE",
         [ MECHANISM_STATUS         ] = "MECHANISM_STATUS",
+        [ GET_EVENT_STATUS_NOTIFICATION ] = "GET_EVENT_STATUS_NOTIFICATION",
+        [ READ_DISC_INFORMATION    ] = "READ_DISC_INFORMATION",
     };
 
     if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index fd82a41..a529ad2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2027,7 +2027,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
                 (long)r->req.cmd.xfer);
         break;
     default:
-        DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
+        DPRINTF("Unknown SCSI command (%2.2x=%s)\n", buf[0],
+                scsi_command_name(buf[0]));
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return 0;
     }
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index f96b7af..048cfc7 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -799,8 +799,9 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req);
     n = scsi_req_enqueue(req->sreq);
 
-    DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x LUN %d ret: %d\n",
-            req->qtag, srp->cmd.cdb[0], lun, n);
+    DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x=%s LUN %d ret: %d\n",
+            req->qtag, srp->cmd.cdb[0], scsi_command_name(srp->cmd.cdb[0]),
+            lun, n);
 
     if (n) {
         /* Transfer direction must be set before preprocessing the
diff --git a/include/block/scsi.h b/include/block/scsi.h
index 9ab045b..edde960 100644
--- a/include/block/scsi.h
+++ b/include/block/scsi.h
@@ -143,6 +143,8 @@
 #define READ_CD               0xbe
 #define SEND_DVD_STRUCTURE    0xbf
 
+const char *scsi_command_name(uint8_t cmd);
+
 /*
  * SERVICE ACTION IN subcodes
  */
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 07/15] megasas: use PCI DMA API
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 06/15] scsi: Print command name in debug Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 08/15] util: add return value to qemu_iovec_concat_iov Paolo Bonzini
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

MegaSAS emulation is not IOMMU-friendly.  Fix this by switching to
pci_dma_* functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/megasas.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index b05c47a..c68a873 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -294,6 +294,7 @@ static void megasas_unmap_sgl(MegasasCmd *cmd)
 static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr,
     uint8_t sense_len)
 {
+    PCIDevice *pcid = PCI_DEVICE(cmd->state);
     uint32_t pa_hi = 0, pa_lo;
     hwaddr pa;
 
@@ -306,7 +307,7 @@ static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr,
             pa_hi = le32_to_cpu(cmd->frame->pass.sense_addr_hi);
         }
         pa = ((uint64_t) pa_hi << 32) | pa_lo;
-        cpu_physical_memory_write(pa, sense_ptr, sense_len);
+        pci_dma_write(pcid, pa, sense_ptr, sense_len);
         cmd->frame->header.sense_len = sense_len;
     }
     return sense_len;
@@ -472,6 +473,7 @@ static MegasasCmd *megasas_next_frame(MegasasState *s,
 static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
     hwaddr frame, uint64_t context, int count)
 {
+    PCIDevice *pcid = PCI_DEVICE(s);
     MegasasCmd *cmd = NULL;
     int frame_size = MFI_FRAME_SIZE * 16;
     hwaddr frame_size_p = frame_size;
@@ -484,11 +486,11 @@ static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
     if (!cmd->pa) {
         cmd->pa = frame;
         /* Map all possible frames */
-        cmd->frame = cpu_physical_memory_map(frame, &frame_size_p, 0);
+        cmd->frame = pci_dma_map(pcid, frame, &frame_size_p, 0);
         if (frame_size_p != frame_size) {
             trace_megasas_qf_map_failed(cmd->index, (unsigned long)frame);
             if (cmd->frame) {
-                cpu_physical_memory_unmap(cmd->frame, frame_size_p, 0, 0);
+                pci_dma_unmap(pcid, cmd->frame, frame_size_p, 0, 0);
                 cmd->frame = NULL;
                 cmd->pa = 0;
             }
@@ -561,13 +563,14 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context)
 
 static void megasas_reset_frames(MegasasState *s)
 {
+    PCIDevice *pcid = PCI_DEVICE(s);
     int i;
     MegasasCmd *cmd;
 
     for (i = 0; i < s->fw_cmds; i++) {
         cmd = &s->frames[i];
         if (cmd->pa) {
-            cpu_physical_memory_unmap(cmd->frame, cmd->pa_size, 0, 0);
+            pci_dma_unmap(pcid, cmd->frame, cmd->pa_size, 0, 0);
             cmd->frame = NULL;
             cmd->pa = 0;
         }
@@ -584,6 +587,7 @@ static void megasas_abort_command(MegasasCmd *cmd)
 
 static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd)
 {
+    PCIDevice *pcid = PCI_DEVICE(s);
     uint32_t pa_hi, pa_lo;
     hwaddr iq_pa, initq_size;
     struct mfi_init_qinfo *initq;
@@ -595,7 +599,7 @@ static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd)
     iq_pa = (((uint64_t) pa_hi << 32) | pa_lo);
     trace_megasas_init_firmware((uint64_t)iq_pa);
     initq_size = sizeof(*initq);
-    initq = cpu_physical_memory_map(iq_pa, &initq_size, 0);
+    initq = pci_dma_map(pcid, iq_pa, &initq_size, 0);
     if (!initq || initq_size != sizeof(*initq)) {
         trace_megasas_initq_map_failed(cmd->index);
         s->event_count++;
@@ -631,7 +635,7 @@ static int megasas_init_firmware(MegasasState *s, MegasasCmd *cmd)
     s->fw_state = MFI_FWSTATE_OPERATIONAL;
 out:
     if (initq) {
-        cpu_physical_memory_unmap(initq, initq_size, 0, 0);
+        pci_dma_unmap(pcid, initq, initq_size, 0, 0);
     }
     return ret;
 }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 08/15] util: add return value to qemu_iovec_concat_iov
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 07/15] megasas: use PCI DMA API Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 09/15] virtio-scsi: start preparing for any_layout Paolo Bonzini
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

This will be necessary later to recognize the case where a
request has both dataout and datain.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu-common.h |  6 +++---
 util/iov.c            | 10 ++++++----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 66ceceb..ae76197 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -315,9 +315,9 @@ void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
 void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
 void qemu_iovec_concat(QEMUIOVector *dst,
                        QEMUIOVector *src, size_t soffset, size_t sbytes);
-void qemu_iovec_concat_iov(QEMUIOVector *dst,
-                           struct iovec *src_iov, unsigned int src_cnt,
-                           size_t soffset, size_t sbytes);
+size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
+                             struct iovec *src_iov, unsigned int src_cnt,
+                             size_t soffset, size_t sbytes);
 bool qemu_iovec_is_zero(QEMUIOVector *qiov);
 void qemu_iovec_destroy(QEMUIOVector *qiov);
 void qemu_iovec_reset(QEMUIOVector *qiov);
diff --git a/util/iov.c b/util/iov.c
index 49f8838..2b4f46d 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -295,15 +295,15 @@ void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len)
  * of src".
  * Only vector pointers are processed, not the actual data buffers.
  */
-void qemu_iovec_concat_iov(QEMUIOVector *dst,
-                           struct iovec *src_iov, unsigned int src_cnt,
-                           size_t soffset, size_t sbytes)
+size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
+                             struct iovec *src_iov, unsigned int src_cnt,
+                             size_t soffset, size_t sbytes)
 {
     int i;
     size_t done;
 
     if (!sbytes) {
-        return;
+        return 0;
     }
     assert(dst->nalloc != -1);
     for (i = 0, done = 0; done < sbytes && i < src_cnt; i++) {
@@ -317,6 +317,8 @@ void qemu_iovec_concat_iov(QEMUIOVector *dst,
         }
     }
     assert(soffset == 0); /* offset beyond end of src */
+
+    return done;
 }
 
 /*
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 09/15] virtio-scsi: start preparing for any_layout
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 08/15] util: add return value to qemu_iovec_concat_iov Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 10/15] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Paolo Bonzini
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

- Introduce virtio_scsi_init_req and virtio_scsi_free_req

- rename qemu_sgl_init_external to qemu_sgl_concat

- move virtio_scsi_parse_req from virtio_scsi_pop_req to callers
  and add header length checks to virtio_scsi_parse_req.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 121 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 49 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b39880a..f013e35 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -15,6 +15,7 @@
 
 #include "hw/virtio/virtio-scsi.h"
 #include "qemu/error-report.h"
+#include "qemu/iov.h"
 #include <hw/scsi/scsi.h>
 #include <block/scsi.h>
 #include <hw/virtio/virtio-bus.h>
@@ -56,18 +57,35 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
     return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
 }
 
+static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
+{
+    VirtIOSCSIReq *req;
+    req = g_malloc(sizeof(*req));
+
+    req->vq = vq;
+    req->dev = s;
+    req->sreq = NULL;
+    qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
+    return req;
+}
+
+static void virtio_scsi_free_req(VirtIOSCSIReq *req)
+{
+    qemu_sglist_destroy(&req->qsgl);
+    g_free(req);
+}
+
 static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
 {
     VirtIOSCSI *s = req->dev;
     VirtQueue *vq = req->vq;
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
     virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
-    qemu_sglist_destroy(&req->qsgl);
     if (req->sreq) {
         req->sreq->hba_private = NULL;
         scsi_req_unref(req->sreq);
     }
-    g_free(req);
+    virtio_scsi_free_req(req);
     virtio_notify(vdev, vq);
 }
 
@@ -77,50 +95,55 @@ static void virtio_scsi_bad_req(void)
     exit(1);
 }
 
-static void qemu_sgl_init_external(VirtIOSCSIReq *req, struct iovec *sg,
+static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg,
                                    hwaddr *addr, int num)
 {
     QEMUSGList *qsgl = &req->qsgl;
 
-    qemu_sglist_init(qsgl, DEVICE(req->dev), num, &address_space_memory);
     while (num--) {
         qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
     }
 }
 
-static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
-                                  VirtIOSCSIReq *req)
+static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
+                                 unsigned req_size, unsigned resp_size)
 {
-    assert(req->elem.in_num);
-    req->vq = vq;
-    req->dev = s;
-    req->sreq = NULL;
+    if (req->elem.in_num == 0) {
+        return -EINVAL;
+    }
+
+    if (req->elem.out_sg[0].iov_len < req_size) {
+        return -EINVAL;
+    }
     if (req->elem.out_num) {
         req->req.buf = req->elem.out_sg[0].iov_base;
     }
+
+    if (req->elem.in_sg[0].iov_len < resp_size) {
+        return -EINVAL;
+    }
     req->resp.buf = req->elem.in_sg[0].iov_base;
 
     if (req->elem.out_num > 1) {
-        qemu_sgl_init_external(req, &req->elem.out_sg[1],
-                               &req->elem.out_addr[1],
-                               req->elem.out_num - 1);
+        qemu_sgl_concat(req, &req->elem.out_sg[1],
+                        &req->elem.out_addr[1],
+                        req->elem.out_num - 1);
     } else {
-        qemu_sgl_init_external(req, &req->elem.in_sg[1],
-                               &req->elem.in_addr[1],
-                               req->elem.in_num - 1);
+        qemu_sgl_concat(req, &req->elem.in_sg[1],
+                        &req->elem.in_addr[1],
+                        req->elem.in_num - 1);
     }
+
+    return 0;
 }
 
 static VirtIOSCSIReq *virtio_scsi_pop_req(VirtIOSCSI *s, VirtQueue *vq)
 {
-    VirtIOSCSIReq *req;
-    req = g_malloc(sizeof(*req));
+    VirtIOSCSIReq *req = virtio_scsi_init_req(s, vq);
     if (!virtqueue_pop(vq, &req->elem)) {
-        g_free(req);
+        virtio_scsi_free_req(req);
         return NULL;
     }
-
-    virtio_scsi_parse_req(s, vq, req);
     return req;
 }
 
@@ -143,9 +166,9 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
     VirtIOSCSIReq *req;
     uint32_t n;
 
-    req = g_malloc(sizeof(*req));
     qemu_get_be32s(f, &n);
     assert(n < vs->conf.num_queues);
+    req = virtio_scsi_init_req(s, vs->cmd_vqs[n]);
     qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem));
     /* TODO: add a way for SCSIBusInfo's load_request to fail,
      * and fail migration instead of asserting here.
@@ -156,7 +179,12 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
 #endif
     assert(req->elem.in_num <= ARRAY_SIZE(req->elem.in_sg));
     assert(req->elem.out_num <= ARRAY_SIZE(req->elem.out_sg));
-    virtio_scsi_parse_req(s, vs->cmd_vqs[n], req);
+
+    if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
+                              sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) {
+        error_report("invalid SCSI request migration data");
+        exit(1);
+    }
 
     scsi_req_ref(sreq);
     req->sreq = sreq;
@@ -281,29 +309,29 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     VirtIOSCSIReq *req;
 
     while ((req = virtio_scsi_pop_req(s, vq))) {
-        int out_size, in_size;
-        if (req->elem.out_num < 1 || req->elem.in_num < 1) {
+        int type;
+
+        if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
+                       &type, sizeof(type)) < sizeof(type)) {
             virtio_scsi_bad_req();
-            continue;
-        }
 
-        out_size = req->elem.out_sg[0].iov_len;
-        in_size = req->elem.in_sg[0].iov_len;
-        if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
-            if (out_size < sizeof(VirtIOSCSICtrlTMFReq) ||
-                in_size < sizeof(VirtIOSCSICtrlTMFResp)) {
+        } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+            if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
+                                      sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
                 virtio_scsi_bad_req();
+            } else {
+                virtio_scsi_do_tmf(s, req);
             }
-            virtio_scsi_do_tmf(s, req);
 
         } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY ||
                    req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
-            if (out_size < sizeof(VirtIOSCSICtrlANReq) ||
-                in_size < sizeof(VirtIOSCSICtrlANResp)) {
+            if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
+                                      sizeof(VirtIOSCSICtrlANResp)) < 0) {
                 virtio_scsi_bad_req();
+            } else {
+                req->resp.an->event_actual = 0;
+                req->resp.an->response = VIRTIO_SCSI_S_OK;
             }
-            req->resp.an->event_actual = 0;
-            req->resp.an->response = VIRTIO_SCSI_S_OK;
         }
         virtio_scsi_complete_req(req);
     }
@@ -373,23 +401,18 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
 
     while ((req = virtio_scsi_pop_req(s, vq))) {
         SCSIDevice *d;
-        int out_size, in_size;
-        if (req->elem.out_num < 1 || req->elem.in_num < 1) {
-            virtio_scsi_bad_req();
-        }
-
-        out_size = req->elem.out_sg[0].iov_len;
-        in_size = req->elem.in_sg[0].iov_len;
-        if (out_size < sizeof(VirtIOSCSICmdReq) + vs->cdb_size ||
-            in_size < sizeof(VirtIOSCSICmdResp) + vs->sense_size) {
-            virtio_scsi_bad_req();
-        }
-
+        int rc;
         if (req->elem.out_num > 1 && req->elem.in_num > 1) {
             virtio_scsi_fail_cmd_req(req);
             continue;
         }
 
+        rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
+                                   sizeof(VirtIOSCSICmdResp) + vs->sense_size);
+        if (rc < 0) {
+            virtio_scsi_bad_req();
+        }
+
         d = virtio_scsi_device_find(s, req->req.cmd->lun);
         if (!d) {
             req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 10/15] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 09/15] virtio-scsi: start preparing for any_layout Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 11/15] virtio-scsi: add extra argument and return type to qemu_sgl_concat Paolo Bonzini
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index f013e35..ec9a536 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -207,6 +207,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
     /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE".  */
     req->resp.tmf->response = VIRTIO_SCSI_S_OK;
 
+    tswap32s(&req->req.tmf->subtype);
     switch (req->req.tmf->subtype) {
     case VIRTIO_SCSI_T_TMF_ABORT_TASK:
     case VIRTIO_SCSI_T_TMF_QUERY_TASK:
@@ -314,8 +315,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
                        &type, sizeof(type)) < sizeof(type)) {
             virtio_scsi_bad_req();
+            continue;
+        }
 
-        } else if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+        tswap32s(&req->req.tmf->type);
+        if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
             if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
                                       sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
                 virtio_scsi_bad_req();
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 11/15] virtio-scsi: add extra argument and return type to qemu_sgl_concat
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 10/15] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 12/15] virtio-scsi: prepare sense data handling for any_layout Paolo Bonzini
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

Will be used for anylayout support.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ec9a536..0718626 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -95,14 +95,27 @@ static void virtio_scsi_bad_req(void)
     exit(1);
 }
 
-static void qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *sg,
-                                   hwaddr *addr, int num)
+static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
+                              hwaddr *addr, int num, size_t skip)
 {
     QEMUSGList *qsgl = &req->qsgl;
-
-    while (num--) {
-        qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
+    size_t copied = 0;
+
+    while (num) {
+        if (skip >= iov->iov_len) {
+            skip -= iov->iov_len;
+        } else {
+            qemu_sglist_add(qsgl, *addr + skip, iov->iov_len - skip);
+            copied += iov->iov_len - skip;
+            skip = 0;
+        }
+        iov++;
+        addr++;
+        num--;
     }
+
+    assert(skip == 0);
+    return copied;
 }
 
 static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
@@ -127,11 +140,11 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
     if (req->elem.out_num > 1) {
         qemu_sgl_concat(req, &req->elem.out_sg[1],
                         &req->elem.out_addr[1],
-                        req->elem.out_num - 1);
+                        req->elem.out_num - 1, 0);
     } else {
         qemu_sgl_concat(req, &req->elem.in_sg[1],
                         &req->elem.in_addr[1],
-                        req->elem.in_num - 1);
+                        req->elem.in_num - 1, 0);
     }
 
     return 0;
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 12/15] virtio-scsi: prepare sense data handling for any_layout
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 11/15] virtio-scsi: add extra argument and return type to qemu_sgl_concat Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 13/15] virtio-scsi: introduce virtio_scsi_complete_cmd_req Paolo Bonzini
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

Retrieve sense and copy it to guest memory, to prepare for when we will use
qemu_iovec_from_buf.

Swap response and request, since we'll use the tail of VirtIOSCSIReq
for the CDB.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0718626..fbc7db7 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -26,12 +26,7 @@ typedef struct VirtIOSCSIReq {
     VirtQueueElement elem;
     QEMUSGList qsgl;
     SCSIRequest *sreq;
-    union {
-        char                  *buf;
-        VirtIOSCSICmdReq      *cmd;
-        VirtIOSCSICtrlTMFReq  *tmf;
-        VirtIOSCSICtrlANReq   *an;
-    } req;
+    size_t resp_size;
     union {
         char                  *buf;
         VirtIOSCSICmdResp     *cmd;
@@ -39,6 +34,12 @@ typedef struct VirtIOSCSIReq {
         VirtIOSCSICtrlANResp  *an;
         VirtIOSCSIEvent       *event;
     } resp;
+    union {
+        char                  *buf;
+        VirtIOSCSICmdReq      *cmd;
+        VirtIOSCSICtrlTMFReq  *tmf;
+        VirtIOSCSICtrlANReq   *an;
+    } req;
 } VirtIOSCSIReq;
 
 static inline int virtio_scsi_get_lun(uint8_t *lun)
@@ -136,6 +137,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
         return -EINVAL;
     }
     req->resp.buf = req->elem.in_sg[0].iov_base;
+    req->resp_size = resp_size;
 
     if (req->elem.out_num > 1) {
         qemu_sgl_concat(req, &req->elem.out_sg[1],
@@ -358,8 +360,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
                                          size_t resid)
 {
     VirtIOSCSIReq *req = r->hba_private;
-    VirtIOSCSI *s = req->dev;
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    uint8_t sense[SCSI_SENSE_BUF_SIZE];
     uint32_t sense_len;
 
     if (r->io_canceled) {
@@ -372,8 +373,9 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
         req->resp.cmd->resid = tswap32(resid);
     } else {
         req->resp.cmd->resid = 0;
-        sense_len = scsi_req_get_sense(r, req->resp.cmd->sense,
-                                       vs->sense_size);
+        sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
+        sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd));
+        memcpy(req->resp.cmd->sense, sense, sense_len);
         req->resp.cmd->sense_len = tswap32(sense_len);
     }
     virtio_scsi_complete_req(req);
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 13/15] virtio-scsi: introduce virtio_scsi_complete_cmd_req
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 12/15] virtio-scsi: prepare sense data handling for any_layout Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 14/15] virtio-scsi: add support for the any_layout feature Paolo Bonzini
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

This is also related to sense handling, and will be used
by anylayout.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index fbc7db7..06fda89 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -356,6 +356,11 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
+{
+    virtio_scsi_complete_req(req);
+}
+
 static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
                                          size_t resid)
 {
@@ -378,7 +383,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
         memcpy(req->resp.cmd->sense, sense, sense_len);
         req->resp.cmd->sense_len = tswap32(sense_len);
     }
-    virtio_scsi_complete_req(req);
+    virtio_scsi_complete_cmd_req(req);
 }
 
 static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r)
@@ -400,13 +405,13 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r)
     } else {
         req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED;
     }
-    virtio_scsi_complete_req(req);
+    virtio_scsi_complete_cmd_req(req);
 }
 
 static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
 {
     req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
-    virtio_scsi_complete_req(req);
+    virtio_scsi_complete_cmd_req(req);
 }
 
 static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
@@ -435,7 +440,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
         d = virtio_scsi_device_find(s, req->req.cmd->lun);
         if (!d) {
             req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
-            virtio_scsi_complete_req(req);
+            virtio_scsi_complete_cmd_req(req);
             continue;
         }
         req->sreq = scsi_req_new(d, req->req.cmd->tag,
@@ -449,7 +454,7 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
             if (req->sreq->cmd.mode != req_mode ||
                 req->sreq->cmd.xfer > req->qsgl.size) {
                 req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN;
-                virtio_scsi_complete_req(req);
+                virtio_scsi_complete_cmd_req(req);
                 continue;
             }
         }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 14/15] virtio-scsi: add support for the any_layout feature
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 13/15] virtio-scsi: introduce virtio_scsi_complete_cmd_req Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-18 16:04 ` [Qemu-devel] [PULL 15/15] block/iscsi: bump libiscsi requirement to 1.9.0 Paolo Bonzini
  2014-06-19 16:14 ` [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Peter Maydell
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel

Store the request and response headers by value, and let
virtio_scsi_parse_req check that there is only one of datain
and dataout.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c           | 193 ++++++++++++++++++++++------------------
 include/hw/i386/pc.h            |   4 +
 include/hw/virtio/virtio-scsi.h |   4 +-
 3 files changed, 109 insertions(+), 92 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 06fda89..3870c47 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -27,21 +27,27 @@ typedef struct VirtIOSCSIReq {
     QEMUSGList qsgl;
     SCSIRequest *sreq;
     size_t resp_size;
+    enum SCSIXferMode mode;
+    QEMUIOVector resp_iov;
     union {
-        char                  *buf;
-        VirtIOSCSICmdResp     *cmd;
-        VirtIOSCSICtrlTMFResp *tmf;
-        VirtIOSCSICtrlANResp  *an;
-        VirtIOSCSIEvent       *event;
+        VirtIOSCSICmdResp     cmd;
+        VirtIOSCSICtrlTMFResp tmf;
+        VirtIOSCSICtrlANResp  an;
+        VirtIOSCSIEvent       event;
     } resp;
     union {
-        char                  *buf;
-        VirtIOSCSICmdReq      *cmd;
-        VirtIOSCSICtrlTMFReq  *tmf;
-        VirtIOSCSICtrlANReq   *an;
+        struct {
+            VirtIOSCSICmdReq  cmd;
+            uint8_t           cdb[];
+        } QEMU_PACKED;
+        VirtIOSCSICtrlTMFReq  tmf;
+        VirtIOSCSICtrlANReq   an;
     } req;
 } VirtIOSCSIReq;
 
+QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) !=
+                  offsetof(VirtIOSCSIReq, req.cmd) + sizeof(VirtIOSCSICmdReq));
+
 static inline int virtio_scsi_get_lun(uint8_t *lun)
 {
     return ((lun[2] << 8) | lun[3]) & 0x3FFF;
@@ -61,17 +67,21 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
 static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
 {
     VirtIOSCSIReq *req;
-    req = g_malloc(sizeof(*req));
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+
+    req = g_malloc0(sizeof(*req) + vs->cdb_size);
 
     req->vq = vq;
     req->dev = s;
     req->sreq = NULL;
     qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory);
+    qemu_iovec_init(&req->resp_iov, 1);
     return req;
 }
 
 static void virtio_scsi_free_req(VirtIOSCSIReq *req)
 {
+    qemu_iovec_destroy(&req->resp_iov);
     qemu_sglist_destroy(&req->qsgl);
     g_free(req);
 }
@@ -81,7 +91,9 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
     VirtIOSCSI *s = req->dev;
     VirtQueue *vq = req->vq;
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
-    virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
+
+    qemu_iovec_from_buf(&req->resp_iov, 0, &req->resp, req->resp_size);
+    virtqueue_push(vq, &req->elem, req->qsgl.size + req->resp_iov.size);
     if (req->sreq) {
         req->sreq->hba_private = NULL;
         scsi_req_unref(req->sreq);
@@ -122,31 +134,35 @@ static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov,
 static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
                                  unsigned req_size, unsigned resp_size)
 {
-    if (req->elem.in_num == 0) {
-        return -EINVAL;
-    }
+    size_t in_size, out_size;
 
-    if (req->elem.out_sg[0].iov_len < req_size) {
+    if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
+                   &req->req, req_size) < req_size) {
         return -EINVAL;
     }
-    if (req->elem.out_num) {
-        req->req.buf = req->elem.out_sg[0].iov_base;
-    }
 
-    if (req->elem.in_sg[0].iov_len < resp_size) {
+    if (qemu_iovec_concat_iov(&req->resp_iov,
+                              req->elem.in_sg, req->elem.in_num, 0,
+                              resp_size) < resp_size) {
         return -EINVAL;
     }
-    req->resp.buf = req->elem.in_sg[0].iov_base;
     req->resp_size = resp_size;
 
-    if (req->elem.out_num > 1) {
-        qemu_sgl_concat(req, &req->elem.out_sg[1],
-                        &req->elem.out_addr[1],
-                        req->elem.out_num - 1, 0);
-    } else {
-        qemu_sgl_concat(req, &req->elem.in_sg[1],
-                        &req->elem.in_addr[1],
-                        req->elem.in_num - 1, 0);
+    out_size = qemu_sgl_concat(req, req->elem.out_sg,
+                               &req->elem.out_addr[0], req->elem.out_num,
+                               req_size);
+    in_size = qemu_sgl_concat(req, req->elem.in_sg,
+                              &req->elem.in_addr[0], req->elem.in_num,
+                              resp_size);
+
+    if (out_size && in_size) {
+        return -ENOTSUP;
+    }
+
+    if (out_size) {
+        req->mode = SCSI_XFER_TO_DEV;
+    } else if (in_size) {
+        req->mode = SCSI_XFER_FROM_DEV;
     }
 
     return 0;
@@ -204,37 +220,34 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
     scsi_req_ref(sreq);
     req->sreq = sreq;
     if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
-        int req_mode =
-            (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV);
-
-        assert(req->sreq->cmd.mode == req_mode);
+        assert(req->sreq->cmd.mode == req->mode);
     }
     return req;
 }
 
 static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
-    SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf->lun);
+    SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
     SCSIRequest *r, *next;
     BusChild *kid;
     int target;
 
     /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE".  */
-    req->resp.tmf->response = VIRTIO_SCSI_S_OK;
+    req->resp.tmf.response = VIRTIO_SCSI_S_OK;
 
-    tswap32s(&req->req.tmf->subtype);
-    switch (req->req.tmf->subtype) {
+    tswap32s(&req->req.tmf.subtype);
+    switch (req->req.tmf.subtype) {
     case VIRTIO_SCSI_T_TMF_ABORT_TASK:
     case VIRTIO_SCSI_T_TMF_QUERY_TASK:
         if (!d) {
             goto fail;
         }
-        if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+        if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
             goto incorrect_lun;
         }
         QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
             VirtIOSCSIReq *cmd_req = r->hba_private;
-            if (cmd_req && cmd_req->req.cmd->tag == req->req.tmf->tag) {
+            if (cmd_req && cmd_req->req.cmd.tag == req->req.tmf.tag) {
                 break;
             }
         }
@@ -244,11 +257,11 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
              * check for it in the loop above.
              */
             assert(r->hba_private);
-            if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK) {
+            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".
                  */
-                req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
+                req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
             } else {
                 scsi_req_cancel(r);
             }
@@ -259,7 +272,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
         if (!d) {
             goto fail;
         }
-        if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+        if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
             goto incorrect_lun;
         }
         s->resetting++;
@@ -273,16 +286,16 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
         if (!d) {
             goto fail;
         }
-        if (d->lun != virtio_scsi_get_lun(req->req.tmf->lun)) {
+        if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
             goto incorrect_lun;
         }
         QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
             if (r->hba_private) {
-                if (req->req.tmf->subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
+                if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
                     /* "If there is any command present in the task set, then
                      * return a service response set to FUNCTION SUCCEEDED".
                      */
-                    req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
+                    req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
                     break;
                 } else {
                     scsi_req_cancel(r);
@@ -292,7 +305,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
         break;
 
     case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
-        target = req->req.tmf->lun[1];
+        target = req->req.tmf.lun[1];
         s->resetting++;
         QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
              d = DO_UPCAST(SCSIDevice, qdev, kid->child);
@@ -305,18 +318,18 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 
     case VIRTIO_SCSI_T_TMF_CLEAR_ACA:
     default:
-        req->resp.tmf->response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
+        req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_REJECTED;
         break;
     }
 
     return;
 
 incorrect_lun:
-    req->resp.tmf->response = VIRTIO_SCSI_S_INCORRECT_LUN;
+    req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
     return;
 
 fail:
-    req->resp.tmf->response = VIRTIO_SCSI_S_BAD_TARGET;
+    req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
 }
 
 static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
@@ -333,8 +346,8 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             continue;
         }
 
-        tswap32s(&req->req.tmf->type);
-        if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+        tswap32s(&req->req.tmf.type);
+        if (req->req.tmf.type == VIRTIO_SCSI_T_TMF) {
             if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq),
                                       sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
                 virtio_scsi_bad_req();
@@ -342,14 +355,14 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
                 virtio_scsi_do_tmf(s, req);
             }
 
-        } else if (req->req.tmf->type == VIRTIO_SCSI_T_AN_QUERY ||
-                   req->req.tmf->type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
+        } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
+                   req->req.tmf.type == VIRTIO_SCSI_T_AN_SUBSCRIBE) {
             if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq),
                                       sizeof(VirtIOSCSICtrlANResp)) < 0) {
                 virtio_scsi_bad_req();
             } else {
-                req->resp.an->event_actual = 0;
-                req->resp.an->response = VIRTIO_SCSI_S_OK;
+                req->resp.an.event_actual = 0;
+                req->resp.an.response = VIRTIO_SCSI_S_OK;
             }
         }
         virtio_scsi_complete_req(req);
@@ -358,6 +371,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 
 static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
 {
+    /* Sense data is not in req->resp and is copied separately
+     * in virtio_scsi_command_complete.
+     */
+    req->resp_size = sizeof(VirtIOSCSICmdResp);
     virtio_scsi_complete_req(req);
 }
 
@@ -372,16 +389,17 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
         return;
     }
 
-    req->resp.cmd->response = VIRTIO_SCSI_S_OK;
-    req->resp.cmd->status = status;
-    if (req->resp.cmd->status == GOOD) {
-        req->resp.cmd->resid = tswap32(resid);
+    req->resp.cmd.response = VIRTIO_SCSI_S_OK;
+    req->resp.cmd.status = status;
+    if (req->resp.cmd.status == GOOD) {
+        req->resp.cmd.resid = tswap32(resid);
     } else {
-        req->resp.cmd->resid = 0;
+        req->resp.cmd.resid = 0;
         sense_len = scsi_req_get_sense(r, sense, sizeof(sense));
-        sense_len = MIN(sense_len, req->resp_size - sizeof(req->resp.cmd));
-        memcpy(req->resp.cmd->sense, sense, sense_len);
-        req->resp.cmd->sense_len = tswap32(sense_len);
+        sense_len = MIN(sense_len, req->resp_iov.size - sizeof(req->resp.cmd));
+        qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd),
+                            &req->resp, sense_len);
+        req->resp.cmd.sense_len = tswap32(sense_len);
     }
     virtio_scsi_complete_cmd_req(req);
 }
@@ -401,16 +419,16 @@ static void virtio_scsi_request_cancelled(SCSIRequest *r)
         return;
     }
     if (req->dev->resetting) {
-        req->resp.cmd->response = VIRTIO_SCSI_S_RESET;
+        req->resp.cmd.response = VIRTIO_SCSI_S_RESET;
     } else {
-        req->resp.cmd->response = VIRTIO_SCSI_S_ABORTED;
+        req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED;
     }
     virtio_scsi_complete_cmd_req(req);
 }
 
 static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
 {
-    req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
+    req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE;
     virtio_scsi_complete_cmd_req(req);
 }
 
@@ -426,37 +444,34 @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
     while ((req = virtio_scsi_pop_req(s, vq))) {
         SCSIDevice *d;
         int rc;
-        if (req->elem.out_num > 1 && req->elem.in_num > 1) {
-            virtio_scsi_fail_cmd_req(req);
-            continue;
-        }
 
         rc = virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
                                    sizeof(VirtIOSCSICmdResp) + vs->sense_size);
         if (rc < 0) {
-            virtio_scsi_bad_req();
+            if (rc == -ENOTSUP) {
+                virtio_scsi_fail_cmd_req(req);
+            } else {
+                virtio_scsi_bad_req();
+            }
+            continue;
         }
 
-        d = virtio_scsi_device_find(s, req->req.cmd->lun);
+        d = virtio_scsi_device_find(s, req->req.cmd.lun);
         if (!d) {
-            req->resp.cmd->response = VIRTIO_SCSI_S_BAD_TARGET;
+            req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
             virtio_scsi_complete_cmd_req(req);
             continue;
         }
-        req->sreq = scsi_req_new(d, req->req.cmd->tag,
-                                 virtio_scsi_get_lun(req->req.cmd->lun),
-                                 req->req.cmd->cdb, req);
-
-        if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
-            int req_mode =
-                (req->elem.in_num > 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV);
-
-            if (req->sreq->cmd.mode != req_mode ||
-                req->sreq->cmd.xfer > req->qsgl.size) {
-                req->resp.cmd->response = VIRTIO_SCSI_S_OVERRUN;
-                virtio_scsi_complete_cmd_req(req);
-                continue;
-            }
+        req->sreq = scsi_req_new(d, req->req.cmd.tag,
+                                 virtio_scsi_get_lun(req->req.cmd.lun),
+                                 req->req.cdb, req);
+
+        if (req->sreq->cmd.mode != SCSI_XFER_NONE
+            && (req->sreq->cmd.mode != req->mode ||
+                req->sreq->cmd.xfer > req->qsgl.size)) {
+            req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
+            virtio_scsi_complete_cmd_req(req);
+            continue;
         }
 
         n = scsi_req_enqueue(req->sreq);
@@ -560,7 +575,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
         return;
     }
 
-    if (req->elem.out_num || req->elem.in_num != 1) {
+    if (req->elem.out_num) {
         virtio_scsi_bad_req();
     }
 
@@ -569,12 +584,12 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
         s->events_dropped = false;
     }
 
-    in_size = req->elem.in_sg[0].iov_len;
+    in_size = iov_size(req->elem.in_sg, req->elem.in_num);
     if (in_size < sizeof(VirtIOSCSIEvent)) {
         virtio_scsi_bad_req();
     }
 
-    evt = req->resp.event;
+    evt = &req->resp.event;
     memset(evt, 0, sizeof(VirtIOSCSIEvent));
     evt->event = event;
     evt->reason = reason;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fa9d997..ca7a0bd 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -268,6 +268,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_COMPAT_2_0 \
         {\
+            .driver   = "virtio-scsi-pci",\
+            .property = "any_layout",\
+            .value    = "off",\
+        },{\
             .driver   = "apic",\
             .property = "version",\
             .value    = stringify(0x11),\
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 42b1024..de0615b 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -84,14 +84,13 @@
 #define VIRTIO_SCSI_EVT_RESET_RESCAN           1
 #define VIRTIO_SCSI_EVT_RESET_REMOVED          2
 
-/* SCSI command request, followed by data-out */
+/* SCSI command request, followed by CDB and data-out */
 typedef struct {
     uint8_t lun[8];              /* Logical Unit Number */
     uint64_t tag;                /* Command identifier */
     uint8_t task_attr;           /* Task attribute */
     uint8_t prio;
     uint8_t crn;
-    uint8_t cdb[];
 } QEMU_PACKED VirtIOSCSICmdReq;
 
 /* Response, followed by sense data and data-in */
@@ -101,7 +100,6 @@ typedef struct {
     uint16_t status_qualifier;   /* Status qualifier */
     uint8_t status;              /* Command completion status */
     uint8_t response;            /* Response values */
-    uint8_t sense[];
 } QEMU_PACKED VirtIOSCSICmdResp;
 
 /* Task Management Request */
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 15/15] block/iscsi: bump libiscsi requirement to 1.9.0
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (13 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 14/15] virtio-scsi: add support for the any_layout feature Paolo Bonzini
@ 2014-06-18 16:04 ` Paolo Bonzini
  2014-06-19 16:14 ` [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Peter Maydell
  15 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2014-06-18 16:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Lieven

From: Peter Lieven <pl@kamp.de>

This patch lifts the minimum supported libiscsi version from 1.4.0 to
1.9.0 since the BUSY patch required that change.

On one this allows us to remove all #ifdefs from the code which
makes the code easier to maintain and read. On the other hand
I would not recommend libiscsi prior to 1.8.0 for production use
because the following important libiscsi fixes for deadlocks and
protocol errors are missing prior to 1.8.0:

dbe9a1e SOCKET queue cmd PDUs directly in waitpdu queue
30df192 DATA-OUT set pdu->cmdsn appropriately
548bd22 ISCSI fix broken send logic in iscsi_scsi_async_command
14bee10 RECONNECT do not increase CmdSN for immediate PDUs
1f4a66a PDU queue out PDUs in order of itt.
562dd46 PDU avoid incrementing itt to 0xffffffff
cd09c0f PDU use serial32 arithmetic for cmdsn, maxcmdsn and expcmdsn.
89e918e SOCKET validate data_size in in_pdu header
91267f5 Limit immediate and unsolicited data to FirstBurstLength

Note that libiscsi 1.9.0 was released on Feb 24th, 2013, about
one month after 1.8.0.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 67 -----------------------------------------------------------
 configure     | 39 +++-------------------------------
 2 files changed, 3 insertions(+), 103 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3875487..83c7992 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -369,17 +369,6 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
 
     lba = sector_qemu2lun(sector_num, iscsilun);
     num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
-#if !defined(LIBISCSI_FEATURE_IOVECTOR)
-    /* if the iovec only contains one buffer we can pass it directly */
-    if (iov->niov == 1) {
-        data = iov->iov[0].iov_base;
-    } else {
-        size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size);
-        buf = g_malloc(size);
-        qemu_iovec_to_buf(iov, 0, buf, size);
-        data = buf;
-    }
-#endif
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
     if (iscsilun->use_16_for_rw) {
@@ -397,10 +386,8 @@ retry:
         g_free(buf);
         return -ENOMEM;
     }
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
     scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov,
                           iov->niov);
-#endif
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
         qemu_coroutine_yield();
@@ -428,7 +415,6 @@ retry:
 }
 
 
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
 static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun,
                                              int64_t sector_num, int nb_sectors)
 {
@@ -538,9 +524,6 @@ out:
     return ret;
 }
 
-#endif /* LIBISCSI_FEATURE_IOVECTOR */
-
-
 static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
                                        int64_t sector_num, int nb_sectors,
                                        QEMUIOVector *iov)
@@ -549,15 +532,11 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
     struct IscsiTask iTask;
     uint64_t lba;
     uint32_t num_sectors;
-#if !defined(LIBISCSI_FEATURE_IOVECTOR)
-    int i;
-#endif
 
     if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
         return -EINVAL;
     }
 
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
     if (iscsilun->lbprz && nb_sectors >= ISCSI_CHECKALLOC_THRES &&
         !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) {
         int64_t ret;
@@ -571,7 +550,6 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
             return 0;
         }
     }
-#endif
 
     lba = sector_qemu2lun(sector_num, iscsilun);
     num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
@@ -587,23 +565,13 @@ retry:
         iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba,
                                        num_sectors * iscsilun->block_size,
                                        iscsilun->block_size,
-#if !defined(CONFIG_LIBISCSI_1_4) /* API change from 1.4.0 to 1.5.0 */
                                        0, 0, 0, 0, 0,
-#endif
                                        iscsi_co_generic_cb, &iTask);
     }
     if (iTask.task == NULL) {
         return -ENOMEM;
     }
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
     scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *) iov->iov, iov->niov);
-#else
-    for (i = 0; i < iov->niov; i++) {
-        scsi_task_add_data_in_buffer(iTask.task,
-                                     iov->iov[i].iov_len,
-                                     iov->iov[i].iov_base);
-    }
-#endif
 
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
@@ -758,18 +726,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
             data.data = acb->ioh->dxferp;
             data.size = acb->ioh->dxfer_len;
         } else {
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
             scsi_task_set_iov_out(acb->task,
                                  (struct scsi_iovec *) acb->ioh->dxferp,
                                  acb->ioh->iovec_count);
-#else
-            struct iovec *iov = (struct iovec *)acb->ioh->dxferp;
-
-            acb->buf = g_malloc(acb->ioh->dxfer_len);
-            data.data = acb->buf;
-            data.size = iov_to_buf(iov, acb->ioh->iovec_count, 0,
-                                   acb->buf, acb->ioh->dxfer_len);
-#endif
         }
     }
 
@@ -789,20 +748,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
                                          acb->ioh->dxfer_len,
                                          acb->ioh->dxferp);
         } else {
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
             scsi_task_set_iov_in(acb->task,
                                  (struct scsi_iovec *) acb->ioh->dxferp,
                                  acb->ioh->iovec_count);
-#else
-            int i;
-            for (i = 0; i < acb->ioh->iovec_count; i++) {
-                struct iovec *iov = (struct iovec *)acb->ioh->dxferp;
-
-                scsi_task_add_data_in_buffer(acb->task,
-                    iov[i].iov_len,
-                    iov[i].iov_base);
-            }
-#endif
         }
     }
 
@@ -811,7 +759,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     return &acb->common;
 }
 
-
 static void ioctl_cb(void *opaque, int status)
 {
     int *p_status = opaque;
@@ -916,8 +863,6 @@ retry:
     return 0;
 }
 
-#if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED)
-
 static int
 coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
                                    int nb_sectors, BdrvRequestFlags flags)
@@ -1012,8 +957,6 @@ retry:
     return 0;
 }
 
-#endif /* SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED */
-
 static void parse_chap(struct iscsi_context *iscsi, const char *target,
                        Error **errp)
 {
@@ -1123,7 +1066,6 @@ static char *parse_initiator_name(const char *target)
     return iscsi_name;
 }
 
-#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
 static void iscsi_nop_timed_event(void *opaque)
 {
     IscsiLun *iscsilun = opaque;
@@ -1141,7 +1083,6 @@ static void iscsi_nop_timed_event(void *opaque)
     timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
     iscsi_set_events(iscsilun);
 }
-#endif
 
 static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
 {
@@ -1280,14 +1221,12 @@ static void iscsi_attach_aio_context(BlockDriverState *bs,
     iscsilun->aio_context = new_context;
     iscsi_set_events(iscsilun);
 
-#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
     /* Set up a timer for sending out iSCSI NOPs */
     iscsilun->nop_timer = aio_timer_new(iscsilun->aio_context,
                                         QEMU_CLOCK_REALTIME, SCALE_MS,
                                         iscsi_nop_timed_event, iscsilun);
     timer_mod(iscsilun->nop_timer,
               qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
-#endif
 }
 
 /*
@@ -1479,13 +1418,11 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) {
         iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran *
                                      iscsilun->block_size) >> BDRV_SECTOR_BITS;
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
         if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) {
             iscsilun->allocationmap =
                 bitmap_new(DIV_ROUND_UP(bs->total_sectors,
                                         iscsilun->cluster_sectors));
         }
-#endif
     }
 
 out:
@@ -1670,13 +1607,9 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_truncate   = iscsi_truncate,
     .bdrv_refresh_limits = iscsi_refresh_limits,
 
-#if defined(LIBISCSI_FEATURE_IOVECTOR)
     .bdrv_co_get_block_status = iscsi_co_get_block_status,
-#endif
     .bdrv_co_discard      = iscsi_co_discard,
-#if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED)
     .bdrv_co_write_zeroes = iscsi_co_write_zeroes,
-#endif
     .bdrv_co_readv         = iscsi_co_readv,
     .bdrv_co_writev        = iscsi_co_writev,
     .bdrv_co_flush_to_disk = iscsi_co_flush,
diff --git a/configure b/configure
index 27d84d9..a69e90b 100755
--- a/configure
+++ b/configure
@@ -3405,46 +3405,20 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
-# Do we have libiscsi
-# We check for iscsi_write16_sync() to make sure we have a
-# at least version 1.4.0 of libiscsi.
+# Do we have libiscsi >= 1.9.0
 if test "$libiscsi" != "no" ; then
-  cat > $TMPC << EOF
-#include <stdio.h>
-#include <iscsi/iscsi.h>
-int main(void) { iscsi_write16_sync(NULL,0,0,NULL,0,0,0,0,0,0,0); return 0; }
-EOF
-  if $pkg_config --atleast-version=1.7.0 libiscsi; then
+  if $pkg_config --atleast-version=1.9.0 libiscsi; then
     libiscsi="yes"
     libiscsi_cflags=$($pkg_config --cflags libiscsi)
     libiscsi_libs=$($pkg_config --libs libiscsi)
-  elif compile_prog "" "-liscsi" ; then
-    libiscsi="yes"
-    libiscsi_libs="-liscsi"
   else
     if test "$libiscsi" = "yes" ; then
-      feature_not_found "libiscsi" "Install libiscsi devel"
+      feature_not_found "libiscsi" "Install libiscsi >= 1.9.0"
     fi
     libiscsi="no"
   fi
 fi
 
-# We also need to know the API version because there was an
-# API change from 1.4.0 to 1.5.0.
-if test "$libiscsi" = "yes"; then
-  cat >$TMPC <<EOF
-#include <iscsi/iscsi.h>
-int main(void)
-{
-  iscsi_read10_task(0, 0, 0, 0, 0, 0, 0);
-  return 0;
-}
-EOF
-  if compile_prog "" "-liscsi"; then
-    libiscsi_version="1.4.0"
-  fi
-fi
-
 ##########################################
 # Do we need libm
 cat > $TMPC << EOF
@@ -4218,11 +4192,7 @@ echo "nss used          $smartcard_nss"
 echo "libusb            $libusb"
 echo "usb net redir     $usb_redir"
 echo "GLX support       $glx"
-if test "$libiscsi_version" = "1.4.0"; then
-echo "libiscsi support  $libiscsi (1.4.0)"
-else
 echo "libiscsi support  $libiscsi"
-fi
 echo "libnfs support    $libnfs"
 echo "build guest agent $guest_agent"
 echo "QGA VSS support   $guest_agent_with_vss"
@@ -4579,9 +4549,6 @@ fi
 
 if test "$libiscsi" = "yes" ; then
   echo "CONFIG_LIBISCSI=m" >> $config_host_mak
-  if test "$libiscsi_version" = "1.4.0"; then
-    echo "CONFIG_LIBISCSI_1_4=y" >> $config_host_mak
-  fi
   echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
   echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak
 fi
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18
  2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
                   ` (14 preceding siblings ...)
  2014-06-18 16:04 ` [Qemu-devel] [PULL 15/15] block/iscsi: bump libiscsi requirement to 1.9.0 Paolo Bonzini
@ 2014-06-19 16:14 ` Peter Maydell
  15 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2014-06-19 16:14 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On 18 June 2014 17:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit af44da87e926ff64260b95f4350d338c4fc113ca:
>
>   Merge remote-tracking branch 'remotes/agraf/tags/signed-ppc-for-upstream' into staging (2014-06-16 18:26:21 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git scsi-next
>
> for you to fetch changes up to e49ab19fcaa617ad6cdfe1ac401327326b6a2552:
>
>   block/iscsi: bump libiscsi requirement to 1.9.0 (2014-06-18 18:03:33 +0200)

Applied, thanks.

-- PMM

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

* Re: [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition
  2014-06-18 16:03 ` [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition Paolo Bonzini
@ 2014-06-30  9:39   ` Alexey Kardashevskiy
  0 siblings, 0 replies; 18+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-30  9:39 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: Peter Lieven

On 06/19/2014 02:03 AM, Paolo Bonzini wrote:
> From: Peter Lieven <pl@kamp.de>
> 
> this patch adds handling of BUSY status reponse from an iSCSI target.
> Currently, we fail with -EIO in case of SCSI_STATUS_BUSY while the
> obvious reaction would be to retry the operation after some time.
> The retry time is randomly choosen from a range with exponential
> growth increasing with each retry.
> 
> This patch includes most of the changes by a an upcoming patch
> from Stefan Hajnoczi:
> 
>  iscsi: implement .bdrv_detach/attach_aio_context()
> 
> because I also need the reference to the aio_context for
> the retry timer to work. I included the changes to maintain
> better mergeability.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/iscsi.c | 55 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 6f87605..e64659f 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -26,6 +26,7 @@
>  #include "config-host.h"
>  
>  #include <poll.h>
> +#include <math.h>
>  #include <arpa/inet.h>
>  #include "qemu-common.h"
>  #include "qemu/config-file.h"
> @@ -75,6 +76,7 @@ typedef struct IscsiTask {
>      Coroutine *co;
>      QEMUBH *bh;
>      IscsiLun *iscsilun;
> +    QEMUTimer retry_timer;
>  } IscsiTask;
>  
>  typedef struct IscsiAIOCB {
> @@ -86,7 +88,6 @@ typedef struct IscsiAIOCB {
>      uint8_t *buf;
>      int status;
>      int canceled;
> -    int retries;
>      int64_t sector_num;
>      int nb_sectors;
>  #ifdef __linux__
> @@ -96,7 +97,8 @@ typedef struct IscsiAIOCB {
>  
>  #define NOP_INTERVAL 5000
>  #define MAX_NOP_FAILURES 3
> -#define ISCSI_CMD_RETRIES 5
> +#define ISCSI_CMD_RETRIES ARRAY_SIZE(iscsi_retry_times)
> +static const unsigned iscsi_retry_times[] = {8, 32, 128, 512, 2048};
>  
>  /* this threshhold is a trade-off knob to choose between
>   * the potential additional overhead of an extra GET_LBA_STATUS request
> @@ -146,6 +148,19 @@ static void iscsi_co_generic_bh_cb(void *opaque)
>      qemu_coroutine_enter(iTask->co, NULL);
>  }
>  
> +static void iscsi_retry_timer_expired(void *opaque)
> +{
> +    struct IscsiTask *iTask = opaque;
> +    if (iTask->co) {
> +        qemu_coroutine_enter(iTask->co, NULL);
> +    }
> +}
> +
> +static inline unsigned exp_random(double mean)
> +{
> +    return -mean * log((double)rand() / RAND_MAX);


This new use of log() breaks linkage on Fedora20/ppc64, qemu-nbd fails (but
qemu-system-ppc64 still links find). Adding "--extra-cflags=-lm" to
./configure invocation helps.

How to fix? Seems like fc20 packages do not have some dependency which
qemu-nbd relies on. Thanks.


[aik@vpl2 qemu]$ make  V=1
cc -DHAS_LIBSSH2_SFTP_FSYNC -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
-Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-fstack-protector-strong -I/usr/include/p11-kit-1  -I/usr/include/p11-kit-1
   -I/usr/include/libpng16  -I/usr/include/libusb-1.0
-I/usr/include/pixman-1   -I/home/aik/p/qemu/tests -pthread
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -g
-Wl,--warn-common -m64 -g  -o qemu-nbd qemu-nbd.o async.o thread-pool.o
nbd.o block.o blockjob.o main-loop.o iohandler.o qemu-timer.o aio-posix.o
qapi-types.o qapi-visit.o qemu-io-cmds.o qemu-coroutine.o
qemu-coroutine-lock.o qemu-coroutine-io.o qemu-coroutine-sleep.o
coroutine-ucontext.o block/raw_bsd.o block/cow.o block/qcow.o block/vdi.o
block/vmdk.o block/cloop.o block/dmg.o block/bochs.o block/vpc.o
block/vvfat.o block/qcow2.o block/qcow2-refcount.o block/qcow2-cluster.o
block/qcow2-snapshot.o block/qcow2-cache.o block/qed.o block/qed-gencb.o
block/qed-l2-cache.o block/qed-table.o block/qed-cluster.o
block/qed-check.o block/vhdx.o block/vhdx-endian.o block/vhdx-log.o
block/quorum.o block/parallels.o block/blkdebug.o block/blkverify.o
block/snapshot.o block/qapi.o block/raw-posix.o block/linux-aio.o
block/nbd.o block/nbd-client.o block/sheepdog.o block/iscsi.o block/curl.o
block/rbd.o block/gluster.o block/ssh.o libqemuutil.a libqemustub.a   -lz
-laio -L/usr/lib64/iscsi -liscsi -lcurl -lrbd -lrados -lgfapi -lglusterfs
-lgfrpc -lgfxdr -lssh2 -lgthread-2.0 -pthread -lglib-2.0   -lz -lrt -lz
-luuid -lgnutls  -lgnutls  -lgnutls  -lSDL -lpthread  -lX11 -lvte2_90
-lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject
-lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lcairo -lX11 -lXext
-lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject
-lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0  -lX11
-lrdmacm -libverbs
/usr/bin/ld: block/iscsi.o: undefined reference to symbol 'log@@GLIBC_2.3'
/usr/bin/ld: note: 'log@@GLIBC_2.3' is defined in DSO /lib64/libm.so.6 so
try adding it to the linker command line
/lib64/libm.so.6: could not read symbols: Invalid operation
collect2: error: ld returned 1 exit status
make: *** [qemu-nbd] Error 1




> +}
> +
>  static void
>  iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
>                          void *command_data, void *opaque)
> @@ -158,14 +173,30 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
>      iTask->do_retry = 0;
>      iTask->task = task;
>  
> -    if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION
> -        && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
> -        error_report("iSCSI CheckCondition: %s", iscsi_get_error(iscsi));
> -        iTask->do_retry = 1;
> -        goto out;
> -    }
> -
>      if (status != SCSI_STATUS_GOOD) {
> +        if (iTask->retries++ < ISCSI_CMD_RETRIES) {
> +            if (status == SCSI_STATUS_CHECK_CONDITION
> +                && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
> +                error_report("iSCSI CheckCondition: %s",
> +                             iscsi_get_error(iscsi));
> +                iTask->do_retry = 1;
> +                goto out;
> +            }
> +            if (status == SCSI_STATUS_BUSY) {
> +                unsigned retry_time =
> +                    exp_random(iscsi_retry_times[iTask->retries - 1]);
> +                error_report("iSCSI Busy (retry #%u in %u ms): %s",
> +                             iTask->retries, retry_time,
> +                             iscsi_get_error(iscsi));
> +                aio_timer_init(iTask->iscsilun->aio_context,
> +                               &iTask->retry_timer, QEMU_CLOCK_REALTIME,
> +                               SCALE_MS, iscsi_retry_timer_expired, iTask);
> +                timer_mod(&iTask->retry_timer,
> +                          qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + retry_time);
> +                iTask->do_retry = 1;
> +                return;
> +            }
> +        }
>          error_report("iSCSI Failure: %s", iscsi_get_error(iscsi));
>      }
>  
> @@ -180,9 +211,9 @@ out:
>  static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
>  {
>      *iTask = (struct IscsiTask) {
> -        .co             = qemu_coroutine_self(),
> -        .retries        = ISCSI_CMD_RETRIES,
> -        .iscsilun       = iscsilun,
> +        .co         = qemu_coroutine_self(),
> +        .retries    = ISCSI_CMD_RETRIES,
> +        .iscsilun   = iscsilun,
>      };
>  }
>  
> 


-- 
Alexey

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

end of thread, other threads:[~2014-06-30  9:39 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-18 16:03 [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Paolo Bonzini
2014-06-18 16:03 ` [Qemu-devel] [PULL 01/15] block/iscsi: handle BUSY condition Paolo Bonzini
2014-06-30  9:39   ` Alexey Kardashevskiy
2014-06-18 16:03 ` [Qemu-devel] [PULL 02/15] block/iscsi: fix potential segfault on early callback Paolo Bonzini
2014-06-18 16:03 ` [Qemu-devel] [PULL 03/15] block/iscsi: use 16 byte CDBs only when necessary Paolo Bonzini
2014-06-18 16:03 ` [Qemu-devel] [PULL 04/15] scsi-disk.c: Fix compilation with -DDEBUG_SCSI Paolo Bonzini
2014-06-18 16:03 ` [Qemu-devel] [PULL 05/15] scsi-disk: fix bug in scsi_block_new_request() introduced by commit 137745c Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 06/15] scsi: Print command name in debug Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 07/15] megasas: use PCI DMA API Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 08/15] util: add return value to qemu_iovec_concat_iov Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 09/15] virtio-scsi: start preparing for any_layout Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 10/15] virtio-scsi: add target swap for VirtIOSCSICtrlTMFReq fields Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 11/15] virtio-scsi: add extra argument and return type to qemu_sgl_concat Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 12/15] virtio-scsi: prepare sense data handling for any_layout Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 13/15] virtio-scsi: introduce virtio_scsi_complete_cmd_req Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 14/15] virtio-scsi: add support for the any_layout feature Paolo Bonzini
2014-06-18 16:04 ` [Qemu-devel] [PULL 15/15] block/iscsi: bump libiscsi requirement to 1.9.0 Paolo Bonzini
2014-06-19 16:14 ` [Qemu-devel] [PULL 00/15] SCSI changes for 2014-06-18 Peter Maydell

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.