All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/15] Protect the block layer with a rwlock: part 3
@ 2022-11-16 14:07 Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry Emanuele Giuseppe Esposito
                   ` (16 more replies)
  0 siblings, 17 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

Please read "Protect the block layer with a rwlock: part 1" and
"Protect the block layer with a rwlock: part 2" for an
additional introduction and aim of this series.

In this serie, we cover the remaining BlockDriver IO callbacks that were not
running in coroutine, therefore not using the graph rdlock.
Therefore convert them to coroutines, using either g_c_w or a new
variant introduced in this serie (see below).

We need to convert these callbacks into coroutine because non-coroutine code
is tied to the main thread, even though it will still delegate I/O accesses to
the iothread (via the bdrv_coroutine_enter call in generated_co_wrappers).
Making callbacks run in coroutines provides more flexibility, because they run
entirely in iothreads and can use CoMutexes for mutual exclusion.

Here we introduce generated_co_wrapper_simple, a simplification of g_c_w that
only considers the case where the caller is not in a coroutine.
This simplifies and clarifies a lot when the caller is a coroutine or not, and
in the future will hopefully replace g_c_w.

While we are at it, try to directly call the _co_ counterpart of a g_c_w when
we know already that the function always run in a coroutine.

Based-on: <20221116135331.3052923-1-eesposit@redhat.com>

Thank you,
Emanuele

Emanuele Giuseppe Esposito (15):
  block/qed: add missing graph rdlock in qed_need_check_timer_entry
  block: rename refresh_total_sectors in bdrv_refresh_total_sectors
  block-backend: use bdrv_getlength instead of blk_getlength
  block: convert bdrv_refresh_total_sectors in generated_co_wrapper
  block: use bdrv_co_refresh_total_sectors when possible
  block: convert bdrv_get_allocated_file_size in
    generated_co_wrapper_simple
  block: convert bdrv_get_info in generated_co_wrapper
  block: convert bdrv_is_inserted in generated_co_wrapper_simple
  block-coroutine-wrapper: support void functions
  block: convert bdrv_eject in generated_co_wrapper_simple
  block: convert bdrv_lock_medium in generated_co_wrapper_simple
  block: convert bdrv_debug_event in generated_co_wrapper
  block: convert bdrv_io_plug in generated_co_wrapper_simple
  block: convert bdrv_io_unplug in generated_co_wrapper_simple
  block: rename newly converted BlockDriver IO coroutine functions

 block.c                            | 93 +++++++++++++++++++-----------
 block/blkdebug.c                   |  4 +-
 block/blkio.c                      |  6 +-
 block/blklogwrites.c               |  2 +-
 block/blkreplay.c                  |  2 +-
 block/blkverify.c                  |  2 +-
 block/block-backend.c              | 43 ++++++++------
 block/commit.c                     |  4 +-
 block/copy-on-read.c               | 12 ++--
 block/crypto.c                     |  6 +-
 block/curl.c                       |  8 +--
 block/file-posix.c                 | 48 +++++++--------
 block/file-win32.c                 |  8 +--
 block/filter-compress.c            | 10 ++--
 block/gluster.c                    | 16 ++---
 block/io.c                         | 78 +++++++++++++------------
 block/iscsi.c                      |  8 +--
 block/meson.build                  |  1 +
 block/mirror.c                     | 17 ++++--
 block/nbd.c                        |  6 +-
 block/nfs.c                        |  2 +-
 block/null.c                       |  8 +--
 block/nvme.c                       |  6 +-
 block/preallocate.c                |  2 +-
 block/qcow.c                       |  2 +-
 block/qcow2-refcount.c             |  2 +-
 block/qcow2.c                      |  6 +-
 block/qed.c                        |  7 ++-
 block/quorum.c                     |  2 +-
 block/raw-format.c                 | 14 ++---
 block/rbd.c                        |  4 +-
 block/replication.c                |  2 +-
 block/ssh.c                        |  2 +-
 block/stream.c                     |  4 +-
 block/throttle.c                   |  2 +-
 block/vdi.c                        |  2 +-
 block/vhdx.c                       |  2 +-
 block/vmdk.c                       |  4 +-
 block/vpc.c                        |  2 +-
 blockdev.c                         |  8 ++-
 hw/scsi/scsi-disk.c                |  5 ++
 include/block/block-io.h           | 40 +++++++++----
 include/block/block_int-common.h   | 37 +++++++-----
 include/block/block_int-io.h       |  5 +-
 include/sysemu/block-backend-io.h  | 32 +++++++---
 scripts/block-coroutine-wrapper.py | 19 ++++--
 tests/unit/test-block-iothread.c   |  7 +++
 47 files changed, 364 insertions(+), 238 deletions(-)

-- 
2.31.1



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

* [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 02/15] block: rename refresh_total_sectors in bdrv_refresh_total_sectors Emanuele Giuseppe Esposito
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

This function is called in two different places:
- timer callback, which does not take the graph rdlock.
- bdrv_qed_drain_begin(), which is a .bdrv_drain_begin()
  callback that will soon take the lock.

Since it calls recursive functions that traverse the
graph, we need to protect them with the graph rdlock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/qed.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qed.c b/block/qed.c
index c2691a85b1..778b23d0f6 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -282,11 +282,13 @@ static void coroutine_fn qed_unplug_allocating_write_reqs(BDRVQEDState *s)
     qemu_co_mutex_unlock(&s->table_lock);
 }
 
+/* Called with graph rdlock taken */
 static void coroutine_fn qed_need_check_timer(BDRVQEDState *s)
 {
     int ret;
 
     trace_qed_need_check_timer_cb(s);
+    assert_bdrv_graph_readable();
 
     if (!qed_plug_allocating_write_reqs(s)) {
         return;
@@ -312,6 +314,7 @@ static void coroutine_fn qed_need_check_timer(BDRVQEDState *s)
 static void coroutine_fn qed_need_check_timer_entry(void *opaque)
 {
     BDRVQEDState *s = opaque;
+    GRAPH_RDLOCK_GUARD();
 
     qed_need_check_timer(opaque);
     bdrv_dec_in_flight(s->bs);
-- 
2.31.1



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

* [PATCH 02/15] block: rename refresh_total_sectors in bdrv_refresh_total_sectors
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 03/15] block-backend: use bdrv_getlength instead of blk_getlength Emanuele Giuseppe Esposito
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

Name is not right, since we are going to convert this in
a generated_co_wrapper.

No functional change intended.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                      | 8 ++++----
 block/io.c                   | 8 +++++---
 include/block/block_int-io.h | 2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 1a6ae08879..ba4bbb42aa 100644
--- a/block.c
+++ b/block.c
@@ -1044,7 +1044,7 @@ static int find_image_format(BlockBackend *file, const char *filename,
  * Set the current 'total_sectors' value
  * Return 0 on success, -errno on error.
  */
-int refresh_total_sectors(BlockDriverState *bs, int64_t hint)
+int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
@@ -1662,7 +1662,7 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
     bs->supported_read_flags |= BDRV_REQ_REGISTERED_BUF;
     bs->supported_write_flags |= BDRV_REQ_REGISTERED_BUF;
 
-    ret = refresh_total_sectors(bs, bs->total_sectors);
+    ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
         return ret;
@@ -5783,7 +5783,7 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs)
         return -ENOMEDIUM;
 
     if (drv->has_variable_length) {
-        int ret = refresh_total_sectors(bs, bs->total_sectors);
+        int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
         if (ret < 0) {
             return ret;
         }
@@ -6565,7 +6565,7 @@ int bdrv_activate(BlockDriverState *bs, Error **errp)
             bdrv_dirty_bitmap_skip_store(bm, false);
         }
 
-        ret = refresh_total_sectors(bs, bs->total_sectors);
+        ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
         if (ret < 0) {
             bs->open_flags |= BDRV_O_INACTIVE;
             error_setg_errno(errp, -ret, "Could not refresh total sector count");
diff --git a/block/io.c b/block/io.c
index 7d1d0c48b0..99867fe148 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3411,15 +3411,17 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
         goto out;
     }
 
-    ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+    ret = bdrv_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
     } else {
         offset = bs->total_sectors * BDRV_SECTOR_SIZE;
     }
-    /* It's possible that truncation succeeded but refresh_total_sectors
+    /*
+     * It's possible that truncation succeeded but bdrv_refresh_total_sectors
      * failed, but the latter doesn't affect how we should finish the request.
-     * Pass 0 as the last parameter so that dirty bitmaps etc. are handled. */
+     * Pass 0 as the last parameter so that dirty bitmaps etc. are handled.
+     */
     bdrv_co_write_req_finish(child, offset - new_bytes, new_bytes, &req, 0);
 
 out:
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index ac6ad3b3ff..453855e651 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -122,7 +122,7 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, int64_t src_offset,
                                        BdrvRequestFlags read_flags,
                                        BdrvRequestFlags write_flags);
 
-int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
+int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint);
 
 BdrvChild *bdrv_cow_child(BlockDriverState *bs);
 BdrvChild *bdrv_filter_child(BlockDriverState *bs);
-- 
2.31.1



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

* [PATCH 03/15] block-backend: use bdrv_getlength instead of blk_getlength
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 02/15] block: rename refresh_total_sectors in bdrv_refresh_total_sectors Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper Emanuele Giuseppe Esposito
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

The only difference is that blk_ checks if the block is available,
but this check is already performed above in blk_check_byte_request().

This is in preparation for the graph rdlock, which will be taken
by both the callers of blk_check_byte_request() and blk_getlength().

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6f0dd15808..4af9a3179e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
     }
 
     if (!blk->allow_write_beyond_eof) {
-        len = blk_getlength(blk);
+        len = bdrv_getlength(blk_bs(blk));
         if (len < 0) {
             return len;
         }
-- 
2.31.1



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

* [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (2 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 03/15] block-backend: use bdrv_getlength instead of blk_getlength Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible Emanuele Giuseppe Esposito
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_getlength is categorized as IO callb
ack, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, sin
ce the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper to automatically
create a wrapper for bdrv_refresh_total_sectors.
Unfortunately we cannot use a generated_co_wrapper_sim
ple,
because the function is called by mixed functions that
run both
in coroutine and non-coroutine context (for example bd
rv_open_driver()).

Unfortunately this callback requires multiple bdrv_* a
nd blk_*
callbacks to be converted, because there are different
mixed
callers (coroutines and not) calling this callback fro
m different
function paths.

Because now this function creates a new coroutine and polls,
we need to take the AioContext lock where it is missing,
for the only reason that internally g_c_w calls AIO_WAIT_WHILE
and it expects to release the AioContext lock.
This is especially messy when a g_c_w creates a coroutine and
polls in bdrv_open_driver, because this function has so many
callers in so many context that it can easily lead to deadlocks.
Therefore the new rule for bdrv_open_driver is that the caller
must always hold the AioContext lock of the given bs (except
if it is a coroutine), because the function calls
bdrv_refresh_total_sectors() which is a g_c_w.

Once the rwlock is ultimated and placed in every place it needs
to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove
the AioContext lock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                           | 29 ++++++++++++++++++++++++-----
 block/block-backend.c             | 12 ++++++++----
 block/commit.c                    |  4 ++--
 block/meson.build                 |  1 +
 block/mirror.c                    |  7 +++++--
 hw/scsi/scsi-disk.c               |  5 +++++
 include/block/block-io.h          |  8 ++++++--
 include/block/block_int-common.h  |  3 ++-
 include/block/block_int-io.h      |  5 ++++-
 include/sysemu/block-backend-io.h | 10 ++++++++--
 tests/unit/test-block-iothread.c  |  7 +++++++
 11 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/block.c b/block.c
index ba4bbb42aa..c7b32ba17a 100644
--- a/block.c
+++ b/block.c
@@ -1044,10 +1044,12 @@ static int find_image_format(BlockBackend *file, const char *filename,
  * Set the current 'total_sectors' value
  * Return 0 on success, -errno on error.
  */
-int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
+int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
+                                               int64_t hint)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv) {
         return -ENOMEDIUM;
@@ -1611,6 +1613,11 @@ out:
     g_free(gen_node_name);
 }
 
+/*
+ * The caller must always hold @bs AioContext lock, because this function calls
+ * bdrv_refresh_total_sectors() which polls when called from non-coroutine
+ * context.
+ */
 static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
                             const char *node_name, QDict *options,
                             int open_flags, Error **errp)
@@ -3750,6 +3757,10 @@ out:
  * The reference parameter may be used to specify an existing block device which
  * should be opened. If specified, neither options nor a filename may be given,
  * nor can an existing BDS be reused (that is, *pbs has to be NULL).
+ *
+ * The caller must always hold @filename AioContext lock, because this
+ * function eventually calls bdrv_refresh_total_sectors() which polls
+ * when called from non-coroutine context.
  */
 static BlockDriverState *bdrv_open_inherit(const char *filename,
                                            const char *reference,
@@ -4038,6 +4049,11 @@ close_and_fail:
     return NULL;
 }
 
+/*
+ * The caller must always hold @filename AioContext lock, because this
+ * function eventually calls bdrv_refresh_total_sectors() which polls
+ * when called from non-coroutine context.
+ */
 BlockDriverState *bdrv_open(const char *filename, const char *reference,
                             QDict *options, int flags, Error **errp)
 {
@@ -5774,16 +5790,17 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
 /**
  * Return number of sectors on success, -errno on error.
  */
-int64_t bdrv_nb_sectors(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_nb_sectors(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv)
         return -ENOMEDIUM;
 
     if (drv->has_variable_length) {
-        int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
+        int ret = bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
         if (ret < 0) {
             return ret;
         }
@@ -5795,11 +5812,13 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs)
  * Return length in bytes on success, -errno on error.
  * The length is always a multiple of BDRV_SECTOR_SIZE.
  */
-int64_t bdrv_getlength(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs)
 {
-    int64_t ret = bdrv_nb_sectors(bs);
+    int64_t ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
+    ret = bdrv_co_nb_sectors(bs);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/block-backend.c b/block/block-backend.c
index 4af9a3179e..fc19cf423e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1603,14 +1603,16 @@ BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                         flags | BDRV_REQ_ZERO_WRITE, cb, opaque);
 }
 
-int64_t blk_getlength(BlockBackend *blk)
+int64_t coroutine_fn blk_co_getlength(BlockBackend *blk)
 {
     IO_CODE();
+    GRAPH_RDLOCK_GUARD();
+
     if (!blk_is_available(blk)) {
         return -ENOMEDIUM;
     }
 
-    return bdrv_getlength(blk_bs(blk));
+    return bdrv_co_getlength(blk_bs(blk));
 }
 
 void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr)
@@ -1623,14 +1625,16 @@ void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr)
     }
 }
 
-int64_t blk_nb_sectors(BlockBackend *blk)
+int64_t coroutine_fn blk_co_nb_sectors(BlockBackend *blk)
 {
     IO_CODE();
+    GRAPH_RDLOCK_GUARD();
+
     if (!blk_is_available(blk)) {
         return -ENOMEDIUM;
     }
 
-    return bdrv_nb_sectors(blk_bs(blk));
+    return bdrv_co_nb_sectors(blk_bs(blk));
 }
 
 BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
diff --git a/block/commit.c b/block/commit.c
index 9d4d908344..986e7502eb 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -123,13 +123,13 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
     QEMU_AUTO_VFREE void *buf = NULL;
     int64_t len, base_len;
 
-    len = blk_getlength(s->top);
+    len = blk_co_getlength(s->top);
     if (len < 0) {
         return len;
     }
     job_progress_set_remaining(&s->common.job, len);
 
-    base_len = blk_getlength(s->base);
+    base_len = blk_co_getlength(s->base);
     if (base_len < 0) {
         return base_len;
     }
diff --git a/block/meson.build b/block/meson.build
index 90011a2805..3662852dc2 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -139,6 +139,7 @@ block_gen_c = custom_target('block-gen.c',
                             input: files(
                                       '../include/block/block-io.h',
                                       '../include/block/dirty-bitmap.h',
+                                      '../include/block/block_int-io.h',
                                       '../include/block/block-global-state.h',
                                       '../include/sysemu/block-backend-io.h',
                                       'coroutines.h'
diff --git a/block/mirror.c b/block/mirror.c
index 02ee7bba08..aecc895b73 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -915,13 +915,16 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
         goto immediate_exit;
     }
 
-    s->bdev_length = bdrv_getlength(bs);
+    bdrv_graph_co_rdlock();
+    s->bdev_length = bdrv_co_getlength(bs);
+    bdrv_graph_co_rdunlock();
+
     if (s->bdev_length < 0) {
         ret = s->bdev_length;
         goto immediate_exit;
     }
 
-    target_length = blk_getlength(s->target);
+    target_length = blk_co_getlength(s->target);
     if (target_length < 0) {
         ret = target_length;
         goto immediate_exit;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e493c28814..d4e360850f 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2332,10 +2332,15 @@ static void scsi_disk_reset(DeviceState *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
     uint64_t nb_sectors;
+    AioContext *ctx;
 
     scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
 
+    ctx = blk_get_aio_context(s->qdev.conf.blk);
+    aio_context_acquire(ctx);
     blk_get_geometry(s->qdev.conf.blk, &nb_sectors);
+    aio_context_release(ctx);
+
     nb_sectors /= s->qdev.blocksize / BDRV_SECTOR_SIZE;
     if (nb_sectors) {
         nb_sectors--;
diff --git a/include/block/block-io.h b/include/block/block-io.h
index f45ef6206f..4fb95e9b7a 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -68,8 +68,12 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
                                   PreallocMode prealloc, BdrvRequestFlags flags,
                                   Error **errp);
 
-int64_t bdrv_nb_sectors(BlockDriverState *bs);
-int64_t bdrv_getlength(BlockDriverState *bs);
+int64_t coroutine_fn bdrv_co_nb_sectors(BlockDriverState *bs);
+int64_t generated_co_wrapper bdrv_nb_sectors(BlockDriverState *bs);
+
+int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs);
+int64_t generated_co_wrapper bdrv_getlength(BlockDriverState *bs);
+
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
                                BlockDriverState *in_bs, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 00c581a712..d1cf52d4f7 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -723,7 +723,8 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
                                          bool exact, PreallocMode prealloc,
                                          BdrvRequestFlags flags, Error **errp);
-    int64_t (*bdrv_getlength)(BlockDriverState *bs);
+    /* Called with graph rdlock held. */
+    int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
     int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
 
     /* Does not need graph rdlock, since it does not traverse the graph */
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 453855e651..5794160a0f 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -122,7 +122,10 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, int64_t src_offset,
                                        BdrvRequestFlags read_flags,
                                        BdrvRequestFlags write_flags);
 
-int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint);
+int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
+                                               int64_t hint);
+int generated_co_wrapper bdrv_refresh_total_sectors(BlockDriverState *bs,
+                                                    int64_t hint);
 
 BdrvChild *bdrv_cow_child(BlockDriverState *bs);
 BdrvChild *bdrv_filter_child(BlockDriverState *bs);
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index 887a29dc59..5456822b07 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -57,9 +57,15 @@ bool blk_is_inserted(BlockBackend *blk);
 bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
-int64_t blk_getlength(BlockBackend *blk);
+
+int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
+int64_t generated_co_wrapper_blk blk_getlength(BlockBackend *blk);
+
 void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
-int64_t blk_nb_sectors(BlockBackend *blk);
+
+int64_t coroutine_fn blk_co_nb_sectors(BlockBackend *blk);
+int64_t generated_co_wrapper_blk blk_nb_sectors(BlockBackend *blk);
+
 void *blk_try_blockalign(BlockBackend *blk, size_t size);
 void *blk_blockalign(BlockBackend *blk, size_t size);
 bool blk_is_writable(BlockBackend *blk);
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 8ca5adec5e..d2d69efd00 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -831,7 +831,14 @@ static void test_attach_second_node(void)
     qdict_put_str(options, "driver", "raw");
     qdict_put_str(options, "file", "base");
 
+    /*
+     * Acquire iothread AioContext, since bs is in ctx and bdrv_open will
+     * call bdrv_refresh_total_sectors(), a generated_co_wrapper function
+     * that therefore expects ctx lock to be held.
+     */
+    aio_context_acquire(ctx);
     filter = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
+    aio_context_release(ctx);
     g_assert(blk_get_aio_context(blk) == ctx);
     g_assert(bdrv_get_aio_context(bs) == ctx);
     g_assert(bdrv_get_aio_context(filter) == ctx);
-- 
2.31.1



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

* [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (3 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 06/15] block: convert bdrv_get_allocated_file_size in generated_co_wrapper_simple Emanuele Giuseppe Esposito
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

In some places we are sure we are always running in a
coroutine, therefore it's useless to call the generated_co_wrapper,
instead call directly the _co_ function.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c | 6 +++---
 block/copy-on-read.c  | 2 +-
 block/io.c            | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index fc19cf423e..6a84a2a019 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1235,8 +1235,8 @@ void blk_set_disable_request_queuing(BlockBackend *blk, bool disable)
     blk->disable_request_queuing = disable;
 }
 
-static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
-                                  int64_t bytes)
+static coroutine_fn int blk_check_byte_request(BlockBackend *blk,
+                                               int64_t offset, int64_t bytes)
 {
     int64_t len;
 
@@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
     }
 
     if (!blk->allow_write_beyond_eof) {
-        len = bdrv_getlength(blk_bs(blk));
+        len = bdrv_co_getlength(blk_bs(blk));
         if (len < 0) {
             return len;
         }
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 815ac1d835..74f7727a02 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -122,7 +122,7 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
 
 static int64_t cor_getlength(BlockDriverState *bs)
 {
-    return bdrv_getlength(bs->file->bs);
+    return bdrv_co_getlength(bs->file->bs);
 }
 
 
diff --git a/block/io.c b/block/io.c
index 99867fe148..3f65c57f82 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3381,7 +3381,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
     if (new_bytes && backing) {
         int64_t backing_len;
 
-        backing_len = bdrv_getlength(backing->bs);
+        backing_len = bdrv_co_getlength(backing->bs);
         if (backing_len < 0) {
             ret = backing_len;
             error_setg_errno(errp, -ret, "Could not get backing file size");
@@ -3411,7 +3411,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
         goto out;
     }
 
-    ret = bdrv_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+    ret = bdrv_co_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
     } else {
-- 
2.31.1



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

* [PATCH 06/15] block: convert bdrv_get_allocated_file_size in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (4 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 07/15] block: convert bdrv_get_info in generated_co_wrapper Emanuele Giuseppe Esposito
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_get_allocated_file_size  is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper_simple to automatically
create a wrapper with the same name that will take care of
always calling the function in a coroutine.
This is a generated_co_wrapper_simple callback, which means
it assumes that bdrv_get_allocated_file_size  is never caled in a coroutine
context.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                          | 7 ++++---
 block/qcow2-refcount.c           | 2 +-
 include/block/block-io.h         | 5 ++++-
 include/block/block_int-common.h | 3 ++-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index c7b32ba17a..5650d6fe63 100644
--- a/block.c
+++ b/block.c
@@ -5708,7 +5708,7 @@ static int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs)
         if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA |
                            BDRV_CHILD_FILTERED))
         {
-            child_size = bdrv_get_allocated_file_size(child->bs);
+            child_size = bdrv_co_get_allocated_file_size(child->bs);
             if (child_size < 0) {
                 return child_size;
             }
@@ -5723,10 +5723,11 @@ static int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs)
  * Length of a allocated file in bytes. Sparse files are counted by actual
  * allocated space. Return < 0 if error or unknown.
  */
-int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv) {
         return -ENOMEDIUM;
@@ -5744,7 +5745,7 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
         return -ENOTSUP;
     } else if (drv->is_filter) {
         /* Filter drivers default to the size of their filtered child */
-        return bdrv_get_allocated_file_size(bdrv_filter_bs(bs));
+        return bdrv_co_get_allocated_file_size(bdrv_filter_bs(bs));
     } else {
         /* Other drivers default to summing their children's sizes */
         return bdrv_sum_allocated_file_size(bs);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 81264740f0..487681d85e 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3719,7 +3719,7 @@ int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs)
         return file_length;
     }
 
-    real_allocation = bdrv_get_allocated_file_size(bs->file->bs);
+    real_allocation = bdrv_co_get_allocated_file_size(bs->file->bs);
     if (real_allocation < 0) {
         return real_allocation;
     }
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 4fb95e9b7a..ac509c461f 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -74,7 +74,10 @@ int64_t generated_co_wrapper bdrv_nb_sectors(BlockDriverState *bs);
 int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs);
 int64_t generated_co_wrapper bdrv_getlength(BlockDriverState *bs);
 
-int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
+int64_t generated_co_wrapper_simple bdrv_get_allocated_file_size(
+                                                        BlockDriverState *bs);
+int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs);
+
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
                                BlockDriverState *in_bs, Error **errp);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index d1cf52d4f7..42daf86c65 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -725,7 +725,8 @@ struct BlockDriver {
                                          BdrvRequestFlags flags, Error **errp);
     /* Called with graph rdlock held. */
     int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
-    int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+    /* Called with graph rdlock held. */
+    int64_t coroutine_fn (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
 
     /* Does not need graph rdlock, since it does not traverse the graph */
     BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
-- 
2.31.1



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

* [PATCH 07/15] block: convert bdrv_get_info in generated_co_wrapper
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (5 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 06/15] block: convert bdrv_get_allocated_file_size in generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple Emanuele Giuseppe Esposito
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_get_info is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper to automatically
create a wrapper with the same name.
Unfortunately we cannot use a generated_co_wrapper_simple,
because the function is called by mixed functions that run both
in coroutine and non-coroutine context (for example block_load
in migration/block.c).

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                          |  6 ++++--
 block/crypto.c                   |  2 +-
 block/io.c                       |  8 ++++----
 block/mirror.c                   | 10 +++++++---
 block/raw-format.c               |  2 +-
 block/stream.c                   |  4 +++-
 include/block/block-io.h         |  6 +++++-
 include/block/block_int-common.h |  4 +++-
 8 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index 5650d6fe63..2cdede9c01 100644
--- a/block.c
+++ b/block.c
@@ -6279,11 +6279,13 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
     pstrcpy(filename, filename_size, bs->backing_file);
 }
 
-int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     int ret;
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
+
     /* if bs->drv == NULL, bs is closed, so there's nothing to do here */
     if (!drv) {
         return -ENOMEDIUM;
@@ -6291,7 +6293,7 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     if (!drv->bdrv_get_info) {
         BlockDriverState *filtered = bdrv_filter_bs(bs);
         if (filtered) {
-            return bdrv_get_info(filtered, bdi);
+            return bdrv_co_get_info(filtered, bdi);
         }
         return -ENOTSUP;
     }
diff --git a/block/crypto.c b/block/crypto.c
index 2fb8add458..12a84dd1cd 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -743,7 +743,7 @@ static int block_crypto_get_info_luks(BlockDriverState *bs,
     BlockDriverInfo subbdi;
     int ret;
 
-    ret = bdrv_get_info(bs->file->bs, &subbdi);
+    ret = bdrv_co_get_info(bs->file->bs, &subbdi);
     if (ret != 0) {
         return ret;
     }
diff --git a/block/io.c b/block/io.c
index 3f65c57f82..99ef9a8cb9 100644
--- a/block/io.c
+++ b/block/io.c
@@ -694,14 +694,14 @@ BdrvTrackedRequest *coroutine_fn bdrv_co_get_self_request(BlockDriverState *bs)
 /**
  * Round a region to cluster boundaries
  */
-void bdrv_round_to_clusters(BlockDriverState *bs,
+void coroutine_fn bdrv_round_to_clusters(BlockDriverState *bs,
                             int64_t offset, int64_t bytes,
                             int64_t *cluster_offset,
                             int64_t *cluster_bytes)
 {
     BlockDriverInfo bdi;
     IO_CODE();
-    if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
+    if (bdrv_co_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
         *cluster_offset = offset;
         *cluster_bytes = bytes;
     } else {
@@ -711,12 +711,12 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
     }
 }
 
-static int bdrv_get_cluster_size(BlockDriverState *bs)
+static coroutine_fn int bdrv_get_cluster_size(BlockDriverState *bs)
 {
     BlockDriverInfo bdi;
     int ret;
 
-    ret = bdrv_get_info(bs, &bdi);
+    ret = bdrv_co_get_info(bs, &bdi);
     if (ret < 0 || bdi.cluster_size == 0) {
         return bs->bl.request_alignment;
     } else {
diff --git a/block/mirror.c b/block/mirror.c
index aecc895b73..8dc136ebbe 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -576,8 +576,10 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         } else if (ret >= 0 && !(ret & BDRV_BLOCK_DATA)) {
             int64_t target_offset;
             int64_t target_bytes;
-            bdrv_round_to_clusters(blk_bs(s->target), offset, io_bytes,
-                                   &target_offset, &target_bytes);
+            WITH_GRAPH_RDLOCK_GUARD() {
+                bdrv_round_to_clusters(blk_bs(s->target), offset, io_bytes,
+                                       &target_offset, &target_bytes);
+            }
             if (target_offset == offset &&
                 target_bytes == io_bytes) {
                 mirror_method = ret & BDRV_BLOCK_ZERO ?
@@ -965,11 +967,13 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
      */
     bdrv_get_backing_filename(target_bs, backing_filename,
                               sizeof(backing_filename));
-    if (!bdrv_get_info(target_bs, &bdi) && bdi.cluster_size) {
+    bdrv_graph_co_rdlock();
+    if (!bdrv_co_get_info(target_bs, &bdi) && bdi.cluster_size) {
         s->target_cluster_size = bdi.cluster_size;
     } else {
         s->target_cluster_size = BDRV_SECTOR_SIZE;
     }
+    bdrv_graph_co_rdunlock();
     if (backing_filename[0] && !bdrv_backing_chain_next(target_bs) &&
         s->granularity < s->target_cluster_size) {
         s->buf_size = MAX(s->buf_size, s->target_cluster_size);
diff --git a/block/raw-format.c b/block/raw-format.c
index a68014ef0b..4773bf9cda 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -369,7 +369,7 @@ static BlockMeasureInfo *raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
 
 static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
-    return bdrv_get_info(bs->file->bs, bdi);
+    return bdrv_co_get_info(bs->file->bs, bdi);
 }
 
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
diff --git a/block/stream.c b/block/stream.c
index 22368ce186..6c9d0eefd1 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -141,7 +141,9 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
         return 0;
     }
 
-    len = bdrv_getlength(s->target_bs);
+    WITH_GRAPH_RDLOCK_GUARD() {
+        len = bdrv_getlength(s->target_bs);
+    }
     if (len < 0) {
         return len;
     }
diff --git a/include/block/block-io.h b/include/block/block-io.h
index ac509c461f..ba60ffa43b 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -129,7 +129,11 @@ bool bdrv_supports_compressed_writes(BlockDriverState *bs);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
 const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
-int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+
+int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+int generated_co_wrapper bdrv_get_info(BlockDriverState *bs,
+                                       BlockDriverInfo *bdi);
+
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
                                           Error **errp);
 BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 42daf86c65..5874531617 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -740,7 +740,9 @@ struct BlockDriver {
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         size_t qiov_offset);
 
-    int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
+    /* Called with graph rdlock held. */
+    int coroutine_fn (*bdrv_get_info)(BlockDriverState *bs,
+                                      BlockDriverInfo *bdi);
 
     /* Does not need graph rdlock, since it does not traverse the graph */
     ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs,
-- 
2.31.1



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

* [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (6 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 07/15] block: convert bdrv_get_info in generated_co_wrapper Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 09/15] block-coroutine-wrapper: support void functions Emanuele Giuseppe Esposito
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_is_inserted is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper_simple to automatically
creates a wrapper with the same name that will take care of
always calling the function in a coroutine.
This is a generated_co_wrapper_simple callback, which means
it assumes that bdrv_is_inserted is never caled in a coroutine
context.

At the same time, add also blk_is_inserted as g_c_w, since it
is called in both coroutine and non contexts.

Because now this function creates a new coroutine and polls,
we need to take the AioContext lock where it is missing,
for the only reason that internally g_c_w calls AIO_WAIT_WHILE
and it expects to release the AioContext lock.
Once the rwlock is ultimated and placed in every place it needs
to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove
the AioContext lock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                           |  5 +++--
 block/block-backend.c             |  5 +++--
 block/io.c                        | 12 ++++++------
 blockdev.c                        |  8 +++++++-
 include/block/block-io.h          |  5 ++++-
 include/block/block_int-common.h  |  4 ++--
 include/sysemu/block-backend-io.h |  5 ++++-
 7 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 2cdede9c01..4205735308 100644
--- a/block.c
+++ b/block.c
@@ -6778,11 +6778,12 @@ out:
 /**
  * Return TRUE if the media is present
  */
-bool bdrv_is_inserted(BlockDriverState *bs)
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
     BdrvChild *child;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv) {
         return false;
@@ -6791,7 +6792,7 @@ bool bdrv_is_inserted(BlockDriverState *bs)
         return drv->bdrv_is_inserted(bs);
     }
     QLIST_FOREACH(child, &bs->children, next) {
-        if (!bdrv_is_inserted(child->bs)) {
+        if (!bdrv_co_is_inserted(child->bs)) {
             return false;
         }
     }
diff --git a/block/block-backend.c b/block/block-backend.c
index 6a84a2a019..9a500fdde3 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1994,12 +1994,13 @@ void blk_activate(BlockBackend *blk, Error **errp)
     bdrv_activate(bs, errp);
 }
 
-bool blk_is_inserted(BlockBackend *blk)
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
-    return bs && bdrv_is_inserted(bs);
+    return bs && bdrv_co_is_inserted(bs);
 }
 
 bool blk_is_available(BlockBackend *blk)
diff --git a/block/io.c b/block/io.c
index 99ef9a8cb9..88da9470c3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1598,7 +1598,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
 
     trace_bdrv_co_preadv_part(bs, offset, bytes, flags);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -2044,7 +2044,7 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
 
     trace_bdrv_co_pwritev_part(child->bs, offset, bytes, flags);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -2764,7 +2764,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
     assert_bdrv_graph_readable();
     bdrv_inc_in_flight(bs);
 
-    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) ||
+    if (!bdrv_co_is_inserted(bs) || bdrv_is_read_only(bs) ||
         bdrv_is_sg(bs)) {
         goto early_exit;
     }
@@ -2889,7 +2889,7 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
     IO_CODE();
     assert_bdrv_graph_readable();
 
-    if (!bs || !bs->drv || !bdrv_is_inserted(bs)) {
+    if (!bs || !bs->drv || !bdrv_co_is_inserted(bs)) {
         return -ENOMEDIUM;
     }
 
@@ -3173,7 +3173,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
     assert(!(read_flags & BDRV_REQ_NO_WAIT));
     assert(!(write_flags & BDRV_REQ_NO_WAIT));
 
-    if (!dst || !dst->bs || !bdrv_is_inserted(dst->bs)) {
+    if (!dst || !dst->bs || !bdrv_co_is_inserted(dst->bs)) {
         return -ENOMEDIUM;
     }
     ret = bdrv_check_request32(dst_offset, bytes, NULL, 0);
@@ -3184,7 +3184,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
         return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, write_flags);
     }
 
-    if (!src || !src->bs || !bdrv_is_inserted(src->bs)) {
+    if (!src || !src->bs || !bdrv_co_is_inserted(src->bs)) {
         return -ENOMEDIUM;
     }
     ret = bdrv_check_request32(src_offset, bytes, NULL, 0);
diff --git a/blockdev.c b/blockdev.c
index 8ffb3d9537..bbe5207e8f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1023,6 +1023,7 @@ fail:
 static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
 {
     BlockDriverState *bs;
+    AioContext *aio_context;
 
     bs = bdrv_lookup_bs(name, name, errp);
     if (bs == NULL) {
@@ -1034,11 +1035,16 @@ static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
         return NULL;
     }
 
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
     if (!bdrv_is_inserted(bs)) {
         error_setg(errp, "Device has no medium");
-        return NULL;
+        bs = NULL;
     }
 
+    aio_context_release(aio_context);
+
     return bs;
 }
 
diff --git a/include/block/block-io.h b/include/block/block-io.h
index ba60ffa43b..3432e6ad3e 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -120,7 +120,10 @@ bool bdrv_is_read_only(BlockDriverState *bs);
 bool bdrv_is_writable(BlockDriverState *bs);
 bool bdrv_is_sg(BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
-bool bdrv_is_inserted(BlockDriverState *bs);
+
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
+bool generated_co_wrapper_simple bdrv_is_inserted(BlockDriverState *bs);
+
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 5874531617..4cad48b2ad 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -759,8 +759,8 @@ struct BlockDriver {
                                           QEMUIOVector *qiov,
                                           int64_t pos);
 
-    /* removable device specific */
-    bool (*bdrv_is_inserted)(BlockDriverState *bs);
+    /* removable device specific. Called with graph rdlock held. */
+    bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
     void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index 5456822b07..bf88f7699e 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -53,7 +53,10 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
 
 void blk_inc_in_flight(BlockBackend *blk);
 void blk_dec_in_flight(BlockBackend *blk);
-bool blk_is_inserted(BlockBackend *blk);
+
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk);
+bool generated_co_wrapper blk_is_inserted(BlockBackend *blk);
+
 bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
-- 
2.31.1



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

* [PATCH 09/15] block-coroutine-wrapper: support void functions
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (7 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 10/15] block: convert bdrv_eject in generated_co_wrapper_simple Emanuele Giuseppe Esposito
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

Just omit the various 'return' when the return type is
void.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 scripts/block-coroutine-wrapper.py | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 05267761f0..8d7aa5d7f4 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -115,7 +115,7 @@ def create_g_c_w(func: FuncDecl) -> str:
 {func.return_type} {func.name}({ func.gen_list('{decl}') })
 {{
     if (qemu_in_coroutine()) {{
-        return {name}({ func.gen_list('{name}') });
+        {func.co_ret}{name}({ func.gen_list('{name}') });
     }} else {{
         {struct_name} s = {{
             .poll_state.bs = {func.bs},
@@ -127,7 +127,7 @@ def create_g_c_w(func: FuncDecl) -> str:
         s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
 
         bdrv_poll_co(&s.poll_state);
-        return s.ret;
+        {func.ret}
     }}
 }}"""
 
@@ -150,7 +150,7 @@ def create_coroutine_only(func: FuncDecl) -> str:
     s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
 
     bdrv_poll_co(&s.poll_state);
-    return s.ret;
+    {func.ret}
 }}"""
 
 
@@ -168,6 +168,15 @@ def gen_wrapper(func: FuncDecl) -> str:
         graph_lock='    bdrv_graph_co_rdlock();'
         graph_unlock='    bdrv_graph_co_rdunlock();'
 
+    func.get_result = 's->ret = '
+    func.ret = 'return s.ret;'
+    func.co_ret = 'return '
+    func.return_field = func.return_type + " ret;"
+    if func.return_type == 'void':
+        func.get_result = ''
+        func.ret = ''
+        func.co_ret = ''
+        func.return_field = ''
 
     t = func.args[0].type
     if t == 'BlockDriverState *':
@@ -193,7 +202,7 @@ def gen_wrapper(func: FuncDecl) -> str:
 
 typedef struct {struct_name} {{
     BdrvPollCo poll_state;
-    {func.return_type} ret;
+    {func.return_field}
 { func.gen_block('    {decl};') }
 }} {struct_name};
 
@@ -202,7 +211,7 @@ def gen_wrapper(func: FuncDecl) -> str:
     {struct_name} *s = opaque;
 
 {graph_lock}
-    s->ret = {name}({ func.gen_list('s->{name}') });
+    {func.get_result}{name}({ func.gen_list('s->{name}') });
 {graph_unlock}
     s->poll_state.in_progress = false;
 
-- 
2.31.1



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

* [PATCH 10/15] block: convert bdrv_eject in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (8 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 09/15] block-coroutine-wrapper: support void functions Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 11/15] block: convert bdrv_lock_medium " Emanuele Giuseppe Esposito
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_eject is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

The only caller of this function is blk_eject, therefore
make blk_eject a generated_co_wrapper_simple, so that
it always creates a new coroutine, and then make bdrv_eject
coroutine_fn.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                           | 3 ++-
 block/block-backend.c             | 5 +++--
 block/copy-on-read.c              | 2 +-
 block/filter-compress.c           | 2 +-
 block/raw-format.c                | 2 +-
 include/block/block-io.h          | 3 ++-
 include/block/block_int-common.h  | 2 +-
 include/sysemu/block-backend-io.h | 4 +++-
 8 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 4205735308..ffbb8c602f 100644
--- a/block.c
+++ b/block.c
@@ -6802,10 +6802,11 @@ bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs)
 /**
  * If eject_flag is TRUE, eject the media. Otherwise, close the tray
  */
-void bdrv_eject(BlockDriverState *bs, bool eject_flag)
+void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (drv && drv->bdrv_eject) {
         drv->bdrv_eject(bs, eject_flag);
diff --git a/block/block-backend.c b/block/block-backend.c
index 9a500fdde3..308dd2070a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2019,14 +2019,15 @@ void blk_lock_medium(BlockBackend *blk, bool locked)
     }
 }
 
-void blk_eject(BlockBackend *blk, bool eject_flag)
+void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag)
 {
     BlockDriverState *bs = blk_bs(blk);
     char *id;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (bs) {
-        bdrv_eject(bs, eject_flag);
+        bdrv_co_eject(bs, eject_flag);
     }
 
     /* Whether or not we ejected on the backend,
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 74f7727a02..76f884a6ae 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -218,7 +218,7 @@ static int coroutine_fn cor_co_pwritev_compressed(BlockDriverState *bs,
 
 static void cor_eject(BlockDriverState *bs, bool eject_flag)
 {
-    bdrv_eject(bs->file->bs, eject_flag);
+    bdrv_co_eject(bs->file->bs, eject_flag);
 }
 
 
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 305716c86c..571e4684dd 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -118,7 +118,7 @@ static void compress_refresh_limits(BlockDriverState *bs, Error **errp)
 
 static void compress_eject(BlockDriverState *bs, bool eject_flag)
 {
-    bdrv_eject(bs->file->bs, eject_flag);
+    bdrv_co_eject(bs->file->bs, eject_flag);
 }
 
 
diff --git a/block/raw-format.c b/block/raw-format.c
index 4773bf9cda..9b23cf17bb 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -405,7 +405,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
 
 static void raw_eject(BlockDriverState *bs, bool eject_flag)
 {
-    bdrv_eject(bs->file->bs, eject_flag);
+    bdrv_co_eject(bs->file->bs, eject_flag);
 }
 
 static void raw_lock_medium(BlockDriverState *bs, bool locked)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 3432e6ad3e..204adeb701 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -125,7 +125,8 @@ bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
 bool generated_co_wrapper_simple bdrv_is_inserted(BlockDriverState *bs);
 
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
-void bdrv_eject(BlockDriverState *bs, bool eject_flag);
+void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag);
+
 const char *bdrv_get_format_name(BlockDriverState *bs);
 
 bool bdrv_supports_compressed_writes(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 4cad48b2ad..d01b3d44f5 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -761,7 +761,7 @@ struct BlockDriver {
 
     /* removable device specific. Called with graph rdlock held. */
     bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
-    void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
+    void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices. Called with graph rdlock taken. */
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index bf88f7699e..cc706c03d8 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -59,7 +59,9 @@ bool generated_co_wrapper blk_is_inserted(BlockBackend *blk);
 
 bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
-void blk_eject(BlockBackend *blk, bool eject_flag);
+
+void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag);
+void generated_co_wrapper_simple blk_eject(BlockBackend *blk, bool eject_flag);
 
 int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
 int64_t generated_co_wrapper_blk blk_getlength(BlockBackend *blk);
-- 
2.31.1



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

* [PATCH 11/15] block: convert bdrv_lock_medium in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (9 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 10/15] block: convert bdrv_eject in generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper Emanuele Giuseppe Esposito
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_lock_medium is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

The only caller of this function is blk_lock_medium, therefore
make blk_lock_medium a generated_co_wrapper_simple, so that
it always creates a new coroutine, and then make bdrv_lock_medium
coroutine_fn.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                           | 3 ++-
 block/block-backend.c             | 5 +++--
 block/copy-on-read.c              | 2 +-
 block/filter-compress.c           | 2 +-
 block/raw-format.c                | 2 +-
 include/block/block-io.h          | 2 +-
 include/block/block_int-common.h  | 2 +-
 include/sysemu/block-backend-io.h | 5 ++++-
 8 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index ffbb8c602f..afc5735b82 100644
--- a/block.c
+++ b/block.c
@@ -6817,10 +6817,11 @@ void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag)
  * Lock or unlock the media (if it is locked, the user won't be able
  * to eject it manually).
  */
-void bdrv_lock_medium(BlockDriverState *bs, bool locked)
+void coroutine_fn bdrv_co_lock_medium(BlockDriverState *bs, bool locked)
 {
     BlockDriver *drv = bs->drv;
     IO_CODE();
+    assert_bdrv_graph_readable();
     trace_bdrv_lock_medium(bs, locked);
 
     if (drv && drv->bdrv_lock_medium) {
diff --git a/block/block-backend.c b/block/block-backend.c
index 308dd2070a..75e2f2124f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2009,13 +2009,14 @@ bool blk_is_available(BlockBackend *blk)
     return blk_is_inserted(blk) && !blk_dev_is_tray_open(blk);
 }
 
-void blk_lock_medium(BlockBackend *blk, bool locked)
+void coroutine_fn blk_co_lock_medium(BlockBackend *blk, bool locked)
 {
     BlockDriverState *bs = blk_bs(blk);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (bs) {
-        bdrv_lock_medium(bs, locked);
+        bdrv_co_lock_medium(bs, locked);
     }
 }
 
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 76f884a6ae..ccc767f37b 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -224,7 +224,7 @@ static void cor_eject(BlockDriverState *bs, bool eject_flag)
 
 static void cor_lock_medium(BlockDriverState *bs, bool locked)
 {
-    bdrv_lock_medium(bs->file->bs, locked);
+    bdrv_co_lock_medium(bs->file->bs, locked);
 }
 
 
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 571e4684dd..e10312c225 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -124,7 +124,7 @@ static void compress_eject(BlockDriverState *bs, bool eject_flag)
 
 static void compress_lock_medium(BlockDriverState *bs, bool locked)
 {
-    bdrv_lock_medium(bs->file->bs, locked);
+    bdrv_co_lock_medium(bs->file->bs, locked);
 }
 
 
diff --git a/block/raw-format.c b/block/raw-format.c
index 9b23cf17bb..96a9b33384 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -410,7 +410,7 @@ static void raw_eject(BlockDriverState *bs, bool eject_flag)
 
 static void raw_lock_medium(BlockDriverState *bs, bool locked)
 {
-    bdrv_lock_medium(bs->file->bs, locked);
+    bdrv_co_lock_medium(bs->file->bs, locked);
 }
 
 static int coroutine_fn raw_co_ioctl(BlockDriverState *bs,
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 204adeb701..497580fc28 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -124,7 +124,7 @@ int bdrv_get_flags(BlockDriverState *bs);
 bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
 bool generated_co_wrapper_simple bdrv_is_inserted(BlockDriverState *bs);
 
-void bdrv_lock_medium(BlockDriverState *bs, bool locked);
+void coroutine_fn bdrv_co_lock_medium(BlockDriverState *bs, bool locked);
 void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag);
 
 const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index d01b3d44f5..3e1eba518c 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -762,7 +762,7 @@ struct BlockDriver {
     /* removable device specific. Called with graph rdlock held. */
     bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
     void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
-    void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
+    void coroutine_fn (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices. Called with graph rdlock taken. */
     BlockAIOCB *coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index cc706c03d8..dd8566ee69 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -58,7 +58,10 @@ bool coroutine_fn blk_co_is_inserted(BlockBackend *blk);
 bool generated_co_wrapper blk_is_inserted(BlockBackend *blk);
 
 bool blk_is_available(BlockBackend *blk);
-void blk_lock_medium(BlockBackend *blk, bool locked);
+
+void coroutine_fn blk_co_lock_medium(BlockBackend *blk, bool locked);
+void generated_co_wrapper_simple blk_lock_medium(BlockBackend *blk,
+                                                 bool locked);
 
 void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag);
 void generated_co_wrapper_simple blk_eject(BlockBackend *blk, bool eject_flag);
-- 
2.31.1



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

* [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (10 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 11/15] block: convert bdrv_lock_medium " Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple Emanuele Giuseppe Esposito
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_debug_event is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

Therefore use generated_co_wrapper to automatically
create a wrapper with the same name.
Unfortunately we cannot use a generated_co_wrapper_simple,
because the function is called by mixed functions that run both
in coroutine and non-coroutine context (for example blkdebug_open).

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                          |  4 +++-
 block/io.c                       | 22 +++++++++++-----------
 include/block/block-io.h         |  7 +++++--
 include/block/block_int-common.h |  4 +++-
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index afc5735b82..a2e3550d5c 100644
--- a/block.c
+++ b/block.c
@@ -6331,9 +6331,11 @@ BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs)
     return drv->bdrv_get_specific_stats(bs);
 }
 
-void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+void coroutine_fn bdrv_co_debug_event(BlockDriverState *bs, BlkdebugEvent event)
 {
     IO_CODE();
+    assert_bdrv_graph_readable();
+
     if (!bs || !bs->drv || !bs->drv->bdrv_debug_event) {
         return;
     }
diff --git a/block/io.c b/block/io.c
index 88da9470c3..11d2c5dcde 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1227,7 +1227,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
                 goto err;
             }
 
-            bdrv_debug_event(bs, BLKDBG_COR_WRITE);
+            bdrv_co_debug_event(bs, BLKDBG_COR_WRITE);
             if (drv->bdrv_co_pwrite_zeroes &&
                 buffer_is_zero(bounce_buffer, pnum)) {
                 /* FIXME: Should we (perhaps conditionally) be setting
@@ -1472,10 +1472,10 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
         qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
 
         if (pad->head) {
-            bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+            bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
         }
         if (pad->merge_reads && pad->tail) {
-            bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+            bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
         }
         ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
                                   align, &local_qiov, 0, 0);
@@ -1483,10 +1483,10 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
             return ret;
         }
         if (pad->head) {
-            bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+            bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
         }
         if (pad->merge_reads && pad->tail) {
-            bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+            bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
         }
 
         if (pad->merge_reads) {
@@ -1497,7 +1497,7 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
     if (pad->tail) {
         qemu_iovec_init_buf(&local_qiov, pad->tail_buf, align);
 
-        bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+        bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
         ret = bdrv_aligned_preadv(
                 child, req,
                 req->overlap_offset + req->overlap_bytes - align,
@@ -1505,7 +1505,7 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
         if (ret < 0) {
             return ret;
         }
-        bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+        bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
     }
 
 zero_mem:
@@ -1908,16 +1908,16 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
     if (ret < 0) {
         /* Do nothing, write notifier decided to fail this request */
     } else if (flags & BDRV_REQ_ZERO_WRITE) {
-        bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
+        bdrv_co_debug_event(bs, BLKDBG_PWRITEV_ZERO);
         ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
     } else if (flags & BDRV_REQ_WRITE_COMPRESSED) {
         ret = bdrv_driver_pwritev_compressed(bs, offset, bytes,
                                              qiov, qiov_offset);
     } else if (bytes <= max_transfer) {
-        bdrv_debug_event(bs, BLKDBG_PWRITEV);
+        bdrv_co_debug_event(bs, BLKDBG_PWRITEV);
         ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, qiov_offset, flags);
     } else {
-        bdrv_debug_event(bs, BLKDBG_PWRITEV);
+        bdrv_co_debug_event(bs, BLKDBG_PWRITEV);
         while (bytes_remaining) {
             int num = MIN(bytes_remaining, max_transfer);
             int local_flags = flags;
@@ -1940,7 +1940,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
             bytes_remaining -= num;
         }
     }
-    bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);
+    bdrv_co_debug_event(bs, BLKDBG_PWRITEV_DONE);
 
     if (ret >= 0) {
         ret = 0;
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 497580fc28..176e3cc734 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -170,12 +170,15 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size);
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
 void bdrv_disable_copy_on_read(BlockDriverState *bs);
 
-void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_fn bdrv_co_debug_event(BlockDriverState *bs,
+                                      BlkdebugEvent event);
+void generated_co_wrapper bdrv_debug_event(BlockDriverState *bs,
+                                                  BlkdebugEvent event);
 
 #define BLKDBG_EVENT(child, evt) \
     do { \
         if (child) { \
-            bdrv_debug_event(child->bs, evt); \
+            bdrv_co_debug_event(child->bs, evt); \
         } \
     } while (0)
 
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 3e1eba518c..b509855c19 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -781,7 +781,9 @@ struct BlockDriver {
                                       BdrvCheckResult *result,
                                       BdrvCheckMode fix);
 
-    void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+    /* Called with graph rdlock taken. */
+    void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs,
+                                          BlkdebugEvent event);
 
     /* io queue for linux-aio */
     void (*bdrv_io_plug)(BlockDriverState *bs);
-- 
2.31.1



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

* [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (11 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 14/15] block: convert bdrv_io_unplug " Emanuele Giuseppe Esposito
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_io_plug is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

The only caller of this function is blk_plug, therefore
make blk_plug a generated_co_wrapper_simple, so that
it always creates a new coroutine, and then make bdrv_plug
coroutine_fn.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c             | 5 +++--
 block/io.c                        | 5 +++--
 include/block/block-io.h          | 3 ++-
 include/block/block_int-common.h  | 4 ++--
 include/sysemu/block-backend-io.h | 4 +++-
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 75e2f2124f..826a936beb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2329,13 +2329,14 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify)
     notifier_list_add(&blk->insert_bs_notifiers, notify);
 }
 
-void blk_io_plug(BlockBackend *blk)
+void coroutine_fn blk_co_io_plug(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (bs) {
-        bdrv_io_plug(bs);
+        bdrv_co_io_plug(bs);
     }
 }
 
diff --git a/block/io.c b/block/io.c
index 11d2c5dcde..d3b8c1e4b2 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3068,13 +3068,14 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
     return mem;
 }
 
-void bdrv_io_plug(BlockDriverState *bs)
+void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
 {
     BdrvChild *child;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     QLIST_FOREACH(child, &bs->children, next) {
-        bdrv_io_plug(child->bs);
+        bdrv_co_io_plug(child->bs);
     }
 
     if (qatomic_fetch_inc(&bs->io_plugged) == 0) {
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 176e3cc734..a045643b26 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -215,7 +215,8 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine *co);
 
 AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 
-void bdrv_io_plug(BlockDriverState *bs);
+void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs);
+
 void bdrv_io_unplug(BlockDriverState *bs);
 
 bool coroutine_fn bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index b509855c19..ed96bc3241 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -785,8 +785,8 @@ struct BlockDriver {
     void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs,
                                           BlkdebugEvent event);
 
-    /* io queue for linux-aio */
-    void (*bdrv_io_plug)(BlockDriverState *bs);
+    /* io queue for linux-aio. Called with graph rdlock taken. */
+    void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
     void (*bdrv_io_unplug)(BlockDriverState *bs);
 
     /**
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index dd8566ee69..703fcc3ac5 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -87,7 +87,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
 int blk_get_max_iov(BlockBackend *blk);
 int blk_get_max_hw_iov(BlockBackend *blk);
 
-void blk_io_plug(BlockBackend *blk);
+void coroutine_fn blk_co_io_plug(BlockBackend *blk);
+void generated_co_wrapper_simple blk_io_plug(BlockBackend *blk);
+
 void blk_io_unplug(BlockBackend *blk);
 AioContext *blk_get_aio_context(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
-- 
2.31.1



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

* [PATCH 14/15] block: convert bdrv_io_unplug in generated_co_wrapper_simple
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (12 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-16 14:07 ` [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions Emanuele Giuseppe Esposito
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

BlockDriver->bdrv_io_unplug is categorized as IO callback, and
it currently doesn't run in a coroutine.
This makes very difficult to add the graph rdlock, since the
callback traverses the block nodes graph.

The only caller of this function is blk_unplug, therefore
make blk_unplug a generated_co_wrapper_simple, so that
it always creates a new coroutine, and then make bdrv_unplug
coroutine_fn.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c             | 5 +++--
 block/io.c                        | 5 +++--
 include/block/block-io.h          | 3 +--
 include/block/block_int-common.h  | 2 +-
 include/sysemu/block-backend-io.h | 4 +++-
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 826a936beb..3b10e35ea4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2340,13 +2340,14 @@ void coroutine_fn blk_co_io_plug(BlockBackend *blk)
     }
 }
 
-void blk_io_unplug(BlockBackend *blk)
+void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (bs) {
-        bdrv_io_unplug(bs);
+        bdrv_co_io_unplug(bs);
     }
 }
 
diff --git a/block/io.c b/block/io.c
index d3b8c1e4b2..48a94dd384 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3086,10 +3086,11 @@ void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
     }
 }
 
-void bdrv_io_unplug(BlockDriverState *bs)
+void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
 {
     BdrvChild *child;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     assert(bs->io_plugged);
     if (qatomic_fetch_dec(&bs->io_plugged) == 1) {
@@ -3100,7 +3101,7 @@ void bdrv_io_unplug(BlockDriverState *bs)
     }
 
     QLIST_FOREACH(child, &bs->children, next) {
-        bdrv_io_unplug(child->bs);
+        bdrv_co_io_unplug(child->bs);
     }
 }
 
diff --git a/include/block/block-io.h b/include/block/block-io.h
index a045643b26..f93357681a 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -216,8 +216,7 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine *co);
 AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 
 void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs);
-
-void bdrv_io_unplug(BlockDriverState *bs);
+void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs);
 
 bool coroutine_fn bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
                                                      const char *name,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index ed96bc3241..3ab3fa45a2 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -787,7 +787,7 @@ struct BlockDriver {
 
     /* io queue for linux-aio. Called with graph rdlock taken. */
     void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
-    void (*bdrv_io_unplug)(BlockDriverState *bs);
+    void coroutine_fn (*bdrv_io_unplug)(BlockDriverState *bs);
 
     /**
      * bdrv_drain_begin is called if implemented in the beginning of a
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index 703fcc3ac5..c8df7320f7 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -90,7 +90,9 @@ int blk_get_max_hw_iov(BlockBackend *blk);
 void coroutine_fn blk_co_io_plug(BlockBackend *blk);
 void generated_co_wrapper_simple blk_io_plug(BlockBackend *blk);
 
-void blk_io_unplug(BlockBackend *blk);
+void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
+void generated_co_wrapper_simple blk_io_unplug(BlockBackend *blk);
+
 AioContext *blk_get_aio_context(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
-- 
2.31.1



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

* [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (13 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 14/15] block: convert bdrv_io_unplug " Emanuele Giuseppe Esposito
@ 2022-11-16 14:07 ` Emanuele Giuseppe Esposito
  2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
  2022-11-21 15:02 ` Emanuele Giuseppe Esposito
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-16 14:07 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration, Emanuele Giuseppe Esposito

Since these functions alwayas run in coroutine context, adjust
their name to include "_co_", just like all other BlockDriver callbacks.

No functional change intended.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                          | 32 ++++++++++-----------
 block/blkdebug.c                 |  4 +--
 block/blkio.c                    |  6 ++--
 block/blklogwrites.c             |  2 +-
 block/blkreplay.c                |  2 +-
 block/blkverify.c                |  2 +-
 block/copy-on-read.c             |  6 ++--
 block/crypto.c                   |  4 +--
 block/curl.c                     |  8 +++---
 block/file-posix.c               | 48 ++++++++++++++++----------------
 block/file-win32.c               |  8 +++---
 block/filter-compress.c          |  6 ++--
 block/gluster.c                  | 16 +++++------
 block/io.c                       | 16 +++++------
 block/iscsi.c                    |  8 +++---
 block/nbd.c                      |  6 ++--
 block/nfs.c                      |  2 +-
 block/null.c                     |  8 +++---
 block/nvme.c                     |  6 ++--
 block/preallocate.c              |  2 +-
 block/qcow.c                     |  2 +-
 block/qcow2.c                    |  6 ++--
 block/qed.c                      |  4 +--
 block/quorum.c                   |  2 +-
 block/raw-format.c               |  8 +++---
 block/rbd.c                      |  4 +--
 block/replication.c              |  2 +-
 block/ssh.c                      |  2 +-
 block/throttle.c                 |  2 +-
 block/vdi.c                      |  2 +-
 block/vhdx.c                     |  2 +-
 block/vmdk.c                     |  4 +--
 block/vpc.c                      |  2 +-
 include/block/block_int-common.h | 27 +++++++++---------
 34 files changed, 131 insertions(+), 130 deletions(-)

diff --git a/block.c b/block.c
index a2e3550d5c..78b3e7fcd6 100644
--- a/block.c
+++ b/block.c
@@ -1055,13 +1055,13 @@ int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
         return -ENOMEDIUM;
     }
 
-    /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */
+    /* Do not attempt drv->bdrv_co_getlength() on scsi-generic devices */
     if (bdrv_is_sg(bs))
         return 0;
 
     /* query actual device if possible, otherwise just trust the hint */
-    if (drv->bdrv_getlength) {
-        int64_t length = drv->bdrv_getlength(bs);
+    if (drv->bdrv_co_getlength) {
+        int64_t length = drv->bdrv_co_getlength(bs);
         if (length < 0) {
             return length;
         }
@@ -5695,7 +5695,7 @@ exit:
 }
 
 /**
- * Implementation of BlockDriver.bdrv_get_allocated_file_size() that
+ * Implementation of BlockDriver.bdrv_co_get_allocated_file_size() that
  * sums the size of all data-bearing children.  (This excludes backing
  * children.)
  */
@@ -5732,8 +5732,8 @@ int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs)
     if (!drv) {
         return -ENOMEDIUM;
     }
-    if (drv->bdrv_get_allocated_file_size) {
-        return drv->bdrv_get_allocated_file_size(bs);
+    if (drv->bdrv_co_get_allocated_file_size) {
+        return drv->bdrv_co_get_allocated_file_size(bs);
     }
 
     if (drv->bdrv_file_open) {
@@ -6290,7 +6290,7 @@ int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     if (!drv) {
         return -ENOMEDIUM;
     }
-    if (!drv->bdrv_get_info) {
+    if (!drv->bdrv_co_get_info) {
         BlockDriverState *filtered = bdrv_filter_bs(bs);
         if (filtered) {
             return bdrv_co_get_info(filtered, bdi);
@@ -6298,7 +6298,7 @@ int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
         return -ENOTSUP;
     }
     memset(bdi, 0, sizeof(*bdi));
-    ret = drv->bdrv_get_info(bs, bdi);
+    ret = drv->bdrv_co_get_info(bs, bdi);
     if (ret < 0) {
         return ret;
     }
@@ -6336,11 +6336,11 @@ void coroutine_fn bdrv_co_debug_event(BlockDriverState *bs, BlkdebugEvent event)
     IO_CODE();
     assert_bdrv_graph_readable();
 
-    if (!bs || !bs->drv || !bs->drv->bdrv_debug_event) {
+    if (!bs || !bs->drv || !bs->drv->bdrv_co_debug_event) {
         return;
     }
 
-    bs->drv->bdrv_debug_event(bs, event);
+    bs->drv->bdrv_co_debug_event(bs, event);
 }
 
 static BlockDriverState *bdrv_find_debug_node(BlockDriverState *bs)
@@ -6790,8 +6790,8 @@ bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs)
     if (!drv) {
         return false;
     }
-    if (drv->bdrv_is_inserted) {
-        return drv->bdrv_is_inserted(bs);
+    if (drv->bdrv_co_is_inserted) {
+        return drv->bdrv_co_is_inserted(bs);
     }
     QLIST_FOREACH(child, &bs->children, next) {
         if (!bdrv_co_is_inserted(child->bs)) {
@@ -6810,8 +6810,8 @@ void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag)
     IO_CODE();
     assert_bdrv_graph_readable();
 
-    if (drv && drv->bdrv_eject) {
-        drv->bdrv_eject(bs, eject_flag);
+    if (drv && drv->bdrv_co_eject) {
+        drv->bdrv_co_eject(bs, eject_flag);
     }
 }
 
@@ -6826,8 +6826,8 @@ void coroutine_fn bdrv_co_lock_medium(BlockDriverState *bs, bool locked)
     assert_bdrv_graph_readable();
     trace_bdrv_lock_medium(bs, locked);
 
-    if (drv && drv->bdrv_lock_medium) {
-        drv->bdrv_lock_medium(bs, locked);
+    if (drv && drv->bdrv_co_lock_medium) {
+        drv->bdrv_co_lock_medium(bs, locked);
     }
 }
 
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 4265ca125e..1b85e84b7a 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -1076,7 +1076,7 @@ static BlockDriver bdrv_blkdebug = {
     .bdrv_reopen_prepare    = blkdebug_reopen_prepare,
     .bdrv_child_perm        = blkdebug_child_perm,
 
-    .bdrv_getlength         = blkdebug_getlength,
+    .bdrv_co_getlength         = blkdebug_getlength,
     .bdrv_refresh_filename  = blkdebug_refresh_filename,
     .bdrv_refresh_limits    = blkdebug_refresh_limits,
 
@@ -1087,7 +1087,7 @@ static BlockDriver bdrv_blkdebug = {
     .bdrv_co_pdiscard       = blkdebug_co_pdiscard,
     .bdrv_co_block_status   = blkdebug_co_block_status,
 
-    .bdrv_debug_event           = blkdebug_debug_event,
+    .bdrv_co_debug_event           = blkdebug_debug_event,
     .bdrv_debug_breakpoint      = blkdebug_debug_breakpoint,
     .bdrv_debug_remove_breakpoint
                                 = blkdebug_debug_remove_breakpoint,
diff --git a/block/blkio.c b/block/blkio.c
index 5eae3adfaf..669b940ce6 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -996,9 +996,9 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
         .instance_size           = sizeof(BDRVBlkioState), \
         .bdrv_file_open          = blkio_file_open, \
         .bdrv_close              = blkio_close, \
-        .bdrv_getlength          = blkio_getlength, \
+        .bdrv_co_getlength          = blkio_getlength, \
         .bdrv_co_truncate        = blkio_truncate, \
-        .bdrv_get_info           = blkio_get_info, \
+        .bdrv_co_get_info           = blkio_get_info, \
         .bdrv_attach_aio_context = blkio_attach_aio_context, \
         .bdrv_detach_aio_context = blkio_detach_aio_context, \
         .bdrv_co_pdiscard        = blkio_co_pdiscard, \
@@ -1006,7 +1006,7 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
         .bdrv_co_pwritev         = blkio_co_pwritev, \
         .bdrv_co_flush_to_disk   = blkio_co_flush, \
         .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
-        .bdrv_io_unplug          = blkio_io_unplug, \
+        .bdrv_co_io_unplug          = blkio_io_unplug, \
         .bdrv_refresh_limits     = blkio_refresh_limits, \
         .bdrv_register_buf       = blkio_register_buf, \
         .bdrv_unregister_buf     = blkio_unregister_buf, \
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index cef9efe55d..72ff8fb6a8 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -497,7 +497,7 @@ static BlockDriver bdrv_blk_log_writes = {
 
     .bdrv_open              = blk_log_writes_open,
     .bdrv_close             = blk_log_writes_close,
-    .bdrv_getlength         = blk_log_writes_getlength,
+    .bdrv_co_getlength         = blk_log_writes_getlength,
     .bdrv_child_perm        = blk_log_writes_child_perm,
     .bdrv_refresh_limits    = blk_log_writes_refresh_limits,
 
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 76a0b8d12a..3c82af100a 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -135,7 +135,7 @@ static BlockDriver bdrv_blkreplay = {
 
     .bdrv_open              = blkreplay_open,
     .bdrv_child_perm        = bdrv_default_perms,
-    .bdrv_getlength         = blkreplay_getlength,
+    .bdrv_co_getlength         = blkreplay_getlength,
 
     .bdrv_co_preadv         = blkreplay_co_preadv,
     .bdrv_co_pwritev        = blkreplay_co_pwritev,
diff --git a/block/blkverify.c b/block/blkverify.c
index c60a2dc624..1a4bd68864 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -313,7 +313,7 @@ static BlockDriver bdrv_blkverify = {
     .bdrv_file_open                   = blkverify_open,
     .bdrv_close                       = blkverify_close,
     .bdrv_child_perm                  = bdrv_default_perms,
-    .bdrv_getlength                   = blkverify_getlength,
+    .bdrv_co_getlength                   = blkverify_getlength,
     .bdrv_refresh_filename            = blkverify_refresh_filename,
     .bdrv_dirname                     = blkverify_dirname,
 
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index ccc767f37b..ba76a1b3ec 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -249,7 +249,7 @@ static BlockDriver bdrv_copy_on_read = {
     .bdrv_close                         = cor_close,
     .bdrv_child_perm                    = cor_child_perm,
 
-    .bdrv_getlength                     = cor_getlength,
+    .bdrv_co_getlength                     = cor_getlength,
 
     .bdrv_co_preadv_part                = cor_co_preadv_part,
     .bdrv_co_pwritev_part               = cor_co_pwritev_part,
@@ -257,8 +257,8 @@ static BlockDriver bdrv_copy_on_read = {
     .bdrv_co_pdiscard                   = cor_co_pdiscard,
     .bdrv_co_pwritev_compressed         = cor_co_pwritev_compressed,
 
-    .bdrv_eject                         = cor_eject,
-    .bdrv_lock_medium                   = cor_lock_medium,
+    .bdrv_co_eject                         = cor_eject,
+    .bdrv_co_lock_medium                   = cor_lock_medium,
 
     .has_variable_length                = true,
     .is_filter                          = true,
diff --git a/block/crypto.c b/block/crypto.c
index 12a84dd1cd..0df4fee46d 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -953,9 +953,9 @@ static BlockDriver bdrv_crypto_luks = {
     .bdrv_refresh_limits = block_crypto_refresh_limits,
     .bdrv_co_preadv     = block_crypto_co_preadv,
     .bdrv_co_pwritev    = block_crypto_co_pwritev,
-    .bdrv_getlength     = block_crypto_getlength,
+    .bdrv_co_getlength     = block_crypto_getlength,
     .bdrv_measure       = block_crypto_measure,
-    .bdrv_get_info      = block_crypto_get_info_luks,
+    .bdrv_co_get_info      = block_crypto_get_info_luks,
     .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
     .bdrv_amend_options = block_crypto_amend_options_luks,
     .bdrv_co_amend      = block_crypto_co_amend_luks,
diff --git a/block/curl.c b/block/curl.c
index cba4c4cac7..79735dfdc4 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -1001,7 +1001,7 @@ static BlockDriver bdrv_http = {
     .bdrv_parse_filename        = curl_parse_filename,
     .bdrv_file_open             = curl_open,
     .bdrv_close                 = curl_close,
-    .bdrv_getlength             = curl_getlength,
+    .bdrv_co_getlength             = curl_getlength,
 
     .bdrv_co_preadv             = curl_co_preadv,
 
@@ -1020,7 +1020,7 @@ static BlockDriver bdrv_https = {
     .bdrv_parse_filename        = curl_parse_filename,
     .bdrv_file_open             = curl_open,
     .bdrv_close                 = curl_close,
-    .bdrv_getlength             = curl_getlength,
+    .bdrv_co_getlength             = curl_getlength,
 
     .bdrv_co_preadv             = curl_co_preadv,
 
@@ -1039,7 +1039,7 @@ static BlockDriver bdrv_ftp = {
     .bdrv_parse_filename        = curl_parse_filename,
     .bdrv_file_open             = curl_open,
     .bdrv_close                 = curl_close,
-    .bdrv_getlength             = curl_getlength,
+    .bdrv_co_getlength             = curl_getlength,
 
     .bdrv_co_preadv             = curl_co_preadv,
 
@@ -1058,7 +1058,7 @@ static BlockDriver bdrv_ftps = {
     .bdrv_parse_filename        = curl_parse_filename,
     .bdrv_file_open             = curl_open,
     .bdrv_close                 = curl_close,
-    .bdrv_getlength             = curl_getlength,
+    .bdrv_co_getlength             = curl_getlength,
 
     .bdrv_co_preadv             = curl_co_preadv,
 
diff --git a/block/file-posix.c b/block/file-posix.c
index b9647c5ffc..09f2615cbe 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3321,14 +3321,14 @@ BlockDriver bdrv_file = {
     .bdrv_co_copy_range_from = raw_co_copy_range_from,
     .bdrv_co_copy_range_to  = raw_co_copy_range_to,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_io_plug = raw_aio_plug,
-    .bdrv_io_unplug = raw_aio_unplug,
+    .bdrv_co_io_plug = raw_aio_plug,
+    .bdrv_co_io_unplug = raw_aio_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate = raw_co_truncate,
-    .bdrv_getlength = raw_getlength,
-    .bdrv_get_info = raw_get_info,
-    .bdrv_get_allocated_file_size
+    .bdrv_co_getlength = raw_getlength,
+    .bdrv_co_get_info = raw_get_info,
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
     .bdrv_get_specific_stats = raw_get_specific_stats,
     .bdrv_check_perm = raw_check_perm,
@@ -3693,14 +3693,14 @@ static BlockDriver bdrv_host_device = {
     .bdrv_co_copy_range_from = raw_co_copy_range_from,
     .bdrv_co_copy_range_to  = raw_co_copy_range_to,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_io_plug = raw_aio_plug,
-    .bdrv_io_unplug = raw_aio_unplug,
+    .bdrv_co_io_plug = raw_aio_plug,
+    .bdrv_co_io_unplug = raw_aio_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate       = raw_co_truncate,
-    .bdrv_getlength	= raw_getlength,
-    .bdrv_get_info = raw_get_info,
-    .bdrv_get_allocated_file_size
+    .bdrv_co_getlength = raw_getlength,
+    .bdrv_co_get_info = raw_get_info,
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
     .bdrv_get_specific_stats = hdev_get_specific_stats,
     .bdrv_check_perm = raw_check_perm,
@@ -3817,20 +3817,20 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_co_pwritev        = raw_co_pwritev,
     .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_io_plug = raw_aio_plug,
-    .bdrv_io_unplug = raw_aio_unplug,
+    .bdrv_co_io_plug = raw_aio_plug,
+    .bdrv_co_io_unplug = raw_aio_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate    = raw_co_truncate,
-    .bdrv_getlength      = raw_getlength,
+    .bdrv_co_getlength      = raw_getlength,
     .has_variable_length = true,
-    .bdrv_get_allocated_file_size
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
     /* removable device support */
-    .bdrv_is_inserted   = cdrom_is_inserted,
-    .bdrv_eject         = cdrom_eject,
-    .bdrv_lock_medium   = cdrom_lock_medium,
+    .bdrv_co_is_inserted   = cdrom_is_inserted,
+    .bdrv_co_eject         = cdrom_eject,
+    .bdrv_co_lock_medium   = cdrom_lock_medium,
 
     /* generic scsi device */
     .bdrv_co_ioctl      = hdev_co_ioctl,
@@ -3947,20 +3947,20 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_co_pwritev        = raw_co_pwritev,
     .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_io_plug = raw_aio_plug,
-    .bdrv_io_unplug = raw_aio_unplug,
+    .bdrv_co_io_plug = raw_aio_plug,
+    .bdrv_co_io_unplug = raw_aio_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate    = raw_co_truncate,
-    .bdrv_getlength      = raw_getlength,
+    .bdrv_co_getlength      = raw_getlength,
     .has_variable_length = true,
-    .bdrv_get_allocated_file_size
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
     /* removable device support */
-    .bdrv_is_inserted   = cdrom_is_inserted,
-    .bdrv_eject         = cdrom_eject,
-    .bdrv_lock_medium   = cdrom_lock_medium,
+    .bdrv_co_is_inserted   = cdrom_is_inserted,
+    .bdrv_co_eject         = cdrom_eject,
+    .bdrv_co_lock_medium   = cdrom_lock_medium,
 };
 #endif /* __FreeBSD__ */
 
diff --git a/block/file-win32.c b/block/file-win32.c
index ec9d64d0e4..f611931631 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -763,8 +763,8 @@ BlockDriver bdrv_file = {
     .bdrv_aio_flush     = raw_aio_flush,
 
     .bdrv_co_truncate   = raw_co_truncate,
-    .bdrv_getlength	= raw_getlength,
-    .bdrv_get_allocated_file_size
+    .bdrv_co_getlength  = raw_getlength,
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
     .create_opts        = &raw_create_opts,
@@ -932,10 +932,10 @@ static BlockDriver bdrv_host_device = {
     .bdrv_detach_aio_context = raw_detach_aio_context,
     .bdrv_attach_aio_context = raw_attach_aio_context,
 
-    .bdrv_getlength      = raw_getlength,
+    .bdrv_co_getlength      = raw_getlength,
     .has_variable_length = true,
 
-    .bdrv_get_allocated_file_size
+    .bdrv_co_get_allocated_file_size
                         = raw_get_allocated_file_size,
 };
 
diff --git a/block/filter-compress.c b/block/filter-compress.c
index e10312c225..7b3fa2fd82 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -134,7 +134,7 @@ static BlockDriver bdrv_compress = {
     .bdrv_open                          = compress_open,
     .bdrv_child_perm                    = bdrv_default_perms,
 
-    .bdrv_getlength                     = compress_getlength,
+    .bdrv_co_getlength                     = compress_getlength,
 
     .bdrv_co_preadv_part                = compress_co_preadv_part,
     .bdrv_co_pwritev_part               = compress_co_pwritev_part,
@@ -142,8 +142,8 @@ static BlockDriver bdrv_compress = {
     .bdrv_co_pdiscard                   = compress_co_pdiscard,
     .bdrv_refresh_limits                = compress_refresh_limits,
 
-    .bdrv_eject                         = compress_eject,
-    .bdrv_lock_medium                   = compress_lock_medium,
+    .bdrv_co_eject                         = compress_eject,
+    .bdrv_co_lock_medium                   = compress_lock_medium,
 
     .has_variable_length                = true,
     .is_filter                          = true,
diff --git a/block/gluster.c b/block/gluster.c
index 7c90f7ba4b..f15b522670 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1561,8 +1561,8 @@ static BlockDriver bdrv_gluster = {
     .bdrv_close                   = qemu_gluster_close,
     .bdrv_co_create               = qemu_gluster_co_create,
     .bdrv_co_create_opts          = qemu_gluster_co_create_opts,
-    .bdrv_getlength               = qemu_gluster_getlength,
-    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_co_getlength               = qemu_gluster_getlength,
+    .bdrv_co_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_co_truncate             = qemu_gluster_co_truncate,
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
@@ -1590,8 +1590,8 @@ static BlockDriver bdrv_gluster_tcp = {
     .bdrv_close                   = qemu_gluster_close,
     .bdrv_co_create               = qemu_gluster_co_create,
     .bdrv_co_create_opts          = qemu_gluster_co_create_opts,
-    .bdrv_getlength               = qemu_gluster_getlength,
-    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_co_getlength               = qemu_gluster_getlength,
+    .bdrv_co_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_co_truncate             = qemu_gluster_co_truncate,
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
@@ -1619,8 +1619,8 @@ static BlockDriver bdrv_gluster_unix = {
     .bdrv_close                   = qemu_gluster_close,
     .bdrv_co_create               = qemu_gluster_co_create,
     .bdrv_co_create_opts          = qemu_gluster_co_create_opts,
-    .bdrv_getlength               = qemu_gluster_getlength,
-    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_co_getlength               = qemu_gluster_getlength,
+    .bdrv_co_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_co_truncate             = qemu_gluster_co_truncate,
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
@@ -1654,8 +1654,8 @@ static BlockDriver bdrv_gluster_rdma = {
     .bdrv_close                   = qemu_gluster_close,
     .bdrv_co_create               = qemu_gluster_co_create,
     .bdrv_co_create_opts          = qemu_gluster_co_create_opts,
-    .bdrv_getlength               = qemu_gluster_getlength,
-    .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
+    .bdrv_co_getlength               = qemu_gluster_getlength,
+    .bdrv_co_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_co_truncate             = qemu_gluster_co_truncate,
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
diff --git a/block/io.c b/block/io.c
index 48a94dd384..0ebe91ad04 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2648,8 +2648,8 @@ bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
 
     bdrv_inc_in_flight(bs);
 
-    if (drv->bdrv_load_vmstate) {
-        ret = drv->bdrv_load_vmstate(bs, qiov, pos);
+    if (drv->bdrv_co_load_vmstate) {
+        ret = drv->bdrv_co_load_vmstate(bs, qiov, pos);
     } else if (child_bs) {
         ret = bdrv_co_readv_vmstate(child_bs, qiov, pos);
     } else {
@@ -2681,8 +2681,8 @@ bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
 
     bdrv_inc_in_flight(bs);
 
-    if (drv->bdrv_save_vmstate) {
-        ret = drv->bdrv_save_vmstate(bs, qiov, pos);
+    if (drv->bdrv_co_save_vmstate) {
+        ret = drv->bdrv_co_save_vmstate(bs, qiov, pos);
     } else if (child_bs) {
         ret = bdrv_co_writev_vmstate(child_bs, qiov, pos);
     } else {
@@ -3080,8 +3080,8 @@ void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
 
     if (qatomic_fetch_inc(&bs->io_plugged) == 0) {
         BlockDriver *drv = bs->drv;
-        if (drv && drv->bdrv_io_plug) {
-            drv->bdrv_io_plug(bs);
+        if (drv && drv->bdrv_co_io_plug) {
+            drv->bdrv_co_io_plug(bs);
         }
     }
 }
@@ -3095,8 +3095,8 @@ void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
     assert(bs->io_plugged);
     if (qatomic_fetch_dec(&bs->io_plugged) == 1) {
         BlockDriver *drv = bs->drv;
-        if (drv && drv->bdrv_io_unplug) {
-            drv->bdrv_io_unplug(bs);
+        if (drv && drv->bdrv_co_io_unplug) {
+            drv->bdrv_co_io_unplug(bs);
         }
     }
 
diff --git a/block/iscsi.c b/block/iscsi.c
index a316d46d96..a5e54150fa 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2433,8 +2433,8 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_reopen_commit     = iscsi_reopen_commit,
     .bdrv_co_invalidate_cache = iscsi_co_invalidate_cache,
 
-    .bdrv_getlength  = iscsi_getlength,
-    .bdrv_get_info   = iscsi_get_info,
+    .bdrv_co_getlength  = iscsi_getlength,
+    .bdrv_co_get_info   = iscsi_get_info,
     .bdrv_co_truncate    = iscsi_co_truncate,
     .bdrv_refresh_limits = iscsi_refresh_limits,
 
@@ -2472,8 +2472,8 @@ static BlockDriver bdrv_iser = {
     .bdrv_reopen_commit     = iscsi_reopen_commit,
     .bdrv_co_invalidate_cache  = iscsi_co_invalidate_cache,
 
-    .bdrv_getlength  = iscsi_getlength,
-    .bdrv_get_info   = iscsi_get_info,
+    .bdrv_co_getlength  = iscsi_getlength,
+    .bdrv_co_get_info   = iscsi_get_info,
     .bdrv_co_truncate    = iscsi_co_truncate,
     .bdrv_refresh_limits = iscsi_refresh_limits,
 
diff --git a/block/nbd.c b/block/nbd.c
index 5cad58aaf6..015875a392 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -2125,7 +2125,7 @@ static BlockDriver bdrv_nbd = {
     .bdrv_co_pdiscard           = nbd_client_co_pdiscard,
     .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_co_truncate           = nbd_co_truncate,
-    .bdrv_getlength             = nbd_getlength,
+    .bdrv_co_getlength             = nbd_getlength,
     .bdrv_refresh_filename      = nbd_refresh_filename,
     .bdrv_co_block_status       = nbd_client_co_block_status,
     .bdrv_dirname               = nbd_dirname,
@@ -2153,7 +2153,7 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_co_pdiscard           = nbd_client_co_pdiscard,
     .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_co_truncate           = nbd_co_truncate,
-    .bdrv_getlength             = nbd_getlength,
+    .bdrv_co_getlength             = nbd_getlength,
     .bdrv_refresh_filename      = nbd_refresh_filename,
     .bdrv_co_block_status       = nbd_client_co_block_status,
     .bdrv_dirname               = nbd_dirname,
@@ -2181,7 +2181,7 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_co_pdiscard           = nbd_client_co_pdiscard,
     .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_co_truncate           = nbd_co_truncate,
-    .bdrv_getlength             = nbd_getlength,
+    .bdrv_co_getlength             = nbd_getlength,
     .bdrv_refresh_filename      = nbd_refresh_filename,
     .bdrv_co_block_status       = nbd_client_co_block_status,
     .bdrv_dirname               = nbd_dirname,
diff --git a/block/nfs.c b/block/nfs.c
index ece22353ac..afecb582b2 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -884,7 +884,7 @@ static BlockDriver bdrv_nfs = {
     .bdrv_has_zero_init             = nfs_has_zero_init,
 /* libnfs does not provide the allocated filesize of a file on win32. */
 #if !defined(_WIN32)
-    .bdrv_get_allocated_file_size   = nfs_get_allocated_file_size,
+    .bdrv_co_get_allocated_file_size   = nfs_get_allocated_file_size,
 #endif
     .bdrv_co_truncate               = nfs_file_co_truncate,
 
diff --git a/block/null.c b/block/null.c
index 75f7d0db40..a2f52e112b 100644
--- a/block/null.c
+++ b/block/null.c
@@ -283,8 +283,8 @@ static BlockDriver bdrv_null_co = {
 
     .bdrv_file_open         = null_file_open,
     .bdrv_parse_filename    = null_co_parse_filename,
-    .bdrv_getlength         = null_getlength,
-    .bdrv_get_allocated_file_size = null_allocated_file_size,
+    .bdrv_co_getlength         = null_getlength,
+    .bdrv_co_get_allocated_file_size = null_allocated_file_size,
 
     .bdrv_co_preadv         = null_co_preadv,
     .bdrv_co_pwritev        = null_co_pwritev,
@@ -304,8 +304,8 @@ static BlockDriver bdrv_null_aio = {
 
     .bdrv_file_open         = null_file_open,
     .bdrv_parse_filename    = null_aio_parse_filename,
-    .bdrv_getlength         = null_getlength,
-    .bdrv_get_allocated_file_size = null_allocated_file_size,
+    .bdrv_co_getlength         = null_getlength,
+    .bdrv_co_get_allocated_file_size = null_allocated_file_size,
 
     .bdrv_aio_preadv        = null_aio_preadv,
     .bdrv_aio_pwritev       = null_aio_pwritev,
diff --git a/block/nvme.c b/block/nvme.c
index 656624c585..9be9722029 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1642,7 +1642,7 @@ static BlockDriver bdrv_nvme = {
     .bdrv_parse_filename      = nvme_parse_filename,
     .bdrv_file_open           = nvme_file_open,
     .bdrv_close               = nvme_close,
-    .bdrv_getlength           = nvme_getlength,
+    .bdrv_co_getlength           = nvme_getlength,
     .bdrv_probe_blocksizes    = nvme_probe_blocksizes,
     .bdrv_co_truncate         = nvme_co_truncate,
 
@@ -1663,8 +1663,8 @@ static BlockDriver bdrv_nvme = {
     .bdrv_detach_aio_context  = nvme_detach_aio_context,
     .bdrv_attach_aio_context  = nvme_attach_aio_context,
 
-    .bdrv_io_plug             = nvme_aio_plug,
-    .bdrv_io_unplug           = nvme_aio_unplug,
+    .bdrv_co_io_plug             = nvme_aio_plug,
+    .bdrv_co_io_unplug           = nvme_aio_unplug,
 
     .bdrv_register_buf        = nvme_register_buf,
     .bdrv_unregister_buf      = nvme_unregister_buf,
diff --git a/block/preallocate.c b/block/preallocate.c
index d50ee7f49b..584a8b5912 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -536,7 +536,7 @@ BlockDriver bdrv_preallocate_filter = {
     .format_name = "preallocate",
     .instance_size = sizeof(BDRVPreallocateState),
 
-    .bdrv_getlength = preallocate_getlength,
+    .bdrv_co_getlength = preallocate_getlength,
     .bdrv_open = preallocate_open,
     .bdrv_close = preallocate_close,
 
diff --git a/block/qcow.c b/block/qcow.c
index daa38839ab..167c135dbe 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -1198,7 +1198,7 @@ static BlockDriver bdrv_qcow = {
 
     .bdrv_make_empty        = qcow_make_empty,
     .bdrv_co_pwritev_compressed = qcow_co_pwritev_compressed,
-    .bdrv_get_info          = qcow_get_info,
+    .bdrv_co_get_info          = qcow_get_info,
 
     .create_opts            = &qcow_create_opts,
     .strong_runtime_opts    = qcow_strong_runtime_opts,
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d6666d3ff..7bdb310db3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -6078,11 +6078,11 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_snapshot_list     = qcow2_snapshot_list,
     .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
     .bdrv_measure           = qcow2_measure,
-    .bdrv_get_info          = qcow2_get_info,
+    .bdrv_co_get_info          = qcow2_get_info,
     .bdrv_get_specific_info = qcow2_get_specific_info,
 
-    .bdrv_save_vmstate    = qcow2_save_vmstate,
-    .bdrv_load_vmstate    = qcow2_load_vmstate,
+    .bdrv_co_save_vmstate    = qcow2_save_vmstate,
+    .bdrv_co_load_vmstate    = qcow2_load_vmstate,
 
     .is_format                  = true,
     .supports_backing           = true,
diff --git a/block/qed.c b/block/qed.c
index 778b23d0f6..f83fcbd4c8 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1656,8 +1656,8 @@ static BlockDriver bdrv_qed = {
     .bdrv_co_writev           = bdrv_qed_co_writev,
     .bdrv_co_pwrite_zeroes    = bdrv_qed_co_pwrite_zeroes,
     .bdrv_co_truncate         = bdrv_qed_co_truncate,
-    .bdrv_getlength           = bdrv_qed_getlength,
-    .bdrv_get_info            = bdrv_qed_get_info,
+    .bdrv_co_getlength           = bdrv_qed_getlength,
+    .bdrv_co_get_info            = bdrv_qed_get_info,
     .bdrv_refresh_limits      = bdrv_qed_refresh_limits,
     .bdrv_change_backing_file = bdrv_qed_change_backing_file,
     .bdrv_co_invalidate_cache = bdrv_qed_co_invalidate_cache,
diff --git a/block/quorum.c b/block/quorum.c
index f9e6539ceb..ce5090d31a 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1283,7 +1283,7 @@ static BlockDriver bdrv_quorum = {
 
     .bdrv_co_flush                      = quorum_co_flush,
 
-    .bdrv_getlength                     = quorum_getlength,
+    .bdrv_co_getlength                     = quorum_getlength,
 
     .bdrv_co_preadv                     = quorum_co_preadv,
     .bdrv_co_pwritev                    = quorum_co_pwritev,
diff --git a/block/raw-format.c b/block/raw-format.c
index 96a9b33384..87d997d640 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -621,16 +621,16 @@ BlockDriver bdrv_raw = {
     .bdrv_co_copy_range_from = &raw_co_copy_range_from,
     .bdrv_co_copy_range_to  = &raw_co_copy_range_to,
     .bdrv_co_truncate     = &raw_co_truncate,
-    .bdrv_getlength       = &raw_getlength,
+    .bdrv_co_getlength       = &raw_getlength,
     .is_format            = true,
     .has_variable_length  = true,
     .bdrv_measure         = &raw_measure,
-    .bdrv_get_info        = &raw_get_info,
+    .bdrv_co_get_info        = &raw_get_info,
     .bdrv_refresh_limits  = &raw_refresh_limits,
     .bdrv_probe_blocksizes = &raw_probe_blocksizes,
     .bdrv_probe_geometry  = &raw_probe_geometry,
-    .bdrv_eject           = &raw_eject,
-    .bdrv_lock_medium     = &raw_lock_medium,
+    .bdrv_co_eject           = &raw_eject,
+    .bdrv_co_lock_medium     = &raw_lock_medium,
     .bdrv_co_ioctl        = &raw_co_ioctl,
     .create_opts          = &raw_create_opts,
     .bdrv_has_zero_init   = &raw_has_zero_init,
diff --git a/block/rbd.c b/block/rbd.c
index f826410f40..a0317d470a 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1655,10 +1655,10 @@ static BlockDriver bdrv_rbd = {
     .bdrv_co_create         = qemu_rbd_co_create,
     .bdrv_co_create_opts    = qemu_rbd_co_create_opts,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
-    .bdrv_get_info          = qemu_rbd_getinfo,
+    .bdrv_co_get_info          = qemu_rbd_getinfo,
     .bdrv_get_specific_info = qemu_rbd_get_specific_info,
     .create_opts            = &qemu_rbd_create_opts,
-    .bdrv_getlength         = qemu_rbd_getlength,
+    .bdrv_co_getlength         = qemu_rbd_getlength,
     .bdrv_co_truncate       = qemu_rbd_co_truncate,
     .protocol_name          = "rbd",
 
diff --git a/block/replication.c b/block/replication.c
index c62f48a874..321db05465 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -758,7 +758,7 @@ static BlockDriver bdrv_replication = {
     .bdrv_close                 = replication_close,
     .bdrv_child_perm            = replication_child_perm,
 
-    .bdrv_getlength             = replication_getlength,
+    .bdrv_co_getlength             = replication_getlength,
     .bdrv_co_readv              = replication_co_readv,
     .bdrv_co_writev             = replication_co_writev,
 
diff --git a/block/ssh.c b/block/ssh.c
index 04726d4ecb..c70db1e4e2 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1363,7 +1363,7 @@ static BlockDriver bdrv_ssh = {
     .bdrv_has_zero_init           = ssh_has_zero_init,
     .bdrv_co_readv                = ssh_co_readv,
     .bdrv_co_writev               = ssh_co_writev,
-    .bdrv_getlength               = ssh_getlength,
+    .bdrv_co_getlength               = ssh_getlength,
     .bdrv_co_truncate             = ssh_co_truncate,
     .bdrv_co_flush_to_disk        = ssh_co_flush,
     .bdrv_refresh_filename        = ssh_refresh_filename,
diff --git a/block/throttle.c b/block/throttle.c
index 88851c84f4..260ffeace2 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -245,7 +245,7 @@ static BlockDriver bdrv_throttle = {
 
     .bdrv_child_perm                    =   bdrv_default_perms,
 
-    .bdrv_getlength                     =   throttle_getlength,
+    .bdrv_co_getlength                     =   throttle_getlength,
 
     .bdrv_co_preadv                     =   throttle_co_preadv,
     .bdrv_co_pwritev                    =   throttle_co_pwritev,
diff --git a/block/vdi.c b/block/vdi.c
index c0c111c4b9..6e4bf24e8c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -1049,7 +1049,7 @@ static BlockDriver bdrv_vdi = {
     .bdrv_co_pwritev    = vdi_co_pwritev,
 #endif
 
-    .bdrv_get_info = vdi_get_info,
+    .bdrv_co_get_info = vdi_get_info,
 
     .is_format = true,
     .create_opts = &vdi_create_opts,
diff --git a/block/vhdx.c b/block/vhdx.c
index bad9ca691b..85c6f8b68e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -2245,7 +2245,7 @@ static BlockDriver bdrv_vhdx = {
     .bdrv_co_writev         = vhdx_co_writev,
     .bdrv_co_create         = vhdx_co_create,
     .bdrv_co_create_opts    = vhdx_co_create_opts,
-    .bdrv_get_info          = vhdx_get_info,
+    .bdrv_co_get_info          = vhdx_get_info,
     .bdrv_co_check          = vhdx_co_check,
     .bdrv_has_zero_init     = vhdx_has_zero_init,
 
diff --git a/block/vmdk.c b/block/vmdk.c
index 0c32bf2e83..768ce89a08 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -3130,11 +3130,11 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_co_create_opts          = vmdk_co_create_opts,
     .bdrv_co_create               = vmdk_co_create,
     .bdrv_co_block_status         = vmdk_co_block_status,
-    .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
+    .bdrv_co_get_allocated_file_size = vmdk_get_allocated_file_size,
     .bdrv_has_zero_init           = vmdk_has_zero_init,
     .bdrv_get_specific_info       = vmdk_get_specific_info,
     .bdrv_refresh_limits          = vmdk_refresh_limits,
-    .bdrv_get_info                = vmdk_get_info,
+    .bdrv_co_get_info                = vmdk_get_info,
     .bdrv_gather_child_options    = vmdk_gather_child_options,
 
     .is_format                    = true,
diff --git a/block/vpc.c b/block/vpc.c
index 95841f259a..96c25975a0 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -1240,7 +1240,7 @@ static BlockDriver bdrv_vpc = {
     .bdrv_co_pwritev            = vpc_co_pwritev,
     .bdrv_co_block_status       = vpc_co_block_status,
 
-    .bdrv_get_info          = vpc_get_info,
+    .bdrv_co_get_info          = vpc_get_info,
 
     .is_format              = true,
     .create_opts            = &vpc_create_opts,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 3ab3fa45a2..0bc180a585 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -724,9 +724,10 @@ struct BlockDriver {
                                          bool exact, PreallocMode prealloc,
                                          BdrvRequestFlags flags, Error **errp);
     /* Called with graph rdlock held. */
-    int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
+    int64_t coroutine_fn (*bdrv_co_getlength)(BlockDriverState *bs);
     /* Called with graph rdlock held. */
-    int64_t coroutine_fn (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+    int64_t coroutine_fn (*bdrv_co_get_allocated_file_size)
+                                                        (BlockDriverState *bs);
 
     /* Does not need graph rdlock, since it does not traverse the graph */
     BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
@@ -741,7 +742,7 @@ struct BlockDriver {
         size_t qiov_offset);
 
     /* Called with graph rdlock held. */
-    int coroutine_fn (*bdrv_get_info)(BlockDriverState *bs,
+    int coroutine_fn (*bdrv_co_get_info)(BlockDriverState *bs,
                                       BlockDriverInfo *bdi);
 
     /* Does not need graph rdlock, since it does not traverse the graph */
@@ -751,18 +752,18 @@ struct BlockDriver {
     BlockStatsSpecific *(*bdrv_get_specific_stats)(BlockDriverState *bs);
 
     /* Called with graph rdlock held. */
-    int coroutine_fn (*bdrv_save_vmstate)(BlockDriverState *bs,
-                                          QEMUIOVector *qiov,
-                                          int64_t pos);
+    int coroutine_fn (*bdrv_co_save_vmstate)(BlockDriverState *bs,
+                                             QEMUIOVector *qiov,
+                                             int64_t pos);
     /* Called with graph rdlock held. */
-    int coroutine_fn (*bdrv_load_vmstate)(BlockDriverState *bs,
+    int coroutine_fn (*bdrv_co_load_vmstate)(BlockDriverState *bs,
                                           QEMUIOVector *qiov,
                                           int64_t pos);
 
     /* removable device specific. Called with graph rdlock held. */
-    bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
-    void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
-    void coroutine_fn (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
+    bool coroutine_fn (*bdrv_co_is_inserted)(BlockDriverState *bs);
+    void coroutine_fn (*bdrv_co_eject)(BlockDriverState *bs, bool eject_flag);
+    void coroutine_fn (*bdrv_co_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices. Called with graph rdlock taken. */
     BlockAIOCB *coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
@@ -782,12 +783,12 @@ struct BlockDriver {
                                       BdrvCheckMode fix);
 
     /* Called with graph rdlock taken. */
-    void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs,
+    void coroutine_fn (*bdrv_co_debug_event)(BlockDriverState *bs,
                                           BlkdebugEvent event);
 
     /* io queue for linux-aio. Called with graph rdlock taken. */
-    void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
-    void coroutine_fn (*bdrv_io_unplug)(BlockDriverState *bs);
+    void coroutine_fn (*bdrv_co_io_plug)(BlockDriverState *bs);
+    void coroutine_fn (*bdrv_co_io_unplug)(BlockDriverState *bs);
 
     /**
      * bdrv_drain_begin is called if implemented in the beginning of a
-- 
2.31.1



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (14 preceding siblings ...)
  2022-11-16 14:07 ` [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions Emanuele Giuseppe Esposito
@ 2022-11-18 10:57 ` Paolo Bonzini
  2022-11-18 15:01   ` Emanuele Giuseppe Esposito
  2022-11-23 11:45   ` Emanuele Giuseppe Esposito
  2022-11-21 15:02 ` Emanuele Giuseppe Esposito
  16 siblings, 2 replies; 24+ messages in thread
From: Paolo Bonzini @ 2022-11-18 10:57 UTC (permalink / raw)
  To: Emanuele Giuseppe Esposito, qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, John Snow, Vladimir Sementsov-Ogievskiy,
	Stefan Weil, Fam Zheng, Ronnie Sahlberg, Peter Lieven,
	Eric Blake, Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
> Here we introduce generated_co_wrapper_simple, a simplification of g_c_w that
> only considers the case where the caller is not in a coroutine.
> This simplifies and clarifies a lot when the caller is a coroutine or not, and
> in the future will hopefully replace g_c_w.

This is a good idea!

Just one thing, though it belongs more in the two series which 
introduced generated_co_wrapper_simple and generated_co_wrapper_blk - I 
would make this the "official" wrapper.  So perhaps:

- generated_co_wrapper_simple -> coroutine_wrapper
- generated_co_wrapper_blk -> coroutine_wrapper_mixed
- generated_co_wrapper -> coroutine_wrapper_mixed_bdrv

?  It is not clear to me yet if you will have bdrv_* functions that take 
the rdlock as well - in which case however coroutine_wrapper_bdrv would 
not be hard to add.

Even without looking at the lock, the three series are going in the 
right direction of ultimately having more "simple" coroutine wrappers at 
the blk_* level and more coroutine functions (ultimately less wrappers, 
too) at the bdrv_* level.

Paolo



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
@ 2022-11-18 15:01   ` Emanuele Giuseppe Esposito
  2022-11-18 15:13     ` Paolo Bonzini
  2022-11-23 11:45   ` Emanuele Giuseppe Esposito
  1 sibling, 1 reply; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-18 15:01 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, John Snow, Vladimir Sementsov-Ogievskiy,
	Stefan Weil, Fam Zheng, Ronnie Sahlberg, Peter Lieven,
	Eric Blake, Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration



Am 18/11/2022 um 11:57 schrieb Paolo Bonzini:
> On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
>> Here we introduce generated_co_wrapper_simple, a simplification of
>> g_c_w that
>> only considers the case where the caller is not in a coroutine.
>> This simplifies and clarifies a lot when the caller is a coroutine or
>> not, and
>> in the future will hopefully replace g_c_w.
> 
> This is a good idea!
> 
> Just one thing, though it belongs more in the two series which
> introduced generated_co_wrapper_simple and generated_co_wrapper_blk - I
> would make this the "official" wrapper.  So perhaps:
> 
> - generated_co_wrapper_simple -> coroutine_wrapper
> - generated_co_wrapper_blk -> coroutine_wrapper_mixed
> - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv
> 
> ?  It is not clear to me yet if you will have bdrv_* functions that take
> the rdlock as well - in which case however coroutine_wrapper_bdrv would
> not be hard to add.

Why _mixed? I thought _blk was a nice name to identify only the blk_*
API that have this slightly different behavior (ie do not take the
rwlock anywhere).

No, I don't think there will be bdrv_* functions that will behave as
blk_*, if that's what you mean.

Regarding the bdrv_* functions in general, there are a couple of
additional places (which should be all in the main loop) where we need
to use assert_bdrv_grapg_readable. In those places we theoretically need
nothing, but if we use the automatic TSA locking checker that is being
tested by kevin, we need some sort of "placeholder" so that cc/gcc (I
don't remember anymore which) won't throw false positives.

> 
> Even without looking at the lock, the three series are going in the
> right direction of ultimately having more "simple" coroutine wrappers at
> the blk_* level and more coroutine functions (ultimately less wrappers,
> too) at the bdrv_* level.
> 
> Paolo
> 



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-18 15:01   ` Emanuele Giuseppe Esposito
@ 2022-11-18 15:13     ` Paolo Bonzini
  0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2022-11-18 15:13 UTC (permalink / raw)
  To: Emanuele Giuseppe Esposito
  Cc: qemu-block, Kevin Wolf, Hanna Reitz, Stefan Hajnoczi,
	Ari Sundholm, Pavel Dovgalyuk, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

On Fri, Nov 18, 2022 at 4:01 PM Emanuele Giuseppe Esposito
<eesposit@redhat.com> wrote:
> > - generated_co_wrapper_simple -> coroutine_wrapper
> > - generated_co_wrapper_blk -> coroutine_wrapper_mixed
> > - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv
> >
> > ?  It is not clear to me yet if you will have bdrv_* functions that take
> > the rdlock as well - in which case however coroutine_wrapper_bdrv would
> > not be hard to add.
>
> Why _mixed? I thought _blk was a nice name to identify only the blk_*
> API that have this slightly different behavior (ie do not take the
> rwlock anywhere).

_mixed means that they are callable from both coroutine and
non-coroutine function, but (unlike "normal" functions) they will
suspend if called in coroutine context.

In fact we could also have a coroutine_mixed_fn static analysis
annotation for functions that *call* coroutine_wrapper_mixed (so
ultimately they can suspend when called from coroutine context, e.g.
vhdx_log_write_and_flush or qcow2's update_refcount):

- coroutine_fn can call coroutine_mixed_fn if needed, but cannot call
any coroutine_wrapper*

- coroutine_mixed_fn can call coroutine_mixed_fn or
coroutine_wrapper_mixed{,_bdrv}, but cannot call coroutine_wrapper

This of course is completely independent of your series, but since the
naming is adjacent it would be nice to only change it once.

> No, I don't think there will be bdrv_* functions that will behave as
> blk_*, if that's what you mean.
>
> Regarding the bdrv_* functions in general, there are a couple of
> additional places (which should be all in the main loop) where we need
> to use assert_bdrv_grapg_readable. In those places we theoretically need
> nothing, but if we use the automatic TSA locking checker that is being
> tested by kevin, we need some sort of "placeholder" so that cc/gcc (I
> don't remember anymore which) won't throw false positives.

clang. :)  That can be sorted out with review, but in general I think
asserting is always okay.

Paolo



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
                   ` (15 preceding siblings ...)
  2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
@ 2022-11-21 15:02 ` Emanuele Giuseppe Esposito
  16 siblings, 0 replies; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-21 15:02 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

Ok, as I expected simple changes in a previous based-on serie provoke a
cascade of changes that inevitably affect these patches too.

While I strongly suggest to have an initial look at these patches
because it gives an idea on what am I trying to accomplish, I would not
go looking at nitpicks and trivial errors that came up from the based-on
series (ie "just as in the previous serie, fix this").

The order of the series is:
1. Still more coroutine and various fixes in block layer
2. Protect the block layer with a rwlock: part 1
3. Protect the block layer with a rwlock: part 2
4. Protect the block layer with a rwlock: part 3

Thank you,
Emanuele

Am 16/11/2022 um 15:07 schrieb Emanuele Giuseppe Esposito:
> Please read "Protect the block layer with a rwlock: part 1" and
> "Protect the block layer with a rwlock: part 2" for an
> additional introduction and aim of this series.
> 
> In this serie, we cover the remaining BlockDriver IO callbacks that were not
> running in coroutine, therefore not using the graph rdlock.
> Therefore convert them to coroutines, using either g_c_w or a new
> variant introduced in this serie (see below).
> 
> We need to convert these callbacks into coroutine because non-coroutine code
> is tied to the main thread, even though it will still delegate I/O accesses to
> the iothread (via the bdrv_coroutine_enter call in generated_co_wrappers).
> Making callbacks run in coroutines provides more flexibility, because they run
> entirely in iothreads and can use CoMutexes for mutual exclusion.
> 
> Here we introduce generated_co_wrapper_simple, a simplification of g_c_w that
> only considers the case where the caller is not in a coroutine.
> This simplifies and clarifies a lot when the caller is a coroutine or not, and
> in the future will hopefully replace g_c_w.
> 
> While we are at it, try to directly call the _co_ counterpart of a g_c_w when
> we know already that the function always run in a coroutine.
> 
> Based-on: <20221116135331.3052923-1-eesposit@redhat.com>
> 
> Thank you,
> Emanuele
> 
> Emanuele Giuseppe Esposito (15):
>   block/qed: add missing graph rdlock in qed_need_check_timer_entry
>   block: rename refresh_total_sectors in bdrv_refresh_total_sectors
>   block-backend: use bdrv_getlength instead of blk_getlength
>   block: convert bdrv_refresh_total_sectors in generated_co_wrapper
>   block: use bdrv_co_refresh_total_sectors when possible
>   block: convert bdrv_get_allocated_file_size in
>     generated_co_wrapper_simple
>   block: convert bdrv_get_info in generated_co_wrapper
>   block: convert bdrv_is_inserted in generated_co_wrapper_simple
>   block-coroutine-wrapper: support void functions
>   block: convert bdrv_eject in generated_co_wrapper_simple
>   block: convert bdrv_lock_medium in generated_co_wrapper_simple
>   block: convert bdrv_debug_event in generated_co_wrapper
>   block: convert bdrv_io_plug in generated_co_wrapper_simple
>   block: convert bdrv_io_unplug in generated_co_wrapper_simple
>   block: rename newly converted BlockDriver IO coroutine functions
> 
>  block.c                            | 93 +++++++++++++++++++-----------
>  block/blkdebug.c                   |  4 +-
>  block/blkio.c                      |  6 +-
>  block/blklogwrites.c               |  2 +-
>  block/blkreplay.c                  |  2 +-
>  block/blkverify.c                  |  2 +-
>  block/block-backend.c              | 43 ++++++++------
>  block/commit.c                     |  4 +-
>  block/copy-on-read.c               | 12 ++--
>  block/crypto.c                     |  6 +-
>  block/curl.c                       |  8 +--
>  block/file-posix.c                 | 48 +++++++--------
>  block/file-win32.c                 |  8 +--
>  block/filter-compress.c            | 10 ++--
>  block/gluster.c                    | 16 ++---
>  block/io.c                         | 78 +++++++++++++------------
>  block/iscsi.c                      |  8 +--
>  block/meson.build                  |  1 +
>  block/mirror.c                     | 17 ++++--
>  block/nbd.c                        |  6 +-
>  block/nfs.c                        |  2 +-
>  block/null.c                       |  8 +--
>  block/nvme.c                       |  6 +-
>  block/preallocate.c                |  2 +-
>  block/qcow.c                       |  2 +-
>  block/qcow2-refcount.c             |  2 +-
>  block/qcow2.c                      |  6 +-
>  block/qed.c                        |  7 ++-
>  block/quorum.c                     |  2 +-
>  block/raw-format.c                 | 14 ++---
>  block/rbd.c                        |  4 +-
>  block/replication.c                |  2 +-
>  block/ssh.c                        |  2 +-
>  block/stream.c                     |  4 +-
>  block/throttle.c                   |  2 +-
>  block/vdi.c                        |  2 +-
>  block/vhdx.c                       |  2 +-
>  block/vmdk.c                       |  4 +-
>  block/vpc.c                        |  2 +-
>  blockdev.c                         |  8 ++-
>  hw/scsi/scsi-disk.c                |  5 ++
>  include/block/block-io.h           | 40 +++++++++----
>  include/block/block_int-common.h   | 37 +++++++-----
>  include/block/block_int-io.h       |  5 +-
>  include/sysemu/block-backend-io.h  | 32 +++++++---
>  scripts/block-coroutine-wrapper.py | 19 ++++--
>  tests/unit/test-block-iothread.c   |  7 +++
>  47 files changed, 364 insertions(+), 238 deletions(-)
> 



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
  2022-11-18 15:01   ` Emanuele Giuseppe Esposito
@ 2022-11-23 11:45   ` Emanuele Giuseppe Esposito
  2022-11-23 13:45     ` Kevin Wolf
  1 sibling, 1 reply; 24+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-11-23 11:45 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, John Snow, Vladimir Sementsov-Ogievskiy,
	Stefan Weil, Fam Zheng, Ronnie Sahlberg, Peter Lieven,
	Eric Blake, Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration



Am 18/11/2022 um 11:57 schrieb Paolo Bonzini:
> On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
>> Here we introduce generated_co_wrapper_simple, a simplification of
>> g_c_w that
>> only considers the case where the caller is not in a coroutine.
>> This simplifies and clarifies a lot when the caller is a coroutine or
>> not, and
>> in the future will hopefully replace g_c_w.
> 
> This is a good idea!
> 
> Just one thing, though it belongs more in the two series which
> introduced generated_co_wrapper_simple and generated_co_wrapper_blk - I
> would make this the "official" wrapper.  So perhaps:
> 
> - generated_co_wrapper_simple -> coroutine_wrapper
> - generated_co_wrapper_blk -> coroutine_wrapper_mixed
> - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv

Ah damn I forgot about this, and of course I just sent v5 for "Still
more coroutine and various fixes in block layer".

To me it sounds good, but before I do a massive edit and then someone
asks to revert it, @Kevin and the others do you agree?

Thank you,
Emanuele

> 
> ?  It is not clear to me yet if you will have bdrv_* functions that take
> the rdlock as well - in which case however coroutine_wrapper_bdrv would
> not be hard to add.
> 
> Even without looking at the lock, the three series are going in the
> right direction of ultimately having more "simple" coroutine wrappers at
> the blk_* level and more coroutine functions (ultimately less wrappers,
> too) at the bdrv_* level.
> 
> Paolo
> 



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-23 11:45   ` Emanuele Giuseppe Esposito
@ 2022-11-23 13:45     ` Kevin Wolf
  2022-11-23 17:04       ` Paolo Bonzini
  0 siblings, 1 reply; 24+ messages in thread
From: Kevin Wolf @ 2022-11-23 13:45 UTC (permalink / raw)
  To: Emanuele Giuseppe Esposito
  Cc: Paolo Bonzini, qemu-block, Hanna Reitz, Stefan Hajnoczi,
	Ari Sundholm, Pavel Dovgalyuk, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

Am 23.11.2022 um 12:45 hat Emanuele Giuseppe Esposito geschrieben:
> 
> 
> Am 18/11/2022 um 11:57 schrieb Paolo Bonzini:
> > On 11/16/22 15:07, Emanuele Giuseppe Esposito wrote:
> >> Here we introduce generated_co_wrapper_simple, a simplification of
> >> g_c_w that
> >> only considers the case where the caller is not in a coroutine.
> >> This simplifies and clarifies a lot when the caller is a coroutine or
> >> not, and
> >> in the future will hopefully replace g_c_w.
> > 
> > This is a good idea!
> > 
> > Just one thing, though it belongs more in the two series which
> > introduced generated_co_wrapper_simple and generated_co_wrapper_blk - I
> > would make this the "official" wrapper.  So perhaps:
> > 
> > - generated_co_wrapper_simple -> coroutine_wrapper
> > - generated_co_wrapper_blk -> coroutine_wrapper_mixed
> > - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv
> 
> Ah damn I forgot about this, and of course I just sent v5 for "Still
> more coroutine and various fixes in block layer".
> 
> To me it sounds good, but before I do a massive edit and then someone
> asks to revert it, @Kevin and the others do you agree?

Makes sense to me in general. I think I originally suggested that the
"basic" version should be the one that doesn't take the graph lock, and
the special version with the longer name is the one that takes it.

This proposal contains the same thought, but extends it to make
generated_co_wrapper_simple the basic thing. This is good, because
eventually we want to get rid of the mixed functions, so they are indeed
the special thing.

I think this means that if we clean up everything, in the end we'll have
coroutine_wrapper and coroutine_wrapper_bdrv (the fourth version not in
the above list, but that Paolo mentioned we may want to have).

The only thing I'm unsure about is whether coroutine_wrapper_bdrv is
descriptive enough as a name or whether it should be something more
explicit like coroutine_wrapper_bdrv_graph_locked.

Kevin



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-23 13:45     ` Kevin Wolf
@ 2022-11-23 17:04       ` Paolo Bonzini
  2022-11-23 18:12         ` Kevin Wolf
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2022-11-23 17:04 UTC (permalink / raw)
  To: Kevin Wolf, Emanuele Giuseppe Esposito
  Cc: qemu-block, Hanna Reitz, Stefan Hajnoczi, Ari Sundholm,
	Pavel Dovgalyuk, John Snow, Vladimir Sementsov-Ogievskiy,
	Stefan Weil, Fam Zheng, Ronnie Sahlberg, Peter Lieven,
	Eric Blake, Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

On 11/23/22 14:45, Kevin Wolf wrote:
> I think this means that if we clean up everything, in the end we'll have
> coroutine_wrapper and coroutine_wrapper_bdrv (the fourth version not in
> the above list, but that Paolo mentioned we may want to have).

Yes, I agree.

> The only thing I'm unsure about is whether coroutine_wrapper_bdrv is
> descriptive enough as a name or whether it should be something more
> explicit like coroutine_wrapper_bdrv_graph_locked.

That's already long and becomes longer if you add "mixed", but perhaps 
co_wrapper_{mixed_,}{bdrv_graph_rdlock,} would be okay?

In other words:

generated_co_wrapper_simple -> co_wrapper
generated_co_wrapper        -> co_wrapper_mixed
generated_co_wrapper_bdrv   -> co_wrapper_mixed_bdrv_graph_rdlock

?

Paolo



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

* Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
  2022-11-23 17:04       ` Paolo Bonzini
@ 2022-11-23 18:12         ` Kevin Wolf
  0 siblings, 0 replies; 24+ messages in thread
From: Kevin Wolf @ 2022-11-23 18:12 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Emanuele Giuseppe Esposito, qemu-block, Hanna Reitz,
	Stefan Hajnoczi, Ari Sundholm, Pavel Dovgalyuk, John Snow,
	Vladimir Sementsov-Ogievskiy, Stefan Weil, Fam Zheng,
	Ronnie Sahlberg, Peter Lieven, Eric Blake,
	Philippe Mathieu-Daudé,
	Alberto Garcia, Ilya Dryomov, Wen Congyang, Xie Changlong,
	Richard W.M. Jones, Jeff Cody, Cleber Rosa, qemu-devel,
	integration

Am 23.11.2022 um 18:04 hat Paolo Bonzini geschrieben:
> On 11/23/22 14:45, Kevin Wolf wrote:
> > I think this means that if we clean up everything, in the end we'll have
> > coroutine_wrapper and coroutine_wrapper_bdrv (the fourth version not in
> > the above list, but that Paolo mentioned we may want to have).
> 
> Yes, I agree.
> 
> > The only thing I'm unsure about is whether coroutine_wrapper_bdrv is
> > descriptive enough as a name or whether it should be something more
> > explicit like coroutine_wrapper_bdrv_graph_locked.
> 
> That's already long and becomes longer if you add "mixed", but perhaps
> co_wrapper_{mixed_,}{bdrv_graph_rdlock,} would be okay?
> 
> In other words:
> 
> generated_co_wrapper_simple -> co_wrapper
> generated_co_wrapper        -> co_wrapper_mixed
> generated_co_wrapper_bdrv   -> co_wrapper_mixed_bdrv_graph_rdlock
> 
> ?

Works for me. Maybe co_wrapper_mixed_bdrv_rdlock (without the "graph")
would be enough, too, if it is too long.

Kevin



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

end of thread, other threads:[~2022-11-23 18:13 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16 14:07 [PATCH 00/15] Protect the block layer with a rwlock: part 3 Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 01/15] block/qed: add missing graph rdlock in qed_need_check_timer_entry Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 02/15] block: rename refresh_total_sectors in bdrv_refresh_total_sectors Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 03/15] block-backend: use bdrv_getlength instead of blk_getlength Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 06/15] block: convert bdrv_get_allocated_file_size in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 07/15] block: convert bdrv_get_info in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 09/15] block-coroutine-wrapper: support void functions Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 10/15] block: convert bdrv_eject in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 11/15] block: convert bdrv_lock_medium " Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 14/15] block: convert bdrv_io_unplug " Emanuele Giuseppe Esposito
2022-11-16 14:07 ` [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions Emanuele Giuseppe Esposito
2022-11-18 10:57 ` [PATCH 00/15] Protect the block layer with a rwlock: part 3 Paolo Bonzini
2022-11-18 15:01   ` Emanuele Giuseppe Esposito
2022-11-18 15:13     ` Paolo Bonzini
2022-11-23 11:45   ` Emanuele Giuseppe Esposito
2022-11-23 13:45     ` Kevin Wolf
2022-11-23 17:04       ` Paolo Bonzini
2022-11-23 18:12         ` Kevin Wolf
2022-11-21 15:02 ` Emanuele Giuseppe Esposito

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.