[PULL,09/25] block: Mark bdrv_(un)freeze_backing_chain() 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_(un)freeze_backing_chain() need to hold a reader lock for the
graph because it calls bdrv_filter_or_cow_child(), which accesses
bs->file/backing.

Use the opportunity to make bdrv_is_backing_chain_frozen() static, it
has no external callers.

Signed-off-by: Kevin Wolf <[e-mail]>
Message-ID: <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
Signed-off-by: Kevin Wolf <[e-mail]>
---
 block/copy-on-read.h               |  3 ++-
 include/block/block-global-state.h | 11 ++++++-----
 block.c                            |  5 +++--
 block/commit.c                     |  6 ++++++
 block/copy-on-read.c               | 19 +++++++++++++++----
 block/mirror.c                     |  3 +++
 block/stream.c                     | 16 +++++++++++-----
 7 files changed, 46 insertions(+), 17 deletions(-)

Patch

diff --git a/block/copy-on-read.h b/block/copy-on-read.h
index 1d8ad38c74..72f9b378ea 100644
--- a/block/copy-on-read.h
+++ b/block/copy-on-read.h
@@ -27,6 +27,7 @@ 
 
 #include "block/block_int.h"
 
-void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs);
+void no_coroutine_fn GRAPH_UNLOCKED
+bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs);
 
 #endif /* BLOCK_COPY_ON_READ_H */
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index b6860ae43b..545708c35a 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -149,11 +149,12 @@  BlockDriverState * GRAPH_RDLOCK
 bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs);
 
 BlockDriverState * GRAPH_RDLOCK bdrv_find_base(BlockDriverState *bs);
-bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
-                                  Error **errp);
-int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
-                              Error **errp);
-void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
+
+int GRAPH_RDLOCK
+bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
+                          Error **errp);
+void GRAPH_RDLOCK
+bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
 
 /*
  * The units of offset and total_work_size may be chosen arbitrarily by the
diff --git a/block.c b/block.c
index 7e8b39711b..dc1980ee42 100644
--- a/block.c
+++ b/block.c
@@ -5843,8 +5843,9 @@  BlockDriverState *bdrv_find_base(BlockDriverState *bs)
  * between @bs and @base is frozen. @errp is set if that's the case.
  * @base must be reachable from @bs, or NULL.
  */
-bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
-                                  Error **errp)
+static bool GRAPH_RDLOCK
+bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
+                             Error **errp)
 {
     BlockDriverState *i;
     BdrvChild *child;
diff --git a/block/commit.c b/block/commit.c
index 05eb57d9ea..d92af02ead 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -48,8 +48,10 @@  static int commit_prepare(Job *job)
 {
     CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
 
+    bdrv_graph_rdlock_main_loop();
     bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
     s->chain_frozen = false;
+    bdrv_graph_rdunlock_main_loop();
 
     /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
      * the normal backing chain can be restored. */
@@ -68,7 +70,9 @@  static void commit_abort(Job *job)
     BlockDriverState *top_bs = blk_bs(s->top);
 
     if (s->chain_frozen) {
+        bdrv_graph_rdlock_main_loop();
         bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
+        bdrv_graph_rdunlock_main_loop();
     }
 
     /* Make sure commit_top_bs and top stay around until bdrv_replace_node() */
@@ -404,7 +408,9 @@  void commit_start(const char *job_id, BlockDriverState *bs,
 
 fail:
     if (s->chain_frozen) {
+        bdrv_graph_rdlock_main_loop();
         bdrv_unfreeze_backing_chain(commit_top_bs, base);
+        bdrv_graph_rdunlock_main_loop();
     }
     if (s->base) {
         blk_unref(s->base);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 5149fcf63a..6f245b629a 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -35,8 +35,8 @@  typedef struct BDRVStateCOR {
 } BDRVStateCOR;
 
 
-static int cor_open(BlockDriverState *bs, QDict *options, int flags,
-                    Error **errp)
+static int GRAPH_UNLOCKED
+cor_open(BlockDriverState *bs, QDict *options, int flags, Error **errp)
 {
     BlockDriverState *bottom_bs = NULL;
     BDRVStateCOR *state = bs->opaque;
@@ -44,6 +44,8 @@  static int cor_open(BlockDriverState *bs, QDict *options, int flags,
     const char *bottom_node = qdict_get_try_str(options, "bottom");
     int ret;
 
+    GLOBAL_STATE_CODE();
+
     ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
     if (ret < 0) {
         return ret;
@@ -59,6 +61,8 @@  static int cor_open(BlockDriverState *bs, QDict *options, int flags,
             bs->file->bs->supported_zero_flags);
 
     if (bottom_node) {
+        GRAPH_RDLOCK_GUARD_MAINLOOP();
+
         bottom_bs = bdrv_find_node(bottom_node);
         if (!bottom_bs) {
             error_setg(errp, "Bottom node '%s' not found", bottom_node);
@@ -227,13 +231,17 @@  cor_co_lock_medium(BlockDriverState *bs, bool locked)
 }
 
 
-static void cor_close(BlockDriverState *bs)
+static void GRAPH_UNLOCKED cor_close(BlockDriverState *bs)
 {
     BDRVStateCOR *s = bs->opaque;
 
+    GLOBAL_STATE_CODE();
+
     if (s->chain_frozen) {
+        bdrv_graph_rdlock_main_loop();
         s->chain_frozen = false;
         bdrv_unfreeze_backing_chain(bs, s->bottom_bs);
+        bdrv_graph_rdunlock_main_loop();
     }
 
     bdrv_unref(s->bottom_bs);
@@ -263,12 +271,15 @@  static BlockDriver bdrv_copy_on_read = {
 };
 
 
-void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
+void no_coroutine_fn bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
 {
     BDRVStateCOR *s = cor_filter_bs->opaque;
 
+    GLOBAL_STATE_CODE();
+
     /* unfreeze, as otherwise bdrv_replace_node() will fail */
     if (s->chain_frozen) {
+        GRAPH_RDLOCK_GUARD_MAINLOOP();
         s->chain_frozen = false;
         bdrv_unfreeze_backing_chain(cor_filter_bs, s->bottom_bs);
     }
diff --git a/block/mirror.c b/block/mirror.c
index 75e826dac8..f8e439371c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -678,6 +678,7 @@  static int mirror_exit_common(Job *job)
     s->prepared = true;
 
     aio_context_acquire(qemu_get_aio_context());
+    bdrv_graph_rdlock_main_loop();
 
     mirror_top_bs = s->mirror_top_bs;
     bs_opaque = mirror_top_bs->opaque;
@@ -696,6 +697,8 @@  static int mirror_exit_common(Job *job)
     bdrv_ref(mirror_top_bs);
     bdrv_ref(target_bs);
 
+    bdrv_graph_rdunlock_main_loop();
+
     /*
      * Remove target parent that still uses BLK_PERM_WRITE/RESIZE before
      * inserting target_bs at s->to_replace, where we might not be able to get
diff --git a/block/stream.c b/block/stream.c
index 5323a9976d..c32c98339a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -266,6 +266,8 @@  void stream_start(const char *job_id, BlockDriverState *bs,
     assert(!(base && bottom));
     assert(!(backing_file_str && bottom));
 
+    bdrv_graph_rdlock_main_loop();
+
     if (bottom) {
         /*
          * New simple interface. The code is written in terms of old interface
@@ -278,13 +280,11 @@  void stream_start(const char *job_id, BlockDriverState *bs,
         assert(!bottom->drv->is_filter);
         base_overlay = above_base = bottom;
     } else {
-        GRAPH_RDLOCK_GUARD_MAINLOOP();
-
         base_overlay = bdrv_find_overlay(bs, base);
         if (!base_overlay) {
             error_setg(errp, "'%s' is not in the backing chain of '%s'",
                        base->node_name, bs->node_name);
-            return;
+            goto out_rdlock;
         }
 
         /*
@@ -306,7 +306,7 @@  void stream_start(const char *job_id, BlockDriverState *bs,
     if (bs_read_only) {
         /* Hold the chain during reopen */
         if (bdrv_freeze_backing_chain(bs, above_base, errp) < 0) {
-            return;
+            goto out_rdlock;
         }
 
         ret = bdrv_reopen_set_read_only(bs, false, errp);
@@ -315,10 +315,12 @@  void stream_start(const char *job_id, BlockDriverState *bs,
         bdrv_unfreeze_backing_chain(bs, above_base);
 
         if (ret < 0) {
-            return;
+            goto out_rdlock;
         }
     }
 
+    bdrv_graph_rdunlock_main_loop();
+
     opts = qdict_new();
 
     qdict_put_str(opts, "driver", "copy-on-read");
@@ -413,4 +415,8 @@  fail:
     if (bs_read_only) {
         bdrv_reopen_set_read_only(bs, true, NULL);
     }
+    return;
+
+out_rdlock:
+    bdrv_graph_rdunlock_main_loop();
 }