diff mbox

[4/5] Convert block functions to coroutine versions

Message ID 1375728247-1306-5-git-send-email-charlie@ctshepherd.com
State New
Headers show

Commit Message

Charlie Shepherd Aug. 5, 2013, 6:44 p.m. UTC 
This patch follows on from the previous one and converts some block layer functions to be
explicitly annotated with coroutine_fn instead of yielding depending upon calling context.
---
 block.c               | 235 ++++++++++++++++++++++++++------------------------
 block/mirror.c        |   4 +-
 include/block/block.h |  37 ++++----
 3 files changed, 148 insertions(+), 128 deletions(-)

Comments

Gabriel Kerneis Aug. 5, 2013, 8:01 p.m. UTC | #1 
On Mon, Aug 05, 2013 at 08:44:06PM +0200, Charlie Shepherd wrote:
> This patch follows on from the previous one and converts some block layer functions to be
> explicitly annotated with coroutine_fn instead of yielding depending upon calling context.

And just like the previous one, it also removes one annotation, which you
might want to mention in the commit message:

> -int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
> +int coroutine_fn bdrv_flush(BlockDriverState *bs)

By the way:

> diff --git a/block.c b/block.c
> index aaa122c..e7011f9 100644
> --- a/block.c
> +++ b/block.c
> @@ -364,7 +364,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
>  
>  typedef struct CreateCo {
>      BlockDriver *drv;
> -    char *filename;
> +    const char *filename;
>      QEMUOptionParameter *options;
>      int ret;
>  } CreateCo;

This looks like an unrelated change which might deserve its own patch.  I'm not
sure what the QEMU policiy is about that kind of minor fix, but maybe send it to
qemu-trivial?
Kevin Wolf Aug. 6, 2013, 9:36 a.m. UTC | #2 
Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:
> This patch follows on from the previous one and converts some block layer functions to be
> explicitly annotated with coroutine_fn instead of yielding depending upon calling context.
> ---
>  block.c               | 235 ++++++++++++++++++++++++++------------------------
>  block/mirror.c        |   4 +-
>  include/block/block.h |  37 ++++----
>  3 files changed, 148 insertions(+), 128 deletions(-)
> 
> diff --git a/block.c b/block.c
> index aaa122c..e7011f9 100644
> --- a/block.c
> +++ b/block.c
> @@ -164,7 +164,7 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
>           || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
>  }
>  
> -static void bdrv_io_limits_intercept(BlockDriverState *bs,
> +static void coroutine_fn bdrv_io_limits_intercept(BlockDriverState *bs,
>                                       bool is_write, int nb_sectors)
>  {
>      int64_t wait_time = -1;
> @@ -364,7 +364,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
>  
>  typedef struct CreateCo {
>      BlockDriver *drv;
> -    char *filename;
> +    const char *filename;
>      QEMUOptionParameter *options;
>      int ret;
>  } CreateCo;

Like Gabriel said, this should be a separate patch. Keeping it in this
series is probably easiest.

> @@ -372,48 +372,48 @@ typedef struct CreateCo {
>  static void coroutine_fn bdrv_create_co_entry(void *opaque)
>  {
>      CreateCo *cco = opaque;
> -    assert(cco->drv);
> -
> -    cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
> +    cco->ret = bdrv_create(cco->drv, cco->filename, cco->options);
>  }
>  
> -int bdrv_create(BlockDriver *drv, const char* filename,
> +int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
>      QEMUOptionParameter *options)
>  {
>      int ret;
> +    char *dup_fn;
> +
> +    assert(drv);
> +    if (!drv->bdrv_co_create) {
> +        return -ENOTSUP;
> +    }
>  
> +    dup_fn = g_strdup(filename);
> +    ret = drv->bdrv_co_create(dup_fn, options);
> +    g_free(dup_fn);
> +    return ret;
> +}
> +
> +
> +int bdrv_create_sync(BlockDriver *drv, const char* filename,
> +    QEMUOptionParameter *options)

bdrv_foo_sync is an unfortunate naming convention, because
bdrv_pwrite_sync already exists and means something totally different:
It's a write directly followed by a disk flush.

> diff --git a/include/block/block.h b/include/block/block.h
> index dd8eca1..57d8ba2 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -125,9 +125,9 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
>  void bdrv_delete(BlockDriverState *bs);
>  int bdrv_parse_cache_flags(const char *mode, int *flags);
>  int bdrv_parse_discard_flags(const char *mode, int *flags);
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> +int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
>                     QDict *options, int flags);
> -int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
> +int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
>  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
>                int flags, BlockDriver *drv);
>  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
> @@ -150,18 +150,24 @@ void bdrv_dev_eject_request(BlockDriverState *bs, bool force);
>  bool bdrv_dev_has_removable_media(BlockDriverState *bs);
>  bool bdrv_dev_is_tray_open(BlockDriverState *bs);
>  bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
> -int bdrv_read(BlockDriverState *bs, int64_t sector_num,
> +int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
>                uint8_t *buf, int nb_sectors);
> -int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
> +int bdrv_read_sync(BlockDriverState *bs, int64_t sector_num,
> +              uint8_t *buf, int nb_sectors);
> +int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
>                            uint8_t *buf, int nb_sectors);
> -int bdrv_write(BlockDriverState *bs, int64_t sector_num,
> +int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
> +               const uint8_t *buf, int nb_sectors);
> +int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
>                 const uint8_t *buf, int nb_sectors);
> -int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
> -int bdrv_pread(BlockDriverState *bs, int64_t offset,
> +int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
> +int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
> +               void *buf, int count);
> +int bdrv_pread_sync(BlockDriverState *bs, int64_t offset,
>                 void *buf, int count);

I haven't checked everything, but bdrv_pread_sync is an example of a
declaration without any user and without implementation.

Proper patch splitting will make review of such things easier, so I'll
defer thorough review until then.

Kevin
Charlie Shepherd Aug. 8, 2013, 1:17 a.m. UTC | #3 
On 06/08/2013 10:36, Kevin Wolf wrote:
> Am 05.08.2013 um 20:44 hat Charlie Shepherd geschrieben:
>> This patch follows on from the previous one and converts some block layer functions to be
>> explicitly annotated with coroutine_fn instead of yielding depending upon calling context.
>> ---
>>   block.c               | 235 ++++++++++++++++++++++++++------------------------
>>   block/mirror.c        |   4 +-
>>   include/block/block.h |  37 ++++----
>>   3 files changed, 148 insertions(+), 128 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index aaa122c..e7011f9 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -164,7 +164,7 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
>>            || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
>>   }
>>   
>> -static void bdrv_io_limits_intercept(BlockDriverState *bs,
>> +static void coroutine_fn bdrv_io_limits_intercept(BlockDriverState *bs,
>>                                        bool is_write, int nb_sectors)
>>   {
>>       int64_t wait_time = -1;
>> @@ -364,7 +364,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
>>   
>>   typedef struct CreateCo {
>>       BlockDriver *drv;
>> -    char *filename;
>> +    const char *filename;
>>       QEMUOptionParameter *options;
>>       int ret;
>>   } CreateCo;
> Like Gabriel said, this should be a separate patch. Keeping it in this
> series is probably easiest.
>
>> @@ -372,48 +372,48 @@ typedef struct CreateCo {
>>   static void coroutine_fn bdrv_create_co_entry(void *opaque)
>>   {
>>       CreateCo *cco = opaque;
>> -    assert(cco->drv);
>> -
>> -    cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
>> +    cco->ret = bdrv_create(cco->drv, cco->filename, cco->options);
>>   }
>>   
>> -int bdrv_create(BlockDriver *drv, const char* filename,
>> +int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
>>       QEMUOptionParameter *options)
>>   {
>>       int ret;
>> +    char *dup_fn;
>> +
>> +    assert(drv);
>> +    if (!drv->bdrv_co_create) {
>> +        return -ENOTSUP;
>> +    }
>>   
>> +    dup_fn = g_strdup(filename);
>> +    ret = drv->bdrv_co_create(dup_fn, options);
>> +    g_free(dup_fn);
>> +    return ret;
>> +}
>> +
>> +
>> +int bdrv_create_sync(BlockDriver *drv, const char* filename,
>> +    QEMUOptionParameter *options)
> bdrv_foo_sync is an unfortunate naming convention, because
> bdrv_pwrite_sync already exists and means something totally different:
> It's a write directly followed by a disk flush.

Yes it's not a great naming convention. I could rename bdrv_pwrite_sync 
to something like bdrv_pwrite_and_sync? Or is that too horrible?
An alternative would be bdrv_foo and bdrv_sync_foo? But bdrv_sync_pwrite 
and bdrv_pwrite_sync would be quite confusing in a hurry.

>> diff --git a/include/block/block.h b/include/block/block.h
>> index dd8eca1..57d8ba2 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -125,9 +125,9 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
>>   void bdrv_delete(BlockDriverState *bs);
>>   int bdrv_parse_cache_flags(const char *mode, int *flags);
>>   int bdrv_parse_discard_flags(const char *mode, int *flags);
>> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
>> +int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
>>                      QDict *options, int flags);
>> -int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
>> +int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
>>   int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
>>                 int flags, BlockDriver *drv);
>>   BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
>> @@ -150,18 +150,24 @@ void bdrv_dev_eject_request(BlockDriverState *bs, bool force);
>>   bool bdrv_dev_has_removable_media(BlockDriverState *bs);
>>   bool bdrv_dev_is_tray_open(BlockDriverState *bs);
>>   bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
>> -int bdrv_read(BlockDriverState *bs, int64_t sector_num,
>> +int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
>>                 uint8_t *buf, int nb_sectors);
>> -int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
>> +int bdrv_read_sync(BlockDriverState *bs, int64_t sector_num,
>> +              uint8_t *buf, int nb_sectors);
>> +int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
>>                             uint8_t *buf, int nb_sectors);
>> -int bdrv_write(BlockDriverState *bs, int64_t sector_num,
>> +int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
>> +               const uint8_t *buf, int nb_sectors);
>> +int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
>>                  const uint8_t *buf, int nb_sectors);
>> -int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
>> -int bdrv_pread(BlockDriverState *bs, int64_t offset,
>> +int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
>> +int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
>> +               void *buf, int count);
>> +int bdrv_pread_sync(BlockDriverState *bs, int64_t offset,
>>                  void *buf, int count);
> I haven't checked everything, but bdrv_pread_sync is an example of a
> declaration without any user and without implementation.
>
> Proper patch splitting will make review of such things easier, so I'll
> defer thorough review until then.

Yes my bad, hopefully splitting them as per your other email will catch 
these issues.
diff mbox

Patch

diff --git a/block.c b/block.c
index aaa122c..e7011f9 100644
--- a/block.c
+++ b/block.c
@@ -164,7 +164,7 @@  bool bdrv_io_limits_enabled(BlockDriverState *bs)
          || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
 }
 
-static void bdrv_io_limits_intercept(BlockDriverState *bs,
+static void coroutine_fn bdrv_io_limits_intercept(BlockDriverState *bs,
                                      bool is_write, int nb_sectors)
 {
     int64_t wait_time = -1;
@@ -364,7 +364,7 @@  BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
 
 typedef struct CreateCo {
     BlockDriver *drv;
-    char *filename;
+    const char *filename;
     QEMUOptionParameter *options;
     int ret;
 } CreateCo;
@@ -372,48 +372,48 @@  typedef struct CreateCo {
 static void coroutine_fn bdrv_create_co_entry(void *opaque)
 {
     CreateCo *cco = opaque;
-    assert(cco->drv);
-
-    cco->ret = cco->drv->bdrv_co_create(cco->filename, cco->options);
+    cco->ret = bdrv_create(cco->drv, cco->filename, cco->options);
 }
 
-int bdrv_create(BlockDriver *drv, const char* filename,
+int coroutine_fn bdrv_create(BlockDriver *drv, const char* filename,
     QEMUOptionParameter *options)
 {
     int ret;
+    char *dup_fn;
+
+    assert(drv);
+    if (!drv->bdrv_co_create) {
+        return -ENOTSUP;
+    }
 
+    dup_fn = g_strdup(filename);
+    ret = drv->bdrv_co_create(dup_fn, options);
+    g_free(dup_fn);
+    return ret;
+}
+
+
+int bdrv_create_sync(BlockDriver *drv, const char* filename,
+    QEMUOptionParameter *options)
+{
     Coroutine *co;
     CreateCo cco = {
         .drv = drv,
-        .filename = g_strdup(filename),
+        .filename = filename,
         .options = options,
         .ret = NOT_DONE,
     };
 
-    if (!drv->bdrv_co_create) {
-        ret = -ENOTSUP;
-        goto out;
-    }
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_create_co_entry(&cco);
-    } else {
-        co = qemu_coroutine_create(bdrv_create_co_entry);
-        qemu_coroutine_enter(co, &cco);
-        while (cco.ret == NOT_DONE) {
-            qemu_aio_wait();
-        }
+    co = qemu_coroutine_create(bdrv_create_co_entry);
+    qemu_coroutine_enter(co, &cco);
+    while (cco.ret == NOT_DONE) {
+        qemu_aio_wait();
     }
 
-    ret = cco.ret;
-
-out:
-    g_free(cco.filename);
-    return ret;
+    return cco.ret;
 }
 
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int coroutine_fn bdrv_create_file(const char* filename, QEMUOptionParameter *options)
 {
     BlockDriver *drv;
 
@@ -522,7 +522,7 @@  BlockDriver *bdrv_find_protocol(const char *filename)
     return NULL;
 }
 
-static int find_image_format(BlockDriverState *bs, const char *filename,
+static int coroutine_fn find_image_format(BlockDriverState *bs, const char *filename,
                              BlockDriver **pdrv)
 {
     int score, score_max;
@@ -676,8 +676,8 @@  static int bdrv_open_flags(BlockDriverState *bs, int flags)
  *
  * Removes all processed options from *options.
  */
-static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
-    QDict *options, int flags, BlockDriver *drv)
+static int coroutine_fn bdrv_open_common(BlockDriverState *bs,
+    BlockDriverState *file, QDict *options, int flags, BlockDriver *drv)
 {
     int ret, open_flags;
     const char *filename;
@@ -778,7 +778,7 @@  free_and_fail:
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
                    QDict *options, int flags)
 {
     BlockDriverState *bs;
@@ -881,7 +881,7 @@  fail:
  * function (even on failure), so if the caller intends to reuse the dictionary,
  * it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
 {
     char backing_filename[PATH_MAX];
     int back_flags, ret;
@@ -955,7 +955,7 @@  static void extract_subqdict(QDict *src, QDict **dst, const char *start)
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_open.
  */
-int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
+int coroutine_fn bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
               int flags, BlockDriver *drv)
 {
     int ret;
@@ -1279,7 +1279,7 @@  int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
     }
 
 
-    ret = bdrv_flush(reopen_state->bs);
+    ret = bdrv_flush_sync(reopen_state->bs);
     if (ret) {
         error_set(errp, ERROR_CLASS_GENERIC_ERROR, "Error (%s) flushing drive",
                   strerror(-ret));
@@ -1358,7 +1358,7 @@  void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 
 void bdrv_close(BlockDriverState *bs)
 {
-    bdrv_flush(bs);
+    bdrv_flush_sync(bs);
     if (bs->job) {
         block_job_cancel_sync(bs->job);
     }
@@ -1754,7 +1754,7 @@  int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)
 #define COMMIT_BUF_SECTORS 2048
 
 /* commit COW file into the raw image */
-int bdrv_commit(BlockDriverState *bs)
+int coroutine_fn bdrv_commit(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
     int64_t sector, total_sectors;
@@ -1805,7 +1805,7 @@  int bdrv_commit(BlockDriverState *bs)
 
     if (drv->bdrv_make_empty) {
         ret = drv->bdrv_make_empty(bs);
-        bdrv_flush(bs);
+        bdrv_flush_sync(bs);
     }
 
     /*
@@ -1813,7 +1813,7 @@  int bdrv_commit(BlockDriverState *bs)
      * stable on disk.
      */
     if (bs->backing_hd)
-        bdrv_flush(bs->backing_hd);
+        bdrv_flush_sync(bs->backing_hd);
 
 ro_cleanup:
     g_free(buf);
@@ -1826,7 +1826,7 @@  ro_cleanup:
     return ret;
 }
 
-int bdrv_commit_all(void)
+int coroutine_fn bdrv_commit_all(void)
 {
     BlockDriverState *bs;
 
@@ -2156,35 +2156,25 @@  typedef struct RwCo {
     int ret;
 } RwCo;
 
-static void coroutine_fn bdrv_rw_co_entry(void *opaque)
+static int bdrv_sync_rwco(void coroutine_fn (*co_fn)(void *), RwCo *rwco)
 {
-    RwCo *rwco = opaque;
-
-    if (!rwco->is_write) {
-        rwco->ret = bdrv_co_do_readv(rwco->bs, rwco->sector_num,
-                                     rwco->nb_sectors, rwco->qiov, 0);
-    } else {
-        rwco->ret = bdrv_co_do_writev(rwco->bs, rwco->sector_num,
-                                      rwco->nb_sectors, rwco->qiov, 0);
+    Coroutine *co;
+    co = qemu_coroutine_create(co_fn);
+    qemu_coroutine_enter(co, rwco);
+    while (rwco->ret == NOT_DONE) {
+        qemu_aio_wait();
     }
+    return rwco->ret;
 }
 
 /*
  * Process a vectored synchronous request using coroutines
  */
-static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
+static int coroutine_fn bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
                        QEMUIOVector *qiov, bool is_write)
 {
-    Coroutine *co;
-    RwCo rwco = {
-        .bs = bs,
-        .sector_num = sector_num,
-        .nb_sectors = qiov->size >> BDRV_SECTOR_BITS,
-        .qiov = qiov,
-        .is_write = is_write,
-        .ret = NOT_DONE,
-    };
     assert((qiov->size & (BDRV_SECTOR_SIZE - 1)) == 0);
+    int nb_sectors = qiov->size >> BDRV_SECTOR_BITS;
 
     /**
      * In sync call context, when the vcpu is blocked, this throttling timer
@@ -2197,23 +2187,40 @@  static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
         bdrv_io_limits_disable(bs);
     }
 
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_rw_co_entry(&rwco);
+    if (!is_write) {
+        return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, 0);
     } else {
-        co = qemu_coroutine_create(bdrv_rw_co_entry);
-        qemu_coroutine_enter(co, &rwco);
-        while (rwco.ret == NOT_DONE) {
-            qemu_aio_wait();
-        }
+        return bdrv_co_do_writev(bs, sector_num, nb_sectors, qiov, 0);
     }
-    return rwco.ret;
+}
+
+static void coroutine_fn bdrv_rw_co_entry(void *opaque)
+{
+    RwCo *rwco = opaque;
+    rwco->ret = bdrv_rwv_co_co(rwco->bs, rwco->sector_num, rwco->qiov, rwco->is_write);
+}
+
+/*
+ * Process a vectored synchronous request synchronously
+ */
+static int bdrv_rwv_sync(BlockDriverState *bs, int64_t sector_num,
+                       QEMUIOVector *qiov, bool is_write)
+{
+    RwCo rwco = {
+        .bs = bs,
+        .sector_num = sector_num,
+        .qiov = qiov,
+        .is_write = is_write,
+        .ret = NOT_DONE,
+    };
+
+    return bdrv_sync_rwco(bdrv_rw_co_entry, &rwco);
 }
 
 /*
  * Process a synchronous request using coroutines
  */
-static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
+static int coroutine_fn bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
                       int nb_sectors, bool is_write)
 {
     QEMUIOVector qiov;
@@ -2226,15 +2233,37 @@  static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
     return bdrv_rwv_co(bs, sector_num, &qiov, is_write);
 }
 
-/* return < 0 if error. See bdrv_write() for the return codes */
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+/*
+ * Process a synchronous request using coroutines
+ */
+static int bdrv_rw_sync(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
+                      int nb_sectors, bool is_write)
+{
+    QEMUIOVector qiov;
+    struct iovec iov = {
+        .iov_base = (void *)buf,
+        .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
+    };
+
+    qemu_iovec_init_external(&qiov, &iov, 1);
+    return bdrv_rwv_sync(bs, sector_num, &qiov, is_write);
+}
+
+int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors)
 {
     return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false);
 }
 
+/* return < 0 if error. See bdrv_write() for the return codes */
+int bdrv_read_sync(BlockDriverState *bs, int64_t sector_num,
+              uint8_t *buf, int nb_sectors)
+{
+    return bdrv_rw_sync(bs, sector_num, buf, nb_sectors, false);
+}
+
 /* Just like bdrv_read(), but with I/O throttling temporarily disabled */
-int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
                           uint8_t *buf, int nb_sectors)
 {
     bool enabled;
@@ -2253,18 +2282,24 @@  int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
   -EINVAL      Invalid sector number or nb_sectors
   -EACCES      Trying to write a read-only device
 */
-int bdrv_write(BlockDriverState *bs, int64_t sector_num,
+int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
+               const uint8_t *buf, int nb_sectors)
+{
+    return bdrv_rw_co_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true);
+}
+
+int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
                const uint8_t *buf, int nb_sectors)
 {
     return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true);
 }
 
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
+int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
 {
     return bdrv_rwv_co(bs, sector_num, qiov, true);
 }
 
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
                void *buf, int count1)
 {
     uint8_t tmp_buf[BDRV_SECTOR_SIZE];
@@ -2309,7 +2344,7 @@  int bdrv_pread(BlockDriverState *bs, int64_t offset,
     return count1;
 }
 
-int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
+int coroutine_fn bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
 {
     uint8_t tmp_buf[BDRV_SECTOR_SIZE];
     int len, nb_sectors, count;
@@ -2366,7 +2401,7 @@  int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
     return qiov->size;
 }
 
-int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pwrite(BlockDriverState *bs, int64_t offset,
                 const void *buf, int count1)
 {
     QEMUIOVector qiov;
@@ -2385,7 +2420,7 @@  int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
  *
  * Returns 0 on success, -errno in error cases.
  */
-int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
     const void *buf, int count)
 {
     int ret;
@@ -2397,7 +2432,7 @@  int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
 
     /* No flush needed for cache modes that already do it */
     if (bs->enable_write_cache) {
-        bdrv_flush(bs);
+        bdrv_flush_sync(bs);
     }
 
     return 0;
@@ -2907,7 +2942,7 @@  void bdrv_flush_all(void)
     BlockDriverState *bs;
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
-        bdrv_flush(bs);
+        bdrv_flush_sync(bs);
     }
 }
 
@@ -3740,7 +3775,7 @@  static void bdrv_aio_bh_cb(void *opaque)
     qemu_aio_release(acb);
 }
 
-static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
+static coroutine_fn BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
                                             int64_t sector_num,
                                             QEMUIOVector *qiov,
                                             int nb_sectors,
@@ -3769,14 +3804,14 @@  static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
     return &acb->common;
 }
 
-static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
+static coroutine_fn BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
     return bdrv_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque, 0);
 }
 
-static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
+static coroutine_fn BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
@@ -4014,7 +4049,7 @@  static void coroutine_fn bdrv_flush_co_entry(void *opaque)
     rwco->ret = bdrv_co_flush(rwco->bs);
 }
 
-int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
+int coroutine_fn bdrv_flush(BlockDriverState *bs)
 {
     int ret;
 
@@ -4102,7 +4137,7 @@  void bdrv_clear_incoming_migration_all(void)
     }
 }
 
-int bdrv_flush(BlockDriverState *bs)
+int bdrv_flush_sync(BlockDriverState *bs)
 {
     Coroutine *co;
     RwCo rwco = {
@@ -4110,18 +4145,7 @@  int bdrv_flush(BlockDriverState *bs)
         .ret = NOT_DONE,
     };
 
-    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();
-        }
-    }
-
-    return rwco.ret;
+    return bdrv_sync_rwco(bdrv_flush_co_entry, &rwco);
 }
 
 static void coroutine_fn bdrv_discard_co_entry(void *opaque)
@@ -4131,7 +4155,7 @@  static void coroutine_fn bdrv_discard_co_entry(void *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 coroutine_fn bdrv_discard(BlockDriverState *bs, int64_t sector_num,
                                  int nb_sectors)
 {
     if (!bs->drv) {
@@ -4172,7 +4196,7 @@  int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     }
 }
 
-int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
+int bdrv_discard_sync(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
 {
     Coroutine *co;
     RwCo rwco = {
@@ -4182,18 +4206,7 @@  int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int 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;
+    return bdrv_sync_rwco(bdrv_discard_co_entry, &rwco);
 }
 
 /**************************************************************/
@@ -4432,7 +4445,7 @@  bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
     bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
 }
 
-void bdrv_img_create(const char *filename, const char *fmt,
+void coroutine_fn bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
                      char *options, uint64_t img_size, int flags,
                      Error **errp, bool quiet)
diff --git a/block/mirror.c b/block/mirror.c
index bed4a7e..3d5da7e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -282,7 +282,7 @@  static void mirror_free_init(MirrorBlockJob *s)
     }
 }
 
-static void mirror_drain(MirrorBlockJob *s)
+static void coroutine_fn mirror_drain(MirrorBlockJob *s)
 {
     while (s->in_flight > 0) {
         qemu_coroutine_yield();
@@ -390,7 +390,7 @@  static void coroutine_fn mirror_run(void *opaque)
         should_complete = false;
         if (s->in_flight == 0 && cnt == 0) {
             trace_mirror_before_flush(s);
-            ret = bdrv_flush(s->target);
+            ret = bdrv_co_flush(s->target);
             if (ret < 0) {
                 if (mirror_error_action(s, false, -ret) == BDRV_ACTION_REPORT) {
                     goto immediate_exit;
diff --git a/include/block/block.h b/include/block/block.h
index dd8eca1..57d8ba2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -125,9 +125,9 @@  void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_delete(BlockDriverState *bs);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+int coroutine_fn bdrv_file_open(BlockDriverState **pbs, const char *filename,
                    QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+int coroutine_fn bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
 int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
               int flags, BlockDriver *drv);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
@@ -150,18 +150,24 @@  void bdrv_dev_eject_request(BlockDriverState *bs, bool force);
 bool bdrv_dev_has_removable_media(BlockDriverState *bs);
 bool bdrv_dev_is_tray_open(BlockDriverState *bs);
 bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
-int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+int bdrv_read_sync(BlockDriverState *bs, int64_t sector_num,
+              uint8_t *buf, int nb_sectors);
+int coroutine_fn bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
                           uint8_t *buf, int nb_sectors);
-int bdrv_write(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_write(BlockDriverState *bs, int64_t sector_num,
+               const uint8_t *buf, int nb_sectors);
+int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
                const uint8_t *buf, int nb_sectors);
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov);
+int coroutine_fn bdrv_pread(BlockDriverState *bs, int64_t offset,
+               void *buf, int count);
+int bdrv_pread_sync(BlockDriverState *bs, int64_t offset,
                void *buf, int count);
-int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
+int coroutine_fn bdrv_pwrite(BlockDriverState *bs, int64_t offset,
                 const void *buf, int count);
-int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
+int coroutine_fn bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
 int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
     const void *buf, int count);
 int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -191,8 +197,8 @@  int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
-int bdrv_commit(BlockDriverState *bs);
-int bdrv_commit_all(void);
+int coroutine_fn bdrv_commit(BlockDriverState *bs);
+int coroutine_fn bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
     const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
@@ -264,13 +270,14 @@  void bdrv_invalidate_cache_all(void);
 void bdrv_clear_incoming_migration_all(void);
 
 /* Ensure contents are flushed to disk.  */
-int bdrv_flush(BlockDriverState *bs);
-int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
+int coroutine_fn bdrv_flush(BlockDriverState *bs);
+int bdrv_flush_sync(BlockDriverState *bs);
 void bdrv_flush_all(void);
 void bdrv_close_all(void);
 void bdrv_drain_all(void);
 
-int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
+int coroutine_fn bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
+int bdrv_discard_sync(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_1(BlockDriverState *bs);
 int bdrv_has_zero_init(BlockDriverState *bs);
@@ -333,7 +340,7 @@  int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
 int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
                       int64_t pos, int size);
 
-void bdrv_img_create(const char *filename, const char *fmt,
+void coroutine_fn bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
                      char *options, uint64_t img_size, int flags,
                      Error **errp, bool quiet);