[Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

jazeltq@gmail.com posted 1 patch 142 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170221065003.26103-1-jazeltq@gmail.com
Test checkpatch passed
Test docker passed
Test s390x passed
block/rbd.c | 80 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 24 deletions(-)

[Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

Posted by jazeltq@gmail.com 142 weeks ago
From: tianqing <tianqing@unitedstack.com>

Rbd can do readv and writev directly, so wo do not need to transform
iov to buf or vice versa any more.

Signed-off-by: tianqing <tianqing@unitedstack.com>
---
 block/rbd.c | 80 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index a57b3e3..22e8e69 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -62,6 +62,13 @@
 #define RBD_MAX_SNAP_NAME_SIZE 128
 #define RBD_MAX_SNAPS 100
 
+/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */
+#ifdef LIBRBD_SUPPORTS_IOVEC
+#define LIBRBD_USE_IOVEC 1
+#else
+#define LIBRBD_USE_IOVEC 0
+#endif
+
 typedef enum {
     RBD_AIO_READ,
     RBD_AIO_WRITE,
@@ -310,6 +317,17 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf,
     return ret;
 }
 
+static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
+{
+    if (LIBRBD_USE_IOVEC) {
+        RBDAIOCB *acb = rcb->acb;
+        iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0,
+                   acb->qiov->size - offs);
+    } else {
+        memset(rcb->buf + offs, 0, rcb->size - offs);
+    }
+}
+
 static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
@@ -426,11 +444,11 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
         }
     } else {
         if (r < 0) {
-            memset(rcb->buf, 0, rcb->size);
+            qemu_rbd_memset(rcb, 0);
             acb->ret = r;
             acb->error = 1;
         } else if (r < rcb->size) {
-            memset(rcb->buf + r, 0, rcb->size - r);
+            qemu_rbd_memset(rcb, r);
             if (!acb->error) {
                 acb->ret = rcb->size;
             }
@@ -441,10 +459,13 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
 
     g_free(rcb);
 
-    if (acb->cmd == RBD_AIO_READ) {
-        qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size);
+    if (!LIBRBD_USE_IOVEC) {
+        if (acb->cmd == RBD_AIO_READ) {
+            qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size);
+        }
+        qemu_vfree(acb->bounce);
     }
-    qemu_vfree(acb->bounce);
+
     acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
 
     qemu_aio_unref(acb);
@@ -655,7 +676,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
     RBDAIOCB *acb;
     RADOSCB *rcb = NULL;
     rbd_completion_t c;
-    char *buf;
     int r;
 
     BDRVRBDState *s = bs->opaque;
@@ -664,27 +684,29 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
     acb->cmd = cmd;
     acb->qiov = qiov;
     assert(!qiov || qiov->size == size);
-    if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
-        acb->bounce = NULL;
-    } else {
-        acb->bounce = qemu_try_blockalign(bs, qiov->size);
-        if (acb->bounce == NULL) {
-            goto failed;
+
+    rcb = g_new(RADOSCB, 1);
+
+    if (!LIBRBD_USE_IOVEC) {
+        if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
+            acb->bounce = NULL;
+        } else {
+            acb->bounce = qemu_try_blockalign(bs, qiov->size);
+            if (acb->bounce == NULL) {
+                goto failed;
+            }
         }
+        if (cmd == RBD_AIO_WRITE) {
+            qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
+        }
+        rcb->buf = acb->bounce;
     }
+
     acb->ret = 0;
     acb->error = 0;
     acb->s = s;
 
-    if (cmd == RBD_AIO_WRITE) {
-        qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
-    }
-
-    buf = acb->bounce;
-
-    rcb = g_new(RADOSCB, 1);
     rcb->acb = acb;
-    rcb->buf = buf;
     rcb->s = acb->s;
     rcb->size = size;
     r = rbd_aio_create_completion(rcb, (rbd_callback_t) rbd_finish_aiocb, &c);
@@ -694,10 +716,18 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
 
     switch (cmd) {
     case RBD_AIO_WRITE:
-        r = rbd_aio_write(s->image, off, size, buf, c);
+#ifdef LIBRBD_SUPPORTS_IOVEC
+            r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c);
+#else
+            r = rbd_aio_write(s->image, off, size, rcb->buf, c);
+#endif
         break;
     case RBD_AIO_READ:
-        r = rbd_aio_read(s->image, off, size, buf, c);
+#ifdef LIBRBD_SUPPORTS_IOVEC
+            r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c);
+#else
+            r = rbd_aio_read(s->image, off, size, rcb->buf, c);
+#endif
         break;
     case RBD_AIO_DISCARD:
         r = rbd_aio_discard_wrapper(s->image, off, size, c);
@@ -712,14 +742,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
     if (r < 0) {
         goto failed_completion;
     }
-
     return &acb->common;
 
 failed_completion:
     rbd_aio_release(c);
 failed:
     g_free(rcb);
-    qemu_vfree(acb->bounce);
+    if (!LIBRBD_USE_IOVEC) {
+        qemu_vfree(acb->bounce);
+    }
+
     qemu_aio_unref(acb);
     return NULL;
 }
-- 
2.10.2


Re: [Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

Posted by Jeff Cody 142 weeks ago
On Tue, Feb 21, 2017 at 02:50:03PM +0800, jazeltq@gmail.com wrote:
> From: tianqing <tianqing@unitedstack.com>
> 
> Rbd can do readv and writev directly, so wo do not need to transform
> iov to buf or vice versa any more.
> 
> Signed-off-by: tianqing <tianqing@unitedstack.com>
> ---


This is marked as an RFC still - is this a series you would like to see in
2.9?

Re: [Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

Posted by Jaze Lee 142 weeks ago
2017-02-24 11:52 GMT+08:00 Jeff Cody <jcody@redhat.com>:
> On Tue, Feb 21, 2017 at 02:50:03PM +0800, jazeltq@gmail.com wrote:
>> From: tianqing <tianqing@unitedstack.com>
>>
>> Rbd can do readv and writev directly, so wo do not need to transform
>> iov to buf or vice versa any more.
>>
>> Signed-off-by: tianqing <tianqing@unitedstack.com>
>> ---
>
>
> This is marked as an RFC still - is this a series you would like to see in
> 2.9?

Yes.   What should i do if i like it in 2.9

-- 
谦谦君子

Re: [Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

Posted by Jeff Cody 142 weeks ago
On Fri, Feb 24, 2017 at 02:09:31PM +0800, Jaze Lee wrote:
> 2017-02-24 11:52 GMT+08:00 Jeff Cody <jcody@redhat.com>:
> > On Tue, Feb 21, 2017 at 02:50:03PM +0800, jazeltq@gmail.com wrote:
> >> From: tianqing <tianqing@unitedstack.com>
> >>
> >> Rbd can do readv and writev directly, so wo do not need to transform
> >> iov to buf or vice versa any more.
> >>
> >> Signed-off-by: tianqing <tianqing@unitedstack.com>
> >> ---
> >
> >
> > This is marked as an RFC still - is this a series you would like to see in
> > 2.9?
> 
> Yes.   What should i do if i like it in 2.9
>

Ideally you would submit the series as a non-RFC patch (that is, it would
have progressed to a normal patch away from RFC).

But in this case, it seems to me that this patch has progressed beyond RFC;
is there any reason it is still marked as RFC instead of just a patch?  It
looks OK to me.


Re: [Qemu-devel] [RFC v6] RBD: Add support readv,writev for rbd

Posted by Jaze Lee 142 weeks ago
Sorry, I do not know. I thought we should always add RFC all though
the patch's life...
What is the prefix i should add? PATH 0 ?

2017-02-24 19:42 GMT+08:00 Jeff Cody <jcody@redhat.com>:
> On Fri, Feb 24, 2017 at 02:09:31PM +0800, Jaze Lee wrote:
>> 2017-02-24 11:52 GMT+08:00 Jeff Cody <jcody@redhat.com>:
>> > On Tue, Feb 21, 2017 at 02:50:03PM +0800, jazeltq@gmail.com wrote:
>> >> From: tianqing <tianqing@unitedstack.com>
>> >>
>> >> Rbd can do readv and writev directly, so wo do not need to transform
>> >> iov to buf or vice versa any more.
>> >>
>> >> Signed-off-by: tianqing <tianqing@unitedstack.com>
>> >> ---
>> >
>> >
>> > This is marked as an RFC still - is this a series you would like to see in
>> > 2.9?
>>
>> Yes.   What should i do if i like it in 2.9
>>
>
> Ideally you would submit the series as a non-RFC patch (that is, it would
> have progressed to a normal patch away from RFC).
>
> But in this case, it seems to me that this patch has progressed beyond RFC;
> is there any reason it is still marked as RFC instead of just a patch?  It
> looks OK to me.
>



-- 
谦谦君子