qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series)
@ 2011-10-17 10:32 Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 1/3] block: unify flush implementations Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Paolo Bonzini @ 2011-10-17 10:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

This series, applying on top of block branch, enables drivers to use
coroutines for flush and discard.  I kept aio_discard after discussing
with Kevin since it should be useful not only for raw-posix-aio, but also
for the userspace iSCSI backend (and in general for backends relying on
an external library that is designed around aio).

BTW, with this patch we get "for free" the invariant that bdrv_aio_*
never returns a NULL acb (Stefan's patches already got to that point
for read/write, of course).

v1->v2:
	add bdrv_co_flush and bdrv_co_discard entry points

Paolo Bonzini (2):
  block: unify flush implementations
  block: add bdrv_co_discard and bdrv_aio_discard support

Stefan Hajnoczi (1):
  block: drop redundant bdrv_flush implementation

 block.c           |  258 +++++++++++++++++++++++++++++++++--------------------
 block.h           |    5 +
 block/blkdebug.c  |    6 --
 block/blkverify.c |    9 --
 block/qcow.c      |    6 --
 block/qcow2.c     |   19 ----
 block/qed.c       |    6 --
 block/raw-posix.c |   18 ----
 block/raw.c       |   23 ++---
 block_int.h       |   10 ++-
 trace-events      |    1 +
 11 files changed, 184 insertions(+), 177 deletions(-)

-- 
1.7.6

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

* [Qemu-devel] [PATCH 1/3] block: unify flush implementations
  2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
@ 2011-10-17 10:32 ` Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 2/3] block: drop redundant bdrv_flush implementation Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2011-10-17 10:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

Add coroutine support for flush and apply the same emulation that
we already do for read/write.  bdrv_aio_flush is simplified to always
go through a coroutine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c     |  164 ++++++++++++++++++++++++++--------------------------------
 block.h     |    1 +
 block_int.h |    1 +
 3 files changed, 76 insertions(+), 90 deletions(-)

diff --git a/block.c b/block.c
index 7184a0f..7b8b14d 100644
--- a/block.c
+++ b/block.c
@@ -53,17 +53,12 @@ static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
 static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
-static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
-        BlockDriverCompletionFunc *cb, void *opaque);
-static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
-        BlockDriverCompletionFunc *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 int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs);
 static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
 static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
@@ -203,9 +198,6 @@ void bdrv_register(BlockDriver *bdrv)
         }
     }
 
-    if (!bdrv->bdrv_aio_flush)
-        bdrv->bdrv_aio_flush = bdrv_aio_flush_em;
-
     QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
 }
 
@@ -1027,11 +1019,6 @@ static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
                                    nb_sectors * BDRV_SECTOR_SIZE);
 }
 
-static inline bool bdrv_has_async_flush(BlockDriver *drv)
-{
-    return drv->bdrv_aio_flush != bdrv_aio_flush_em;
-}
-
 typedef struct RwCo {
     BlockDriverState *bs;
     int64_t sector_num;
@@ -1759,33 +1746,6 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
     return bs->device_name;
 }
 
-int bdrv_flush(BlockDriverState *bs)
-{
-    if (bs->open_flags & BDRV_O_NO_FLUSH) {
-        return 0;
-    }
-
-    if (bs->drv && bdrv_has_async_flush(bs->drv) && qemu_in_coroutine()) {
-        return bdrv_co_flush_em(bs);
-    }
-
-    if (bs->drv && bs->drv->bdrv_flush) {
-        return bs->drv->bdrv_flush(bs);
-    }
-
-    /*
-     * Some block drivers always operate in either writethrough or unsafe mode
-     * and don't support bdrv_flush therefore. Usually qemu doesn't know how
-     * the server works (because the behaviour is hardcoded or depends on
-     * server-side configuration), so we can't ensure that everything is safe
-     * on disk. Returning an error doesn't work because that would break guests
-     * even if the server operates in writethrough mode.
-     *
-     * Let's hope the user knows what he's doing.
-     */
-    return 0;
-}
-
 void bdrv_flush_all(void)
 {
     BlockDriverState *bs;
@@ -2610,22 +2570,6 @@ fail:
     return -1;
 }
 
-BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
-        BlockDriverCompletionFunc *cb, void *opaque)
-{
-    BlockDriver *drv = bs->drv;
-
-    trace_bdrv_aio_flush(bs, opaque);
-
-    if (bs->open_flags & BDRV_O_NO_FLUSH) {
-        return bdrv_aio_noop_em(bs, cb, opaque);
-    }
-
-    if (!drv)
-        return NULL;
-    return drv->bdrv_aio_flush(bs, cb, opaque);
-}
-
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 {
     acb->pool->cancel(acb);
@@ -2785,41 +2729,28 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
     return &acb->common;
 }
 
-static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
-        BlockDriverCompletionFunc *cb, void *opaque)
+static void coroutine_fn bdrv_aio_flush_co_entry(void *opaque)
 {
-    BlockDriverAIOCBSync *acb;
-
-    acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
-    acb->is_write = 1; /* don't bounce in the completion hadler */
-    acb->qiov = NULL;
-    acb->bounce = NULL;
-    acb->ret = 0;
-
-    if (!acb->bh)
-        acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+    BlockDriverAIOCBCoroutine *acb = opaque;
+    BlockDriverState *bs = acb->common.bs;
 
-    bdrv_flush(bs);
+    acb->req.error = bdrv_co_flush(bs);
+    acb->bh = qemu_bh_new(bdrv_co_em_bh, acb);
     qemu_bh_schedule(acb->bh);
-    return &acb->common;
 }
 
-static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
-    BlockDriverAIOCBSync *acb;
+    trace_bdrv_aio_flush(bs, opaque);
 
-    acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
-    acb->is_write = 1; /* don't bounce in the completion handler */
-    acb->qiov = NULL;
-    acb->bounce = NULL;
-    acb->ret = 0;
+    Coroutine *co;
+    BlockDriverAIOCBCoroutine *acb;
 
-    if (!acb->bh) {
-        acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
-    }
+    acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
+    co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
+    qemu_coroutine_enter(co, acb);
 
-    qemu_bh_schedule(acb->bh);
     return &acb->common;
 }
 
@@ -2916,19 +2847,72 @@ static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
     return bdrv_co_io_em(bs, sector_num, nb_sectors, iov, true);
 }
 
-static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs)
+static void coroutine_fn bdrv_flush_co_entry(void *opaque)
 {
-    CoroutineIOCompletion co = {
-        .coroutine = qemu_coroutine_self(),
+    RwCo *rwco = opaque;
+
+    rwco->ret = bdrv_co_flush(rwco->bs);
+}
+
+int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
+{
+    if (bs->open_flags & BDRV_O_NO_FLUSH) {
+        return 0;
+    } else if (!bs->drv) {
+        return 0;
+    } else if (bs->drv->bdrv_co_flush) {
+        return bs->drv->bdrv_co_flush(bs);
+    } else if (bs->drv->bdrv_aio_flush) {
+        BlockDriverAIOCB *acb;
+        CoroutineIOCompletion co = {
+            .coroutine = qemu_coroutine_self(),
+        };
+
+        acb = bs->drv->bdrv_aio_flush(bs, bdrv_co_io_em_complete, &co);
+        if (acb == NULL) {
+            return -EIO;
+        } else {
+            qemu_coroutine_yield();
+            return co.ret;
+        }
+    } else if (bs->drv->bdrv_flush) {
+        return bs->drv->bdrv_flush(bs);
+    } else {
+        /*
+         * Some block drivers always operate in either writethrough or unsafe
+         * mode and don't support bdrv_flush therefore. Usually qemu doesn't
+         * know how the server works (because the behaviour is hardcoded or
+         * depends on server-side configuration), so we can't ensure that
+         * everything is safe on disk. Returning an error doesn't work because
+         * that would break guests even if the server operates in writethrough
+         * mode.
+         *
+         * Let's hope the user knows what he's doing.
+         */
+        return 0;
+    }
+}
+
+int bdrv_flush(BlockDriverState *bs)
+{
+    Coroutine *co;
+    RwCo rwco = {
+        .bs = bs,
+        .ret = NOT_DONE,
     };
-    BlockDriverAIOCB *acb;
 
-    acb = bdrv_aio_flush(bs, bdrv_co_io_em_complete, &co);
-    if (!acb) {
-        return -EIO;
+    if (qemu_in_coroutine()) {
+        /* Fast-path if already in coroutine context */
+        bdrv_flush_co_entry(&rwco);
+    } else {
+        co = qemu_coroutine_create(bdrv_flush_co_entry);
+        qemu_coroutine_enter(co, &rwco);
+        while (rwco.ret == NOT_DONE) {
+            qemu_aio_wait();
+        }
     }
-    qemu_coroutine_yield();
-    return co.ret;
+
+    return rwco.ret;
 }
 
 /**************************************************************/
diff --git a/block.h b/block.h
index e77988e..65c5166 100644
--- a/block.h
+++ b/block.h
@@ -191,6 +191,7 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
+int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
 void bdrv_flush_all(void);
 void bdrv_close_all(void);
 
diff --git a/block_int.h b/block_int.h
index f2f4f2d..9cb536d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -83,6 +83,7 @@ struct BlockDriver {
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+    int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
 
     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
         int num_reqs);
-- 
1.7.6

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

* [Qemu-devel] [PATCH 2/3] block: drop redundant bdrv_flush implementation
  2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 1/3] block: unify flush implementations Paolo Bonzini
@ 2011-10-17 10:32 ` Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 3/3] block: add bdrv_co_discard and bdrv_aio_discard support Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2011-10-17 10:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, Stefan Hajnoczi

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Block drivers now only need to provide either of .bdrv_co_flush,
.bdrv_aio_flush() or for legacy drivers .bdrv_flush().  Remove
the redundant .bdrv_flush() implementations.

[Paolo Bonzini: change raw driver to bdrv_co_flush]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c  |    6 ------
 block/blkverify.c |    9 ---------
 block/qcow.c      |    6 ------
 block/qcow2.c     |   19 -------------------
 block/qed.c       |    6 ------
 block/raw-posix.c |   18 ------------------
 block/raw.c       |   13 +++----------
 7 files changed, 3 insertions(+), 74 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index b3c5d42..9b88535 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -397,11 +397,6 @@ static void blkdebug_close(BlockDriverState *bs)
     }
 }
 
-static int blkdebug_flush(BlockDriverState *bs)
-{
-    return bdrv_flush(bs->file);
-}
-
 static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque)
 {
@@ -454,7 +449,6 @@ static BlockDriver bdrv_blkdebug = {
 
     .bdrv_file_open     = blkdebug_open,
     .bdrv_close         = blkdebug_close,
-    .bdrv_flush         = blkdebug_flush,
 
     .bdrv_aio_readv     = blkdebug_aio_readv,
     .bdrv_aio_writev    = blkdebug_aio_writev,
diff --git a/block/blkverify.c b/block/blkverify.c
index c7522b4..483f3b3 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,14 +116,6 @@ static void blkverify_close(BlockDriverState *bs)
     s->test_file = NULL;
 }
 
-static int blkverify_flush(BlockDriverState *bs)
-{
-    BDRVBlkverifyState *s = bs->opaque;
-
-    /* Only flush test file, the raw file is not important */
-    return bdrv_flush(s->test_file);
-}
-
 static int64_t blkverify_getlength(BlockDriverState *bs)
 {
     BDRVBlkverifyState *s = bs->opaque;
@@ -368,7 +360,6 @@ static BlockDriver bdrv_blkverify = {
 
     .bdrv_file_open     = blkverify_open,
     .bdrv_close         = blkverify_close,
-    .bdrv_flush         = blkverify_flush,
 
     .bdrv_aio_readv     = blkverify_aio_readv,
     .bdrv_aio_writev    = blkverify_aio_writev,
diff --git a/block/qcow.c b/block/qcow.c
index c8bfecc..9b71116 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -781,11 +781,6 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static int qcow_flush(BlockDriverState *bs)
-{
-    return bdrv_flush(bs->file);
-}
-
 static BlockDriverAIOCB *qcow_aio_flush(BlockDriverState *bs,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
@@ -826,7 +821,6 @@ static BlockDriver bdrv_qcow = {
     .bdrv_open		= qcow_open,
     .bdrv_close		= qcow_close,
     .bdrv_create	= qcow_create,
-    .bdrv_flush		= qcow_flush,
     .bdrv_is_allocated	= qcow_is_allocated,
     .bdrv_set_key	= qcow_set_key,
     .bdrv_make_empty	= qcow_make_empty,
diff --git a/block/qcow2.c b/block/qcow2.c
index 510ff68..4dc980c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1092,24 +1092,6 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static int qcow2_flush(BlockDriverState *bs)
-{
-    BDRVQcowState *s = bs->opaque;
-    int ret;
-
-    ret = qcow2_cache_flush(bs, s->l2_table_cache);
-    if (ret < 0) {
-        return ret;
-    }
-
-    ret = qcow2_cache_flush(bs, s->refcount_block_cache);
-    if (ret < 0) {
-        return ret;
-    }
-
-    return bdrv_flush(bs->file);
-}
-
 static BlockDriverAIOCB *qcow2_aio_flush(BlockDriverState *bs,
                                          BlockDriverCompletionFunc *cb,
                                          void *opaque)
@@ -1242,7 +1224,6 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_open          = qcow2_open,
     .bdrv_close         = qcow2_close,
     .bdrv_create        = qcow2_create,
-    .bdrv_flush         = qcow2_flush,
     .bdrv_is_allocated  = qcow2_is_allocated,
     .bdrv_set_key       = qcow2_set_key,
     .bdrv_make_empty    = qcow2_make_empty,
diff --git a/block/qed.c b/block/qed.c
index e87dc4d..2e06992 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -533,11 +533,6 @@ static void bdrv_qed_close(BlockDriverState *bs)
     qemu_vfree(s->l1_table);
 }
 
-static int bdrv_qed_flush(BlockDriverState *bs)
-{
-    return bdrv_flush(bs->file);
-}
-
 static int qed_create(const char *filename, uint32_t cluster_size,
                       uint64_t image_size, uint32_t table_size,
                       const char *backing_file, const char *backing_fmt)
@@ -1479,7 +1474,6 @@ static BlockDriver bdrv_qed = {
     .bdrv_open                = bdrv_qed_open,
     .bdrv_close               = bdrv_qed_close,
     .bdrv_create              = bdrv_qed_create,
-    .bdrv_flush               = bdrv_qed_flush,
     .bdrv_is_allocated        = bdrv_qed_is_allocated,
     .bdrv_make_empty          = bdrv_qed_make_empty,
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index c7f5544..afcb4c1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -583,19 +583,6 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
     return result;
 }
 
-static int raw_flush(BlockDriverState *bs)
-{
-    BDRVRawState *s = bs->opaque;
-    int ret;
-
-    ret = qemu_fdatasync(s->fd);
-    if (ret < 0) {
-        return -errno;
-    }
-
-    return 0;
-}
-
 #ifdef CONFIG_XFS
 static int xfs_discard(BDRVRawState *s, int64_t sector_num, int nb_sectors)
 {
@@ -645,7 +632,6 @@ static BlockDriver bdrv_file = {
     .bdrv_file_open = raw_open,
     .bdrv_close = raw_close,
     .bdrv_create = raw_create,
-    .bdrv_flush = raw_flush,
     .bdrv_discard = raw_discard,
 
     .bdrv_aio_readv = raw_aio_readv,
@@ -915,7 +901,6 @@ static BlockDriver bdrv_host_device = {
     .bdrv_create        = hdev_create,
     .create_options     = raw_create_options,
     .bdrv_has_zero_init = hdev_has_zero_init,
-    .bdrv_flush         = raw_flush,
 
     .bdrv_aio_readv	= raw_aio_readv,
     .bdrv_aio_writev	= raw_aio_writev,
@@ -1035,7 +1020,6 @@ static BlockDriver bdrv_host_floppy = {
     .bdrv_create        = hdev_create,
     .create_options     = raw_create_options,
     .bdrv_has_zero_init = hdev_has_zero_init,
-    .bdrv_flush         = raw_flush,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
@@ -1135,7 +1119,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_create        = hdev_create,
     .create_options     = raw_create_options,
     .bdrv_has_zero_init = hdev_has_zero_init,
-    .bdrv_flush         = raw_flush,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
@@ -1255,7 +1238,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_create        = hdev_create,
     .create_options     = raw_create_options,
     .bdrv_has_zero_init = hdev_has_zero_init,
-    .bdrv_flush         = raw_flush,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
diff --git a/block/raw.c b/block/raw.c
index 5ca606b..39a3a9a 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -25,15 +25,9 @@ static void raw_close(BlockDriverState *bs)
 {
 }
 
-static int raw_flush(BlockDriverState *bs)
+static int coroutine_fn raw_co_flush(BlockDriverState *bs)
 {
-    return bdrv_flush(bs->file);
-}
-
-static BlockDriverAIOCB *raw_aio_flush(BlockDriverState *bs,
-    BlockDriverCompletionFunc *cb, void *opaque)
-{
-    return bdrv_aio_flush(bs->file, cb, opaque);
+    return bdrv_co_flush(bs->file);
 }
 
 static int64_t raw_getlength(BlockDriverState *bs)
@@ -117,12 +111,11 @@ static BlockDriver bdrv_raw = {
     .bdrv_close         = raw_close,
     .bdrv_co_readv      = raw_co_readv,
     .bdrv_co_writev     = raw_co_writev,
-    .bdrv_flush         = raw_flush,
+    .bdrv_co_flush      = raw_co_flush,
     .bdrv_probe         = raw_probe,
     .bdrv_getlength     = raw_getlength,
     .bdrv_truncate      = raw_truncate,
 
-    .bdrv_aio_flush     = raw_aio_flush,
     .bdrv_discard       = raw_discard,
 
     .bdrv_is_inserted   = raw_is_inserted,
-- 
1.7.6

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

* [Qemu-devel] [PATCH 3/3] block: add bdrv_co_discard and bdrv_aio_discard support
  2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 1/3] block: unify flush implementations Paolo Bonzini
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 2/3] block: drop redundant bdrv_flush implementation Paolo Bonzini
@ 2011-10-17 10:32 ` Paolo Bonzini
  2011-10-17 10:43 ` [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Kevin Wolf
  2011-10-17 10:57 ` Kevin Wolf
  4 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2011-10-17 10:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

This similarly adds support for coroutine and asynchronous discard.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c      |  102 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 block.h      |    4 ++
 block/raw.c  |   10 +++--
 block_int.h  |    9 ++++-
 trace-events |    1 +
 5 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 7b8b14d..28508f2 100644
--- a/block.c
+++ b/block.c
@@ -1768,17 +1768,6 @@ int bdrv_has_zero_init(BlockDriverState *bs)
     return 1;
 }
 
-int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
-{
-    if (!bs->drv) {
-        return -ENOMEDIUM;
-    }
-    if (!bs->drv->bdrv_discard) {
-        return 0;
-    }
-    return bs->drv->bdrv_discard(bs, sector_num, nb_sectors);
-}
-
 /*
  * Returns true iff the specified sector is present in the disk image. Drivers
  * not implementing the functionality are assumed to not support backing files,
@@ -2754,6 +2743,34 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
     return &acb->common;
 }
 
+static void coroutine_fn bdrv_aio_discard_co_entry(void *opaque)
+{
+    BlockDriverAIOCBCoroutine *acb = opaque;
+    BlockDriverState *bs = acb->common.bs;
+
+    acb->req.error = bdrv_co_discard(bs, acb->req.sector, acb->req.nb_sectors);
+    acb->bh = qemu_bh_new(bdrv_co_em_bh, acb);
+    qemu_bh_schedule(acb->bh);
+}
+
+BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    Coroutine *co;
+    BlockDriverAIOCBCoroutine *acb;
+
+    trace_bdrv_aio_discard(bs, sector_num, nb_sectors, opaque);
+
+    acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
+    acb->req.sector = sector_num;
+    acb->req.nb_sectors = nb_sectors;
+    co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
+    qemu_coroutine_enter(co, acb);
+
+    return &acb->common;
+}
+
 void bdrv_init(void)
 {
     module_call_init(MODULE_INIT_BLOCK);
@@ -2915,6 +2932,69 @@ int bdrv_flush(BlockDriverState *bs)
     return rwco.ret;
 }
 
+static void coroutine_fn bdrv_discard_co_entry(void *opaque)
+{
+    RwCo *rwco = opaque;
+
+    rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors);
+}
+
+int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
+                                 int nb_sectors)
+{
+    if (!bs->drv) {
+        return -ENOMEDIUM;
+    } else if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+        return -EIO;
+    } else if (bs->read_only) {
+        return -EROFS;
+    } else if (bs->drv->bdrv_co_discard) {
+        return bs->drv->bdrv_co_discard(bs, sector_num, nb_sectors);
+    } else if (bs->drv->bdrv_aio_discard) {
+        BlockDriverAIOCB *acb;
+        CoroutineIOCompletion co = {
+            .coroutine = qemu_coroutine_self(),
+        };
+
+        acb = bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors,
+                                        bdrv_co_io_em_complete, &co);
+        if (acb == NULL) {
+            return -EIO;
+        } else {
+            qemu_coroutine_yield();
+            return co.ret;
+        }
+    } else if (bs->drv->bdrv_discard) {
+        return bs->drv->bdrv_discard(bs, sector_num, nb_sectors);
+    } else {
+        return 0;
+    }
+}
+
+int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+{
+    Coroutine *co;
+    RwCo rwco = {
+        .bs = bs,
+        .sector_num = sector_num,
+        .nb_sectors = nb_sectors,
+        .ret = NOT_DONE,
+    };
+
+    if (qemu_in_coroutine()) {
+        /* Fast-path if already in coroutine context */
+        bdrv_discard_co_entry(&rwco);
+    } else {
+        co = qemu_coroutine_create(bdrv_discard_co_entry);
+        qemu_coroutine_enter(co, &rwco);
+        while (rwco.ret == NOT_DONE) {
+            qemu_aio_wait();
+        }
+    }
+
+    return rwco.ret;
+}
+
 /**************************************************************/
 /* removable device support */
 
diff --git a/block.h b/block.h
index 65c5166..5a042c9 100644
--- a/block.h
+++ b/block.h
@@ -166,6 +166,9 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
                                   BlockDriverCompletionFunc *cb, void *opaque);
 BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
                                  BlockDriverCompletionFunc *cb, void *opaque);
+BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
+                                   int64_t sector_num, int nb_sectors,
+                                   BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
 
 typedef struct BlockRequest {
@@ -196,6 +199,7 @@ void bdrv_flush_all(void);
 void bdrv_close_all(void);
 
 int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
+int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
diff --git a/block/raw.c b/block/raw.c
index 39a3a9a..33cc471 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -45,9 +45,10 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
    return 1; /* everything can be opened as raw image */
 }
 
-static int raw_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+static int coroutine_fn raw_co_discard(BlockDriverState *bs,
+                                       int64_t sector_num, int nb_sectors)
 {
-    return bdrv_discard(bs->file, sector_num, nb_sectors);
+    return bdrv_co_discard(bs->file, sector_num, nb_sectors);
 }
 
 static int raw_is_inserted(BlockDriverState *bs)
@@ -109,15 +110,16 @@ static BlockDriver bdrv_raw = {
 
     .bdrv_open          = raw_open,
     .bdrv_close         = raw_close,
+
     .bdrv_co_readv      = raw_co_readv,
     .bdrv_co_writev     = raw_co_writev,
     .bdrv_co_flush      = raw_co_flush,
+    .bdrv_co_discard    = raw_co_discard,
+
     .bdrv_probe         = raw_probe,
     .bdrv_getlength     = raw_getlength,
     .bdrv_truncate      = raw_truncate,
 
-    .bdrv_discard       = raw_discard,
-
     .bdrv_is_inserted   = raw_is_inserted,
     .bdrv_media_changed = raw_media_changed,
     .bdrv_eject         = raw_eject,
diff --git a/block_int.h b/block_int.h
index 9cb536d..384598f 100644
--- a/block_int.h
+++ b/block_int.h
@@ -63,6 +63,8 @@ struct BlockDriver {
     void (*bdrv_close)(BlockDriverState *bs);
     int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
     int (*bdrv_flush)(BlockDriverState *bs);
+    int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
+                        int nb_sectors);
     int (*bdrv_is_allocated)(BlockDriverState *bs, int64_t sector_num,
                              int nb_sectors, int *pnum);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
@@ -76,14 +78,17 @@ struct BlockDriver {
         BlockDriverCompletionFunc *cb, void *opaque);
     BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
         BlockDriverCompletionFunc *cb, void *opaque);
-    int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
-                        int nb_sectors);
+    BlockDriverAIOCB *(*bdrv_aio_discard)(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors,
+        BlockDriverCompletionFunc *cb, void *opaque);
 
     int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
+    int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors);
 
     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
         int num_reqs);
diff --git a/trace-events b/trace-events
index 63d8c8e..e6160ad 100644
--- a/trace-events
+++ b/trace-events
@@ -61,6 +61,7 @@ multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
 bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"
 bdrv_aio_multiwrite_earlyfail(void *mcb) "mcb %p"
 bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
+bdrv_aio_discard(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
 bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
 bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
 bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
-- 
1.7.6

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

* Re: [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series)
  2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
                   ` (2 preceding siblings ...)
  2011-10-17 10:32 ` [Qemu-devel] [PATCH 3/3] block: add bdrv_co_discard and bdrv_aio_discard support Paolo Bonzini
@ 2011-10-17 10:43 ` Kevin Wolf
  2011-10-17 10:43   ` Paolo Bonzini
  2011-10-17 10:57 ` Kevin Wolf
  4 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2011-10-17 10:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Am 17.10.2011 12:32, schrieb Paolo Bonzini:
> This series, applying on top of block branch, enables drivers to use
> coroutines for flush and discard.  I kept aio_discard after discussing
> with Kevin since it should be useful not only for raw-posix-aio, but also
> for the userspace iSCSI backend (and in general for backends relying on
> an external library that is designed around aio).
> 
> BTW, with this patch we get "for free" the invariant that bdrv_aio_*
> never returns a NULL acb (Stefan's patches already got to that point
> for read/write, of course).

Cool, I wasn't aware of that. That's a very nice side effect!

Maybe we should write this down in a comment and remove the now
unnecessary error handling from callers.

Kevin

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

* Re: [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series)
  2011-10-17 10:43 ` [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Kevin Wolf
@ 2011-10-17 10:43   ` Paolo Bonzini
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2011-10-17 10:43 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel

On 10/17/2011 12:43 PM, Kevin Wolf wrote:
> Cool, I wasn't aware of that. That's a very nice side effect!
>
> Maybe we should write this down in a comment and remove the now
> unnecessary error handling from callers.

Looks like I finally have an excuse to play with Coccinelle!

Paolo

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

* Re: [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series)
  2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
                   ` (3 preceding siblings ...)
  2011-10-17 10:43 ` [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Kevin Wolf
@ 2011-10-17 10:57 ` Kevin Wolf
  4 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2011-10-17 10:57 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Am 17.10.2011 12:32, schrieb Paolo Bonzini:
> This series, applying on top of block branch, enables drivers to use
> coroutines for flush and discard.  I kept aio_discard after discussing
> with Kevin since it should be useful not only for raw-posix-aio, but also
> for the userspace iSCSI backend (and in general for backends relying on
> an external library that is designed around aio).
> 
> BTW, with this patch we get "for free" the invariant that bdrv_aio_*
> never returns a NULL acb (Stefan's patches already got to that point
> for read/write, of course).
> 
> v1->v2:
> 	add bdrv_co_flush and bdrv_co_discard entry points
> 
> Paolo Bonzini (2):
>   block: unify flush implementations
>   block: add bdrv_co_discard and bdrv_aio_discard support
> 
> Stefan Hajnoczi (1):
>   block: drop redundant bdrv_flush implementation
> 
>  block.c           |  258 +++++++++++++++++++++++++++++++++--------------------
>  block.h           |    5 +
>  block/blkdebug.c  |    6 --
>  block/blkverify.c |    9 --
>  block/qcow.c      |    6 --
>  block/qcow2.c     |   19 ----
>  block/qed.c       |    6 --
>  block/raw-posix.c |   18 ----
>  block/raw.c       |   23 ++---
>  block_int.h       |   10 ++-
>  trace-events      |    1 +
>  11 files changed, 184 insertions(+), 177 deletions(-)
> 

Thanks, applied all to the block branch.

Kevin

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

end of thread, other threads:[~2011-10-17 10:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-17 10:32 [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Paolo Bonzini
2011-10-17 10:32 ` [Qemu-devel] [PATCH 1/3] block: unify flush implementations Paolo Bonzini
2011-10-17 10:32 ` [Qemu-devel] [PATCH 2/3] block: drop redundant bdrv_flush implementation Paolo Bonzini
2011-10-17 10:32 ` [Qemu-devel] [PATCH 3/3] block: add bdrv_co_discard and bdrv_aio_discard support Paolo Bonzini
2011-10-17 10:43 ` [Qemu-devel] [PATCH v2 0/3] coroutinization of flush and discard (split out of NBD series) Kevin Wolf
2011-10-17 10:43   ` Paolo Bonzini
2011-10-17 10:57 ` 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).