block: Mark bdrv_chain_contains() and callers GRAPH_RDLOCK

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_chain_contains() need to hold a reader lock for the graph because
it calls bdrv_filter_or_cow_bs(), which accesses bs->file/backing.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20231027155333.420094-11-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
master
Kevin Wolf 2023-10-27 17:53:19 +02:00
parent 9275fc72bd
commit 79bb762727
4 changed files with 32 additions and 23 deletions

View File

@ -6524,7 +6524,6 @@ bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base)
{
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
while (top && top != base) {
top = bdrv_filter_or_cow_bs(top);

View File

@ -399,7 +399,9 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
* For more information see commit f8d59dfb40bb and test
* tests/qemu-iotests/222
*/
bdrv_graph_rdlock_main_loop();
is_fleecing = bdrv_chain_contains(target->bs, source->bs);
bdrv_graph_rdunlock_main_loop();
s = g_new(BlockCopyState, 1);
*s = (BlockCopyState) {

View File

@ -2450,11 +2450,12 @@ void qmp_block_stream(const char *job_id, const char *device,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
bdrv_graph_rdlock_main_loop();
if (base) {
base_bs = bdrv_find_backing_image(bs, base);
if (base_bs == NULL) {
error_setg(errp, "Can't find '%s' in the backing chain", base);
goto out;
goto out_rdlock;
}
assert(bdrv_get_aio_context(base_bs) == aio_context);
}
@ -2462,38 +2463,36 @@ void qmp_block_stream(const char *job_id, const char *device,
if (base_node) {
base_bs = bdrv_lookup_bs(NULL, base_node, errp);
if (!base_bs) {
goto out;
goto out_rdlock;
}
if (bs == base_bs || !bdrv_chain_contains(bs, base_bs)) {
error_setg(errp, "Node '%s' is not a backing image of '%s'",
base_node, device);
goto out;
goto out_rdlock;
}
assert(bdrv_get_aio_context(base_bs) == aio_context);
bdrv_graph_rdlock_main_loop();
bdrv_refresh_filename(base_bs);
bdrv_graph_rdunlock_main_loop();
}
if (bottom) {
bottom_bs = bdrv_lookup_bs(NULL, bottom, errp);
if (!bottom_bs) {
goto out;
goto out_rdlock;
}
if (!bottom_bs->drv) {
error_setg(errp, "Node '%s' is not open", bottom);
goto out;
goto out_rdlock;
}
if (bottom_bs->drv->is_filter) {
error_setg(errp, "Node '%s' is a filter, use a non-filter node "
"as 'bottom'", bottom);
goto out;
goto out_rdlock;
}
if (!bdrv_chain_contains(bs, bottom_bs)) {
error_setg(errp, "Node '%s' is not in a chain starting from '%s'",
bottom, device);
goto out;
goto out_rdlock;
}
assert(bdrv_get_aio_context(bottom_bs) == aio_context);
}
@ -2501,14 +2500,12 @@ 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)
*/
bdrv_graph_rdlock_main_loop();
iter_end = bottom ? bdrv_filter_or_cow_bs(bottom_bs) : base_bs;
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;
goto out_rdlock;
}
}
bdrv_graph_rdunlock_main_loop();
@ -2540,6 +2537,11 @@ void qmp_block_stream(const char *job_id, const char *device,
out:
aio_context_release(aio_context);
return;
out_rdlock:
bdrv_graph_rdunlock_main_loop();
aio_context_release(aio_context);
}
void qmp_block_commit(const char *job_id, const char *device,
@ -3439,39 +3441,38 @@ void qmp_change_backing_file(const char *device,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
bdrv_graph_rdlock_main_loop();
image_bs = bdrv_lookup_bs(NULL, image_node_name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
goto out_rdlock;
}
if (!image_bs) {
error_setg(errp, "image file not found");
goto out;
goto out_rdlock;
}
bdrv_graph_rdlock_main_loop();
if (bdrv_find_base(image_bs) == image_bs) {
error_setg(errp, "not allowing backing file change on an image "
"without a backing file");
bdrv_graph_rdunlock_main_loop();
goto out;
goto out_rdlock;
}
/* even though we are not necessarily operating on bs, we need it to
* determine if block ops are currently prohibited on the chain */
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
bdrv_graph_rdunlock_main_loop();
goto out;
goto out_rdlock;
}
bdrv_graph_rdunlock_main_loop();
/* final sanity check */
if (!bdrv_chain_contains(bs, image_bs)) {
error_setg(errp, "'%s' and image file are not in the same chain",
device);
goto out;
goto out_rdlock;
}
bdrv_graph_rdunlock_main_loop();
/* if not r/w, reopen to make r/w */
ro = bdrv_is_read_only(image_bs);
@ -3499,6 +3500,11 @@ void qmp_change_backing_file(const char *device,
out:
aio_context_release(aio_context);
return;
out_rdlock:
bdrv_graph_rdunlock_main_loop();
aio_context_release(aio_context);
}
void qmp_blockdev_add(BlockdevOptions *options, Error **errp)

View File

@ -199,7 +199,9 @@ XDbgBlockGraph * GRAPH_RDLOCK bdrv_get_xdbg_block_graph(Error **errp);
BlockDriverState *bdrv_lookup_bs(const char *device,
const char *node_name,
Error **errp);
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
bool GRAPH_RDLOCK
bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
BlockDriverState *bdrv_next_node(BlockDriverState *bs);
BlockDriverState *bdrv_next_all_states(BlockDriverState *bs);