Patchwork [RFC,04/17] qed: implement bdrv_aio_discard

login
register
mail settings
Submitter Paolo Bonzini
Date March 8, 2012, 5:15 p.m.
Message ID <1331226917-6658-5-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/145598/
State New
Headers show

Comments

Paolo Bonzini - March 8, 2012, 5:15 p.m.
Move the bdrv_co_write_zeroes operation to bdrv_aio_discard, so that
the ugly coroutine loop can be eliminated.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/qed.c |   54 ++++++++++++++++++------------------------------------
 1 files changed, 18 insertions(+), 36 deletions(-)
Kevin Wolf - March 9, 2012, 4:31 p.m.
Am 08.03.2012 18:15, schrieb Paolo Bonzini:
> Move the bdrv_co_write_zeroes operation to bdrv_aio_discard, so that
> the ugly coroutine loop can be eliminated.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

But the logically correct (and therefore easier to understand) way would
be to let bdrv_aio_discard call bdrv_co_write_zeroes instead of the
other way round. Mapping write_zeroes to discard is only correct as long
as QED implements discard as zero writes instead of really discarding data.

Kevin
Paolo Bonzini - March 9, 2012, 5:53 p.m.
Il 09/03/2012 17:31, Kevin Wolf ha scritto:
>> > Move the bdrv_co_write_zeroes operation to bdrv_aio_discard, so that
>> > the ugly coroutine loop can be eliminated.
>> > 
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> But the logically correct (and therefore easier to understand) way would
> be to let bdrv_aio_discard call bdrv_co_write_zeroes instead of the
> other way round. Mapping write_zeroes to discard is only correct as long
> as QED implements discard as zero writes instead of really discarding data.

Well, bdrv_co_write_zeroes will disappear soon.

Paolo

Patch

diff --git a/block/qed.c b/block/qed.c
index 4f3d88d..9944be5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1374,6 +1374,20 @@  static BlockDriverAIOCB *bdrv_qed_aio_writev(BlockDriverState *bs,
                          opaque, QED_AIOCB_WRITE);
 }
 
+static BlockDriverAIOCB *bdrv_qed_aio_discard(BlockDriverState *bs,
+                                              int64_t sector_num,
+                                              int nb_sectors,
+                                              BlockDriverCompletionFunc *cb,
+                                              void *opaque)
+{
+    /* Zero writes start without an I/O buffer.  If a buffer becomes necessary
+     * then it will be allocated during request processing.
+     */
+    return qed_aio_setup(bs, sector_num, NULL, nb_sectors,
+                         cb, opaque,
+                         QED_AIOCB_WRITE | QED_AIOCB_ZERO);
+}
+
 static BlockDriverAIOCB *bdrv_qed_aio_flush(BlockDriverState *bs,
                                             BlockDriverCompletionFunc *cb,
                                             void *opaque)
@@ -1381,45 +1395,11 @@  static BlockDriverAIOCB *bdrv_qed_aio_flush(BlockDriverState *bs,
     return bdrv_aio_flush(bs->file, cb, opaque);
 }
 
-typedef struct {
-    Coroutine *co;
-    int ret;
-    bool done;
-} QEDWriteZeroesCB;
-
-static void coroutine_fn qed_co_write_zeroes_cb(void *opaque, int ret)
-{
-    QEDWriteZeroesCB *cb = opaque;
-
-    cb->done = true;
-    cb->ret = ret;
-    if (cb->co) {
-        qemu_coroutine_enter(cb->co, NULL);
-    }
-}
-
 static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs,
                                                  int64_t sector_num,
                                                  int nb_sectors)
 {
-    BlockDriverAIOCB *blockacb;
-    QEDWriteZeroesCB cb = { .done = false };
-
-    /* Zero writes start without an I/O buffer.  If a buffer becomes necessary
-     * then it will be allocated during request processing.
-     */
-    blockacb = qed_aio_setup(bs, sector_num, NULL, nb_sectors,
-                             qed_co_write_zeroes_cb, &cb,
-                             QED_AIOCB_WRITE | QED_AIOCB_ZERO);
-    if (!blockacb) {
-        return -EIO;
-    }
-    if (!cb.done) {
-        cb.co = qemu_coroutine_self();
-        qemu_coroutine_yield();
-    }
-    assert(cb.done);
-    return cb.ret;
+    return bdrv_co_discard(bs, sector_num, nb_sectors);
 }
 
 static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset)
@@ -1457,8 +1437,9 @@  static int bdrv_qed_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     BDRVQEDState *s = bs->opaque;
 
-    memset(bdi, 0, sizeof(*bdi));
     bdi->cluster_size = s->header.cluster_size;
+    bdi->discard_zeroes_data = true;
+    bdi->discard_granularity = 1;
     return 0;
 }
 
@@ -1581,6 +1562,7 @@  static BlockDriver bdrv_qed = {
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
     .bdrv_aio_writev          = bdrv_qed_aio_writev,
     .bdrv_aio_flush           = bdrv_qed_aio_flush,
+    .bdrv_aio_discard         = bdrv_qed_aio_discard,
     .bdrv_co_write_zeroes     = bdrv_qed_co_write_zeroes,
     .bdrv_truncate            = bdrv_qed_truncate,
     .bdrv_getlength           = bdrv_qed_getlength,