[Qemu-devel] [PATCH v4 0/9] virtio-blk: Converge VirtIOBlockRequest into VirtIOBlockReq

[Qemu-devel] [PATCH v4 0/9] virtio-blk: Converge VirtIOBlockRequest into VirtIOBlockReq

[Fam Zheng]

This unifies the request structure used by dataplane and non-dataplane code,
while dropping unnessary fields for bounce buffer and read flag.

v4: Address Stefan's comments as below, and add Paolo's rev-by lines to others.

    [01/09] virtio-blk: Move VirtIOBlockReq to header

            Include "block/block.h".

    [02/09] virtio-blk: Convert VirtIOBlockReq.elem to pointer

            Use g_slice_new0 for req as well.

    [06/09] virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr

            Check guest input, don't assert.

    [09/09] virtio-blk: Fix and clean up the in_sg and out_sg check

            Use error_report + exit.

Fam Zheng (9):
  virtio-blk: Move VirtIOBlockReq to header
  virtio-blk: Convert VirtIOBlockReq.elem to pointer
  virtio-blk: Drop bounce buffer from dataplane code
  virtio-blk: Drop VirtIOBlockRequest.read
  virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq
  virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr
  virtio-blk: Convert VirtIOBlockReq.out to structrue
  virtio-blk: Fill in VirtIOBlockReq.out in dataplane code
  virtio-blk: Fix and clean up the in_sg and out_sg check

 hw/block/dataplane/virtio-blk.c | 169 ++++++++++++++--------------------------
 hw/block/virtio-blk.c           | 117 +++++++++++++++-------------
 include/hw/virtio/virtio-blk.h  |  11 +++
 3 files changed, 132 insertions(+), 165 deletions(-)

-- 
2.0.0

[Qemu-devel] [PATCH v4 1/9] virtio-blk: Move VirtIOBlockReq to header

[Fam Zheng]

For later reusing by dataplane code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/virtio-blk.c          | 11 -----------
 include/hw/virtio/virtio-blk.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 85aa871..a5bae47 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -27,17 +27,6 @@
 #endif
 #include "hw/virtio/virtio-bus.h"
 
-typedef struct VirtIOBlockReq
-{
-    VirtIOBlock *dev;
-    VirtQueueElement elem;
-    struct virtio_blk_inhdr *in;
-    struct virtio_blk_outhdr *out;
-    QEMUIOVector qiov;
-    struct VirtIOBlockReq *next;
-    BlockAcctCookie acct;
-} VirtIOBlockReq;
-
 static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 4bc9b54..d05d177 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -17,6 +17,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/block/block.h"
 #include "sysemu/iothread.h"
+#include "block/block.h"
 
 #define TYPE_VIRTIO_BLK "virtio-blk-device"
 #define VIRTIO_BLK(obj) \
@@ -133,6 +134,16 @@ typedef struct VirtIOBlock {
 #endif
 } VirtIOBlock;
 
+typedef struct VirtIOBlockReq {
+    VirtIOBlock *dev;
+    VirtQueueElement elem;
+    struct virtio_blk_inhdr *in;
+    struct virtio_blk_outhdr *out;
+    QEMUIOVector qiov;
+    struct VirtIOBlockReq *next;
+    BlockAcctCookie acct;
+} VirtIOBlockReq;
+
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
 
-- 
2.0.0

[Qemu-devel] [PATCH v4 2/9] virtio-blk: Convert VirtIOBlockReq.elem to pointer

[Fam Zheng]

This will make converging with dataplane code easier.

Add virtio_blk_free_request to handle the freeing of request internal
fields.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/virtio-blk.c          | 85 +++++++++++++++++++++++-------------------
 include/hw/virtio/virtio-blk.h |  2 +-
 2 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index a5bae47..26285ff 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -27,6 +27,22 @@
 #endif
 #include "hw/virtio/virtio-bus.h"
 
+static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
+{
+    VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
+    req->dev = s;
+    req->elem = g_slice_new0(VirtQueueElement);
+    return req;
+}
+
+static void virtio_blk_free_request(VirtIOBlockReq *req)
+{
+    if (req) {
+        g_slice_free(VirtQueueElement, req->elem);
+        g_slice_free(VirtIOBlockReq, req);
+    }
+}
+
 static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
@@ -35,7 +51,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
     trace_virtio_blk_req_complete(req, status);
 
     stb_p(&req->in->status, status);
-    virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
+    virtqueue_push(s->vq, req->elem, req->qiov.size + sizeof(*req->in));
     virtio_notify(vdev, s->vq);
 }
 
@@ -51,7 +67,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     } else if (action == BDRV_ACTION_REPORT) {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
         bdrv_acct_done(s->bs, &req->acct);
-        g_free(req);
+        virtio_blk_free_request(req);
     }
 
     bdrv_error_action(s->bs, action, is_read, error);
@@ -72,7 +88,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 
     virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
     bdrv_acct_done(req->dev->bs, &req->acct);
-    g_free(req);
+    virtio_blk_free_request(req);
 }
 
 static void virtio_blk_flush_complete(void *opaque, int ret)
@@ -87,27 +103,16 @@ static void virtio_blk_flush_complete(void *opaque, int ret)
 
     virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
     bdrv_acct_done(req->dev->bs, &req->acct);
-    g_free(req);
-}
-
-static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
-{
-    VirtIOBlockReq *req = g_malloc(sizeof(*req));
-    req->dev = s;
-    req->qiov.size = 0;
-    req->next = NULL;
-    return req;
+    virtio_blk_free_request(req);
 }
 
 static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
 {
     VirtIOBlockReq *req = virtio_blk_alloc_request(s);
 
-    if (req != NULL) {
-        if (!virtqueue_pop(s->vq, &req->elem)) {
-            g_free(req);
-            return NULL;
-        }
+    if (!virtqueue_pop(s->vq, req->elem)) {
+        virtio_blk_free_request(req);
+        return NULL;
     }
 
     return req;
@@ -236,9 +241,9 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 {
     int status;
 
-    status = virtio_blk_handle_scsi_req(req->dev, &req->elem);
+    status = virtio_blk_handle_scsi_req(req->dev, req->elem);
     virtio_blk_req_complete(req, status);
-    g_free(req);
+    virtio_blk_free_request(req);
 }
 
 typedef struct MultiReqBuffer {
@@ -340,19 +345,19 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
 {
     uint32_t type;
 
-    if (req->elem.out_num < 1 || req->elem.in_num < 1) {
+    if (req->elem->out_num < 1 || req->elem->in_num < 1) {
         error_report("virtio-blk missing headers");
         exit(1);
     }
 
-    if (req->elem.out_sg[0].iov_len < sizeof(*req->out) ||
-        req->elem.in_sg[req->elem.in_num - 1].iov_len < sizeof(*req->in)) {
+    if (req->elem->out_sg[0].iov_len < sizeof(*req->out) ||
+        req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) {
         error_report("virtio-blk header not in correct element");
         exit(1);
     }
 
-    req->out = (void *)req->elem.out_sg[0].iov_base;
-    req->in = (void *)req->elem.in_sg[req->elem.in_num - 1].iov_base;
+    req->out = (void *)req->elem->out_sg[0].iov_base;
+    req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base;
 
     type = ldl_p(&req->out->type);
 
@@ -367,23 +372,23 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
          * NB: per existing s/n string convention the string is
          * terminated by '\0' only when shorter than buffer.
          */
-        strncpy(req->elem.in_sg[0].iov_base,
+        strncpy(req->elem->in_sg[0].iov_base,
                 s->blk.serial ? s->blk.serial : "",
-                MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
+                MIN(req->elem->in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
-        g_free(req);
+        virtio_blk_free_request(req);
     } else if (type & VIRTIO_BLK_T_OUT) {
-        qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
-                                 req->elem.out_num - 1);
+        qemu_iovec_init_external(&req->qiov, &req->elem->out_sg[1],
+                                 req->elem->out_num - 1);
         virtio_blk_handle_write(req, mrb);
     } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
         /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
-        qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
-                                 req->elem.in_num - 1);
+        qemu_iovec_init_external(&req->qiov, &req->elem->in_sg[0],
+                                 req->elem->in_num - 1);
         virtio_blk_handle_read(req);
     } else {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
-        g_free(req);
+        virtio_blk_free_request(req);
     }
 }
 
@@ -598,7 +603,8 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
     
     while (req) {
         qemu_put_sbyte(f, 1);
-        qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
+        qemu_put_buffer(f, (unsigned char *)req->elem,
+                        sizeof(VirtQueueElement));
         req = req->next;
     }
     qemu_put_sbyte(f, 0);
@@ -620,14 +626,15 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 
     while (qemu_get_sbyte(f)) {
         VirtIOBlockReq *req = virtio_blk_alloc_request(s);
-        qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
+        qemu_get_buffer(f, (unsigned char *)req->elem,
+                        sizeof(VirtQueueElement));
         req->next = s->rq;
         s->rq = req;
 
-        virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr,
-            req->elem.in_num, 1);
-        virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr,
-            req->elem.out_num, 0);
+        virtqueue_map_sg(req->elem->in_sg, req->elem->in_addr,
+            req->elem->in_num, 1);
+        virtqueue_map_sg(req->elem->out_sg, req->elem->out_addr,
+            req->elem->out_num, 0);
     }
 
     return 0;
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index d05d177..b495e42 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -136,7 +136,7 @@ typedef struct VirtIOBlock {
 
 typedef struct VirtIOBlockReq {
     VirtIOBlock *dev;
-    VirtQueueElement elem;
+    VirtQueueElement *elem;
     struct virtio_blk_inhdr *in;
     struct virtio_blk_outhdr *out;
     QEMUIOVector qiov;
-- 
2.0.0

[Qemu-devel] [PATCH v4 3/9] virtio-blk: Drop bounce buffer from dataplane code

[Fam Zheng]

The block layer will handle the unaligned request.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index c10b7b7..3d1e9e1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -29,8 +29,6 @@ typedef struct {
     QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
     VirtQueueElement *elem;         /* saved data from the virtqueue */
     QEMUIOVector qiov;              /* original request iovecs */
-    struct iovec bounce_iov;        /* used if guest buffers are unaligned */
-    QEMUIOVector bounce_qiov;       /* bounce buffer iovecs */
     bool read;                      /* read or write? */
 } VirtIOBlockRequest;
 
@@ -85,14 +83,6 @@ static void complete_rdwr(void *opaque, int ret)
 
     trace_virtio_blk_data_plane_complete_request(req->s, req->elem->index, ret);
 
-    if (req->read && req->bounce_iov.iov_base) {
-        qemu_iovec_from_buf(&req->qiov, 0, req->bounce_iov.iov_base, len);
-    }
-
-    if (req->bounce_iov.iov_base) {
-        qemu_vfree(req->bounce_iov.iov_base);
-    }
-
     qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
     qemu_iovec_destroy(req->inhdr);
     g_slice_free(QEMUIOVector, req->inhdr);
@@ -152,21 +142,6 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
 
     qiov = &req->qiov;
 
-    if (!bdrv_qiov_is_aligned(s->blk->conf.bs, qiov)) {
-        void *bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov->size);
-
-        /* Populate bounce buffer with data for writes */
-        if (!read) {
-            qemu_iovec_to_buf(qiov, 0, bounce_buffer, qiov->size);
-        }
-
-        /* Redirect I/O to aligned bounce buffer */
-        req->bounce_iov.iov_base = bounce_buffer;
-        req->bounce_iov.iov_len = qiov->size;
-        qemu_iovec_init_external(&req->bounce_qiov, &req->bounce_iov, 1);
-        qiov = &req->bounce_qiov;
-    }
-
     nb_sectors = qiov->size / BDRV_SECTOR_SIZE;
 
     if (read) {
-- 
2.0.0

[Qemu-devel] [PATCH v4 4/9] virtio-blk: Drop VirtIOBlockRequest.read

[Fam Zheng]

Since it's set but not used.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 3d1e9e1..4e5e458 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -29,7 +29,6 @@ typedef struct {
     QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
     VirtQueueElement *elem;         /* saved data from the virtqueue */
     QEMUIOVector qiov;              /* original request iovecs */
-    bool read;                      /* read or write? */
 } VirtIOBlockRequest;
 
 struct VirtIOBlockDataPlane {
@@ -137,7 +136,6 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
     req->s = s;
     req->elem = elem;
     req->inhdr = inhdr;
-    req->read = read;
     qemu_iovec_init_external(&req->qiov, iov, iov_cnt);
 
     qiov = &req->qiov;
-- 
2.0.0

[Qemu-devel] [PATCH v4 5/9] virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq

[Fam Zheng]

Field "inhdr" is added temporarily for a more mechanical change, and
will be dropped in the next commit.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 34 +++++++++++++++-------------------
 include/hw/virtio/virtio-blk.h  |  4 ++++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 4e5e458..70e8c14 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -24,13 +24,6 @@
 #include "hw/virtio/virtio-bus.h"
 #include "qom/object_interfaces.h"
 
-typedef struct {
-    VirtIOBlockDataPlane *s;
-    QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
-    VirtQueueElement *elem;         /* saved data from the virtqueue */
-    QEMUIOVector qiov;              /* original request iovecs */
-} VirtIOBlockRequest;
-
 struct VirtIOBlockDataPlane {
     bool started;
     bool starting;
@@ -68,7 +61,7 @@ static void notify_guest(VirtIOBlockDataPlane *s)
 
 static void complete_rdwr(void *opaque, int ret)
 {
-    VirtIOBlockRequest *req = opaque;
+    VirtIOBlockReq *req = opaque;
     struct virtio_blk_inhdr hdr;
     int len;
 
@@ -80,7 +73,8 @@ static void complete_rdwr(void *opaque, int ret)
         len = 0;
     }
 
-    trace_virtio_blk_data_plane_complete_request(req->s, req->elem->index, ret);
+    trace_virtio_blk_data_plane_complete_request(req->dev->dataplane,
+                                                 req->elem->index, ret);
 
     qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
     qemu_iovec_destroy(req->inhdr);
@@ -90,9 +84,9 @@ static void complete_rdwr(void *opaque, int ret)
      * written to, but for virtio-blk it seems to be the number of bytes
      * transferred plus the status bytes.
      */
-    vring_push(&req->s->vring, req->elem, len + sizeof(hdr));
-    notify_guest(req->s);
-    g_slice_free(VirtIOBlockRequest, req);
+    vring_push(&req->dev->dataplane->vring, req->elem, len + sizeof(hdr));
+    notify_guest(req->dev->dataplane);
+    g_slice_free(VirtIOBlockReq, req);
 }
 
 static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
@@ -128,14 +122,15 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
                         int64_t sector_num, VirtQueueElement *elem,
                         QEMUIOVector *inhdr)
 {
-    VirtIOBlockRequest *req = g_slice_new0(VirtIOBlockRequest);
+    VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
+    VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
     QEMUIOVector *qiov;
     int nb_sectors;
 
     /* Fill in virtio block metadata needed for completion */
-    req->s = s;
     req->elem = elem;
     req->inhdr = inhdr;
+    req->dev = dev;
     qemu_iovec_init_external(&req->qiov, iov, iov_cnt);
 
     qiov = &req->qiov;
@@ -153,7 +148,7 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
 
 static void complete_flush(void *opaque, int ret)
 {
-    VirtIOBlockRequest *req = opaque;
+    VirtIOBlockReq *req = opaque;
     unsigned char status;
 
     if (ret == 0) {
@@ -162,15 +157,16 @@ static void complete_flush(void *opaque, int ret)
         status = VIRTIO_BLK_S_IOERR;
     }
 
-    complete_request_early(req->s, req->elem, req->inhdr, status);
-    g_slice_free(VirtIOBlockRequest, req);
+    complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status);
+    g_slice_free(VirtIOBlockReq, req);
 }
 
 static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
                          QEMUIOVector *inhdr)
 {
-    VirtIOBlockRequest *req = g_slice_new(VirtIOBlockRequest);
-    req->s = s;
+    VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
+    VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
+    req->dev = dev;
     req->elem = elem;
     req->inhdr = inhdr;
 
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index b495e42..4211cd6 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -142,6 +142,10 @@ typedef struct VirtIOBlockReq {
     QEMUIOVector qiov;
     struct VirtIOBlockReq *next;
     BlockAcctCookie acct;
+
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
+#endif
 } VirtIOBlockReq;
 
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
-- 
2.0.0

[Qemu-devel] [PATCH v4 6/9] virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr

[Fam Zheng]

In current virtio spec, inhdr is a single byte, and is unlikely to
change for both functionality and compatibility considerations.
Non-dataplane uses .in, and we are on the way to converge them. So
let's unify it to get cleaner code.

Remove .inhdr and use .in.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 54 +++++++++++++++++------------------------
 include/hw/virtio/virtio-blk.h  |  4 ---
 2 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 70e8c14..cef707f 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -76,9 +76,7 @@ static void complete_rdwr(void *opaque, int ret)
     trace_virtio_blk_data_plane_complete_request(req->dev->dataplane,
                                                  req->elem->index, ret);
 
-    qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
-    qemu_iovec_destroy(req->inhdr);
-    g_slice_free(QEMUIOVector, req->inhdr);
+    stb_p(&req->in->status, hdr.status);
 
     /* According to the virtio specification len should be the number of bytes
      * written to, but for virtio-blk it seems to be the number of bytes
@@ -90,24 +88,20 @@ static void complete_rdwr(void *opaque, int ret)
 }
 
 static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                                   QEMUIOVector *inhdr, unsigned char status)
+                                   struct virtio_blk_inhdr *inhdr,
+                                   unsigned char status)
 {
-    struct virtio_blk_inhdr hdr = {
-        .status = status,
-    };
+    stb_p(&inhdr->status, status);
 
-    qemu_iovec_from_buf(inhdr, 0, &hdr, sizeof(hdr));
-    qemu_iovec_destroy(inhdr);
-    g_slice_free(QEMUIOVector, inhdr);
-
-    vring_push(&s->vring, elem, sizeof(hdr));
+    vring_push(&s->vring, elem, sizeof(*inhdr));
     notify_guest(s);
 }
 
 /* Get disk serial number */
 static void do_get_id_cmd(VirtIOBlockDataPlane *s,
                           struct iovec *iov, unsigned int iov_cnt,
-                          VirtQueueElement *elem, QEMUIOVector *inhdr)
+                          VirtQueueElement *elem,
+                          struct virtio_blk_inhdr *inhdr)
 {
     char id[VIRTIO_BLK_ID_BYTES];
 
@@ -120,7 +114,7 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s,
 static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
                         struct iovec *iov, unsigned iov_cnt,
                         int64_t sector_num, VirtQueueElement *elem,
-                        QEMUIOVector *inhdr)
+                        struct virtio_blk_inhdr *inhdr)
 {
     VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
     VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
@@ -129,8 +123,8 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
 
     /* Fill in virtio block metadata needed for completion */
     req->elem = elem;
-    req->inhdr = inhdr;
     req->dev = dev;
+    req->in = inhdr;
     qemu_iovec_init_external(&req->qiov, iov, iov_cnt);
 
     qiov = &req->qiov;
@@ -157,24 +151,24 @@ static void complete_flush(void *opaque, int ret)
         status = VIRTIO_BLK_S_IOERR;
     }
 
-    complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status);
+    complete_request_early(req->dev->dataplane, req->elem, req->in, status);
     g_slice_free(VirtIOBlockReq, req);
 }
 
 static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                         QEMUIOVector *inhdr)
+                         struct virtio_blk_inhdr *inhdr)
 {
     VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
     VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
     req->dev = dev;
     req->elem = elem;
-    req->inhdr = inhdr;
+    req->in = inhdr;
 
     bdrv_aio_flush(s->blk->conf.bs, complete_flush, req);
 }
 
 static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                        QEMUIOVector *inhdr)
+                        struct virtio_blk_inhdr *inhdr)
 {
     int status;
 
@@ -189,8 +183,7 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
     unsigned out_num = elem->out_num;
     unsigned in_num = elem->in_num;
     struct virtio_blk_outhdr outhdr;
-    QEMUIOVector *inhdr;
-    size_t in_size;
+    struct virtio_blk_inhdr *inhdr;
 
     /* Copy in outhdr */
     if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr,
@@ -200,17 +193,16 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
     }
     iov_discard_front(&iov, &out_num, sizeof(outhdr));
 
-    /* Grab inhdr for later */
-    in_size = iov_size(in_iov, in_num);
-    if (in_size < sizeof(struct virtio_blk_inhdr)) {
-        error_report("virtio_blk request inhdr too short");
+    /* We are likely safe with the iov_len check, because inhdr is only 1 byte,
+     * but checking here in case the header gets bigger in the future. */
+    if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*inhdr)) {
+        error_report("virtio-blk request inhdr too short");
         return -EFAULT;
     }
-    inhdr = g_slice_new(QEMUIOVector);
-    qemu_iovec_init(inhdr, 1);
-    qemu_iovec_concat_iov(inhdr, in_iov, in_num,
-            in_size - sizeof(struct virtio_blk_inhdr),
-            sizeof(struct virtio_blk_inhdr));
+
+    /* Grab inhdr for later */
+    inhdr = (void *)in_iov[in_num - 1].iov_base
+            + in_iov[in_num - 1].iov_len - sizeof(*inhdr);
     iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));
 
     /* TODO Linux sets the barrier bit even when not advertised! */
@@ -243,8 +235,6 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
 
     default:
         error_report("virtio-blk unsupported request type %#x", outhdr.type);
-        qemu_iovec_destroy(inhdr);
-        g_slice_free(QEMUIOVector, inhdr);
         return -EFAULT;
     }
 }
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 4211cd6..b495e42 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -142,10 +142,6 @@ typedef struct VirtIOBlockReq {
     QEMUIOVector qiov;
     struct VirtIOBlockReq *next;
     BlockAcctCookie acct;
-
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
-#endif
 } VirtIOBlockReq;
 
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
-- 
2.0.0

[Qemu-devel] [PATCH v4 7/9] virtio-blk: Convert VirtIOBlockReq.out to structrue

[Fam Zheng]

The virtio code currently assumes that the outhdr is in its own iovec.
This is not guaranteed by the spec, so we should relax this assumption.

Convert the VirtIOBlockReq.out field to structrue so that we can use
iov_to_buf and then discard the header from the beginning of iovec.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/virtio-blk.c          | 20 ++++++++++++++------
 include/hw/virtio/virtio-blk.h |  2 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 26285ff..09b6b60 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu-common.h"
+#include "qemu/iov.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 #include "hw/block/block.h"
@@ -81,7 +82,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
     trace_virtio_blk_rw_complete(req, ret);
 
     if (ret) {
-        bool is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT);
+        bool is_read = !(ldl_p(&req->out.type) & VIRTIO_BLK_T_OUT);
         if (virtio_blk_handle_rw_error(req, -ret, is_read))
             return;
     }
@@ -287,7 +288,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
     BlockRequest *blkreq;
     uint64_t sector;
 
-    sector = ldq_p(&req->out->sector);
+    sector = ldq_p(&req->out.sector);
 
     bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE);
 
@@ -321,7 +322,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
 {
     uint64_t sector;
 
-    sector = ldq_p(&req->out->sector);
+    sector = ldq_p(&req->out.sector);
 
     bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ);
 
@@ -344,22 +345,29 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
     MultiReqBuffer *mrb)
 {
     uint32_t type;
+    struct iovec *iov = req->elem->out_sg;
+    unsigned out_num = req->elem->out_num;
 
     if (req->elem->out_num < 1 || req->elem->in_num < 1) {
         error_report("virtio-blk missing headers");
         exit(1);
     }
 
-    if (req->elem->out_sg[0].iov_len < sizeof(*req->out) ||
+    if (req->elem->out_sg[0].iov_len < sizeof(req->out) ||
         req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) {
         error_report("virtio-blk header not in correct element");
         exit(1);
     }
 
-    req->out = (void *)req->elem->out_sg[0].iov_base;
+    if (unlikely(iov_to_buf(iov, out_num, 0, &req->out,
+                            sizeof(req->out)) != sizeof(req->out))) {
+        error_report("virtio-blk request outhdr too short");
+        exit(1);
+    }
+    iov_discard_front(&iov, &out_num, sizeof(req->out));
     req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base;
 
-    type = ldl_p(&req->out->type);
+    type = ldl_p(&req->out.type);
 
     if (type & VIRTIO_BLK_T_FLUSH) {
         virtio_blk_handle_flush(req, mrb);
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index b495e42..2571e96 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -138,7 +138,7 @@ typedef struct VirtIOBlockReq {
     VirtIOBlock *dev;
     VirtQueueElement *elem;
     struct virtio_blk_inhdr *in;
-    struct virtio_blk_outhdr *out;
+    struct virtio_blk_outhdr out;
     QEMUIOVector qiov;
     struct VirtIOBlockReq *next;
     BlockAcctCookie acct;
-- 
2.0.0

[Qemu-devel] [PATCH v4 8/9] virtio-blk: Fill in VirtIOBlockReq.out in dataplane code

[Fam Zheng]

VirtIOBlockReq is allocated in process_request, and freed in command
functions.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 102 +++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 58 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index cef707f..29e7ec3 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -87,55 +87,49 @@ static void complete_rdwr(void *opaque, int ret)
     g_slice_free(VirtIOBlockReq, req);
 }
 
-static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                                   struct virtio_blk_inhdr *inhdr,
-                                   unsigned char status)
+static void complete_request_early(VirtIOBlockReq *req, unsigned char status)
 {
-    stb_p(&inhdr->status, status);
+    stb_p(&req->in->status, status);
 
-    vring_push(&s->vring, elem, sizeof(*inhdr));
-    notify_guest(s);
+    vring_push(&req->dev->dataplane->vring, req->elem, sizeof(*req->in));
+    notify_guest(req->dev->dataplane);
+    g_slice_free(VirtIOBlockReq, req);
 }
 
 /* Get disk serial number */
-static void do_get_id_cmd(VirtIOBlockDataPlane *s,
-                          struct iovec *iov, unsigned int iov_cnt,
-                          VirtQueueElement *elem,
-                          struct virtio_blk_inhdr *inhdr)
+static void do_get_id_cmd(VirtIOBlockReq *req,
+                          struct iovec *iov, unsigned int iov_cnt)
 {
     char id[VIRTIO_BLK_ID_BYTES];
 
     /* Serial number not NUL-terminated when longer than buffer */
-    strncpy(id, s->blk->serial ? s->blk->serial : "", sizeof(id));
+    strncpy(id, req->dev->blk.serial ? req->dev->blk.serial : "", sizeof(id));
     iov_from_buf(iov, iov_cnt, 0, id, sizeof(id));
-    complete_request_early(s, elem, inhdr, VIRTIO_BLK_S_OK);
+    complete_request_early(req, VIRTIO_BLK_S_OK);
 }
 
-static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
-                        struct iovec *iov, unsigned iov_cnt,
-                        int64_t sector_num, VirtQueueElement *elem,
-                        struct virtio_blk_inhdr *inhdr)
+
+static void do_rdwr_cmd(bool read, VirtIOBlockReq *req,
+                        struct iovec *iov, unsigned iov_cnt)
 {
-    VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
-    VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
     QEMUIOVector *qiov;
     int nb_sectors;
+    int64_t sector_num;
 
-    /* Fill in virtio block metadata needed for completion */
-    req->elem = elem;
-    req->dev = dev;
-    req->in = inhdr;
     qemu_iovec_init_external(&req->qiov, iov, iov_cnt);
 
     qiov = &req->qiov;
 
+    sector_num = req->out.sector * 512 / BDRV_SECTOR_SIZE;
     nb_sectors = qiov->size / BDRV_SECTOR_SIZE;
 
     if (read) {
-        bdrv_aio_readv(s->blk->conf.bs, sector_num, qiov, nb_sectors,
+        bdrv_aio_readv(req->dev->blk.conf.bs,
+                       sector_num, qiov, nb_sectors,
                        complete_rdwr, req);
     } else {
-        bdrv_aio_writev(s->blk->conf.bs, sector_num, qiov, nb_sectors,
+        bdrv_aio_writev(req->dev->blk.conf.bs,
+                        sector_num, qiov, nb_sectors,
                         complete_rdwr, req);
     }
 }
@@ -151,29 +145,21 @@ static void complete_flush(void *opaque, int ret)
         status = VIRTIO_BLK_S_IOERR;
     }
 
-    complete_request_early(req->dev->dataplane, req->elem, req->in, status);
-    g_slice_free(VirtIOBlockReq, req);
+    complete_request_early(req, status);
 }
 
-static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                         struct virtio_blk_inhdr *inhdr)
+static void do_flush_cmd(VirtIOBlockReq *req)
 {
-    VirtIOBlock *dev = VIRTIO_BLK(s->vdev);
-    VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq);
-    req->dev = dev;
-    req->elem = elem;
-    req->in = inhdr;
 
-    bdrv_aio_flush(s->blk->conf.bs, complete_flush, req);
+    bdrv_aio_flush(req->dev->blk.conf.bs, complete_flush, req);
 }
 
-static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem,
-                        struct virtio_blk_inhdr *inhdr)
+static void do_scsi_cmd(VirtIOBlockReq *req)
 {
     int status;
 
-    status = virtio_blk_handle_scsi_req(VIRTIO_BLK(s->vdev), elem);
-    complete_request_early(s, elem, inhdr, status);
+    status = virtio_blk_handle_scsi_req(req->dev, req->elem);
+    complete_request_early(req, status);
 }
 
 static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
@@ -182,59 +168,59 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem)
     struct iovec *in_iov = elem->in_sg;
     unsigned out_num = elem->out_num;
     unsigned in_num = elem->in_num;
-    struct virtio_blk_outhdr outhdr;
-    struct virtio_blk_inhdr *inhdr;
+    VirtIOBlockReq *req;
 
+    req = g_slice_new(VirtIOBlockReq);
+    req->dev = VIRTIO_BLK(s->vdev);
+    req->elem = elem;
     /* Copy in outhdr */
-    if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr,
-                            sizeof(outhdr)) != sizeof(outhdr))) {
+    if (unlikely(iov_to_buf(iov, out_num, 0, &req->out,
+                            sizeof(req->out)) != sizeof(req->out))) {
         error_report("virtio-blk request outhdr too short");
+        g_slice_free(VirtIOBlockReq, req);
         return -EFAULT;
     }
-    iov_discard_front(&iov, &out_num, sizeof(outhdr));
+    iov_discard_front(&iov, &out_num, sizeof(req->out));
 
     /* We are likely safe with the iov_len check, because inhdr is only 1 byte,
      * but checking here in case the header gets bigger in the future. */
-    if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*inhdr)) {
+    if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*req->in)) {
         error_report("virtio-blk request inhdr too short");
         return -EFAULT;
     }
 
     /* Grab inhdr for later */
-    inhdr = (void *)in_iov[in_num - 1].iov_base
-            + in_iov[in_num - 1].iov_len - sizeof(*inhdr);
+    req->in = (void *)in_iov[in_num - 1].iov_base
+            + in_iov[in_num - 1].iov_len - sizeof(*req->in);
     iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));
 
     /* TODO Linux sets the barrier bit even when not advertised! */
-    outhdr.type &= ~VIRTIO_BLK_T_BARRIER;
+    req->out.type &= ~VIRTIO_BLK_T_BARRIER;
 
-    switch (outhdr.type) {
+    switch (req->out.type) {
     case VIRTIO_BLK_T_IN:
-        do_rdwr_cmd(s, true, in_iov, in_num,
-                    outhdr.sector * 512 / BDRV_SECTOR_SIZE,
-                    elem, inhdr);
+        do_rdwr_cmd(true, req, in_iov, in_num);
         return 0;
 
     case VIRTIO_BLK_T_OUT:
-        do_rdwr_cmd(s, false, iov, out_num,
-                    outhdr.sector * 512 / BDRV_SECTOR_SIZE,
-                    elem, inhdr);
+        do_rdwr_cmd(false, req, iov, out_num);
         return 0;
 
     case VIRTIO_BLK_T_SCSI_CMD:
-        do_scsi_cmd(s, elem, inhdr);
+        do_scsi_cmd(req);
         return 0;
 
     case VIRTIO_BLK_T_FLUSH:
-        do_flush_cmd(s, elem, inhdr);
+        do_flush_cmd(req);
         return 0;
 
     case VIRTIO_BLK_T_GET_ID:
-        do_get_id_cmd(s, in_iov, in_num, elem, inhdr);
+        do_get_id_cmd(req, in_iov, in_num);
         return 0;
 
     default:
-        error_report("virtio-blk unsupported request type %#x", outhdr.type);
+        error_report("virtio-blk unsupported request type %#x", req->out.type);
+        g_slice_free(VirtIOBlockReq, req);
         return -EFAULT;
     }
 }
-- 
2.0.0

[Qemu-devel] [PATCH v4 9/9] virtio-blk: Fix and clean up the in_sg and out_sg check

[Fam Zheng]

out_sg is checked by iov_to_buf below, so it can be dropped.

Add assert and iov_discard_back around in_sg, as the in_sg is handled in
dataplane code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/virtio-blk.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09b6b60..b9d0d0a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -345,7 +345,9 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
     MultiReqBuffer *mrb)
 {
     uint32_t type;
+    struct iovec *in_iov = req->elem->in_sg;
     struct iovec *iov = req->elem->out_sg;
+    unsigned in_num = req->elem->in_num;
     unsigned out_num = req->elem->out_num;
 
     if (req->elem->out_num < 1 || req->elem->in_num < 1) {
@@ -353,19 +355,24 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
         exit(1);
     }
 
-    if (req->elem->out_sg[0].iov_len < sizeof(req->out) ||
-        req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) {
-        error_report("virtio-blk header not in correct element");
-        exit(1);
-    }
-
     if (unlikely(iov_to_buf(iov, out_num, 0, &req->out,
                             sizeof(req->out)) != sizeof(req->out))) {
         error_report("virtio-blk request outhdr too short");
         exit(1);
     }
+
     iov_discard_front(&iov, &out_num, sizeof(req->out));
-    req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base;
+
+    if (in_num < 1 ||
+        in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
+        error_report("virtio-blk request inhdr too short");
+        exit(1);
+    }
+
+    req->in = (void *)in_iov[in_num - 1].iov_base
+              + in_iov[in_num - 1].iov_len
+              - sizeof(struct virtio_blk_inhdr);
+    iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));
 
     type = ldl_p(&req->out.type);
 
-- 
2.0.0

Re: [Qemu-devel] [PATCH v4 0/9] virtio-blk: Converge VirtIOBlockRequest into VirtIOBlockReq

[Stefan Hajnoczi]

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

On Wed, Jun 11, 2014 at 12:11:41PM +0800, Fam Zheng wrote:
> This unifies the request structure used by dataplane and non-dataplane code,
> while dropping unnessary fields for bounce buffer and read flag.
> 
> v4: Address Stefan's comments as below, and add Paolo's rev-by lines to others.
> 
>     [01/09] virtio-blk: Move VirtIOBlockReq to header
> 
>             Include "block/block.h".
> 
>     [02/09] virtio-blk: Convert VirtIOBlockReq.elem to pointer
> 
>             Use g_slice_new0 for req as well.
> 
>     [06/09] virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr
> 
>             Check guest input, don't assert.
> 
>     [09/09] virtio-blk: Fix and clean up the in_sg and out_sg check
> 
>             Use error_report + exit.
> 
> Fam Zheng (9):
>   virtio-blk: Move VirtIOBlockReq to header
>   virtio-blk: Convert VirtIOBlockReq.elem to pointer
>   virtio-blk: Drop bounce buffer from dataplane code
>   virtio-blk: Drop VirtIOBlockRequest.read
>   virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq
>   virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr
>   virtio-blk: Convert VirtIOBlockReq.out to structrue
>   virtio-blk: Fill in VirtIOBlockReq.out in dataplane code
>   virtio-blk: Fix and clean up the in_sg and out_sg check
> 
>  hw/block/dataplane/virtio-blk.c | 169 ++++++++++++++--------------------------
>  hw/block/virtio-blk.c           | 117 +++++++++++++++-------------
>  include/hw/virtio/virtio-blk.h  |  11 +++
>  3 files changed, 132 insertions(+), 165 deletions(-)
> 
> -- 
> 2.0.0
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v4 0/9] virtio-blk: Converge VirtIOBlockRequest into VirtIOBlockReq

[Kevin Wolf]

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

Am 27.06.2014 um 15:29 hat Stefan Hajnoczi geschrieben:
> On Wed, Jun 11, 2014 at 12:11:41PM +0800, Fam Zheng wrote:
> > This unifies the request structure used by dataplane and non-dataplane code,
> > while dropping unnessary fields for bounce buffer and read flag.
> > 
> > v4: Address Stefan's comments as below, and add Paolo's rev-by lines to others.
> > 
> >     [01/09] virtio-blk: Move VirtIOBlockReq to header
> > 
> >             Include "block/block.h".
> > 
> >     [02/09] virtio-blk: Convert VirtIOBlockReq.elem to pointer
> > 
> >             Use g_slice_new0 for req as well.
> > 
> >     [06/09] virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr
> > 
> >             Check guest input, don't assert.
> > 
> >     [09/09] virtio-blk: Fix and clean up the in_sg and out_sg check
> > 
> >             Use error_report + exit.
> > 
> > Fam Zheng (9):
> >   virtio-blk: Move VirtIOBlockReq to header
> >   virtio-blk: Convert VirtIOBlockReq.elem to pointer
> >   virtio-blk: Drop bounce buffer from dataplane code
> >   virtio-blk: Drop VirtIOBlockRequest.read
> >   virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq
> >   virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr
> >   virtio-blk: Convert VirtIOBlockReq.out to structrue
> >   virtio-blk: Fill in VirtIOBlockReq.out in dataplane code
> >   virtio-blk: Fix and clean up the in_sg and out_sg check
> > 
> >  hw/block/dataplane/virtio-blk.c | 169 ++++++++++++++--------------------------
> >  hw/block/virtio-blk.c           | 117 +++++++++++++++-------------
> >  include/hw/virtio/virtio-blk.h  |  11 +++
> >  3 files changed, 132 insertions(+), 165 deletions(-)
> > 
> > -- 
> > 2.0.0
> > 
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks, applied all to the block branch.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]