[Qemu-devel] [PATCH 0/5] block: remove unused emulation and synchronous functions

[Qemu-devel] [PATCH 0/5] block: remove unused emulation and synchronous functions

[Stefan Hajnoczi]

Now that the block layer processes requests in coroutine context, some of the
emulation wrappers and duplicate code paths can be dropped.  Paraphrasing a
wise man, Arnold Schwarzenegger, "I will go to the block layer and I will clean
house" :).

They key thing behind this series is that the block layer processes requests in
coroutine context and will try to use .brdv_co_readv()/.bdrv_co_writev() when
possible.  Should the BlockDriver not implement those interfaces, an emulation
function will be used to provide them using aio.  If the BlockDriver does not
implement aio interfaces, then an emulation function will be used to provide
them using synchronous I/O.

This means:

1. A BlockDriver that implements coroutine interfaces does not need to
   implement aio or synchronous interfaces.

2. A BlockDriver that implements aio interfaces does not need to implement
   synchronous interfaces.

3. Coroutine interfaces are preferred and do not require any emulation
   functions.

This patch series propagates these rules across existing BlockDrivers and
removes unused emulation functions from block.c.

Stefan Hajnoczi (5):
  block: drop emulation functions that use coroutines
  raw-posix: remove bdrv_read()/bdrv_write()
  block: use coroutine interface for raw format
  block: drop .bdrv_read()/.bdrv_write() emulation
  block: drop bdrv_has_async_rw()

 block.c           |  142 ++-------------------------
 block/raw-posix.c |  277 -----------------------------------------------------
 block/raw.c       |   32 ++-----
 3 files changed, 18 insertions(+), 433 deletions(-)

-- 
1.7.6.3

[Qemu-devel] [PATCH 1/5] block: drop emulation functions that use coroutines

[Stefan Hajnoczi]

Block drivers that implement coroutine functions used to get sync and
aio wrappers.  This is no longer necessary since all request processing
now happens in a coroutine.  If a block driver implements the coroutine
interface then none of the other interfaces will be invoked.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |   61 +++++++------------------------------------------------------
 1 files changed, 7 insertions(+), 54 deletions(-)

diff --git a/block.c b/block.c
index 8c7d05e..3d27bab 100644
--- a/block.c
+++ b/block.c
@@ -61,12 +61,6 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
                         uint8_t *buf, int nb_sectors);
 static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
                          const uint8_t *buf, int nb_sectors);
-static BlockDriverAIOCB *bdrv_co_aio_readv_em(BlockDriverState *bs,
-        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        BlockDriverCompletionFunc *cb, void *opaque);
-static BlockDriverAIOCB *bdrv_co_aio_writev_em(BlockDriverState *bs,
-        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        BlockDriverCompletionFunc *cb, void *opaque);
 static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
                                          int64_t sector_num, int nb_sectors,
                                          QEMUIOVector *iov);
@@ -84,8 +78,7 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
                                                int nb_sectors,
                                                BlockDriverCompletionFunc *cb,
                                                void *opaque,
-                                               bool is_write,
-                                               CoroutineEntry *entry);
+                                               bool is_write);
 static void coroutine_fn bdrv_co_do_rw(void *opaque);
 
 static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
@@ -199,13 +192,8 @@ void path_combine(char *dest, int dest_size,
 
 void bdrv_register(BlockDriver *bdrv)
 {
-    if (bdrv->bdrv_co_readv) {
-        /* Emulate AIO by coroutines, and sync by AIO */
-        bdrv->bdrv_aio_readv = bdrv_co_aio_readv_em;
-        bdrv->bdrv_aio_writev = bdrv_co_aio_writev_em;
-        bdrv->bdrv_read = bdrv_read_em;
-        bdrv->bdrv_write = bdrv_write_em;
-     } else {
+    /* 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;
 
@@ -2382,7 +2370,7 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
     trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
 
     return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors,
-                                 cb, opaque, false, bdrv_co_do_rw);
+                                 cb, opaque, false);
 }
 
 BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
@@ -2392,7 +2380,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
     trace_bdrv_aio_writev(bs, sector_num, nb_sectors, opaque);
 
     return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors,
-                                 cb, opaque, true, bdrv_co_do_rw);
+                                 cb, opaque, true);
 }
 
 
@@ -2767,24 +2755,6 @@ static void bdrv_co_rw_bh(void *opaque)
     qemu_aio_release(acb);
 }
 
-/* Invoke .bdrv_co_readv/.bdrv_co_writev */
-static void coroutine_fn bdrv_co_rw(void *opaque)
-{
-    BlockDriverAIOCBCoroutine *acb = opaque;
-    BlockDriverState *bs = acb->common.bs;
-
-    if (!acb->is_write) {
-        acb->req.error = bs->drv->bdrv_co_readv(bs, acb->req.sector,
-            acb->req.nb_sectors, acb->req.qiov);
-    } else {
-        acb->req.error = bs->drv->bdrv_co_writev(bs, acb->req.sector,
-            acb->req.nb_sectors, acb->req.qiov);
-    }
-
-    acb->bh = qemu_bh_new(bdrv_co_rw_bh, acb);
-    qemu_bh_schedule(acb->bh);
-}
-
 /* Invoke bdrv_co_do_readv/bdrv_co_do_writev */
 static void coroutine_fn bdrv_co_do_rw(void *opaque)
 {
@@ -2809,8 +2779,7 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
                                                int nb_sectors,
                                                BlockDriverCompletionFunc *cb,
                                                void *opaque,
-                                               bool is_write,
-                                               CoroutineEntry *entry)
+                                               bool is_write)
 {
     Coroutine *co;
     BlockDriverAIOCBCoroutine *acb;
@@ -2821,28 +2790,12 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
     acb->req.qiov = qiov;
     acb->is_write = is_write;
 
-    co = qemu_coroutine_create(entry);
+    co = qemu_coroutine_create(bdrv_co_do_rw);
     qemu_coroutine_enter(co, acb);
 
     return &acb->common;
 }
 
-static BlockDriverAIOCB *bdrv_co_aio_readv_em(BlockDriverState *bs,
-        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        BlockDriverCompletionFunc *cb, void *opaque)
-{
-    return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque,
-                                 false, bdrv_co_rw);
-}
-
-static BlockDriverAIOCB *bdrv_co_aio_writev_em(BlockDriverState *bs,
-        int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-        BlockDriverCompletionFunc *cb, void *opaque)
-{
-    return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque,
-                                 true, bdrv_co_rw);
-}
-
 static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
-- 
1.7.6.3

[Qemu-devel] [PATCH 2/5] raw-posix: remove bdrv_read()/bdrv_write()

[Stefan Hajnoczi]

Block drivers only need to provide one of sync, aio, or coroutine
interfaces.  Since raw-posix.c provides aio interfaces, simply drop the
synchronous interfaces since they can be emulated using aio and
coroutines.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/raw-posix.c |  277 -----------------------------------------------------
 1 files changed, 0 insertions(+), 277 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0b5e225b..c7f5544 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -297,273 +297,6 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
 */
 
 /*
- * offset and count are in bytes, but must be multiples of 512 for files
- * opened with O_DIRECT. buf must be aligned to 512 bytes then.
- *
- * This function may be called without alignment if the caller ensures
- * that O_DIRECT is not in effect.
- */
-static int raw_pread_aligned(BlockDriverState *bs, int64_t offset,
-                     uint8_t *buf, int count)
-{
-    BDRVRawState *s = bs->opaque;
-    int ret;
-
-    ret = fd_open(bs);
-    if (ret < 0)
-        return ret;
-
-    ret = pread(s->fd, buf, count, offset);
-    if (ret == count)
-        return ret;
-
-    /* Allow reads beyond the end (needed for pwrite) */
-    if ((ret == 0) && bs->growable) {
-        int64_t size = raw_getlength(bs);
-        if (offset >= size) {
-            memset(buf, 0, count);
-            return count;
-        }
-    }
-
-    DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
-                      "] read failed %d : %d = %s\n",
-                      s->fd, bs->filename, offset, buf, count,
-                      bs->total_sectors, ret, errno, strerror(errno));
-
-    /* Try harder for CDrom. */
-    if (s->type != FTYPE_FILE) {
-        ret = pread(s->fd, buf, count, offset);
-        if (ret == count)
-            return ret;
-        ret = pread(s->fd, buf, count, offset);
-        if (ret == count)
-            return ret;
-
-        DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
-                          "] retry read failed %d : %d = %s\n",
-                          s->fd, bs->filename, offset, buf, count,
-                          bs->total_sectors, ret, errno, strerror(errno));
-    }
-
-    return  (ret < 0) ? -errno : ret;
-}
-
-/*
- * offset and count are in bytes, but must be multiples of the sector size
- * for files opened with O_DIRECT. buf must be aligned to sector size bytes
- * then.
- *
- * This function may be called without alignment if the caller ensures
- * that O_DIRECT is not in effect.
- */
-static int raw_pwrite_aligned(BlockDriverState *bs, int64_t offset,
-                      const uint8_t *buf, int count)
-{
-    BDRVRawState *s = bs->opaque;
-    int ret;
-
-    ret = fd_open(bs);
-    if (ret < 0)
-        return -errno;
-
-    ret = pwrite(s->fd, buf, count, offset);
-    if (ret == count)
-        return ret;
-
-    DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
-                      "] write failed %d : %d = %s\n",
-                      s->fd, bs->filename, offset, buf, count,
-                      bs->total_sectors, ret, errno, strerror(errno));
-
-    return  (ret < 0) ? -errno : ret;
-}
-
-
-/*
- * offset and count are in bytes and possibly not aligned. For files opened
- * with O_DIRECT, necessary alignments are ensured before calling
- * raw_pread_aligned to do the actual read.
- */
-static int raw_pread(BlockDriverState *bs, int64_t offset,
-                     uint8_t *buf, int count)
-{
-    BDRVRawState *s = bs->opaque;
-    unsigned sector_mask = bs->buffer_alignment - 1;
-    int size, ret, shift, sum;
-
-    sum = 0;
-
-    if (s->aligned_buf != NULL)  {
-
-        if (offset & sector_mask) {
-            /* align offset on a sector size bytes boundary */
-
-            shift = offset & sector_mask;
-            size = (shift + count + sector_mask) & ~sector_mask;
-            if (size > s->aligned_buf_size)
-                size = s->aligned_buf_size;
-            ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf, size);
-            if (ret < 0)
-                return ret;
-
-            size = bs->buffer_alignment - shift;
-            if (size > count)
-                size = count;
-            memcpy(buf, s->aligned_buf + shift, size);
-
-            buf += size;
-            offset += size;
-            count -= size;
-            sum += size;
-
-            if (count == 0)
-                return sum;
-        }
-        if (count & sector_mask || (uintptr_t) buf & sector_mask) {
-
-            /* read on aligned buffer */
-
-            while (count) {
-
-                size = (count + sector_mask) & ~sector_mask;
-                if (size > s->aligned_buf_size)
-                    size = s->aligned_buf_size;
-
-                ret = raw_pread_aligned(bs, offset, s->aligned_buf, size);
-                if (ret < 0) {
-                    return ret;
-                } else if (ret == 0) {
-                    fprintf(stderr, "raw_pread: read beyond end of file\n");
-                    abort();
-                }
-
-                size = ret;
-                if (size > count)
-                    size = count;
-
-                memcpy(buf, s->aligned_buf, size);
-
-                buf += size;
-                offset += size;
-                count -= size;
-                sum += size;
-            }
-
-            return sum;
-        }
-    }
-
-    return raw_pread_aligned(bs, offset, buf, count) + sum;
-}
-
-static int raw_read(BlockDriverState *bs, int64_t sector_num,
-                    uint8_t *buf, int nb_sectors)
-{
-    int ret;
-
-    ret = raw_pread(bs, sector_num * BDRV_SECTOR_SIZE, buf,
-                    nb_sectors * BDRV_SECTOR_SIZE);
-    if (ret == (nb_sectors * BDRV_SECTOR_SIZE))
-        ret = 0;
-    return ret;
-}
-
-/*
- * offset and count are in bytes and possibly not aligned. For files opened
- * with O_DIRECT, necessary alignments are ensured before calling
- * raw_pwrite_aligned to do the actual write.
- */
-static int raw_pwrite(BlockDriverState *bs, int64_t offset,
-                      const uint8_t *buf, int count)
-{
-    BDRVRawState *s = bs->opaque;
-    unsigned sector_mask = bs->buffer_alignment - 1;
-    int size, ret, shift, sum;
-
-    sum = 0;
-
-    if (s->aligned_buf != NULL) {
-
-        if (offset & sector_mask) {
-            /* align offset on a sector size bytes boundary */
-            shift = offset & sector_mask;
-            ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf,
-                                    bs->buffer_alignment);
-            if (ret < 0)
-                return ret;
-
-            size = bs->buffer_alignment - shift;
-            if (size > count)
-                size = count;
-            memcpy(s->aligned_buf + shift, buf, size);
-
-            ret = raw_pwrite_aligned(bs, offset - shift, s->aligned_buf,
-                                     bs->buffer_alignment);
-            if (ret < 0)
-                return ret;
-
-            buf += size;
-            offset += size;
-            count -= size;
-            sum += size;
-
-            if (count == 0)
-                return sum;
-        }
-        if (count & sector_mask || (uintptr_t) buf & sector_mask) {
-
-            while ((size = (count & ~sector_mask)) != 0) {
-
-                if (size > s->aligned_buf_size)
-                    size = s->aligned_buf_size;
-
-                memcpy(s->aligned_buf, buf, size);
-
-                ret = raw_pwrite_aligned(bs, offset, s->aligned_buf, size);
-                if (ret < 0)
-                    return ret;
-
-                buf += ret;
-                offset += ret;
-                count -= ret;
-                sum += ret;
-            }
-            /* here, count < sector_size because (count & ~sector_mask) == 0 */
-            if (count) {
-                ret = raw_pread_aligned(bs, offset, s->aligned_buf,
-                                     bs->buffer_alignment);
-                if (ret < 0)
-                    return ret;
-                 memcpy(s->aligned_buf, buf, count);
-
-                 ret = raw_pwrite_aligned(bs, offset, s->aligned_buf,
-                                     bs->buffer_alignment);
-                 if (ret < 0)
-                     return ret;
-                 if (count < ret)
-                     ret = count;
-
-                 sum += ret;
-            }
-            return sum;
-        }
-    }
-    return raw_pwrite_aligned(bs, offset, buf, count) + sum;
-}
-
-static int raw_write(BlockDriverState *bs, int64_t sector_num,
-                     const uint8_t *buf, int nb_sectors)
-{
-    int ret;
-    ret = raw_pwrite(bs, sector_num * BDRV_SECTOR_SIZE, buf,
-                     nb_sectors * BDRV_SECTOR_SIZE);
-    if (ret == (nb_sectors * BDRV_SECTOR_SIZE))
-        ret = 0;
-    return ret;
-}
-
-/*
  * Check if all memory in this vector is sector aligned.
  */
 static int qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
@@ -910,8 +643,6 @@ static BlockDriver bdrv_file = {
     .instance_size = sizeof(BDRVRawState),
     .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_file_open = raw_open,
-    .bdrv_read = raw_read,
-    .bdrv_write = raw_write,
     .bdrv_close = raw_close,
     .bdrv_create = raw_create,
     .bdrv_flush = raw_flush,
@@ -1190,8 +921,6 @@ static BlockDriver bdrv_host_device = {
     .bdrv_aio_writev	= raw_aio_writev,
     .bdrv_aio_flush	= raw_aio_flush,
 
-    .bdrv_read          = raw_read,
-    .bdrv_write         = raw_write,
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength	= raw_getlength,
     .bdrv_get_allocated_file_size
@@ -1312,8 +1041,6 @@ static BlockDriver bdrv_host_floppy = {
     .bdrv_aio_writev    = raw_aio_writev,
     .bdrv_aio_flush	= raw_aio_flush,
 
-    .bdrv_read          = raw_read,
-    .bdrv_write         = raw_write,
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength	= raw_getlength,
     .bdrv_get_allocated_file_size
@@ -1414,8 +1141,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_aio_writev    = raw_aio_writev,
     .bdrv_aio_flush	= raw_aio_flush,
 
-    .bdrv_read          = raw_read,
-    .bdrv_write         = raw_write,
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength     = raw_getlength,
     .bdrv_get_allocated_file_size
@@ -1536,8 +1261,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_aio_writev    = raw_aio_writev,
     .bdrv_aio_flush	= raw_aio_flush,
 
-    .bdrv_read          = raw_read,
-    .bdrv_write         = raw_write,
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength     = raw_getlength,
     .bdrv_get_allocated_file_size
-- 
1.7.6.3

[Qemu-devel] [PATCH 3/5] block: use coroutine interface for raw format

[Stefan Hajnoczi]

The raw format delegates all operations to bs->file (the protocol).
Previously this block driver exposed both sync and aio interfaces.
Since the block layer now works in terms of coroutines, expose the
coroutine interfaces and drop the others.  This avoids unnecessary
emulation of sync and aio interfaces.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/raw.c |   32 ++++++++------------------------
 1 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/block/raw.c b/block/raw.c
index 63cf2d3..5ca606b 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -9,30 +9,16 @@ static int raw_open(BlockDriverState *bs, int flags)
     return 0;
 }
 
-static int raw_read(BlockDriverState *bs, int64_t sector_num,
-                    uint8_t *buf, int nb_sectors)
+static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
+                                     int nb_sectors, QEMUIOVector *qiov)
 {
-    return bdrv_read(bs->file, sector_num, buf, nb_sectors);
+    return bdrv_co_readv(bs->file, sector_num, nb_sectors, qiov);
 }
 
-static int raw_write(BlockDriverState *bs, int64_t sector_num,
-                     const uint8_t *buf, int nb_sectors)
+static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
+                                      int nb_sectors, QEMUIOVector *qiov)
 {
-    return bdrv_write(bs->file, sector_num, buf, nb_sectors);
-}
-
-static BlockDriverAIOCB *raw_aio_readv(BlockDriverState *bs,
-    int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-    BlockDriverCompletionFunc *cb, void *opaque)
-{
-    return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
-}
-
-static BlockDriverAIOCB *raw_aio_writev(BlockDriverState *bs,
-    int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-    BlockDriverCompletionFunc *cb, void *opaque)
-{
-    return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
+    return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
 }
 
 static void raw_close(BlockDriverState *bs)
@@ -129,15 +115,13 @@ static BlockDriver bdrv_raw = {
 
     .bdrv_open          = raw_open,
     .bdrv_close         = raw_close,
-    .bdrv_read          = raw_read,
-    .bdrv_write         = raw_write,
+    .bdrv_co_readv      = raw_co_readv,
+    .bdrv_co_writev     = raw_co_writev,
     .bdrv_flush         = raw_flush,
     .bdrv_probe         = raw_probe,
     .bdrv_getlength     = raw_getlength,
     .bdrv_truncate      = raw_truncate,
 
-    .bdrv_aio_readv     = raw_aio_readv,
-    .bdrv_aio_writev    = raw_aio_writev,
     .bdrv_aio_flush     = raw_aio_flush,
     .bdrv_discard       = raw_discard,
 
-- 
1.7.6.3

[Qemu-devel] [PATCH 4/5] block: drop .bdrv_read()/.bdrv_write() emulation

[Stefan Hajnoczi]

There is no need to emulate .bdrv_read()/.bdrv_write() since these
interfaces are only called if aio and coroutine interfaces are not
present.  All valid BlockDrivers must implement either sync, aio, or
coroutine interfaces.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |   75 ++------------------------------------------------------------
 1 files changed, 3 insertions(+), 72 deletions(-)

diff --git a/block.c b/block.c
index 3d27bab..4d4d61a 100644
--- a/block.c
+++ b/block.c
@@ -57,10 +57,6 @@ 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 bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
-                        uint8_t *buf, int nb_sectors);
-static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
-                         const uint8_t *buf, int nb_sectors);
 static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
                                          int64_t sector_num, int nb_sectors,
                                          QEMUIOVector *iov);
@@ -197,14 +193,13 @@ void bdrv_register(BlockDriver *bdrv)
         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;
-        } else if (!bdrv->bdrv_read) {
-            /* add synchronous IO emulation layer */
-            bdrv->bdrv_read = bdrv_read_em;
-            bdrv->bdrv_write = bdrv_write_em;
         }
     }
 
@@ -2834,70 +2829,6 @@ static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
     return &acb->common;
 }
 
-/**************************************************************/
-/* sync block device emulation */
-
-static void bdrv_rw_em_cb(void *opaque, int ret)
-{
-    *(int *)opaque = ret;
-}
-
-static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
-                        uint8_t *buf, int nb_sectors)
-{
-    int async_ret;
-    BlockDriverAIOCB *acb;
-    struct iovec iov;
-    QEMUIOVector qiov;
-
-    async_ret = NOT_DONE;
-    iov.iov_base = (void *)buf;
-    iov.iov_len = nb_sectors * BDRV_SECTOR_SIZE;
-    qemu_iovec_init_external(&qiov, &iov, 1);
-
-    acb = bs->drv->bdrv_aio_readv(bs, sector_num, &qiov, nb_sectors,
-                                  bdrv_rw_em_cb, &async_ret);
-    if (acb == NULL) {
-        async_ret = -1;
-        goto fail;
-    }
-
-    while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
-    }
-
-
-fail:
-    return async_ret;
-}
-
-static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
-                         const uint8_t *buf, int nb_sectors)
-{
-    int async_ret;
-    BlockDriverAIOCB *acb;
-    struct iovec iov;
-    QEMUIOVector qiov;
-
-    async_ret = NOT_DONE;
-    iov.iov_base = (void *)buf;
-    iov.iov_len = nb_sectors * BDRV_SECTOR_SIZE;
-    qemu_iovec_init_external(&qiov, &iov, 1);
-
-    acb = bs->drv->bdrv_aio_writev(bs, sector_num, &qiov, nb_sectors,
-                                   bdrv_rw_em_cb, &async_ret);
-    if (acb == NULL) {
-        async_ret = -1;
-        goto fail;
-    }
-    while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
-    }
-
-fail:
-    return async_ret;
-}
-
 void bdrv_init(void)
 {
     module_call_init(MODULE_INIT_BLOCK);
-- 
1.7.6.3

[Qemu-devel] [PATCH 5/5] block: drop bdrv_has_async_rw()

[Stefan Hajnoczi]

Commit cd74d83345e0e3b708330ab8c4cd9111bb82cda6 ("block: switch
bdrv_read()/bdrv_write() to coroutines") removed the bdrv_has_async_rw()
callers.  This patch removes bdrv_has_async_rw() since it is no longer
used.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 4d4d61a..9873b57 100644
--- a/block.c
+++ b/block.c
@@ -1027,12 +1027,6 @@ static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
                                    nb_sectors * BDRV_SECTOR_SIZE);
 }
 
-static inline bool bdrv_has_async_rw(BlockDriver *drv)
-{
-    return drv->bdrv_co_readv != bdrv_co_readv_em
-        || drv->bdrv_aio_readv != bdrv_aio_readv_em;
-}
-
 static inline bool bdrv_has_async_flush(BlockDriver *drv)
 {
     return drv->bdrv_aio_flush != bdrv_aio_flush_em;
-- 
1.7.6.3

Re: [Qemu-devel] [PATCH 0/5] block: remove unused emulation and synchronous functions

[Kevin Wolf]

Am 13.10.2011 22:09, schrieb Stefan Hajnoczi:
> Now that the block layer processes requests in coroutine context, some of the
> emulation wrappers and duplicate code paths can be dropped.  Paraphrasing a
> wise man, Arnold Schwarzenegger, "I will go to the block layer and I will clean
> house" :).
> 
> They key thing behind this series is that the block layer processes requests in
> coroutine context and will try to use .brdv_co_readv()/.bdrv_co_writev() when
> possible.  Should the BlockDriver not implement those interfaces, an emulation
> function will be used to provide them using aio.  If the BlockDriver does not
> implement aio interfaces, then an emulation function will be used to provide
> them using synchronous I/O.
> 
> This means:
> 
> 1. A BlockDriver that implements coroutine interfaces does not need to
>    implement aio or synchronous interfaces.
> 
> 2. A BlockDriver that implements aio interfaces does not need to implement
>    synchronous interfaces.
> 
> 3. Coroutine interfaces are preferred and do not require any emulation
>    functions.
> 
> This patch series propagates these rules across existing BlockDrivers and
> removes unused emulation functions from block.c.
> 
> Stefan Hajnoczi (5):
>   block: drop emulation functions that use coroutines
>   raw-posix: remove bdrv_read()/bdrv_write()
>   block: use coroutine interface for raw format
>   block: drop .bdrv_read()/.bdrv_write() emulation
>   block: drop bdrv_has_async_rw()

Thanks, applied all to the block branch.

>  block.c           |  142 ++-------------------------
>  block/raw-posix.c |  277 -----------------------------------------------------
>  block/raw.c       |   32 ++-----
>  3 files changed, 18 insertions(+), 433 deletions(-)

Awesome. We need more series like this! :-)

Kevin