block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCK
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 <kwolf@redhat.com>
Message-ID: <20231027155333.420094-10-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block.c b/block.c
index 7e8b397..dc1980e 100644
--- a/block.c
+++ b/block.c
@@ -5843,8 +5843,9 @@
* 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 05eb57d..d92af02 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -48,8 +48,10 @@
{
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 @@
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 @@
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 5149fcf..6f245b6 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -35,8 +35,8 @@
} 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 @@
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 @@
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 @@
}
-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 @@
};
-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/copy-on-read.h b/block/copy-on-read.h
index 1d8ad38..72f9b37 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/block/mirror.c b/block/mirror.c
index 75e826d..f8e4393 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -678,6 +678,7 @@
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 @@
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 5323a99..c32c983 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -266,6 +266,8 @@
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 @@
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 @@
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 @@
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 @@
if (bs_read_only) {
bdrv_reopen_set_read_only(bs, true, NULL);
}
+ return;
+
+out_rdlock:
+ bdrv_graph_rdunlock_main_loop();
}
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index b6860ae..545708c 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -149,11 +149,12 @@
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
