[PULL,06/25] block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK

Commit Message

Kevin Wolf Nov. 7, 2023, 6:45 p.m. UTC

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because
it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing.

Signed-off-by: Kevin Wolf <[e-mail]>
Message-ID: <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
Signed-off-by: Kevin Wolf <[e-mail]>
---
 include/block/block_int-io.h |  3 ++-
 block.c                      | 31 ++++++++++++++++++-------------
 block/stream.c               |  4 ++++
 blockdev.c                   |  2 +-
 nbd/server.c                 |  6 ++++++
 5 files changed, 31 insertions(+), 15 deletions(-)

Patch

diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 26bff94e4e..6800af7590 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -150,7 +150,8 @@  bdrv_filter_bs(BlockDriverState *bs)
     return child_bs(bdrv_filter_child(bs));
 }
 
-static inline BlockDriverState *bdrv_filter_or_cow_bs(BlockDriverState *bs)
+static inline BlockDriverState * GRAPH_RDLOCK
+bdrv_filter_or_cow_bs(BlockDriverState *bs)
 {
     IO_CODE();
     return child_bs(bdrv_filter_or_cow_child(bs));
diff --git a/block.c b/block.c
index 5f92eb4950..a6060eddbc 100644
--- a/block.c
+++ b/block.c
@@ -5435,17 +5435,6 @@  static int bdrv_replace_node_common(BlockDriverState *from,
 
     GLOBAL_STATE_CODE();
 
-    if (detach_subchain) {
-        assert(bdrv_chain_contains(from, to));
-        assert(from != to);
-        for (to_cow_parent = from;
-             bdrv_filter_or_cow_bs(to_cow_parent) != to;
-             to_cow_parent = bdrv_filter_or_cow_bs(to_cow_parent))
-        {
-            ;
-        }
-    }
-
     /* Make sure that @from doesn't go away until we have successfully attached
      * all of its parents to @to. */
     bdrv_ref(from);
@@ -5457,6 +5446,17 @@  static int bdrv_replace_node_common(BlockDriverState *from,
 
     bdrv_graph_wrlock(to);
 
+    if (detach_subchain) {
+        assert(bdrv_chain_contains(from, to));
+        assert(from != to);
+        for (to_cow_parent = from;
+             bdrv_filter_or_cow_bs(to_cow_parent) != to;
+             to_cow_parent = bdrv_filter_or_cow_bs(to_cow_parent))
+        {
+            ;
+        }
+    }
+
     /*
      * Do the replacement without permission update.
      * Replacement may influence the permissions, we should calculate new
@@ -5504,10 +5504,14 @@  int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
 
 int bdrv_drop_filter(BlockDriverState *bs, Error **errp)
 {
+    BlockDriverState *child_bs;
+
     GLOBAL_STATE_CODE();
+    bdrv_graph_rdlock_main_loop();
+    child_bs = bdrv_filter_or_cow_bs(bs);
+    bdrv_graph_rdunlock_main_loop();
 
-    return bdrv_replace_node_common(bs, bdrv_filter_or_cow_bs(bs), true, true,
-                                    errp);
+    return bdrv_replace_node_common(bs, child_bs, true, true, errp);
 }
 
 /*
@@ -6509,6 +6513,7 @@  bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base)
 {
 
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     while (top && top != base) {
         top = bdrv_filter_or_cow_bs(top);
diff --git a/block/stream.c b/block/stream.c
index 51333e460b..2781441191 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -60,6 +60,8 @@  static int stream_prepare(Job *job)
     Error *local_err = NULL;
     int ret = 0;
 
+    GLOBAL_STATE_CODE();
+
     /* We should drop filter at this point, as filter hold the backing chain */
     bdrv_cor_filter_drop(s->cor_filter_bs);
     s->cor_filter_bs = NULL;
@@ -78,8 +80,10 @@  static int stream_prepare(Job *job)
         bdrv_drained_begin(unfiltered_bs_cow);
     }
 
+    bdrv_graph_rdlock_main_loop();
     base = bdrv_filter_or_cow_bs(s->above_base);
     unfiltered_base = bdrv_skip_filters(base);
+    bdrv_graph_rdunlock_main_loop();
 
     if (bdrv_cow_child(unfiltered_bs)) {
         const char *base_id = NULL, *base_fmt = NULL;
diff --git a/blockdev.c b/blockdev.c
index 148df99e00..4cb8e1d91a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2501,8 +2501,8 @@  void qmp_block_stream(const char *job_id, const char *device,
     /*
      * Check for op blockers in the whole chain between bs and base (or bottom)
      */
-    iter_end = bottom ? bdrv_filter_or_cow_bs(bottom_bs) : base_bs;
     bdrv_graph_rdlock_main_loop();
+    iter_end = bottom ? bdrv_filter_or_cow_bs(bottom_bs) : base_bs;
     for (iter = bs; iter && iter != iter_end;
          iter = bdrv_filter_or_cow_bs(iter))
     {
diff --git a/nbd/server.c b/nbd/server.c
index 859c163d19..895cf0a752 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1689,6 +1689,7 @@  static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
     size_t i;
     int ret;
 
+    GLOBAL_STATE_CODE();
     assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
 
     if (!nbd_server_is_running()) {
@@ -1743,6 +1744,8 @@  static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
     }
     exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
 
+    bdrv_graph_rdlock_main_loop();
+
     for (bitmaps = arg->bitmaps; bitmaps; bitmaps = bitmaps->next) {
         exp->nr_export_bitmaps++;
     }
@@ -1825,9 +1828,12 @@  static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
 
     QTAILQ_INSERT_TAIL(&exports, exp, next);
 
+    bdrv_graph_rdunlock_main_loop();
+
     return 0;
 
 fail:
+    bdrv_graph_rdunlock_main_loop();
     g_free(exp->export_bitmaps);
     g_free(exp->name);
     g_free(exp->description);