From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com, eesposit@redhat.com,
eblake@redhat.com, pbonzini@redhat.com,
vsementsov@yandex-team.ru, qemu-devel@nongnu.org
Subject: [PATCH 09/24] block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCK
Date: Fri, 27 Oct 2023 17:53:18 +0200 [thread overview]
Message-ID: <20231027155333.420094-10-kwolf@redhat.com> (raw)
In-Reply-To: <20231027155333.420094-1-kwolf@redhat.com>
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>
---
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(-)
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 4d11a30508..304bd3208a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -670,6 +670,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;
@@ -688,6 +689,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();
}
--
2.41.0
next prev parent reply other threads:[~2023-10-27 15:54 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-27 15:53 [PATCH 00/24] block: Graph locking part 6 (bs->file/backing) Kevin Wolf
2023-10-27 15:53 ` [PATCH 01/24] block: Mark bdrv_probe_blocksizes() and callers GRAPH_RDLOCK Kevin Wolf
2023-10-27 19:40 ` Eric Blake
2023-10-27 15:53 ` [PATCH 02/24] block: Mark bdrv_has_zero_init() " Kevin Wolf
2023-10-27 19:59 ` Eric Blake
2023-10-27 15:53 ` [PATCH 03/24] block: Mark bdrv_filter_bs() " Kevin Wolf
2023-10-27 20:02 ` Eric Blake
2023-10-27 20:45 ` Eric Blake
2023-10-27 15:53 ` [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK Kevin Wolf
2023-10-27 20:22 ` Eric Blake
2023-11-03 9:45 ` Kevin Wolf
2023-11-03 12:33 ` Eric Blake
2023-10-27 15:53 ` [PATCH 05/24] block: Mark block_job_add_bdrv() GRAPH_WRLOCK Kevin Wolf
2023-10-27 20:27 ` Eric Blake
2023-10-27 15:53 ` [PATCH 06/24] block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK Kevin Wolf
2023-10-27 20:33 ` Eric Blake
2023-10-27 15:53 ` [PATCH 07/24] block: Mark bdrv_skip_implicit_filters() " Kevin Wolf
2023-10-27 20:37 ` Eric Blake
2023-10-27 15:53 ` [PATCH 08/24] block: Mark bdrv_skip_filters() " Kevin Wolf
2023-10-27 20:52 ` Eric Blake
2023-10-27 15:53 ` Kevin Wolf [this message]
2023-10-27 21:00 ` [PATCH 09/24] block: Mark bdrv_(un)freeze_backing_chain() " Eric Blake
2023-11-03 9:54 ` Kevin Wolf
2023-10-27 15:53 ` [PATCH 10/24] block: Mark bdrv_chain_contains() " Kevin Wolf
2023-10-27 21:17 ` Eric Blake
2023-10-27 15:53 ` [PATCH 11/24] block: Mark bdrv_filter_child() " Kevin Wolf
2023-10-27 21:19 ` Eric Blake
2023-10-27 15:53 ` [PATCH 12/24] block: Mark bdrv_cow_child() " Kevin Wolf
2023-10-27 21:20 ` Eric Blake
2023-10-27 15:53 ` [PATCH 13/24] block: Mark bdrv_set_backing_hd_drained() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:22 ` Eric Blake
2023-10-27 15:53 ` [PATCH 14/24] block: Inline bdrv_set_backing_noperm() Kevin Wolf
2023-10-27 21:23 ` Eric Blake
2023-10-27 15:53 ` [PATCH 15/24] block: Mark bdrv_replace_node_common() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:27 ` Eric Blake
2023-10-27 15:53 ` [PATCH 16/24] block: Mark bdrv_replace_node() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:33 ` Eric Blake
2023-11-03 10:32 ` Kevin Wolf
2023-11-03 12:37 ` Eric Blake
2023-10-27 15:53 ` [PATCH 17/24] block: Protect bs->backing with graph_lock Kevin Wolf
2023-10-27 21:46 ` Eric Blake
2023-10-27 15:53 ` [PATCH 18/24] blkverify: Add locking for request_fn Kevin Wolf
2023-10-30 13:51 ` Eric Blake
2023-10-27 15:53 ` [PATCH 19/24] block: Introduce bdrv_co_change_backing_file() Kevin Wolf
2023-10-30 13:57 ` Eric Blake
2023-11-03 10:33 ` Kevin Wolf
2023-11-03 12:38 ` Eric Blake
2023-10-27 15:53 ` [PATCH 20/24] block: Add missing GRAPH_RDLOCK annotations Kevin Wolf
2023-10-30 21:19 ` Eric Blake
2023-10-27 15:53 ` [PATCH 21/24] qcow2: Take locks for accessing bs->file Kevin Wolf
2023-10-30 21:25 ` Eric Blake
2023-10-27 15:53 ` [PATCH 22/24] vhdx: " Kevin Wolf
2023-10-30 21:26 ` Eric Blake
2023-10-27 15:53 ` [PATCH 23/24] block: Take graph lock for most of .bdrv_open Kevin Wolf
2023-10-30 21:34 ` Eric Blake
2023-11-03 10:05 ` Kevin Wolf
2023-10-27 15:53 ` [PATCH 24/24] block: Protect bs->file with graph_lock Kevin Wolf
2023-10-30 21:37 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231027155333.420094-10-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=eblake@redhat.com \
--cc=eesposit@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@yandex-team.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.