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 17/22] block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK
Date: Fri, 29 Sep 2023 16:51:52 +0200 [thread overview]
Message-ID: <20230929145157.45443-18-kwolf@redhat.com> (raw)
In-Reply-To: <20230929145157.45443-1-kwolf@redhat.com>
This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_op_is_blocked() need to hold a reader lock for the graph
because it calls bdrv_get_device_or_node_name(), which accesses the
parents list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-global-state.h | 4 +++-
block.c | 2 --
block/block-backend.c | 1 +
block/commit.c | 1 +
block/monitor/block-hmp-cmds.c | 3 +++
block/qapi-sysemu.c | 9 +++++++--
blockdev.c | 15 +++++++++++++++
7 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 794ef34ded..6bfafe781d 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -246,7 +246,9 @@ bdrv_attach_child(BlockDriverState *parent_bs,
BdrvChildRole child_role,
Error **errp);
-bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+bool GRAPH_RDLOCK
+bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+
void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
diff --git a/block.c b/block.c
index 39c28a8dde..0814b0704e 100644
--- a/block.c
+++ b/block.c
@@ -7250,8 +7250,6 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
BdrvOpBlocker *blocker;
GLOBAL_STATE_CODE();
- assume_graph_lock(); /* FIXME */
-
assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0fd1b4a437..a7057400d7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2388,6 +2388,7 @@ bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp)
{
BlockDriverState *bs = blk_bs(blk);
GLOBAL_STATE_CODE();
+ GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!bs) {
return false;
diff --git a/block/commit.c b/block/commit.c
index aa45beb0f0..43d1de7577 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -434,6 +434,7 @@ int bdrv_commit(BlockDriverState *bs)
Error *local_err = NULL;
GLOBAL_STATE_CODE();
+ GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!drv)
return -ENOMEDIUM;
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 6b7d7dd072..7645c7e5fb 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -144,6 +144,9 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
AioContext *aio_context;
Error *local_err = NULL;
+ GLOBAL_STATE_CODE();
+ GRAPH_RDLOCK_GUARD_MAINLOOP();
+
bs = bdrv_find_node(id);
if (bs) {
qmp_blockdev_del(id, &local_err);
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index e885a64c32..3f614cbc04 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -169,14 +169,16 @@ void qmp_blockdev_close_tray(const char *device,
}
}
-static void blockdev_remove_medium(const char *device, const char *id,
- Error **errp)
+static void GRAPH_UNLOCKED
+blockdev_remove_medium(const char *device, const char *id, Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
AioContext *aio_context;
bool has_attached_device;
+ GLOBAL_STATE_CODE();
+
blk = qmp_get_blk(device, id, errp);
if (!blk) {
return;
@@ -205,9 +207,12 @@ static void blockdev_remove_medium(const char *device, const char *id,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
+ bdrv_graph_rdlock_main_loop();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+ bdrv_graph_rdunlock_main_loop();
goto out;
}
+ bdrv_graph_rdunlock_main_loop();
blk_remove_bs(blk);
diff --git a/blockdev.c b/blockdev.c
index 51c58dd432..a01c62596b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1696,9 +1696,12 @@ static void drive_backup_action(DriveBackup *backup,
}
/* Early check to avoid creating target */
+ bdrv_graph_rdlock_main_loop();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+ bdrv_graph_rdunlock_main_loop();
goto out;
}
+ bdrv_graph_rdunlock_main_loop();
flags = bs->open_flags | BDRV_O_RDWR;
@@ -2360,10 +2363,13 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name,
return;
}
+ bdrv_graph_co_rdlock();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
error_setg(errp, QERR_DEVICE_IN_USE, device);
+ bdrv_graph_co_rdunlock();
return;
}
+ bdrv_graph_co_rdunlock();
blk = blk_co_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp);
if (!blk) {
@@ -2487,13 +2493,16 @@ 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();
for (iter = bs; iter && iter != iter_end;
iter = bdrv_filter_or_cow_bs(iter))
{
if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) {
+ bdrv_graph_rdunlock_main_loop();
goto out;
}
}
+ bdrv_graph_rdunlock_main_loop();
/* if we are streaming the entire chain, the result will have no backing
* file, and specifying one is therefore an error */
@@ -3021,9 +3030,12 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
}
/* Early check to avoid creating target */
+ bdrv_graph_rdlock_main_loop();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
+ bdrv_graph_rdunlock_main_loop();
return;
}
+ bdrv_graph_rdunlock_main_loop();
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -3409,9 +3421,12 @@ void qmp_change_backing_file(const char *device,
/* even though we are not necessarily operating on bs, we need it to
* determine if block ops are currently prohibited on the chain */
+ bdrv_graph_rdlock_main_loop();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
+ bdrv_graph_rdunlock_main_loop();
goto out;
}
+ bdrv_graph_rdunlock_main_loop();
/* final sanity check */
if (!bdrv_chain_contains(bs, image_bs)) {
--
2.41.0
next prev parent reply other threads:[~2023-09-29 14:53 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-29 14:51 [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) Kevin Wolf
2023-09-29 14:51 ` [PATCH 01/22] test-bdrv-drain: Don't call bdrv_graph_wrlock() in coroutine context Kevin Wolf
2023-09-29 14:51 ` [PATCH 02/22] block-coroutine-wrapper: Add no_co_wrapper_bdrv_rdlock functions Kevin Wolf
2023-09-29 14:51 ` [PATCH 03/22] block: Take graph rdlock in bdrv_inactivate_all() Kevin Wolf
2023-09-29 14:51 ` [PATCH 04/22] block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 05/22] block: Mark drain related functions GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 06/22] block: Mark bdrv_parent_cb_resize() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 07/22] block: Mark bdrv_snapshot_fallback() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 08/22] block: Take graph rdlock in parts of reopen Kevin Wolf
2023-09-29 14:51 ` [PATCH 09/22] block: Mark bdrv_get_xdbg_block_graph() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 10/22] block: Mark bdrv_refresh_filename() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 11/22] block: Mark bdrv_primary_child() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 12/22] block: Mark bdrv_get_parent_name() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 13/22] block: Mark bdrv_amend_options() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 14/22] qcow2: Mark qcow2_signal_corruption() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 15/22] qcow2: Mark qcow2_inactivate() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 16/22] qcow2: Mark check_constraints_on_bitmap() GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` Kevin Wolf [this message]
2023-09-29 14:51 ` [PATCH 18/22] block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 19/22] block: Mark bdrv_get_specific_info() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 20/22] block: Protect bs->parents with graph_lock Kevin Wolf
2023-09-29 14:51 ` [PATCH 21/22] block: Protect bs->children " Kevin Wolf
2023-09-29 14:51 ` [PATCH 22/22] block: Add assertion for bdrv_graph_wrlock() Kevin Wolf
2023-10-10 20:48 ` [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) Stefan Hajnoczi
2023-10-11 11:05 ` Kevin Wolf
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=20230929145157.45443-18-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.