[Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests

[Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests

[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

[Qemu-devel] [PATCH 1/3] block: make qiov_is_aligned() public

[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

[Qemu-devel] [PATCH 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd()

[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

[Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[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

Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[Paolo Bonzini]

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
> 
> 

[Qemu-devel] libqos - was: [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[Andreas Färber]

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

Re: [Qemu-devel] libqos - was: [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests

[Stefan Hajnoczi]

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

Re: [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests

[Kevin Wolf]

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>