This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_skip_implicit_filters() need to hold a reader lock for the graph
because it calls bdrv_filter_child(), which accesses bs->file/backing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int-global-state.h | 3 ++-
block.c | 28 +++++++++++++++++---------
block/monitor/block-hmp-cmds.c | 3 +++
blockdev.c | 14 +++++++------
4 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index afce6c4416..ef31c58bb3 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -277,7 +277,8 @@ BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name,
Error **errp);
-BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
+BlockDriverState * GRAPH_RDLOCK
+bdrv_skip_implicit_filters(BlockDriverState *bs);
/**
* bdrv_add_aio_context_notifier:
diff --git a/block.c b/block.c
index a6060eddbc..7e8b39711b 100644
--- a/block.c
+++ b/block.c
@@ -4778,6 +4778,8 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
return 0;
}
+ bdrv_graph_rdlock_main_loop();
+
switch (qobject_type(value)) {
case QTYPE_QNULL:
assert(is_backing); /* The 'file' option does not allow a null value */
@@ -4787,17 +4789,16 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
str = qstring_get_str(qobject_to(QString, value));
new_child_bs = bdrv_lookup_bs(NULL, str, errp);
if (new_child_bs == NULL) {
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_rdlock;
}
- bdrv_graph_rdlock_main_loop();
has_child = bdrv_recurse_has_child(new_child_bs, bs);
- bdrv_graph_rdunlock_main_loop();
-
if (has_child) {
error_setg(errp, "Making '%s' a %s child of '%s' would create a "
"cycle", str, child_name, bs->node_name);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_rdlock;
}
break;
default:
@@ -4809,18 +4810,21 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
}
if (old_child_bs == new_child_bs) {
- return 0;
+ ret = 0;
+ goto out_rdlock;
}
if (old_child_bs) {
if (bdrv_skip_implicit_filters(old_child_bs) == new_child_bs) {
- return 0;
+ ret = 0;
+ goto out_rdlock;
}
if (old_child_bs->implicit) {
error_setg(errp, "Cannot replace implicit %s child of %s",
child_name, bs->node_name);
- return -EPERM;
+ ret = -EPERM;
+ goto out_rdlock;
}
}
@@ -4831,7 +4835,8 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
*/
error_setg(errp, "'%s' is a %s filter node that does not support a "
"%s child", bs->node_name, bs->drv->format_name, child_name);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_rdlock;
}
if (is_backing) {
@@ -4852,6 +4857,7 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
aio_context_acquire(ctx);
}
+ bdrv_graph_rdunlock_main_loop();
bdrv_graph_wrlock(new_child_bs);
ret = bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing,
@@ -4870,6 +4876,10 @@ bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
}
return ret;
+
+out_rdlock:
+ bdrv_graph_rdunlock_main_loop();
+ return ret;
}
/*
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 7645c7e5fb..d55278099d 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -206,6 +206,9 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
BlockBackend *blk;
int ret;
+ GLOBAL_STATE_CODE();
+ GRAPH_RDLOCK_GUARD_MAINLOOP();
+
if (!strcmp(device, "all")) {
ret = blk_commit_all();
} else {
diff --git a/blockdev.c b/blockdev.c
index 240834b6d4..52236f2639 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1737,10 +1737,10 @@ static void drive_backup_action(DriveBackup *backup,
assert(format);
if (source) {
/* Implicit filters should not appear in the filename */
- BlockDriverState *explicit_backing =
- bdrv_skip_implicit_filters(source);
+ BlockDriverState *explicit_backing;
bdrv_graph_rdlock_main_loop();
+ explicit_backing = bdrv_skip_implicit_filters(source);
bdrv_refresh_filename(explicit_backing);
bdrv_graph_rdunlock_main_loop();
@@ -3099,16 +3099,18 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
bdrv_img_create(arg->target, format,
NULL, NULL, NULL, size, flags, false, &local_err);
} else {
- /* Implicit filters should not appear in the filename */
- BlockDriverState *explicit_backing =
- bdrv_skip_implicit_filters(target_backing_bs);
+ BlockDriverState *explicit_backing;
switch (arg->mode) {
case NEW_IMAGE_MODE_EXISTING:
break;
case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
- /* create new image with backing file */
+ /*
+ * Create new image with backing file.
+ * Implicit filters should not appear in the filename.
+ */
bdrv_graph_rdlock_main_loop();
+ explicit_backing = bdrv_skip_implicit_filters(target_backing_bs);
bdrv_refresh_filename(explicit_backing);
bdrv_graph_rdunlock_main_loop();
--
2.41.0