diff mbox

[v2,03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev()

Message ID 1461849406-29743-4-git-send-email-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin Wolf April 28, 2016, 1:16 p.m. UTC 
Instead of registering emulation functions as .bdrv_co_writev, just
directly check whether the function is there or not, and use the AIO
interface if it isn't. This makes the read/write functions more
consistent with how things are done in other places (flush, discard,
etc.)

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/io.c   | 126 ++++++++++++++++++++++++-----------------------------------
 trace-events |   1 -
 2 files changed, 52 insertions(+), 75 deletions(-)

Comments

Fam Zheng April 29, 2016, 1:22 a.m. UTC | #1 
On Thu, 04/28 15:16, Kevin Wolf wrote:
> Instead of registering emulation functions as .bdrv_co_writev, just
> directly check whether the function is there or not, and use the AIO
> interface if it isn't. This makes the read/write functions more
> consistent with how things are done in other places (flush, discard,
> etc.)
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block/io.c   | 126 ++++++++++++++++++++++++-----------------------------------
>  trace-events |   1 -
>  2 files changed, 52 insertions(+), 75 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index c9b2864..86d97c3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -40,12 +40,6 @@ static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
>  static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
>          int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
>          BlockCompletionFunc *cb, void *opaque);
> -static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
> -                                         int64_t sector_num, int nb_sectors,
> -                                         QEMUIOVector *iov);
> -static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
> -                                         int64_t sector_num, int nb_sectors,
> -                                         QEMUIOVector *iov);
>  static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
>                                           int64_t sector_num,
>                                           QEMUIOVector *qiov,
> @@ -125,19 +119,13 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
>  
>  void bdrv_setup_io_funcs(BlockDriver *bdrv)
>  {
> -    /* Block drivers without coroutine functions need emulation */
> -    if (!bdrv->bdrv_co_readv) {
> -        bdrv->bdrv_co_readv = bdrv_co_readv_em;
> -        bdrv->bdrv_co_writev = bdrv_co_writev_em;
> -
> -        /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> -         * the block driver lacks aio we need to emulate that too.
> -         */
> -        if (!bdrv->bdrv_aio_readv) {
> -            /* add AIO emulation layer */
> -            bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
> -            bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
> -        }
> +    /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> +     * the block driver lacks aio we need to emulate that.
> +     */
> +    if (!bdrv->bdrv_aio_readv) {

I'd also AND with !bdrv->bdrv_co_readv, because in that case the em functions
are assigned but not used.

> +        /* add AIO emulation layer */
> +        bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
> +        bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
>      }
>  }
>
Fam Zheng April 29, 2016, 3:25 a.m. UTC | #2 
On Fri, 04/29 09:22, Fam Zheng wrote:
> > @@ -125,19 +119,13 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
> >  
> >  void bdrv_setup_io_funcs(BlockDriver *bdrv)
> >  {
> > -    /* Block drivers without coroutine functions need emulation */
> > -    if (!bdrv->bdrv_co_readv) {
> > -        bdrv->bdrv_co_readv = bdrv_co_readv_em;
> > -        bdrv->bdrv_co_writev = bdrv_co_writev_em;
> > -
> > -        /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> > -         * the block driver lacks aio we need to emulate that too.
> > -         */
> > -        if (!bdrv->bdrv_aio_readv) {
> > -            /* add AIO emulation layer */
> > -            bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
> > -            bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
> > -        }
> > +    /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
> > +     * the block driver lacks aio we need to emulate that.
> > +     */
> > +    if (!bdrv->bdrv_aio_readv) {
> 
> I'd also AND with !bdrv->bdrv_co_readv, because in that case the em functions
> are assigned but not used.

Never mind, I see the last patch removes this altogether.

Fam
diff mbox

Patch

diff --git a/block/io.c b/block/io.c
index c9b2864..86d97c3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -40,12 +40,6 @@  static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
 static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque);
-static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
-                                         int64_t sector_num, int nb_sectors,
-                                         QEMUIOVector *iov);
-static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
-                                         int64_t sector_num, int nb_sectors,
-                                         QEMUIOVector *iov);
 static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
                                          int64_t sector_num,
                                          QEMUIOVector *qiov,
@@ -125,19 +119,13 @@  void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
 
 void bdrv_setup_io_funcs(BlockDriver *bdrv)
 {
-    /* Block drivers without coroutine functions need emulation */
-    if (!bdrv->bdrv_co_readv) {
-        bdrv->bdrv_co_readv = bdrv_co_readv_em;
-        bdrv->bdrv_co_writev = bdrv_co_writev_em;
-
-        /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
-         * the block driver lacks aio we need to emulate that too.
-         */
-        if (!bdrv->bdrv_aio_readv) {
-            /* add AIO emulation layer */
-            bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
-            bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
-        }
+    /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
+     * the block driver lacks aio we need to emulate that.
+     */
+    if (!bdrv->bdrv_aio_readv) {
+        /* add AIO emulation layer */
+        bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
+        bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
     }
 }
 
@@ -800,6 +788,19 @@  int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
     return 0;
 }
 
+typedef struct CoroutineIOCompletion {
+    Coroutine *coroutine;
+    int ret;
+} CoroutineIOCompletion;
+
+static void bdrv_co_io_em_complete(void *opaque, int ret)
+{
+    CoroutineIOCompletion *co = opaque;
+
+    co->ret = ret;
+    qemu_coroutine_enter(co->coroutine, NULL);
+}
+
 static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
                                            uint64_t offset, uint64_t bytes,
                                            QEMUIOVector *qiov, int flags)
@@ -812,7 +813,23 @@  static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
     assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
     assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
 
-    return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+    if (drv->bdrv_co_readv) {
+        return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+    } else {
+        BlockAIOCB *acb;
+        CoroutineIOCompletion co = {
+            .coroutine = qemu_coroutine_self(),
+        };
+
+        acb = bs->drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
+                                      bdrv_co_io_em_complete, &co);
+        if (acb == NULL) {
+            return -EIO;
+        } else {
+            qemu_coroutine_yield();
+            return co.ret;
+        }
+    }
 }
 
 static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
@@ -831,9 +848,23 @@  static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
     if (drv->bdrv_co_writev_flags) {
         ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
                                         flags);
-    } else {
+    } else if (drv->bdrv_co_writev) {
         assert(drv->supported_write_flags == 0);
         ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+    } else {
+        BlockAIOCB *acb;
+        CoroutineIOCompletion co = {
+            .coroutine = qemu_coroutine_self(),
+        };
+
+        acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
+                                       bdrv_co_io_em_complete, &co);
+        if (acb == NULL) {
+            return -EIO;
+        } else {
+            qemu_coroutine_yield();
+            return co.ret;
+        }
     }
 
     if (ret == 0 && (flags & BDRV_REQ_FUA) &&
@@ -2351,59 +2382,6 @@  void qemu_aio_unref(void *p)
 /**************************************************************/
 /* Coroutine block device emulation */
 
-typedef struct CoroutineIOCompletion {
-    Coroutine *coroutine;
-    int ret;
-} CoroutineIOCompletion;
-
-static void bdrv_co_io_em_complete(void *opaque, int ret)
-{
-    CoroutineIOCompletion *co = opaque;
-
-    co->ret = ret;
-    qemu_coroutine_enter(co->coroutine, NULL);
-}
-
-static int coroutine_fn bdrv_co_io_em(BlockDriverState *bs, int64_t sector_num,
-                                      int nb_sectors, QEMUIOVector *iov,
-                                      bool is_write)
-{
-    CoroutineIOCompletion co = {
-        .coroutine = qemu_coroutine_self(),
-    };
-    BlockAIOCB *acb;
-
-    if (is_write) {
-        acb = bs->drv->bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
-                                       bdrv_co_io_em_complete, &co);
-    } else {
-        acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
-                                      bdrv_co_io_em_complete, &co);
-    }
-
-    trace_bdrv_co_io_em(bs, sector_num, nb_sectors, is_write, acb);
-    if (!acb) {
-        return -EIO;
-    }
-    qemu_coroutine_yield();
-
-    return co.ret;
-}
-
-static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
-                                         int64_t sector_num, int nb_sectors,
-                                         QEMUIOVector *iov)
-{
-    return bdrv_co_io_em(bs, sector_num, nb_sectors, iov, false);
-}
-
-static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
-                                         int64_t sector_num, int nb_sectors,
-                                         QEMUIOVector *iov)
-{
-    return bdrv_co_io_em(bs, sector_num, nb_sectors, iov, true);
-}
-
 static void coroutine_fn bdrv_flush_co_entry(void *opaque)
 {
     RwCo *rwco = opaque;
diff --git a/trace-events b/trace-events
index 8350743..b4acd2a 100644
--- a/trace-events
+++ b/trace-events
@@ -74,7 +74,6 @@  bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector
 bdrv_co_readv_no_serialising(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x"
-bdrv_co_io_em(void *bs, int64_t sector_num, int nb_sectors, int is_write, void *acb) "bs %p sector_num %"PRId64" nb_sectors %d is_write %d acb %p"
 bdrv_co_do_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d"
 
 # block/stream.c