[PATCH 00/20] Protect the block layer with a rwlock: part 1

[PATCH 00/20] Protect the block layer with a rwlock: part 1

[Emanuele Giuseppe Esposito]

This serie is the first of four series that aim to introduce and use a new
graph rwlock in the QEMU block layer.
The aim is to replace the current AioContext lock with much fine-grained locks,
aimed to protect only specific data.
Currently the AioContext lock is used pretty much everywhere, and it's not
even clear what it is protecting exactly.

The aim of the rwlock is to cover graph modifications: more precisely,
when a BlockDriverState parent or child list is modified or read, since it can
be concurrently accessed by the main loop and iothreads.

The main assumption is that the main loop is the only one allowed to perform
graph modifications, and so far this has always been held by the current code.

The rwlock is inspired from cpus-common.c implementation, and aims to
reduce cacheline bouncing by having per-aiocontext counter of readers.
All details and implementation of the lock are in patch 1.

We distinguish between writer (main loop, under BQL) that modifies the
graph, and readers (all other coroutines running in various AioContext),
that go through the graph edges, reading ->parents and->children.
The writer (main loop)  has an "exclusive" access, so it first waits for
current read to finish, and then prevents incoming ones from
entering while it has the exclusive access.
The readers (coroutines in multiple AioContext) are free to
access the graph as long the writer is not modifying the graph.
In case it is, they go in a CoQueue and sleep until the writer
is done.

In this first serie, my aim is to introduce the lock (patches 1-3,6), cover the
main graph writer (patch 4), define assertions (patch 5) and start using the
read lock in the generated_co_wrapper functions (7-20).
Such functions recursively traverse the BlockDriverState graph, so they must
take the graph rdlock.

We distinguish two cases related to the generated_co_wrapper (often shortened
to g_c_w):
- qemu_in_coroutine(), which means the function is already running in a
  coroutine. This means we don't take the lock, because the coroutine must
  have taken it when it started
- !qemu_in_coroutine(), which means we need to create a new coroutine that
  performs the operation requested. In this case we take the rdlock as soon as
  the coroutine starts, and release only before finishing.

In this and following series, we try to follow the following locking pattern:
- bdrv_co_* functions that call BlockDriver callbacks always expect the lock
  to be taken, therefore they assert.
- blk_co_* functions usually call blk_wait_while_drained(), therefore they must
  take the lock internally. Therefore we introduce generated_co_wrapper_blk,
  which does not take the rdlock when starting the coroutine.

The long term goal of this series is to eventually replace the AioContext lock,
so that we can get rid of it once and for all.

This serie is based on v4 of "Still more coroutine and various fixes in block layer".

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

Thank you,
Emanuele

Emanuele Giuseppe Esposito (19):
  graph-lock: introduce BdrvGraphRWlock structure
  async: register/unregister aiocontext in graph lock list
  block.c: wrlock in bdrv_replace_child_noperm
  block: remove unnecessary assert_bdrv_graph_writable()
  block: assert that graph read and writes are performed correctly
  graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD
    macros
  block-coroutine-wrapper.py: take the graph rdlock in bdrv_* functions
  block-backend: introduce new generated_co_wrapper_blk annotation
  block-gen: assert that {bdrv/blk}_co_truncate is always called with
    graph rdlock taken
  block-gen: assert that bdrv_co_{check/invalidate_cache} are always
    called with graph rdlock taken
  block-gen: assert that bdrv_co_pwrite is always called with graph
    rdlock taken
  block-gen: assert that bdrv_co_pwrite_{zeros/sync} is always called
    with graph rdlock taken
  block-gen: assert that bdrv_co_pread is always called with graph
    rdlock taken
  block-gen: assert that {bdrv/blk}_co_flush is always called with graph
    rdlock taken
  block-gen: assert that bdrv_co_{read/write}v_vmstate are always called
    with graph rdlock taken
  block-gen: assert that bdrv_co_pdiscard is always called with graph
    rdlock taken
  block-gen: assert that bdrv_co_common_block_status_above is always
    called with graph rdlock taken
  block-gen: assert that bdrv_co_ioctl is always called with graph
    rdlock taken
  block-gen: assert that nbd_co_do_establish_connection is always called
    with graph rdlock taken

Paolo Bonzini (1):
  block: introduce a lock to protect graph operations

 block.c                                |  15 +-
 block/backup.c                         |   3 +
 block/block-backend.c                  |  10 +-
 block/block-copy.c                     |  10 +-
 block/graph-lock.c                     | 255 +++++++++++++++++++++++++
 block/io.c                             |  15 ++
 block/meson.build                      |   1 +
 block/mirror.c                         |  20 +-
 block/nbd.c                            |   1 +
 block/stream.c                         |  32 ++--
 include/block/aio.h                    |   9 +
 include/block/block-common.h           |   1 +
 include/block/block_int-common.h       |  36 +++-
 include/block/block_int-global-state.h |  17 --
 include/block/block_int-io.h           |   2 +
 include/block/block_int.h              |   1 +
 include/block/graph-lock.h             | 180 +++++++++++++++++
 include/sysemu/block-backend-io.h      |  69 +++----
 qemu-img.c                             |   4 +-
 scripts/block-coroutine-wrapper.py     |  13 +-
 tests/unit/test-bdrv-drain.c           |   2 +
 util/async.c                           |   4 +
 util/meson.build                       |   1 +
 23 files changed, 615 insertions(+), 86 deletions(-)
 create mode 100644 block/graph-lock.c
 create mode 100644 include/block/graph-lock.h

-- 
2.31.1

[PATCH 01/20] block: introduce a lock to protect graph operations

[Emanuele Giuseppe Esposito]

From: Paolo Bonzini <pbonzini@redhat.com>

block layer graph operations are always run under BQL in the
main loop. This is proved by the assertion qemu_in_main_thread()
and its wrapper macro GLOBAL_STATE_CODE.
However, there are also concurrent coroutines running in other
iothreads that always try to traverse the graph.
Currently this is protected (among various other things) by
the AioContext lock, but once this is removed we need to make
sure that reads do not happen while modifying the graph.

We distinguish between writer (main loop, under BQL) that modifies the
graph, and readers (all other coroutines running in various AioContext),
that go through the graph edges, reading ->parents and->children.

The writer (main loop)  has an "exclusive" access, so it first waits for
current read to finish, and then prevents incoming ones from
entering while it has the exclusive access.

The readers (coroutines in multiple AioContext) are free to
access the graph as long the writer is not modifying the graph.
In case it is, they go in a CoQueue and sleep until the writer
is done.

If a coroutine changes AioContext, the counter in the original and new
AioContext are left intact, since the writer does not care where is the
reader, but only if there is one.
As a result, some AioContexts might have a negative reader count, to
balance the positive count of the AioContext that took the lock.
This also means that when an AioContext is deleted it may have a nonzero
reader count. In that case we transfer the count to a global shared counter
so that the writer is always aware of all readers.

Co-developed-with: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/graph-lock.c         | 221 +++++++++++++++++++++++++++++++++++++
 block/meson.build          |   1 +
 include/block/aio.h        |   9 ++
 include/block/block_int.h  |   1 +
 include/block/graph-lock.h | 129 ++++++++++++++++++++++
 5 files changed, 361 insertions(+)
 create mode 100644 block/graph-lock.c
 create mode 100644 include/block/graph-lock.h

diff --git a/block/graph-lock.c b/block/graph-lock.c
new file mode 100644
index 0000000000..b608a89d7c
--- /dev/null
+++ b/block/graph-lock.c
@@ -0,0 +1,221 @@
+/*
+ * Graph lock: rwlock to protect block layer graph manipulations (add/remove
+ * edges and nodes)
+ *
+ *  Copyright (c) 2022 Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "block/graph-lock.h"
+#include "block/block.h"
+#include "block/block_int.h"
+
+/* Protects the list of aiocontext and orphaned_reader_count */
+static QemuMutex aio_context_list_lock;
+
+/* Written and read with atomic operations. */
+static int has_writer;
+
+/*
+ * A reader coroutine could move from an AioContext to another.
+ * If this happens, there is no problem from the point of view of
+ * counters. The problem is that the total count becomes
+ * unbalanced if one of the two AioContexts gets deleted.
+ * The count of readers must remain correct, so the AioContext's
+ * balance is transferred to this glboal variable.
+ * Protected by aio_context_list_lock.
+ */
+static uint32_t orphaned_reader_count;
+
+/* Queue of readers waiting for the writer to finish */
+static CoQueue reader_queue;
+
+/*
+ * List of AioContext. This list ensures that each AioContext
+ * can safely modify only its own counter, avoid reading/writing
+ * others and thus improving performances by avoiding cacheline bounces.
+ */
+static QTAILQ_HEAD(, AioContext) aio_context_list =
+    QTAILQ_HEAD_INITIALIZER(aio_context_list);
+
+static void __attribute__((__constructor__)) bdrv_init_graph_lock(void)
+{
+    qemu_mutex_init(&aio_context_list_lock);
+    qemu_co_queue_init(&reader_queue);
+}
+
+void register_aiocontext(AioContext *ctx)
+{
+    QEMU_LOCK_GUARD(&aio_context_list_lock);
+    assert(ctx->reader_count == 0);
+    QTAILQ_INSERT_TAIL(&aio_context_list, ctx, next_aio);
+}
+
+void unregister_aiocontext(AioContext *ctx)
+{
+    QEMU_LOCK_GUARD(&aio_context_list_lock);
+    orphaned_reader_count += ctx->reader_count;
+    QTAILQ_REMOVE(&aio_context_list, ctx, next_aio);
+}
+
+static uint32_t reader_count(void)
+{
+    AioContext *ctx;
+    uint32_t rd;
+
+    QEMU_LOCK_GUARD(&aio_context_list_lock);
+
+    /* rd can temporarly be negative, but the total will *always* be >= 0 */
+    rd = orphaned_reader_count;
+    QTAILQ_FOREACH(ctx, &aio_context_list, next_aio) {
+        rd += qatomic_read(&ctx->reader_count);
+    }
+
+    /* shouldn't overflow unless there are 2^31 readers */
+    assert((int32_t)rd >= 0);
+    return rd;
+}
+
+void bdrv_graph_wrlock(void)
+{
+    GLOBAL_STATE_CODE();
+    assert(!qatomic_read(&has_writer));
+
+    /*
+     * reader_count == 0: this means writer will read has_reader as 1
+     * reader_count >= 1: we don't know if writer read has_writer == 0 or 1,
+     *                    but we need to wait.
+     * Wait by allowing other coroutine (and possible readers) to continue.
+     */
+    do {
+        /*
+         * has_writer must be 0 while polling, otherwise we get a deadlock if
+         * any callback involved during AIO_WAIT_WHILE() tries to acquire the
+         * reader lock.
+         */
+        qatomic_set(&has_writer, 0);
+        AIO_WAIT_WHILE(qemu_get_aio_context(), reader_count() >= 1);
+        qatomic_set(&has_writer, 1);
+
+        /*
+         * We want to only check reader_count() after has_writer = 1 is visible
+         * to other threads. That way no more readers can sneak in after we've
+         * determined reader_count() == 0.
+         */
+        smp_mb();
+    } while (reader_count() >= 1);
+}
+
+void bdrv_graph_wrunlock(void)
+{
+    GLOBAL_STATE_CODE();
+    QEMU_LOCK_GUARD(&aio_context_list_lock);
+    assert(qatomic_read(&has_writer));
+
+    /*
+     * No need for memory barriers, this works in pair with
+     * the slow path of rdlock() and both take the lock.
+     */
+    qatomic_store_release(&has_writer, 0);
+
+    /* Wake up all coroutine that are waiting to read the graph */
+    qemu_co_enter_all(&reader_queue, &aio_context_list_lock);
+}
+
+void coroutine_fn bdrv_graph_co_rdlock(void)
+{
+    AioContext *aiocontext;
+    aiocontext = qemu_get_current_aio_context();
+
+    for (;;) {
+        qatomic_set(&aiocontext->reader_count,
+                    aiocontext->reader_count + 1);
+        /* make sure writer sees reader_count before we check has_writer */
+        smp_mb();
+
+        /*
+         * has_writer == 0: this means writer will read reader_count as >= 1
+         * has_writer == 1: we don't know if writer read reader_count == 0
+         *                  or > 0, but we need to wait anyways because
+         *                  it will write.
+         */
+        if (!qatomic_read(&has_writer)) {
+            break;
+        }
+
+        /*
+         * Synchronize access with reader_count() in bdrv_graph_wrlock().
+         * Case 1:
+         * If this critical section gets executed first, reader_count will
+         * decrease and the reader will go to sleep.
+         * Then the writer will read reader_count that does not take into
+         * account this reader, and if there's no other reader it will
+         * enter the write section.
+         * Case 2:
+         * If reader_count() critical section gets executed first,
+         * then writer will read reader_count >= 1.
+         * It will wait in AIO_WAIT_WHILE(), but once it releases the lock
+         * we will enter this critical section and call aio_wait_kick().
+         */
+        WITH_QEMU_LOCK_GUARD(&aio_context_list_lock) {
+            /*
+             * Additional check when we use the above lock to synchronize
+             * with bdrv_graph_wrunlock().
+             * Case 1:
+             * If this gets executed first, has_writer is still 1, so we reduce
+             * reader_count and go to sleep.
+             * Then the writer will set has_writer to 0 and wake up all readers,
+             * us included.
+             * Case 2:
+             * If bdrv_graph_wrunlock() critical section gets executed first,
+             * then it will set has_writer to 0 and wake up all other readers.
+             * Then we execute this critical section, and therefore must check
+             * again for has_writer, otherwise we sleep without any writer
+             * actually running.
+             */
+            if (!qatomic_read(&has_writer)) {
+                return;
+            }
+
+            /* slow path where reader sleeps */
+            aiocontext->reader_count--;
+            aio_wait_kick();
+            qemu_co_queue_wait(&reader_queue, &aio_context_list_lock);
+        }
+    }
+}
+
+void coroutine_fn bdrv_graph_co_rdunlock(void)
+{
+    AioContext *aiocontext;
+    aiocontext = qemu_get_current_aio_context();
+
+    qatomic_store_release(&aiocontext->reader_count,
+                          aiocontext->reader_count - 1);
+    /* make sure writer sees reader_count before we check has_writer */
+    smp_mb();
+
+    /*
+     * has_writer == 0: this means reader will read reader_count decreased
+     * has_writer == 1: we don't know if writer read reader_count old or
+     *                  new. Therefore, kick again so on next iteration
+     *                  writer will for sure read the updated value.
+     */
+    if (qatomic_read(&has_writer)) {
+        aio_wait_kick();
+    }
+}
diff --git a/block/meson.build b/block/meson.build
index c48a59571e..90011a2805 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -10,6 +10,7 @@ block_ss.add(files(
   'blkverify.c',
   'block-backend.c',
   'block-copy.c',
+  'graph-lock.c',
   'commit.c',
   'copy-on-read.c',
   'preallocate.c',
diff --git a/include/block/aio.h b/include/block/aio.h
index d128558f1d..8e64f81d01 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -127,6 +127,15 @@ struct AioContext {
     /* Used by AioContext users to protect from multi-threaded access.  */
     QemuRecMutex lock;
 
+    /* How many readers in this AioContext are currently reading the graph. */
+    uint32_t reader_count;
+
+    /*
+     * List of AioContext kept in graph-lock.c
+     * Protected by aio_context_list_lock
+     */
+    QTAILQ_ENTRY(AioContext) next_aio;
+
     /* The list of registered AIO handlers.  Protected by ctx->list_lock. */
     AioHandlerList aio_handlers;
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7d50b6bbd1..b35b0138ed 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -26,6 +26,7 @@
 
 #include "block_int-global-state.h"
 #include "block_int-io.h"
+#include "block/graph-lock.h"
 
 /* DO NOT ADD ANYTHING IN HERE. USE ONE OF THE HEADERS INCLUDED ABOVE */
 
diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
new file mode 100644
index 0000000000..f975312bb6
--- /dev/null
+++ b/include/block/graph-lock.h
@@ -0,0 +1,129 @@
+/*
+ * Graph lock: rwlock to protect block layer graph manipulations (add/remove
+ * edges and nodes)
+ *
+ *  Copyright (c) 2022 Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef GRAPH_LOCK_H
+#define GRAPH_LOCK_H
+
+#include "qemu/osdep.h"
+
+/**
+ * Graph Lock API
+ * This API provides a rwlock used to protect block layer
+ * graph modifications like edge (BdrvChild) and node (BlockDriverState)
+ * addition and removal.
+ * Currently we have 1 writer only, the Main loop, and many
+ * readers, mostly coroutines running in other AioContext thus other threads.
+ *
+ * We distinguish between writer (main loop, under BQL) that modifies the
+ * graph, and readers (all other coroutines running in various AioContext),
+ * that go through the graph edges, reading
+ * BlockDriverState ->parents and->children.
+ *
+ * The writer (main loop)  has an "exclusive" access, so it first waits for
+ * current read to finish, and then prevents incoming ones from
+ * entering while it has the exclusive access.
+ *
+ * The readers (coroutines in multiple AioContext) are free to
+ * access the graph as long the writer is not modifying the graph.
+ * In case it is, they go in a CoQueue and sleep until the writer
+ * is done.
+ *
+ * If a coroutine changes AioContext, the counter in the original and new
+ * AioContext are left intact, since the writer does not care where is the
+ * reader, but only if there is one.
+ * As a result, some AioContexts might have a negative reader count, to
+ * balance the positive count of the AioContext that took the lock.
+ * This also means that when an AioContext is deleted it may have a nonzero
+ * reader count. In that case we transfer the count to a global shared counter
+ * so that the writer is always aware of all readers.
+ */
+
+/*
+ * register_aiocontext:
+ * Add AioContext @ctx to the list of AioContext.
+ * This list is used to obtain the total number of readers
+ * currently running the graph.
+ */
+void register_aiocontext(AioContext *ctx);
+
+/*
+ * unregister_aiocontext:
+ * Removes AioContext @ctx to the list of AioContext.
+ */
+void unregister_aiocontext(AioContext *ctx);
+
+/*
+ * bdrv_graph_wrlock:
+ * Start an exclusive write operation to modify the graph.
+ * This means we are adding or removing an edge or a node (bs)
+ * from the block layer graph.
+ * Nobody else is allowed to access the graph.
+ * Set global has_writer to 1, so that the next readers will wait
+ * that writer is done in a coroutine queue.
+ * Then keep track of the running readers by counting what is the total
+ * amount of readers (sum of all aiocontext readers), and wait until
+ * they all finish with AIO_WAIT_WHILE.
+ *
+ * Must only be called from outside bdrv_graph_co_rdlock.
+ * The wrlock is only taken from the main loop, with BQL held,
+ * as only the main loop is allowed to modify the graph.
+ */
+void bdrv_graph_wrlock(void);
+
+/*
+ * bdrv_graph_wrunlock:
+ * Write finished, reset global has_writer to 0 and restart
+ * all readers that are waiting.
+ */
+void bdrv_graph_wrunlock(void);
+
+/*
+ * bdrv_graph_co_rdlock:
+ * Read the bs graph. This usually means traversing all nodes in
+ * the graph, therefore it can't happen while another thread is
+ * modifying it.
+ * Increases the reader counter of the current aiocontext,
+ * and if has_writer is set, it means that the writer is modifying
+ * the graph, therefore wait in a coroutine queue.
+ * The writer will then wake this coroutine once it is done.
+ *
+ * This lock should be taken from Iothreads (IO_CODE() class of functions)
+ * because it signals the writer that there are some
+ * readers currently running, or waits until the current
+ * write is finished before continuing.
+ * Calling this function from the Main Loop with BQL held
+ * is not necessary, since the Main Loop itself is the only
+ * writer, thus won't be able to read and write at the same time.
+ * The only exception to that is when we can't take the lock in the
+ * function/coroutine itself, and need to delegate the caller (usually main
+ * loop) to take it and wait that the coroutine ends, so that
+ * we always signal that a reader is running.
+ */
+void coroutine_fn bdrv_graph_co_rdlock(void);
+
+/*
+ * bdrv_graph_rdunlock:
+ * Read terminated, decrease the count of readers in the current aiocontext.
+ * If the writer is waiting for reads to finish (has_writer == 1), signal
+ * the writer that we are done via aio_wait_kick() to let it continue.
+ */
+void coroutine_fn bdrv_graph_co_rdunlock(void);
+
+#endif /* GRAPH_LOCK_H */
+
-- 
2.31.1

[PATCH 02/20] graph-lock: introduce BdrvGraphRWlock structure

[Emanuele Giuseppe Esposito]

Just a wrapper to simplify what is available to the struct AioContext.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/graph-lock.c         | 59 ++++++++++++++++++++++++++------------
 include/block/aio.h        | 12 ++++----
 include/block/graph-lock.h |  1 +
 3 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/block/graph-lock.c b/block/graph-lock.c
index b608a89d7c..c3c6eeedad 100644
--- a/block/graph-lock.c
+++ b/block/graph-lock.c
@@ -44,12 +44,23 @@ static uint32_t orphaned_reader_count;
 /* Queue of readers waiting for the writer to finish */
 static CoQueue reader_queue;
 
+struct BdrvGraphRWlock {
+    /* How many readers are currently reading the graph. */
+    uint32_t reader_count;
+
+    /*
+     * List of BdrvGraphRWlock kept in graph-lock.c
+     * Protected by aio_context_list_lock
+     */
+    QTAILQ_ENTRY(BdrvGraphRWlock) next_aio;
+};
+
 /*
- * List of AioContext. This list ensures that each AioContext
+ * List of BdrvGraphRWlock. This list ensures that each BdrvGraphRWlock
  * can safely modify only its own counter, avoid reading/writing
  * others and thus improving performances by avoiding cacheline bounces.
  */
-static QTAILQ_HEAD(, AioContext) aio_context_list =
+static QTAILQ_HEAD(, BdrvGraphRWlock) aio_context_list =
     QTAILQ_HEAD_INITIALIZER(aio_context_list);
 
 static void __attribute__((__constructor__)) bdrv_init_graph_lock(void)
@@ -60,29 +71,31 @@ static void __attribute__((__constructor__)) bdrv_init_graph_lock(void)
 
 void register_aiocontext(AioContext *ctx)
 {
+    ctx->bdrv_graph = g_new0(BdrvGraphRWlock, 1);
     QEMU_LOCK_GUARD(&aio_context_list_lock);
-    assert(ctx->reader_count == 0);
-    QTAILQ_INSERT_TAIL(&aio_context_list, ctx, next_aio);
+    assert(ctx->bdrv_graph->reader_count == 0);
+    QTAILQ_INSERT_TAIL(&aio_context_list, ctx->bdrv_graph, next_aio);
 }
 
 void unregister_aiocontext(AioContext *ctx)
 {
     QEMU_LOCK_GUARD(&aio_context_list_lock);
-    orphaned_reader_count += ctx->reader_count;
-    QTAILQ_REMOVE(&aio_context_list, ctx, next_aio);
+    orphaned_reader_count += ctx->bdrv_graph->reader_count;
+    QTAILQ_REMOVE(&aio_context_list, ctx->bdrv_graph, next_aio);
+    g_free(ctx->bdrv_graph);
 }
 
 static uint32_t reader_count(void)
 {
-    AioContext *ctx;
+    BdrvGraphRWlock *brdv_graph;
     uint32_t rd;
 
     QEMU_LOCK_GUARD(&aio_context_list_lock);
 
     /* rd can temporarly be negative, but the total will *always* be >= 0 */
     rd = orphaned_reader_count;
-    QTAILQ_FOREACH(ctx, &aio_context_list, next_aio) {
-        rd += qatomic_read(&ctx->reader_count);
+    QTAILQ_FOREACH(brdv_graph, &aio_context_list, next_aio) {
+        rd += qatomic_read(&brdv_graph->reader_count);
     }
 
     /* shouldn't overflow unless there are 2^31 readers */
@@ -138,12 +151,17 @@ void bdrv_graph_wrunlock(void)
 
 void coroutine_fn bdrv_graph_co_rdlock(void)
 {
-    AioContext *aiocontext;
-    aiocontext = qemu_get_current_aio_context();
+    BdrvGraphRWlock *bdrv_graph;
+    bdrv_graph = qemu_get_current_aio_context()->bdrv_graph;
+
+    /* Do not lock if in main thread */
+    if (qemu_in_main_thread()) {
+        return;
+    }
 
     for (;;) {
-        qatomic_set(&aiocontext->reader_count,
-                    aiocontext->reader_count + 1);
+        qatomic_set(&bdrv_graph->reader_count,
+                    bdrv_graph->reader_count + 1);
         /* make sure writer sees reader_count before we check has_writer */
         smp_mb();
 
@@ -192,7 +210,7 @@ void coroutine_fn bdrv_graph_co_rdlock(void)
             }
 
             /* slow path where reader sleeps */
-            aiocontext->reader_count--;
+            bdrv_graph->reader_count--;
             aio_wait_kick();
             qemu_co_queue_wait(&reader_queue, &aio_context_list_lock);
         }
@@ -201,11 +219,16 @@ void coroutine_fn bdrv_graph_co_rdlock(void)
 
 void coroutine_fn bdrv_graph_co_rdunlock(void)
 {
-    AioContext *aiocontext;
-    aiocontext = qemu_get_current_aio_context();
+    BdrvGraphRWlock *bdrv_graph;
+    bdrv_graph = qemu_get_current_aio_context()->bdrv_graph;
+
+    /* Do not lock if in main thread */
+    if (qemu_in_main_thread()) {
+        return;
+    }
 
-    qatomic_store_release(&aiocontext->reader_count,
-                          aiocontext->reader_count - 1);
+    qatomic_store_release(&bdrv_graph->reader_count,
+                          bdrv_graph->reader_count - 1);
     /* make sure writer sees reader_count before we check has_writer */
     smp_mb();
 
diff --git a/include/block/aio.h b/include/block/aio.h
index 8e64f81d01..0f65a3cc9e 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -22,6 +22,7 @@
 #include "qemu/event_notifier.h"
 #include "qemu/thread.h"
 #include "qemu/timer.h"
+#include "block/graph-lock.h"
 
 typedef struct BlockAIOCB BlockAIOCB;
 typedef void BlockCompletionFunc(void *opaque, int ret);
@@ -127,14 +128,13 @@ struct AioContext {
     /* Used by AioContext users to protect from multi-threaded access.  */
     QemuRecMutex lock;
 
-    /* How many readers in this AioContext are currently reading the graph. */
-    uint32_t reader_count;
-
     /*
-     * List of AioContext kept in graph-lock.c
-     * Protected by aio_context_list_lock
+     * Keep track of readers and writers of the block layer graph.
+     * This is essential to avoid performing additions and removal
+     * of nodes and edges from block graph while some
+     * other thread is traversing it.
      */
-    QTAILQ_ENTRY(AioContext) next_aio;
+    BdrvGraphRWlock *bdrv_graph;
 
     /* The list of registered AIO handlers.  Protected by ctx->list_lock. */
     AioHandlerList aio_handlers;
diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
index f975312bb6..fc806aefa3 100644
--- a/include/block/graph-lock.h
+++ b/include/block/graph-lock.h
@@ -53,6 +53,7 @@
  * reader count. In that case we transfer the count to a global shared counter
  * so that the writer is always aware of all readers.
  */
+typedef struct BdrvGraphRWlock BdrvGraphRWlock;
 
 /*
  * register_aiocontext:
-- 
2.31.1

[PATCH 03/20] async: register/unregister aiocontext in graph lock list

[Emanuele Giuseppe Esposito]

Add/remove the AioContext in aio_context_list in graph-lock.c only when
it is being effectively created/destroyed.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 util/async.c     | 4 ++++
 util/meson.build | 1 +
 2 files changed, 5 insertions(+)

diff --git a/util/async.c b/util/async.c
index 63434ddae4..14d63b3091 100644
--- a/util/async.c
+++ b/util/async.c
@@ -27,6 +27,7 @@
 #include "qapi/error.h"
 #include "block/aio.h"
 #include "block/thread-pool.h"
+#include "block/graph-lock.h"
 #include "qemu/main-loop.h"
 #include "qemu/atomic.h"
 #include "qemu/rcu_queue.h"
@@ -376,6 +377,7 @@ aio_ctx_finalize(GSource     *source)
     qemu_rec_mutex_destroy(&ctx->lock);
     qemu_lockcnt_destroy(&ctx->list_lock);
     timerlistgroup_deinit(&ctx->tlg);
+    unregister_aiocontext(ctx);
     aio_context_destroy(ctx);
 }
 
@@ -574,6 +576,8 @@ AioContext *aio_context_new(Error **errp)
     ctx->thread_pool_min = 0;
     ctx->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
 
+    register_aiocontext(ctx);
+
     return ctx;
 fail:
     g_source_destroy(&ctx->source);
diff --git a/util/meson.build b/util/meson.build
index 59c1f467bb..ecee2ba899 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -70,6 +70,7 @@ endif
 
 if have_block
   util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
+  util_ss.add(files('../block/graph-lock.c'))
   util_ss.add(files('base64.c'))
   util_ss.add(files('buffer.c'))
   util_ss.add(files('bufferiszero.c'))
-- 
2.31.1

[PATCH 04/20] block.c: wrlock in bdrv_replace_child_noperm

[Emanuele Giuseppe Esposito]

Protect the main function where graph is modified.

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

diff --git a/block.c b/block.c
index d3e168408a..4ef537a9f2 100644
--- a/block.c
+++ b/block.c
@@ -1416,6 +1416,7 @@ static void bdrv_child_cb_attach(BdrvChild *child)
 
     assert_bdrv_graph_writable(bs);
     QLIST_INSERT_HEAD(&bs->children, child, next);
+
     if (bs->drv->is_filter || (child->role & BDRV_CHILD_FILTERED)) {
         /*
          * Here we handle filters and block/raw-format.c when it behave like
@@ -2829,24 +2830,25 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
         assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
     }
 
+    bdrv_graph_wrlock();
     if (old_bs) {
         if (child->klass->detach) {
             child->klass->detach(child);
         }
-        assert_bdrv_graph_writable(old_bs);
+
         QLIST_REMOVE(child, next_parent);
     }
 
     child->bs = new_bs;
 
     if (new_bs) {
-        assert_bdrv_graph_writable(new_bs);
         QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent);
 
         if (child->klass->attach) {
             child->klass->attach(child);
         }
     }
+    bdrv_graph_wrunlock();
 
     /*
      * If the old child node was drained but the new one is not, allow
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 791dddfd7d..fd9f40a815 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -71,6 +71,7 @@ enum BdrvTrackedRequestType {
     BDRV_TRACKED_TRUNCATE,
 };
 
+
 /*
  * That is not quite good that BdrvTrackedRequest structure is public,
  * as block/io.c is very careful about incoming offset/bytes being
-- 
2.31.1

[PATCH 05/20] block: remove unnecessary assert_bdrv_graph_writable()

[Emanuele Giuseppe Esposito]

We don't protect bdrv->aio_context with the graph rwlock,
so these assertions are not needed

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

diff --git a/block.c b/block.c
index 4ef537a9f2..afab74d4da 100644
--- a/block.c
+++ b/block.c
@@ -7183,7 +7183,6 @@ static void bdrv_detach_aio_context(BlockDriverState *bs)
     if (bs->quiesce_counter) {
         aio_enable_external(bs->aio_context);
     }
-    assert_bdrv_graph_writable(bs);
     bs->aio_context = NULL;
 }
 
@@ -7197,7 +7196,6 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
         aio_disable_external(new_context);
     }
 
-    assert_bdrv_graph_writable(bs);
     bs->aio_context = new_context;
 
     if (bs->drv && bs->drv->bdrv_attach_aio_context) {
@@ -7278,7 +7276,6 @@ static void bdrv_set_aio_context_commit(void *opaque)
     BlockDriverState *bs = (BlockDriverState *) state->bs;
     AioContext *new_context = state->new_ctx;
     AioContext *old_context = bdrv_get_aio_context(bs);
-    assert_bdrv_graph_writable(bs);
 
     /*
      * Take the old AioContex when detaching it from bs.
-- 
2.31.1

[PATCH 06/20] block: assert that graph read and writes are performed correctly

[Emanuele Giuseppe Esposito]

Remove the old assert_bdrv_graph_writable, and replace it with
the new version using graph-lock API.
See the function documentation for more information.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c                                |  4 ++--
 block/graph-lock.c                     | 11 +++++++++++
 include/block/block_int-global-state.h | 17 -----------------
 include/block/graph-lock.h             | 15 +++++++++++++++
 4 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/block.c b/block.c
index afab74d4da..1c870d85e6 100644
--- a/block.c
+++ b/block.c
@@ -1414,7 +1414,7 @@ static void bdrv_child_cb_attach(BdrvChild *child)
 {
     BlockDriverState *bs = child->opaque;
 
-    assert_bdrv_graph_writable(bs);
+    assert_bdrv_graph_writable();
     QLIST_INSERT_HEAD(&bs->children, child, next);
 
     if (bs->drv->is_filter || (child->role & BDRV_CHILD_FILTERED)) {
@@ -1461,7 +1461,7 @@ static void bdrv_child_cb_detach(BdrvChild *child)
         bdrv_backing_detach(child);
     }
 
-    assert_bdrv_graph_writable(bs);
+    assert_bdrv_graph_writable();
     QLIST_REMOVE(child, next);
     if (child == bs->backing) {
         assert(child != bs->file);
diff --git a/block/graph-lock.c b/block/graph-lock.c
index c3c6eeedad..07476fd7c8 100644
--- a/block/graph-lock.c
+++ b/block/graph-lock.c
@@ -242,3 +242,14 @@ void coroutine_fn bdrv_graph_co_rdunlock(void)
         aio_wait_kick();
     }
 }
+
+void assert_bdrv_graph_readable(void)
+{
+    assert(qemu_in_main_thread() || reader_count());
+}
+
+void assert_bdrv_graph_writable(void)
+{
+    assert(qemu_in_main_thread());
+    assert(qatomic_read(&has_writer));
+}
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index b49f4eb35b..2f0993f6e9 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -310,21 +310,4 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
  */
 void bdrv_drain_all_end_quiesce(BlockDriverState *bs);
 
-/**
- * Make sure that the function is running under both drain and BQL.
- * The latter protects from concurrent writings
- * from the GS API, while the former prevents concurrent reads
- * from I/O.
- */
-static inline void assert_bdrv_graph_writable(BlockDriverState *bs)
-{
-    /*
-     * TODO: this function is incomplete. Because the users of this
-     * assert lack the necessary drains, check only for BQL.
-     * Once the necessary drains are added,
-     * assert also for qatomic_read(&bs->quiesce_counter) > 0
-     */
-    assert(qemu_in_main_thread());
-}
-
 #endif /* BLOCK_INT_GLOBAL_STATE_H */
diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
index fc806aefa3..9430707dca 100644
--- a/include/block/graph-lock.h
+++ b/include/block/graph-lock.h
@@ -126,5 +126,20 @@ void coroutine_fn bdrv_graph_co_rdlock(void);
  */
 void coroutine_fn bdrv_graph_co_rdunlock(void);
 
+/*
+ * assert_bdrv_graph_readable:
+ * Make sure that the reader is either the main loop,
+ * or there is at least a reader helding the rdlock.
+ * In this way an incoming writer is aware of the read and waits.
+ */
+void assert_bdrv_graph_readable(void);
+
+/*
+ * assert_bdrv_graph_writable:
+ * Make sure that the writer is the main loop and has set @has_writer,
+ * so that incoming readers will pause.
+ */
+void assert_bdrv_graph_writable(void);
+
 #endif /* GRAPH_LOCK_H */
 
-- 
2.31.1

[PATCH 07/20] graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD macros

[Emanuele Giuseppe Esposito]

Similar to the implementation in lockable.h, implement macros to
automatically take and release the rdlock.
Create the empty GraphLockable struct only to use it as a type for
G_DEFINE_AUTOPTR_CLEANUP_FUNC.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/block/graph-lock.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
index 9430707dca..0d886a9ca3 100644
--- a/include/block/graph-lock.h
+++ b/include/block/graph-lock.h
@@ -141,5 +141,40 @@ void assert_bdrv_graph_readable(void);
  */
 void assert_bdrv_graph_writable(void);
 
+typedef struct GraphLockable { } GraphLockable;
+
+/*
+ * In C, compound literals have the lifetime of an automatic variable.
+ * In C++ it would be different, but then C++ wouldn't need QemuLockable
+ * either...
+ */
+#define GML_OBJ_() (&(GraphLockable) { })
+
+static inline GraphLockable *graph_lockable_auto_lock(GraphLockable *x)
+{
+    bdrv_graph_co_rdlock();
+    return x;
+}
+
+static inline void graph_lockable_auto_unlock(GraphLockable *x)
+{
+    bdrv_graph_co_rdunlock();
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(GraphLockable, graph_lockable_auto_unlock)
+
+#define WITH_GRAPH_RDLOCK_GUARD_(var)                                         \
+    for (g_autoptr(GraphLockable) var = graph_lockable_auto_lock(GML_OBJ_()); \
+         var;                                                                 \
+         graph_lockable_auto_unlock(var), var = NULL)
+
+#define WITH_GRAPH_RDLOCK_GUARD() \
+    WITH_GRAPH_RDLOCK_GUARD_(glue(graph_lockable_auto, __COUNTER__))
+
+#define GRAPH_RDLOCK_GUARD(x)                                       \
+    g_autoptr(GraphLockable)                                        \
+    glue(graph_lockable_auto, __COUNTER__) G_GNUC_UNUSED =          \
+            graph_lockable_auto_lock(GML_OBJ_())
+
 #endif /* GRAPH_LOCK_H */
 
-- 
2.31.1

[PATCH 08/20] block-coroutine-wrapper.py: take the graph rdlock in bdrv_* functions

[Emanuele Giuseppe Esposito]

All generated_co_wrapper functions create a coroutine when
called from non-coroutine context.

The format can be one of the two:

bdrv_something()
    if(qemu_in_coroutine()):
        bdrv_co_something();
    else:
        // create coroutine that calls bdrv_co_something();

blk_something()
    if(qemu_in_coroutine()):
        blk_co_something();
    else:
        // create coroutine that calls blk_co_something();
	// blk_co_something() then eventually calls bdrv_co_something()

The bdrv_co_something functions are recursively traversing the graph,
therefore they all need to be protected with the graph rdlock.
Instead, blk_co_something() calls bdrv_co_something(), so given that and
being always called at the root of the graph (not in recursive
callbacks), they should take the graph rdlock.

The contract is simple, from now on, all bdrv_co_* functions called by g_c_w
callbacks assume that the graph rdlock is taken at the coroutine
creation, i.e. in g_c_w or in specific coroutines (right now we just
consider the g_c_w case).

All the blk_co_* are responsible of taking the rdlock (at this point is still a TBD).

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 scripts/block-coroutine-wrapper.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 21ecb3e896..05267761f0 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -67,8 +67,11 @@ def __init__(self, return_type: str, name: str, args: str,
         self.return_type = return_type.strip()
         self.name = name.strip()
         self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
+        self.lock = True
         self.create_only_co = False
 
+        if variant == '_blk':
+            self.lock = False
         if variant == '_simple':
             self.create_only_co = True
 
@@ -86,7 +89,6 @@ def gen_block(self, format: str) -> str:
                           r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
                           r'\((?P<args>[^)]*)\);$', re.MULTILINE)
 
-
 def func_decl_iter(text: str) -> Iterator:
     for m in func_decl_re.finditer(text):
         yield FuncDecl(return_type=m.group('return_type'),
@@ -160,6 +162,13 @@ def gen_wrapper(func: FuncDecl) -> str:
     func.co_name = f'{subsystem}_co_{subname}'
     name = func.co_name
 
+    graph_lock=''
+    graph_unlock=''
+    if func.lock:
+        graph_lock='    bdrv_graph_co_rdlock();'
+        graph_unlock='    bdrv_graph_co_rdunlock();'
+
+
     t = func.args[0].type
     if t == 'BlockDriverState *':
         bs = 'bs'
@@ -192,7 +201,9 @@ def gen_wrapper(func: FuncDecl) -> str:
 {{
     {struct_name} *s = opaque;
 
+{graph_lock}
     s->ret = {name}({ func.gen_list('s->{name}') });
+{graph_unlock}
     s->poll_state.in_progress = false;
 
     aio_wait_kick();
-- 
2.31.1

[PATCH 09/20] block-backend: introduce new generated_co_wrapper_blk annotation

[Emanuele Giuseppe Esposito]

This annotation will be used to distinguish the blk_* API from the
bdrv_* API in block-gen.c. The reason for this distinction is that
blk_* API eventually result in always calling bdrv_*, which has
implications when we introduce the read graph lock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/block/block-common.h      |  1 +
 include/sysemu/block-backend-io.h | 69 ++++++++++++++++---------------
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/include/block/block-common.h b/include/block/block-common.h
index 683e3d1c51..f70f1560c5 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -42,6 +42,7 @@
  */
 #define generated_co_wrapper
 #define generated_co_wrapper_simple
+#define generated_co_wrapper_blk
 
 #include "block/dirty-bitmap.h"
 #include "block/blockjob.h"
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index a47cb825e5..887a29dc59 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -110,77 +110,78 @@ int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
  * the "I/O or GS" API.
  */
 
-int generated_co_wrapper blk_pread(BlockBackend *blk, int64_t offset,
-                                   int64_t bytes, void *buf,
-                                   BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_pread(BlockBackend *blk, int64_t offset,
+                                       int64_t bytes, void *buf,
+                                       BdrvRequestFlags flags);
 int coroutine_fn blk_co_pread(BlockBackend *blk, int64_t offset, int64_t bytes,
                               void *buf, BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_preadv(BlockBackend *blk, int64_t offset,
-                                    int64_t bytes, QEMUIOVector *qiov,
-                                    BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_preadv(BlockBackend *blk, int64_t offset,
+                                        int64_t bytes, QEMUIOVector *qiov,
+                                        BdrvRequestFlags flags);
 int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
                                int64_t bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_preadv_part(BlockBackend *blk, int64_t offset,
-                                         int64_t bytes, QEMUIOVector *qiov,
-                                         size_t qiov_offset,
-                                         BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_preadv_part(BlockBackend *blk, int64_t offset,
+                                             int64_t bytes, QEMUIOVector *qiov,
+                                             size_t qiov_offset,
+                                             BdrvRequestFlags flags);
 int coroutine_fn blk_co_preadv_part(BlockBackend *blk, int64_t offset,
                                     int64_t bytes, QEMUIOVector *qiov,
                                     size_t qiov_offset, BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_pwrite(BlockBackend *blk, int64_t offset,
-                                    int64_t bytes, const void *buf,
-                                    BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_pwrite(BlockBackend *blk, int64_t offset,
+                                        int64_t bytes, const void *buf,
+                                        BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset, int64_t bytes,
                                const void *buf, BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_pwritev(BlockBackend *blk, int64_t offset,
-                                     int64_t bytes, QEMUIOVector *qiov,
-                                     BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_pwritev(BlockBackend *blk, int64_t offset,
+                                         int64_t bytes, QEMUIOVector *qiov,
+                                         BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
                                 int64_t bytes, QEMUIOVector *qiov,
                                 BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_pwritev_part(BlockBackend *blk, int64_t offset,
-                                          int64_t bytes, QEMUIOVector *qiov,
-                                          size_t qiov_offset,
-                                          BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_pwritev_part(BlockBackend *blk, int64_t offset,
+                                              int64_t bytes, QEMUIOVector *qiov,
+                                              size_t qiov_offset,
+                                              BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
                                      int64_t bytes,
                                      QEMUIOVector *qiov, size_t qiov_offset,
                                      BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_pwrite_compressed(BlockBackend *blk,
-                                               int64_t offset, int64_t bytes,
-                                               const void *buf);
+int generated_co_wrapper_blk blk_pwrite_compressed(BlockBackend *blk,
+                                                int64_t offset, int64_t bytes,
+                                                const void *buf);
 int coroutine_fn blk_co_pwrite_compressed(BlockBackend *blk, int64_t offset,
                                           int64_t bytes, const void *buf);
 
-int generated_co_wrapper blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                                           int64_t bytes,
-                                           BdrvRequestFlags flags);
+int generated_co_wrapper_blk blk_pwrite_zeroes(BlockBackend *blk,
+                                               int64_t offset,
+                                               int64_t bytes,
+                                               BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                       int64_t bytes, BdrvRequestFlags flags);
 
-int generated_co_wrapper blk_pdiscard(BlockBackend *blk, int64_t offset,
-                                      int64_t bytes);
+int generated_co_wrapper_blk blk_pdiscard(BlockBackend *blk, int64_t offset,
+                                          int64_t bytes);
 int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
                                  int64_t bytes);
 
-int generated_co_wrapper blk_flush(BlockBackend *blk);
+int generated_co_wrapper_blk blk_flush(BlockBackend *blk);
 int coroutine_fn blk_co_flush(BlockBackend *blk);
 
-int generated_co_wrapper blk_ioctl(BlockBackend *blk, unsigned long int req,
-                                   void *buf);
+int generated_co_wrapper_blk blk_ioctl(BlockBackend *blk, unsigned long int req,
+                                       void *buf);
 int coroutine_fn blk_co_ioctl(BlockBackend *blk, unsigned long int req,
                               void *buf);
 
-int generated_co_wrapper blk_truncate(BlockBackend *blk, int64_t offset,
-                                      bool exact, PreallocMode prealloc,
-                                      BdrvRequestFlags flags, Error **errp);
+int generated_co_wrapper_blk blk_truncate(BlockBackend *blk, int64_t offset,
+                                          bool exact, PreallocMode prealloc,
+                                          BdrvRequestFlags flags, Error **errp);
 int coroutine_fn blk_co_truncate(BlockBackend *blk, int64_t offset, bool exact,
                                  PreallocMode prealloc, BdrvRequestFlags flags,
                                  Error **errp);
-- 
2.31.1

[PATCH 10/20] block-gen: assert that {bdrv/blk}_co_truncate is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called by the blk_* API.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

Protecting bdrv_co_truncate() implies that
BlockDriver->bdrv_co_truncate() is always called with
graph rdlock taken.

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

diff --git a/block/block-backend.c b/block/block-backend.c
index 333d50fb3f..0686cd6942 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2370,6 +2370,7 @@ int coroutine_fn blk_co_truncate(BlockBackend *blk, int64_t offset, bool exact,
                                  Error **errp)
 {
     IO_OR_GS_CODE();
+    GRAPH_RDLOCK_GUARD();
     if (!blk_is_available(blk)) {
         error_setg(errp, "No medium inserted");
         return -ENOMEDIUM;
diff --git a/block/io.c b/block/io.c
index 9bcb19e5ee..ac12725fb2 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3295,6 +3295,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
     int64_t old_size, new_bytes;
     int ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     /* if bs->drv == NULL, bs is closed, so there's nothing to do here */
     if (!drv) {
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index fd9f40a815..d666b0c441 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -681,6 +681,8 @@ struct BlockDriver {
      *
      * If @exact is true and this function fails but would succeed
      * with @exact = false, it should return -ENOTSUP.
+     *
+     * Called with graph rdlock held.
      */
     int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
                                          bool exact, PreallocMode prealloc,
-- 
2.31.1

[PATCH 11/20] block-gen: assert that bdrv_co_{check/invalidate_cache} are always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

The only callers of these functions are the respective
generated_co_wrapper, and they already take the lock.

Protecting bdrv_co_{check/invalidate_cache}() implies that
BlockDriver->bdrv_co_{check/invalidate_cache}() is always called with
graph rdlock taken.

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

diff --git a/block.c b/block.c
index 1c870d85e6..c7611bed9e 100644
--- a/block.c
+++ b/block.c
@@ -5375,6 +5375,7 @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs,
                                BdrvCheckResult *res, BdrvCheckMode fix)
 {
     IO_CODE();
+    assert_bdrv_graph_readable();
     if (bs->drv == NULL) {
         return -ENOMEDIUM;
     }
@@ -6590,6 +6591,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
     IO_CODE();
 
     assert(!(bs->open_flags & BDRV_O_INACTIVE));
+    assert_bdrv_graph_readable();
 
     if (bs->drv->bdrv_co_invalidate_cache) {
         bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index d666b0c441..f285a6b8f7 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -641,6 +641,7 @@ struct BlockDriver {
 
     /*
      * Invalidate any cached meta-data.
+     * Called with graph rdlock held.
      */
     void coroutine_fn (*bdrv_co_invalidate_cache)(BlockDriverState *bs,
                                                   Error **errp);
@@ -726,6 +727,7 @@ struct BlockDriver {
     /*
      * Returns 0 for completed check, -errno for internal errors.
      * The check results are stored in result.
+     * Called with graph rdlock held.
      */
     int coroutine_fn (*bdrv_co_check)(BlockDriverState *bs,
                                       BdrvCheckResult *result,
-- 
2.31.1

[PATCH 12/20] block-gen: assert that bdrv_co_pwrite is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called elsewhere else.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

By protecting brdv_co_pwrite, we also automatically protect
the following other generated_co_wrappers:
blk_co_pwrite
blk_co_pwritev
blk_co_pwritev_part
blk_co_pwrite_compressed
blk_co_pwrite_zeroes

Protecting bdrv_driver_pwritev_compressed() and bdrv_driver_pwritev_compressed()
implies that the following BlockDriver callbacks always called with graph rdlock
taken:
- bdrv_aio_pwritev
- bdrv_co_writev
- bdrv_co_pwritev
- bdrv_co_pwritev_part
- bdrv_co_pwritev_compressed
- bdrv_co_pwritev_compressed_part

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

diff --git a/block/block-backend.c b/block/block-backend.c
index 0686cd6942..d48ec3a2ac 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1363,6 +1363,7 @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
     IO_CODE();
 
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     /* Call blk_bs() only after waiting, the graph may have changed */
     bs = blk_bs(blk);
diff --git a/block/block-copy.c b/block/block-copy.c
index f33ab1d0b6..dabf461112 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -464,6 +464,8 @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool,
  * a full-size buffer or disabled if the copy_range attempt fails.  The output
  * value of @method should be used for subsequent tasks.
  * Returns 0 on success.
+ *
+ * Called with graph rdlock taken.
  */
 static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
                                            int64_t offset, int64_t bytes,
@@ -554,8 +556,10 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
     BlockCopyMethod method = t->method;
     int ret;
 
-    ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method,
-                             &error_is_read);
+    WITH_GRAPH_RDLOCK_GUARD() {
+        ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method,
+                                 &error_is_read);
+    }
 
     WITH_QEMU_LOCK_GUARD(&s->lock) {
         if (s->method == t->method) {
diff --git a/block/io.c b/block/io.c
index ac12725fb2..9280fb9f38 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1012,6 +1012,7 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
     unsigned int nb_sectors;
     QEMUIOVector local_qiov;
     int ret;
+    assert_bdrv_graph_readable();
 
     bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort);
 
@@ -1090,6 +1091,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
     BlockDriver *drv = bs->drv;
     QEMUIOVector local_qiov;
     int ret;
+    assert_bdrv_graph_readable();
 
     bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort);
 
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index f285a6b8f7..d44f825d95 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -479,6 +479,7 @@ struct BlockDriver {
     BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
+    /* Called with graph rdlock taken. */
     BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
@@ -515,6 +516,7 @@ struct BlockDriver {
         QEMUIOVector *qiov, size_t qiov_offset,
         BdrvRequestFlags flags);
 
+    /* Called with graph rdlock taken. */
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
         int flags);
@@ -532,10 +534,12 @@ struct BlockDriver {
      * no larger than 'max_transfer'.
      *
      * The buffer in @qiov may point directly to guest memory.
+     * Called with graph rdlock taken.
      */
     int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags);
+    /* Called with graph rdlock taken. */
     int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
         BdrvRequestFlags flags);
@@ -693,8 +697,10 @@ struct BlockDriver {
     BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
                                       Error **errp);
 
+    /* Called with graph rdlock held. */
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov);
+    /* Called with graph rdlock held. */
     int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         size_t qiov_offset);
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 8bc061ebb8..ae88507d6a 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -69,6 +69,7 @@ static inline int coroutine_fn bdrv_co_pwrite(BdrvChild *child,
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     return bdrv_co_pwritev(child, offset, bytes, &qiov, flags);
 }
-- 
2.31.1

[PATCH 13/20] block-gen: assert that bdrv_co_pwrite_{zeros/sync} is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

Already protected by bdrv_co_pwrite callers.

Protecting bdrv_co_do_pwrite_zeroes() implies that
BlockDriver->bdrv_co_pwrite_zeroes() is always called with
graph rdlock taken.

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

diff --git a/block/io.c b/block/io.c
index 9280fb9f38..92c74648fb 100644
--- a/block/io.c
+++ b/block/io.c
@@ -904,6 +904,7 @@ int coroutine_fn bdrv_co_pwrite_sync(BdrvChild *child, int64_t offset,
 {
     int ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     ret = bdrv_co_pwrite(child, offset, bytes, buf, flags);
     if (ret < 0) {
@@ -1660,6 +1661,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
                         bs->bl.request_alignment);
     int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
 
+    assert_bdrv_graph_readable();
     bdrv_check_request(offset, bytes, &error_abort);
 
     if (!drv) {
@@ -2124,6 +2126,7 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
 {
     IO_CODE();
     trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
+    assert_bdrv_graph_readable();
 
     if (!(child->bs->open_flags & BDRV_O_UNMAP)) {
         flags &= ~BDRV_REQ_MAY_UNMAP;
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index d44f825d95..e8d2e4b6c7 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -549,6 +549,8 @@ struct BlockDriver {
      * would use a compact metadata representation to implement this.  This
      * function pointer may be NULL or return -ENOSUP and .bdrv_co_writev()
      * will be called instead.
+     *
+     * Called with graph rdlock taken.
      */
     int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, BdrvRequestFlags flags);
-- 
2.31.1

[PATCH 14/20] block-gen: assert that bdrv_co_pread is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called elsewhere else.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

By protecting brdv_co_pread, we also automatically protect
the following other generated_co_wrappers:
blk_co_pread
blk_co_preadv
blk_co_preadv_part

Protecting bdrv_driver_preadv() implies that the following BlockDriver
callbacks always called with graph rdlock taken:
- bdrv_co_preadv_part
- bdrv_co_preadv
- bdrv_aio_preadv
- bdrv_co_readv

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c            | 1 +
 block/io.c                       | 1 +
 block/mirror.c                   | 6 ++++--
 include/block/block_int-common.h | 5 +++++
 include/block/block_int-io.h     | 1 +
 tests/unit/test-bdrv-drain.c     | 2 ++
 6 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index d48ec3a2ac..083ed6009e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1289,6 +1289,7 @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes,
     IO_CODE();
 
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     /* Call blk_bs() only after waiting, the graph may have changed */
     bs = blk_bs(blk);
diff --git a/block/io.c b/block/io.c
index 92c74648fb..cfc201ef91 100644
--- a/block/io.c
+++ b/block/io.c
@@ -942,6 +942,7 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
     unsigned int nb_sectors;
     QEMUIOVector local_qiov;
     int ret;
+    assert_bdrv_graph_readable();
 
     bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort);
     assert(!(flags & ~bs->supported_read_flags));
diff --git a/block/mirror.c b/block/mirror.c
index 251adc5ae0..f509cc1cb1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -389,8 +389,10 @@ static void coroutine_fn mirror_co_read(void *opaque)
     op->is_in_flight = true;
     trace_mirror_one_iteration(s, op->offset, op->bytes);
 
-    ret = bdrv_co_preadv(s->mirror_top_bs->backing, op->offset, op->bytes,
-                         &op->qiov, 0);
+    WITH_GRAPH_RDLOCK_GUARD() {
+        ret = bdrv_co_preadv(s->mirror_top_bs->backing, op->offset, op->bytes,
+                             &op->qiov, 0);
+    }
     mirror_read_complete(op, ret);
 }
 
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index e8d2e4b6c7..64c5bb64de 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -476,6 +476,7 @@ struct BlockDriver {
                                       Error **errp);
 
     /* aio */
+    /* Called with graph rdlock held. */
     BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
@@ -489,6 +490,7 @@ struct BlockDriver {
         int64_t offset, int bytes,
         BlockCompletionFunc *cb, void *opaque);
 
+    /* Called with graph rdlock held. */
     int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
 
@@ -506,11 +508,14 @@ struct BlockDriver {
      * no larger than 'max_transfer'.
      *
      * The buffer in @qiov may point directly to guest memory.
+     *
+     * Called with graph rdlock held.
      */
     int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags);
 
+    /* Called with graph rdlock held. */
     int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
         int64_t offset, int64_t bytes,
         QEMUIOVector *qiov, size_t qiov_offset,
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index ae88507d6a..ac6ad3b3ff 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -60,6 +60,7 @@ static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     return bdrv_co_preadv(child, offset, bytes, &qiov, flags);
 }
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 2686a8acee..90edc2f5bf 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -967,6 +967,8 @@ static void coroutine_fn test_co_delete_by_drain(void *opaque)
     void *buffer = g_malloc(65536);
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buffer, 65536);
 
+    GRAPH_RDLOCK_GUARD();
+
     /* Pretend some internal write operation from parent to child.
      * Important: We have to read from the child, not from the parent!
      * Draining works by first propagating it all up the tree to the
-- 
2.31.1

[PATCH 15/20] block-gen: assert that {bdrv/blk}_co_flush is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called by the blk_* API.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

Protecting bdrv_co_flush() implies that the following BlockDriver
callbacks always called with graph rdlock taken:
- bdrv_co_flush
- bdrv_co_flush_to_os
- bdrv_co_flush_to_disk

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/block-backend.c            | 3 ++-
 block/io.c                       | 1 +
 include/block/block_int-common.h | 6 ++++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 083ed6009e..d660772375 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1759,8 +1759,9 @@ int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn blk_co_do_flush(BlockBackend *blk)
 {
-    blk_wait_while_drained(blk);
     IO_CODE();
+    blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     if (!blk_is_available(blk)) {
         return -ENOMEDIUM;
diff --git a/block/io.c b/block/io.c
index cfc201ef91..0bf3919939 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2757,6 +2757,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
     int ret = 0;
     IO_CODE();
 
+    assert_bdrv_graph_readable();
     bdrv_inc_in_flight(bs);
 
     if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) ||
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 64c5bb64de..bab0521943 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -661,6 +661,8 @@ struct BlockDriver {
      * Flushes all data for all layers by calling bdrv_co_flush for underlying
      * layers, if needed. This function is needed for deterministic
      * synchronization of the flush finishing callback.
+     *
+     * Called with graph rdlock taken.
      */
     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
 
@@ -671,6 +673,8 @@ struct BlockDriver {
     /*
      * Flushes all data that was already written to the OS all the way down to
      * the disk (for example file-posix.c calls fsync()).
+     *
+     * Called with graph rdlock taken.
      */
     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
 
@@ -678,6 +682,8 @@ struct BlockDriver {
      * Flushes all internal caches to the OS. The data may still sit in a
      * writeback cache of the host OS, but it will survive a crash of the qemu
      * process.
+     *
+     * Called with graph rdlock held.
      */
     int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
 
-- 
2.31.1

[PATCH 16/20] block-gen: assert that bdrv_co_{read/write}v_vmstate are always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

The only caller of these functions is bdrv_{read/write}v_vmstate, a
generated_co_wrapper function that already takes the
graph read lock.

Protecting bdrv_co_{read/write}v_vmstate() implies that
BlockDriver->bdrv_{load/save}_vmstate() is always called with
graph rdlock taken.

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

diff --git a/block/io.c b/block/io.c
index 0bf3919939..c9b451fecd 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2633,6 +2633,7 @@ bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
     BlockDriverState *child_bs = bdrv_primary_bs(bs);
     int ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL);
     if (ret < 0) {
@@ -2665,6 +2666,7 @@ bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
     BlockDriverState *child_bs = bdrv_primary_bs(bs);
     int ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL);
     if (ret < 0) {
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index bab0521943..568c2d3092 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -724,9 +724,11 @@ struct BlockDriver {
                                                  Error **errp);
     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);
+    /* Called with graph rdlock held. */
     int coroutine_fn (*bdrv_load_vmstate)(BlockDriverState *bs,
                                           QEMUIOVector *qiov,
                                           int64_t pos);
-- 
2.31.1

[PATCH 17/20] block-gen: assert that bdrv_co_pdiscard is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called by the blk_* API.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

Protecting bdrv_co_pdiscard{_snapshot}() implies that the following BlockDriver
callbacks always called with graph rdlock taken:
- bdrv_co_pdiscard
- bdrv_aio_pdiscard
- bdrv_co_pdiscard_snapshot

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

diff --git a/block/block-backend.c b/block/block-backend.c
index d660772375..211a813523 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1716,6 +1716,7 @@ blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
     IO_CODE();
 
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     ret = blk_check_byte_request(blk, offset, bytes);
     if (ret < 0) {
diff --git a/block/io.c b/block/io.c
index c9b451fecd..bc9f47538c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2885,6 +2885,7 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
     int head, tail, align;
     BlockDriverState *bs = child->bs;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!bs || !bs->drv || !bdrv_is_inserted(bs)) {
         return -ENOMEDIUM;
@@ -3488,6 +3489,7 @@ bdrv_co_pdiscard_snapshot(BlockDriverState *bs, int64_t offset, int64_t bytes)
     BlockDriver *drv = bs->drv;
     int ret;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     if (!drv) {
         return -ENOMEDIUM;
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 568c2d3092..7c34a8e40f 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -486,6 +486,7 @@ struct BlockDriver {
         BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque);
+    /* Called with graph rdlock taken. */
     BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
         int64_t offset, int bytes,
         BlockCompletionFunc *cb, void *opaque);
@@ -559,6 +560,7 @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, BdrvRequestFlags flags);
+    /* Called with graph rdlock taken. */
     int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
         int64_t offset, int64_t bytes);
 
@@ -647,6 +649,7 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_snapshot_block_status)(BlockDriverState *bs,
         bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum,
         int64_t *map, BlockDriverState **file);
+    /* Called with graph rdlock taken. */
     int coroutine_fn (*bdrv_co_pdiscard_snapshot)(BlockDriverState *bs,
         int64_t offset, int64_t bytes);
 
-- 
2.31.1

[PATCH 18/20] block-gen: assert that bdrv_co_common_block_status_above is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

This function, in addition to be called by a generated_co_wrapper,
is also called elsewhere else.
The strategy is to always take the lock at the function called
when the coroutine is created, to avoid recursive locking.

Protecting bdrv_co_block_status() called by
bdrv_co_common_block_status_above() implies that
BlockDriver->bdrv_co_block_status() is always called with
graph rdlock taken.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block/backup.c                   |  3 +++
 block/block-backend.c            |  2 ++
 block/block-copy.c               |  2 ++
 block/io.c                       |  2 ++
 block/mirror.c                   | 14 +++++++++-----
 block/stream.c                   | 32 ++++++++++++++++++--------------
 include/block/block_int-common.h |  2 ++
 qemu-img.c                       |  4 +++-
 8 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 6a9ad97a53..42b16d0136 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -269,7 +269,10 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
                 return -ECANCELED;
             }
 
+            /* rdlock protects the subsequent call to bdrv_is_allocated() */
+            bdrv_graph_co_rdlock();
             ret = block_copy_reset_unallocated(s->bcs, offset, &count);
+            bdrv_graph_co_rdunlock();
             if (ret < 0) {
                 return ret;
             }
diff --git a/block/block-backend.c b/block/block-backend.c
index 211a813523..20b772a476 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1433,6 +1433,7 @@ int coroutine_fn blk_block_status_above(BlockBackend *blk,
                                         BlockDriverState **file)
 {
     IO_CODE();
+    GRAPH_RDLOCK_GUARD();
     return bdrv_block_status_above(blk_bs(blk), base, offset, bytes, pnum, map,
                                    file);
 }
@@ -1443,6 +1444,7 @@ int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
                                         int64_t bytes, int64_t *pnum)
 {
     IO_CODE();
+    GRAPH_RDLOCK_GUARD();
     return bdrv_is_allocated_above(blk_bs(blk), base, include_base, offset,
                                    bytes, pnum);
 }
diff --git a/block/block-copy.c b/block/block-copy.c
index dabf461112..e20d2b2f78 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -630,6 +630,7 @@ static int coroutine_fn block_copy_is_cluster_allocated(BlockCopyState *s,
     assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
 
     while (true) {
+        /* protected in backup_run() */
         ret = bdrv_is_allocated(bs, offset, bytes, &count);
         if (ret < 0) {
             return ret;
@@ -892,6 +893,7 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state)
 
 static void coroutine_fn block_copy_async_co_entry(void *opaque)
 {
+    GRAPH_RDLOCK_GUARD();
     block_copy_common(opaque);
 }
 
diff --git a/block/io.c b/block/io.c
index bc9f47538c..c5b3bb0a6d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2215,6 +2215,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
     bool has_filtered_child;
 
     assert(pnum);
+    assert_bdrv_graph_readable();
     *pnum = 0;
     total_size = bdrv_getlength(bs);
     if (total_size < 0) {
@@ -2445,6 +2446,7 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
     IO_CODE();
 
     assert(!include_base || base); /* Can't include NULL base */
+    assert_bdrv_graph_readable();
 
     if (!depth) {
         depth = &dummy;
diff --git a/block/mirror.c b/block/mirror.c
index f509cc1cb1..02ee7bba08 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -559,9 +559,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         MirrorMethod mirror_method = MIRROR_METHOD_COPY;
 
         assert(!(offset % s->granularity));
-        ret = bdrv_block_status_above(source, NULL, offset,
-                                      nb_chunks * s->granularity,
-                                      &io_bytes, NULL, NULL);
+        WITH_GRAPH_RDLOCK_GUARD() {
+            ret = bdrv_block_status_above(source, NULL, offset,
+                                        nb_chunks * s->granularity,
+                                        &io_bytes, NULL, NULL);
+        }
         if (ret < 0) {
             io_bytes = MIN(nb_chunks * s->granularity, max_io_bytes);
         } else if (ret & BDRV_BLOCK_DATA) {
@@ -864,8 +866,10 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
             return 0;
         }
 
-        ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset, bytes,
-                                      &count);
+        WITH_GRAPH_RDLOCK_GUARD() {
+            ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset,
+                                          bytes, &count);
+        }
         if (ret < 0) {
             return ret;
         }
diff --git a/block/stream.c b/block/stream.c
index 8744ad103f..22368ce186 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -161,21 +161,25 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
 
         copy = false;
 
-        ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n);
-        if (ret == 1) {
-            /* Allocated in the top, no need to copy.  */
-        } else if (ret >= 0) {
-            /* Copy if allocated in the intermediate images.  Limit to the
-             * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE).  */
-            ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
-                                          s->base_overlay, true,
-                                          offset, n, &n);
-            /* Finish early if end of backing file has been reached */
-            if (ret == 0 && n == 0) {
-                n = len - offset;
+        WITH_GRAPH_RDLOCK_GUARD() {
+            ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n);
+            if (ret == 1) {
+                /* Allocated in the top, no need to copy.  */
+            } else if (ret >= 0) {
+                /*
+                 * Copy if allocated in the intermediate images.  Limit to the
+                 * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE).
+                 */
+                ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
+                                            s->base_overlay, true,
+                                            offset, n, &n);
+                /* Finish early if end of backing file has been reached */
+                if (ret == 0 && n == 0) {
+                    n = len - offset;
+                }
+
+                copy = (ret > 0);
             }
-
-            copy = (ret > 0);
         }
         trace_stream_one_iteration(s, offset, n, ret);
         if (copy) {
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 7c34a8e40f..9d9cd59f1e 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -623,6 +623,8 @@ struct BlockDriver {
      * block/io.c's bdrv_co_block_status() will utilize an unclamped
      * *pnum value for the block-status cache on protocol nodes, prior
      * to clamping *pnum for return to its caller.
+     *
+     * Called with graph rdlock taken.
      */
     int coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bs,
         bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum,
diff --git a/qemu-img.c b/qemu-img.c
index 4282a34bc0..33703a6d92 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1977,7 +1977,9 @@ static void coroutine_fn convert_co_do_copy(void *opaque)
             qemu_co_mutex_unlock(&s->lock);
             break;
         }
-        n = convert_iteration_sectors(s, s->sector_num);
+        WITH_GRAPH_RDLOCK_GUARD() {
+            n = convert_iteration_sectors(s, s->sector_num);
+        }
         if (n < 0) {
             qemu_co_mutex_unlock(&s->lock);
             s->ret = n;
-- 
2.31.1

[PATCH 19/20] block-gen: assert that bdrv_co_ioctl is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

The only caller of this function is blk_ioctl, a generated_co_wrapper
functions that needs to take the graph read lock.

Protecting bdrv_co_ioctl() implies that
BlockDriver->bdrv_co_ioctl() is always called with
graph rdlock taken, and BlockDriver->bdrv_aio_ioctl is
a coroutine_fn callback (called too with rdlock taken).

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

diff --git a/block/block-backend.c b/block/block-backend.c
index 20b772a476..9e1c689e84 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1672,6 +1672,7 @@ blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
     IO_CODE();
 
     blk_wait_while_drained(blk);
+    GRAPH_RDLOCK_GUARD();
 
     if (!blk_is_available(blk)) {
         return -ENOMEDIUM;
diff --git a/block/io.c b/block/io.c
index c5b3bb0a6d..831f277e85 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3007,6 +3007,7 @@ int coroutine_fn bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
     };
     BlockAIOCB *acb;
     IO_CODE();
+    assert_bdrv_graph_readable();
 
     bdrv_inc_in_flight(bs);
     if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 9d9cd59f1e..db97d61836 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -743,10 +743,11 @@ struct BlockDriver {
     void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
-    /* to control generic scsi devices */
-    BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
+    /* to control generic scsi devices. Called with graph rdlock taken. */
+    BlockAIOCB *coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
         unsigned long int req, void *buf,
         BlockCompletionFunc *cb, void *opaque);
+    /* Called with graph rdlock taken. */
     int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
                                       unsigned long int req, void *buf);
 
-- 
2.31.1

[PATCH 20/20] block-gen: assert that nbd_co_do_establish_connection is always called with graph rdlock taken

[Emanuele Giuseppe Esposito]

The only caller of this function is nbd_do_establish_connection, a
generated_co_wrapper that already take the graph read lock.

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

diff --git a/block/nbd.c b/block/nbd.c
index 7d485c86d2..5cad58aaf6 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -322,6 +322,7 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
     int ret;
     IO_CODE();
 
+    assert_bdrv_graph_readable();
     assert(!s->ioc);
 
     s->ioc = nbd_co_establish_connection(s->conn, &s->info, blocking, errp);
-- 
2.31.1

Re: [PATCH 00/20] Protect the block layer with a rwlock: part 1

[Emanuele Giuseppe Esposito]

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 14:48 schrieb Emanuele Giuseppe Esposito:
> This serie is the first of four series that aim to introduce and use a new
> graph rwlock in the QEMU block layer.
> The aim is to replace the current AioContext lock with much fine-grained locks,
> aimed to protect only specific data.
> Currently the AioContext lock is used pretty much everywhere, and it's not
> even clear what it is protecting exactly.
> 
> The aim of the rwlock is to cover graph modifications: more precisely,
> when a BlockDriverState parent or child list is modified or read, since it can
> be concurrently accessed by the main loop and iothreads.
> 
> The main assumption is that the main loop is the only one allowed to perform
> graph modifications, and so far this has always been held by the current code.
> 
> The rwlock is inspired from cpus-common.c implementation, and aims to
> reduce cacheline bouncing by having per-aiocontext counter of readers.
> All details and implementation of the lock are in patch 1.
> 
> We distinguish between writer (main loop, under BQL) that modifies the
> graph, and readers (all other coroutines running in various AioContext),
> that go through the graph edges, reading ->parents and->children.
> The writer (main loop)  has an "exclusive" access, so it first waits for
> current read to finish, and then prevents incoming ones from
> entering while it has the exclusive access.
> The readers (coroutines in multiple AioContext) are free to
> access the graph as long the writer is not modifying the graph.
> In case it is, they go in a CoQueue and sleep until the writer
> is done.
> 
> In this first serie, my aim is to introduce the lock (patches 1-3,6), cover the
> main graph writer (patch 4), define assertions (patch 5) and start using the
> read lock in the generated_co_wrapper functions (7-20).
> Such functions recursively traverse the BlockDriverState graph, so they must
> take the graph rdlock.
> 
> We distinguish two cases related to the generated_co_wrapper (often shortened
> to g_c_w):
> - qemu_in_coroutine(), which means the function is already running in a
>   coroutine. This means we don't take the lock, because the coroutine must
>   have taken it when it started
> - !qemu_in_coroutine(), which means we need to create a new coroutine that
>   performs the operation requested. In this case we take the rdlock as soon as
>   the coroutine starts, and release only before finishing.
> 
> In this and following series, we try to follow the following locking pattern:
> - bdrv_co_* functions that call BlockDriver callbacks always expect the lock
>   to be taken, therefore they assert.
> - blk_co_* functions usually call blk_wait_while_drained(), therefore they must
>   take the lock internally. Therefore we introduce generated_co_wrapper_blk,
>   which does not take the rdlock when starting the coroutine.
> 
> The long term goal of this series is to eventually replace the AioContext lock,
> so that we can get rid of it once and for all.
> 
> This serie is based on v4 of "Still more coroutine and various fixes in block layer".
> 
> Based-on: <20221116122241.2856527-1-eesposit@redhat.com>
> 
> Thank you,
> Emanuele
> 
> Emanuele Giuseppe Esposito (19):
>   graph-lock: introduce BdrvGraphRWlock structure
>   async: register/unregister aiocontext in graph lock list
>   block.c: wrlock in bdrv_replace_child_noperm
>   block: remove unnecessary assert_bdrv_graph_writable()
>   block: assert that graph read and writes are performed correctly
>   graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD
>     macros
>   block-coroutine-wrapper.py: take the graph rdlock in bdrv_* functions
>   block-backend: introduce new generated_co_wrapper_blk annotation
>   block-gen: assert that {bdrv/blk}_co_truncate is always called with
>     graph rdlock taken
>   block-gen: assert that bdrv_co_{check/invalidate_cache} are always
>     called with graph rdlock taken
>   block-gen: assert that bdrv_co_pwrite is always called with graph
>     rdlock taken
>   block-gen: assert that bdrv_co_pwrite_{zeros/sync} is always called
>     with graph rdlock taken
>   block-gen: assert that bdrv_co_pread is always called with graph
>     rdlock taken
>   block-gen: assert that {bdrv/blk}_co_flush is always called with graph
>     rdlock taken
>   block-gen: assert that bdrv_co_{read/write}v_vmstate are always called
>     with graph rdlock taken
>   block-gen: assert that bdrv_co_pdiscard is always called with graph
>     rdlock taken
>   block-gen: assert that bdrv_co_common_block_status_above is always
>     called with graph rdlock taken
>   block-gen: assert that bdrv_co_ioctl is always called with graph
>     rdlock taken
>   block-gen: assert that nbd_co_do_establish_connection is always called
>     with graph rdlock taken
> 
> Paolo Bonzini (1):
>   block: introduce a lock to protect graph operations
> 
>  block.c                                |  15 +-
>  block/backup.c                         |   3 +
>  block/block-backend.c                  |  10 +-
>  block/block-copy.c                     |  10 +-
>  block/graph-lock.c                     | 255 +++++++++++++++++++++++++
>  block/io.c                             |  15 ++
>  block/meson.build                      |   1 +
>  block/mirror.c                         |  20 +-
>  block/nbd.c                            |   1 +
>  block/stream.c                         |  32 ++--
>  include/block/aio.h                    |   9 +
>  include/block/block-common.h           |   1 +
>  include/block/block_int-common.h       |  36 +++-
>  include/block/block_int-global-state.h |  17 --
>  include/block/block_int-io.h           |   2 +
>  include/block/block_int.h              |   1 +
>  include/block/graph-lock.h             | 180 +++++++++++++++++
>  include/sysemu/block-backend-io.h      |  69 +++----
>  qemu-img.c                             |   4 +-
>  scripts/block-coroutine-wrapper.py     |  13 +-
>  tests/unit/test-bdrv-drain.c           |   2 +
>  util/async.c                           |   4 +
>  util/meson.build                       |   1 +
>  23 files changed, 615 insertions(+), 86 deletions(-)
>  create mode 100644 block/graph-lock.c
>  create mode 100644 include/block/graph-lock.h
>