qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests
@ 2013-01-10 16:48 Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-01-10 16:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi

Windows guests do not work with x-data-plane=on because misaligned request
support is missing in hw/dataplane/virtio-blk.c.  This series adds a bounce
buffer when the request is misaligned.  Windows guests now work with
x-data-plane=on.

Stefan Hajnoczi (3):
  block: make qiov_is_aligned() public
  dataplane: extract virtio-blk read/write processing into do_rdwr_cmd()  dataplane: handle misaligned virtio-blk requests

 block.c                   | 16 +++++++++++
 block/raw-posix.c         | 18 +------------
 hw/dataplane/virtio-blk.c | 67 +++++++++++++++++++++++++++++++++++++++--------
 include/block/block.h     |  1 +
 4 files changed, 74 insertions(+), 28 deletions(-)

-- 
1.8.0.2

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

* [Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public
  2013-01-10 16:48 [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Stefan Hajnoczi
@ 2013-01-10 16:48 ` Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd() Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-01-10 16:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi

The qiov_is_aligned() function checks whether a QEMUIOVector meets a
BlockDriverState's alignment requirements.  This is needed by
virtio-blk-data-plane so:

1. Move the function from block/raw-posix.c to block/block.c.
2. Make it public in block/block.h.
3. Rename to bdrv_qiov_is_aligned().
4. Change return type from int to bool.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c               | 16 ++++++++++++++++
 block/raw-posix.c     | 18 +-----------------
 include/block/block.h |  1 +
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 4e28c55..11ca661 100644
--- a/block.c
+++ b/block.c
@@ -4323,6 +4323,22 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
     return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment : 512, size);
 }
 
+/*
+ * Check if all memory in this vector is sector aligned.
+ */
+bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
+{
+    int i;
+
+    for (i = 0; i < qiov->niov; i++) {
+        if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable)
 {
     int64_t bitmap_size;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 87d888e..02d2a4e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -430,22 +430,6 @@ static void raw_reopen_abort(BDRVReopenState *state)
 #endif
 */
 
-/*
- * Check if all memory in this vector is sector aligned.
- */
-static int qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
-{
-    int i;
-
-    for (i = 0; i < qiov->niov; i++) {
-        if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
-            return 0;
-        }
-    }
-
-    return 1;
-}
-
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 {
     int ret;
@@ -722,7 +706,7 @@ static BlockDriverAIOCB *raw_aio_submit(BlockDriverState *bs,
      * driver that it needs to copy the buffer.
      */
     if ((bs->open_flags & BDRV_O_NOCACHE)) {
-        if (!qiov_is_aligned(bs, qiov)) {
+        if (!bdrv_qiov_is_aligned(bs, qiov)) {
             type |= QEMU_AIO_MISALIGNED;
 #ifdef CONFIG_LINUX_AIO
         } else if (s->use_aio) {
diff --git a/include/block/block.h b/include/block/block.h
index 0719339..ffd1936 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -349,6 +349,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
 
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
+bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
 
 #define BDRV_SECTORS_PER_DIRTY_CHUNK 2048
 
-- 
1.8.0.2

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

* [Qemu-devel] [PATCH 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd()
  2013-01-10 16:48 [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public Stefan Hajnoczi
@ 2013-01-10 16:48 ` Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests Stefan Hajnoczi
  2013-01-11 12:43 ` [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Kevin Wolf
  3 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-01-10 16:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi

Extract code for read/write command processing into do_rdwr_cmd().  This
brings together pieces that are spread across process_request().

The real motivation is to set the stage for handling misaligned
requests, which the next patch tackles.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/dataplane/virtio-blk.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index 4c4ad84..a6696b8 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -130,6 +130,22 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s,
     complete_request_early(s, head, inhdr, VIRTIO_BLK_S_OK);
 }
 
+static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
+                       struct iovec *iov, unsigned int iov_cnt,
+                       long long offset, unsigned int head,
+                       QEMUIOVector *inhdr)
+{
+    struct iocb *iocb;
+
+    iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
+
+    /* Fill in virtio block metadata needed for completion */
+    VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
+    req->head = head;
+    req->inhdr = inhdr;
+    return 0;
+}
+
 static int process_request(IOQueue *ioq, struct iovec iov[],
                            unsigned int out_num, unsigned int in_num,
                            unsigned int head)
@@ -139,7 +155,6 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
     struct virtio_blk_outhdr outhdr;
     QEMUIOVector *inhdr;
     size_t in_size;
-    struct iocb *iocb;
 
     /* Copy in outhdr */
     if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr,
@@ -167,12 +182,12 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
 
     switch (outhdr.type) {
     case VIRTIO_BLK_T_IN:
-        iocb = ioq_rdwr(ioq, true, in_iov, in_num, outhdr.sector * 512);
-        break;
+        do_rdwr_cmd(s, true, in_iov, in_num, outhdr.sector * 512, head, inhdr);
+        return 0;
 
     case VIRTIO_BLK_T_OUT:
-        iocb = ioq_rdwr(ioq, false, iov, out_num, outhdr.sector * 512);
-        break;
+        do_rdwr_cmd(s, false, iov, out_num, outhdr.sector * 512, head, inhdr);
+        return 0;
 
     case VIRTIO_BLK_T_SCSI_CMD:
         /* TODO support SCSI commands */
@@ -198,12 +213,6 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
         g_slice_free(QEMUIOVector, inhdr);
         return -EFAULT;
     }
-
-    /* Fill in virtio block metadata needed for completion */
-    VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
-    req->head = head;
-    req->inhdr = inhdr;
-    return 0;
 }
 
 static void handle_notify(EventHandler *handler)
-- 
1.8.0.2

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

* [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-10 16:48 [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public Stefan Hajnoczi
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd() Stefan Hajnoczi
@ 2013-01-10 16:48 ` Stefan Hajnoczi
  2013-01-11 12:39   ` Kevin Wolf
  2013-01-11 12:43 ` [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Kevin Wolf
  3 siblings, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-01-10 16:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi

O_DIRECT on Linux has alignment requirements on I/O buffers and
misaligned requests result in -EINVAL.  The Linux virtio_blk guest
driver usually submits aligned requests so I forgot to handle misaligned
requests.

It turns out that virtio-win guest drivers submit misaligned requests.
Handle them using a bounce buffer that meets alignment requirements.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/dataplane/virtio-blk.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index a6696b8..991ab5f 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -34,6 +34,8 @@ typedef struct {
     struct iocb iocb;               /* Linux AIO control block */
     QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
     unsigned int head;              /* vring descriptor index */
+    void *bounce_buffer;            /* used if guest buffers are unaligned */
+    QEMUIOVector *read_qiov;        /* for read completion /w bounce buffer */
 } VirtIOBlockRequest;
 
 struct VirtIOBlockDataPlane {
@@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t ret, void *opaque)
 
     trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
 
+    if (req->read_qiov) {
+        assert(req->bounce_buffer);
+        qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);
+        qemu_iovec_destroy(req->read_qiov);
+        g_slice_free(QEMUIOVector, req->read_qiov);
+    }
+
+    qemu_vfree(req->bounce_buffer);
+
     qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
     qemu_iovec_destroy(req->inhdr);
     g_slice_free(QEMUIOVector, req->inhdr);
@@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
                        QEMUIOVector *inhdr)
 {
     struct iocb *iocb;
+    QEMUIOVector qiov;
+    struct iovec bounce_iov;
+    void *bounce_buffer = NULL;
+    QEMUIOVector *read_qiov = NULL;
+
+    qemu_iovec_init_external(&qiov, iov, iov_cnt);
+    if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
+        /* Redirect I/O to aligned bounce buffer */
+        bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
+        bounce_iov.iov_base = bounce_buffer;
+        bounce_iov.iov_len = qiov.size;
+        iov = &bounce_iov;
+        iov_cnt = 1;
+
+        if (read) {
+            /* Need to copy back from bounce buffer on completion */
+            read_qiov = g_slice_new(QEMUIOVector);
+            qemu_iovec_init(read_qiov, iov_cnt);
+            qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
+        } else {
+            qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
+        }
+    }
 
     iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
 
@@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
     VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
     req->head = head;
     req->inhdr = inhdr;
+    req->bounce_buffer = bounce_buffer;
+    req->read_qiov = read_qiov;
     return 0;
 }
 
-- 
1.8.0.2

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

* Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests Stefan Hajnoczi
@ 2013-01-11 12:39   ` Kevin Wolf
  2013-01-11 13:27     ` Paolo Bonzini
  2013-01-11 14:38     ` [Qemu-devel] " Stefan Hajnoczi
  0 siblings, 2 replies; 10+ messages in thread
From: Kevin Wolf @ 2013-01-11 12:39 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel

Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
> O_DIRECT on Linux has alignment requirements on I/O buffers and
> misaligned requests result in -EINVAL.  The Linux virtio_blk guest
> driver usually submits aligned requests so I forgot to handle misaligned
> requests.
> 
> It turns out that virtio-win guest drivers submit misaligned requests.
> Handle them using a bounce buffer that meets alignment requirements.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/dataplane/virtio-blk.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
> index a6696b8..991ab5f 100644
> --- a/hw/dataplane/virtio-blk.c
> +++ b/hw/dataplane/virtio-blk.c
> @@ -34,6 +34,8 @@ typedef struct {
>      struct iocb iocb;               /* Linux AIO control block */
>      QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
>      unsigned int head;              /* vring descriptor index */
> +    void *bounce_buffer;            /* used if guest buffers are unaligned */
> +    QEMUIOVector *read_qiov;        /* for read completion /w bounce buffer */
>  } VirtIOBlockRequest;
>  
>  struct VirtIOBlockDataPlane {
> @@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t ret, void *opaque)
>  
>      trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
>  
> +    if (req->read_qiov) {
> +        assert(req->bounce_buffer);
> +        qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);

Shouldn't it be qemu_iovec_from_buf()?

Makes me wonder if qtest cases might be doable...

> +        qemu_iovec_destroy(req->read_qiov);
> +        g_slice_free(QEMUIOVector, req->read_qiov);
> +    }
> +
> +    qemu_vfree(req->bounce_buffer);
> +
>      qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
>      qemu_iovec_destroy(req->inhdr);
>      g_slice_free(QEMUIOVector, req->inhdr);
> @@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
>                         QEMUIOVector *inhdr)
>  {
>      struct iocb *iocb;
> +    QEMUIOVector qiov;
> +    struct iovec bounce_iov;
> +    void *bounce_buffer = NULL;
> +    QEMUIOVector *read_qiov = NULL;
> +
> +    qemu_iovec_init_external(&qiov, iov, iov_cnt);
> +    if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
> +        /* Redirect I/O to aligned bounce buffer */
> +        bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
> +        bounce_iov.iov_base = bounce_buffer;
> +        bounce_iov.iov_len = qiov.size;
> +        iov = &bounce_iov;
> +        iov_cnt = 1;
> +
> +        if (read) {
> +            /* Need to copy back from bounce buffer on completion */
> +            read_qiov = g_slice_new(QEMUIOVector);
> +            qemu_iovec_init(read_qiov, iov_cnt);
> +            qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);

This would have to be the original iov and iov_cnt, but they are already
overwritten here.

> +        } else {
> +            qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
> +        }
> +    }
>  
>      iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
>  
> @@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
>      VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
>      req->head = head;
>      req->inhdr = inhdr;
> +    req->bounce_buffer = bounce_buffer;
> +    req->read_qiov = read_qiov;
>      return 0;
>  }

Kevin

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

* Re: [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests
  2013-01-10 16:48 [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2013-01-10 16:48 ` [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests Stefan Hajnoczi
@ 2013-01-11 12:43 ` Kevin Wolf
  3 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2013-01-11 12:43 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel

Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
> Windows guests do not work with x-data-plane=on because misaligned request
> support is missing in hw/dataplane/virtio-blk.c.  This series adds a bounce
> buffer when the request is misaligned.  Windows guests now work with
> x-data-plane=on.
> 
> Stefan Hajnoczi (3):
>   block: make qiov_is_aligned() public
>   dataplane: extract virtio-blk read/write processing into do_rdwr_cmd()  dataplane: handle misaligned virtio-blk requests
> 
>  block.c                   | 16 +++++++++++
>  block/raw-posix.c         | 18 +------------
>  hw/dataplane/virtio-blk.c | 67 +++++++++++++++++++++++++++++++++++++++--------
>  include/block/block.h     |  1 +
>  4 files changed, 74 insertions(+), 28 deletions(-)

Patches 1 and 2:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

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

* Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-11 12:39   ` Kevin Wolf
@ 2013-01-11 13:27     ` Paolo Bonzini
  2013-01-11 15:44       ` [Qemu-devel] libqos - was: " Andreas Färber
  2013-01-11 14:38     ` [Qemu-devel] " Stefan Hajnoczi
  1 sibling, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2013-01-11 13:27 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: Anthony Liguori, qemu-devel, Stefan Hajnoczi

Il 11/01/2013 13:39, Kevin Wolf ha scritto:
> Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
>> O_DIRECT on Linux has alignment requirements on I/O buffers and
>> misaligned requests result in -EINVAL.  The Linux virtio_blk guest
>> driver usually submits aligned requests so I forgot to handle misaligned
>> requests.
>>
>> It turns out that virtio-win guest drivers submit misaligned requests.
>> Handle them using a bounce buffer that meets alignment requirements.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  hw/dataplane/virtio-blk.c | 36 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
>> index a6696b8..991ab5f 100644
>> --- a/hw/dataplane/virtio-blk.c
>> +++ b/hw/dataplane/virtio-blk.c
>> @@ -34,6 +34,8 @@ typedef struct {
>>      struct iocb iocb;               /* Linux AIO control block */
>>      QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
>>      unsigned int head;              /* vring descriptor index */
>> +    void *bounce_buffer;            /* used if guest buffers are unaligned */
>> +    QEMUIOVector *read_qiov;        /* for read completion /w bounce buffer */
>>  } VirtIOBlockRequest;
>>  
>>  struct VirtIOBlockDataPlane {
>> @@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t ret, void *opaque)
>>  
>>      trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
>>  
>> +    if (req->read_qiov) {
>> +        assert(req->bounce_buffer);
>> +        qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);
> 
> Shouldn't it be qemu_iovec_from_buf()?

Yes.

> Makes me wonder if qtest cases might be doable...

Anthony, what's going on with libqos? :)

In the meanwhile...

>> +        qemu_iovec_destroy(req->read_qiov);
>> +        g_slice_free(QEMUIOVector, req->read_qiov);
>> +    }
>> +
>> +    qemu_vfree(req->bounce_buffer);
>> +
>>      qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
>>      qemu_iovec_destroy(req->inhdr);
>>      g_slice_free(QEMUIOVector, req->inhdr);
>> @@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
>>                         QEMUIOVector *inhdr)
>>  {
>>      struct iocb *iocb;
>> +    QEMUIOVector qiov;
>> +    struct iovec bounce_iov;
>> +    void *bounce_buffer = NULL;
>> +    QEMUIOVector *read_qiov = NULL;
>> +
>> +    qemu_iovec_init_external(&qiov, iov, iov_cnt);
>> +    if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {

... changing this to "if (1)" would be enough for testing with Linux guests.

Paolo

>> +        /* Redirect I/O to aligned bounce buffer */
>> +        bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
>> +        bounce_iov.iov_base = bounce_buffer;
>> +        bounce_iov.iov_len = qiov.size;
>> +        iov = &bounce_iov;
>> +        iov_cnt = 1;
>> +
>> +        if (read) {
>> +            /* Need to copy back from bounce buffer on completion */
>> +            read_qiov = g_slice_new(QEMUIOVector);
>> +            qemu_iovec_init(read_qiov, iov_cnt);
>> +            qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
> 
> This would have to be the original iov and iov_cnt, but they are already
> overwritten here.
> 
>> +        } else {
>> +            qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
>> +        }
>> +    }
>>  
>>      iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
>>  
>> @@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
>>      VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
>>      req->head = head;
>>      req->inhdr = inhdr;
>> +    req->bounce_buffer = bounce_buffer;
>> +    req->read_qiov = read_qiov;
>>      return 0;
>>  }
> 
> Kevin
> 
> 

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

* Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-11 12:39   ` Kevin Wolf
  2013-01-11 13:27     ` Paolo Bonzini
@ 2013-01-11 14:38     ` Stefan Hajnoczi
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-01-11 14:38 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel

On Fri, Jan 11, 2013 at 01:39:40PM +0100, Kevin Wolf wrote:
> Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
> > O_DIRECT on Linux has alignment requirements on I/O buffers and
> > misaligned requests result in -EINVAL.  The Linux virtio_blk guest
> > driver usually submits aligned requests so I forgot to handle misaligned
> > requests.
> > 
> > It turns out that virtio-win guest drivers submit misaligned requests.
> > Handle them using a bounce buffer that meets alignment requirements.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  hw/dataplane/virtio-blk.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
> > index a6696b8..991ab5f 100644
> > --- a/hw/dataplane/virtio-blk.c
> > +++ b/hw/dataplane/virtio-blk.c
> > @@ -34,6 +34,8 @@ typedef struct {
> >      struct iocb iocb;               /* Linux AIO control block */
> >      QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
> >      unsigned int head;              /* vring descriptor index */
> > +    void *bounce_buffer;            /* used if guest buffers are unaligned */
> > +    QEMUIOVector *read_qiov;        /* for read completion /w bounce buffer */
> >  } VirtIOBlockRequest;
> >  
> >  struct VirtIOBlockDataPlane {
> > @@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t ret, void *opaque)
> >  
> >      trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
> >  
> > +    if (req->read_qiov) {
> > +        assert(req->bounce_buffer);
> > +        qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);
> 
> Shouldn't it be qemu_iovec_from_buf()?
> 
> Makes me wonder if qtest cases might be doable...
> 
> > +        qemu_iovec_destroy(req->read_qiov);
> > +        g_slice_free(QEMUIOVector, req->read_qiov);
> > +    }
> > +
> > +    qemu_vfree(req->bounce_buffer);
> > +
> >      qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
> >      qemu_iovec_destroy(req->inhdr);
> >      g_slice_free(QEMUIOVector, req->inhdr);
> > @@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
> >                         QEMUIOVector *inhdr)
> >  {
> >      struct iocb *iocb;
> > +    QEMUIOVector qiov;
> > +    struct iovec bounce_iov;
> > +    void *bounce_buffer = NULL;
> > +    QEMUIOVector *read_qiov = NULL;
> > +
> > +    qemu_iovec_init_external(&qiov, iov, iov_cnt);
> > +    if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
> > +        /* Redirect I/O to aligned bounce buffer */
> > +        bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
> > +        bounce_iov.iov_base = bounce_buffer;
> > +        bounce_iov.iov_len = qiov.size;
> > +        iov = &bounce_iov;
> > +        iov_cnt = 1;
> > +
> > +        if (read) {
> > +            /* Need to copy back from bounce buffer on completion */
> > +            read_qiov = g_slice_new(QEMUIOVector);
> > +            qemu_iovec_init(read_qiov, iov_cnt);
> > +            qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
> 
> This would have to be the original iov and iov_cnt, but they are already
> overwritten here.

Good point.  What's really weird is that I tested this with a Windows
guest - probably the buffer cache fooled me so I didn't notice read was
completely broken.

Stefan

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

* [Qemu-devel] libqos - was: [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-11 13:27     ` Paolo Bonzini
@ 2013-01-11 15:44       ` Andreas Färber
  2013-01-11 16:25         ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Färber @ 2013-01-11 15:44 UTC (permalink / raw)
  To: Paolo Bonzini, Anthony Liguori
  Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Alexander Graf

Am 11.01.2013 14:27, schrieb Paolo Bonzini:
> Il 11/01/2013 13:39, Kevin Wolf ha scritto:
>> Makes me wonder if qtest cases might be doable...
> 
> Anthony, what's going on with libqos? :)

I posted a rebased version of I2C libqos support after Anthony approved
the pre-header-reorg version and am hoping it will get applied now.

For virtio here you're obviously waiting for PCI libqos, haven't seen a
new patch series yet. If the issue is just lack of time on Anthony's
part and not some major design issue, I'd be willing to help post 1.4.

Unfortunately I've been hearing that s390 qtest cases are not possible
due to it not using MMIO or PIO...

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] libqos - was: [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
  2013-01-11 15:44       ` [Qemu-devel] libqos - was: " Andreas Färber
@ 2013-01-11 16:25         ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-01-11 16:25 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Anthony Liguori, qemu-devel, Stefan Hajnoczi,
	Alexander Graf

Il 11/01/2013 16:44, Andreas Färber ha scritto:
>>> >> Makes me wonder if qtest cases might be doable...
>> > 
>> > Anthony, what's going on with libqos? :)
> I posted a rebased version of I2C libqos support after Anthony approved
> the pre-header-reorg version and am hoping it will get applied now.
> 
> For virtio here you're obviously waiting for PCI libqos, haven't seen a
> new patch series yet. If the issue is just lack of time on Anthony's
> part and not some major design issue, I'd be willing to help post 1.4.
> 
> Unfortunately I've been hearing that s390 qtest cases are not possible
> due to it not using MMIO or PIO...

You can add s390 magic to qtest, just like it has I/O ports for x86...

Paolo

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

end of thread, other threads:[~2013-01-11 16:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-10 16:48 [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Stefan Hajnoczi
2013-01-10 16:48 ` [Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public Stefan Hajnoczi
2013-01-10 16:48 ` [Qemu-devel] [PATCH 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd() Stefan Hajnoczi
2013-01-10 16:48 ` [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests Stefan Hajnoczi
2013-01-11 12:39   ` Kevin Wolf
2013-01-11 13:27     ` Paolo Bonzini
2013-01-11 15:44       ` [Qemu-devel] libqos - was: " Andreas Färber
2013-01-11 16:25         ` Paolo Bonzini
2013-01-11 14:38     ` [Qemu-devel] " Stefan Hajnoczi
2013-01-11 12:43 ` [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).