system/qemu
system/qemu
1
0
Fork 0
mirror of https://gitlab.com/qemu-project/qemu synced 2024-07-24 03:44:38 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCK

Browse source 
The function reads the parents list, so it needs to hold the graph lock.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230911094620.45040-14-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Kevin Wolf 2023-09-11 11:46:12 +02:00
parent afdaeb9ea0
commit 3804e3cf54
9 changed files with 66 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
block.c
View file

@ -2202,7 +2202,8 @@ static bool bdrv_a_allow_b(BdrvChild *a, BdrvChild *b, Error **errp)
return false; return false;
} }
static bool bdrv_parent_perms_conflict(BlockDriverState *bs, Error **errp) static bool GRAPH_RDLOCK
bdrv_parent_perms_conflict(BlockDriverState *bs, Error **errp)
{ {
BdrvChild *a, *b; BdrvChild *a, *b;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
@ -2255,8 +2256,8 @@ static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
* simplest way to satisfy this criteria: use only result of * simplest way to satisfy this criteria: use only result of
* bdrv_topological_dfs() or NULL as @list parameter. * bdrv_topological_dfs() or NULL as @list parameter.
*/ */
static GSList *bdrv_topological_dfs(GSList *list, GHashTable *found, static GSList * GRAPH_RDLOCK
BlockDriverState *bs) bdrv_topological_dfs(GSList *list, GHashTable *found, BlockDriverState *bs)
{ {
BdrvChild *child; BdrvChild *child;
g_autoptr(GHashTable) local_found = NULL; g_autoptr(GHashTable) local_found = NULL;
@ -2532,8 +2533,9 @@ static int bdrv_node_refresh_perm(BlockDriverState *bs, BlockReopenQueue *q,
* @list is a product of bdrv_topological_dfs() (may be called several times) - * @list is a product of bdrv_topological_dfs() (may be called several times) -
* a topologically sorted subgraph. * a topologically sorted subgraph.
*/ */
static int bdrv_do_refresh_perms(GSList *list, BlockReopenQueue *q, static int GRAPH_RDLOCK
Transaction *tran, Error **errp) bdrv_do_refresh_perms(GSList *list, BlockReopenQueue *q, Transaction *tran,
Error **errp)
{ {
int ret; int ret;
BlockDriverState *bs; BlockDriverState *bs;
@ -2560,8 +2562,9 @@ static int bdrv_do_refresh_perms(GSList *list, BlockReopenQueue *q,
* topologically sorted. It's not a problem if some node occurs in the @list * topologically sorted. It's not a problem if some node occurs in the @list
* several times. * several times.
*/ */
static int bdrv_list_refresh_perms(GSList *list, BlockReopenQueue *q, static int GRAPH_RDLOCK
Transaction *tran, Error **errp) bdrv_list_refresh_perms(GSList *list, BlockReopenQueue *q, Transaction *tran,
Error **errp)
{ {
g_autoptr(GHashTable) found = g_hash_table_new(NULL, NULL); g_autoptr(GHashTable) found = g_hash_table_new(NULL, NULL);
g_autoptr(GSList) refresh_list = NULL; g_autoptr(GSList) refresh_list = NULL;
@ -2621,8 +2624,8 @@ char *bdrv_perm_names(uint64_t perm)
/* @tran is allowed to be NULL. In this case no rollback is possible */ /* @tran is allowed to be NULL. In this case no rollback is possible */
static int bdrv_refresh_perms(BlockDriverState *bs, Transaction *tran, static int GRAPH_RDLOCK
Error **errp) bdrv_refresh_perms(BlockDriverState *bs, Transaction *tran, Error **errp)
{ {
int ret; int ret;
Transaction *local_tran = NULL; Transaction *local_tran = NULL;
@ -3247,7 +3250,6 @@ void bdrv_root_unref_child(BdrvChild *child)
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
bdrv_graph_wrlock(NULL); bdrv_graph_wrlock(NULL);
bdrv_replace_child_noperm(child, NULL); bdrv_replace_child_noperm(child, NULL);
bdrv_graph_wrunlock();
bdrv_child_free(child); bdrv_child_free(child);
if (child_bs) { if (child_bs) {
@ -3266,6 +3268,7 @@ void bdrv_root_unref_child(BdrvChild *child)
NULL); NULL);
} }
bdrv_graph_wrunlock();
bdrv_unref(child_bs); bdrv_unref(child_bs);
} }
@ -4585,7 +4588,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
* reconfiguring the fd and that's why it does it in raw_check_perm(), not * reconfiguring the fd and that's why it does it in raw_check_perm(), not
* in raw_reopen_prepare() which is called with "old" permissions. * in raw_reopen_prepare() which is called with "old" permissions.
*/ */
bdrv_graph_rdlock_main_loop();
ret = bdrv_list_refresh_perms(refresh_list, bs_queue, tran, errp); ret = bdrv_list_refresh_perms(refresh_list, bs_queue, tran, errp);
bdrv_graph_rdunlock_main_loop();
if (ret < 0) { if (ret < 0) {
goto abort; goto abort;
} }
@ -6833,6 +6839,7 @@ int bdrv_activate(BlockDriverState *bs, Error **errp)
BdrvDirtyBitmap *bm; BdrvDirtyBitmap *bm;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!bs->drv) { if (!bs->drv) {
return -ENOMEDIUM; return -ENOMEDIUM;
@ -6963,6 +6970,7 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
uint64_t cumulative_perms, cumulative_shared_perms; uint64_t cumulative_perms, cumulative_shared_perms;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!bs->drv) { if (!bs->drv) {
return -ENOMEDIUM; return -ENOMEDIUM;

26
block/block-backend.c
View file

@ -121,6 +121,10 @@ static QTAILQ_HEAD(, BlockBackend) block_backends =
static QTAILQ_HEAD(, BlockBackend) monitor_block_backends = static QTAILQ_HEAD(, BlockBackend) monitor_block_backends =
QTAILQ_HEAD_INITIALIZER(monitor_block_backends); QTAILQ_HEAD_INITIALIZER(monitor_block_backends);
static int coroutine_mixed_fn GRAPH_RDLOCK
blk_set_perm_locked(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
Error **errp);
static void blk_root_inherit_options(BdrvChildRole role, bool parent_is_format, static void blk_root_inherit_options(BdrvChildRole role, bool parent_is_format,
int *child_flags, QDict *child_options, int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options) int parent_flags, QDict *parent_options)
@ -186,7 +190,7 @@ static void blk_vm_state_changed(void *opaque, bool running, RunState state)
* *
* If an error is returned, the VM cannot be allowed to be resumed. * If an error is returned, the VM cannot be allowed to be resumed.
*/ */
static void blk_root_activate(BdrvChild *child, Error **errp) static void GRAPH_RDLOCK blk_root_activate(BdrvChild *child, Error **errp)
{ {
BlockBackend *blk = child->opaque; BlockBackend *blk = child->opaque;
Error *local_err = NULL; Error *local_err = NULL;
@ -207,7 +211,7 @@ static void blk_root_activate(BdrvChild *child, Error **errp)
*/ */
saved_shared_perm = blk->shared_perm; saved_shared_perm = blk->shared_perm;
blk_set_perm(blk, blk->perm, BLK_PERM_ALL, &local_err); blk_set_perm_locked(blk, blk->perm, BLK_PERM_ALL, &local_err);
if (local_err) { if (local_err) {
error_propagate(errp, local_err); error_propagate(errp, local_err);
blk->disable_perm = true; blk->disable_perm = true;
@ -226,7 +230,7 @@ static void blk_root_activate(BdrvChild *child, Error **errp)
return; return;
} }
blk_set_perm(blk, blk->perm, blk->shared_perm, &local_err); blk_set_perm_locked(blk, blk->perm, blk->shared_perm, &local_err);
if (local_err) { if (local_err) {
error_propagate(errp, local_err); error_propagate(errp, local_err);
blk->disable_perm = true; blk->disable_perm = true;
@ -259,7 +263,7 @@ static bool blk_can_inactivate(BlockBackend *blk)
return blk->force_allow_inactivate; return blk->force_allow_inactivate;
} }
static int blk_root_inactivate(BdrvChild *child) static int GRAPH_RDLOCK blk_root_inactivate(BdrvChild *child)
{ {
BlockBackend *blk = child->opaque; BlockBackend *blk = child->opaque;
@ -953,8 +957,9 @@ int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp)
/* /*
* Sets the permission bitmasks that the user of the BlockBackend needs. * Sets the permission bitmasks that the user of the BlockBackend needs.
*/ */
int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm, static int coroutine_mixed_fn GRAPH_RDLOCK
Error **errp) blk_set_perm_locked(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
Error **errp)
{ {
int ret; int ret;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
@ -972,6 +977,15 @@ int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
return 0; return 0;
} }
int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
Error **errp)
{
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
return blk_set_perm_locked(blk, perm, shared_perm, errp);
}
void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm) void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
{ {
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();

6
block/crypto.c
View file

@ -777,7 +777,7 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
return spec_info; return spec_info;
} }
static int static int GRAPH_RDLOCK
block_crypto_amend_prepare(BlockDriverState *bs, Error **errp) block_crypto_amend_prepare(BlockDriverState *bs, Error **errp)
{ {
BlockCrypto *crypto = bs->opaque; BlockCrypto *crypto = bs->opaque;
@ -793,7 +793,7 @@ block_crypto_amend_prepare(BlockDriverState *bs, Error **errp)
return ret; return ret;
} }
static void static void GRAPH_RDLOCK
block_crypto_amend_cleanup(BlockDriverState *bs) block_crypto_amend_cleanup(BlockDriverState *bs)
{ {
BlockCrypto *crypto = bs->opaque; BlockCrypto *crypto = bs->opaque;
@ -841,6 +841,8 @@ block_crypto_amend_options_luks(BlockDriverState *bs,
QCryptoBlockAmendOptions *amend_options = NULL; QCryptoBlockAmendOptions *amend_options = NULL;
int ret = -EINVAL; int ret = -EINVAL;
assume_graph_lock(); /* FIXME */
assert(crypto); assert(crypto);
assert(crypto->block); assert(crypto->block);

8
block/mirror.c
View file

@ -702,8 +702,12 @@ static int mirror_exit_common(Job *job)
* mirror_top_bs from now on, so keep it drained. */ * mirror_top_bs from now on, so keep it drained. */
bdrv_drained_begin(mirror_top_bs); bdrv_drained_begin(mirror_top_bs);
bs_opaque->stop = true; bs_opaque->stop = true;
bdrv_graph_rdlock_main_loop();
bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing, bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
&error_abort); &error_abort);
bdrv_graph_rdunlock_main_loop();
if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
BlockDriverState *backing = s->is_none_mode ? src : s->base; BlockDriverState *backing = s->is_none_mode ? src : s->base;
BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs); BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs);
@ -1670,6 +1674,8 @@ static BlockJob *mirror_start_job(
uint64_t target_perms, target_shared_perms; uint64_t target_perms, target_shared_perms;
int ret; int ret;
GLOBAL_STATE_CODE();
if (granularity == 0) { if (granularity == 0) {
granularity = bdrv_get_default_bitmap_granularity(target); granularity = bdrv_get_default_bitmap_granularity(target);
} }
@ -1906,8 +1912,10 @@ fail:
} }
bs_opaque->stop = true; bs_opaque->stop = true;
bdrv_graph_rdlock_main_loop();
bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing, bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
&error_abort); &error_abort);
bdrv_graph_rdunlock_main_loop();
bdrv_replace_node(mirror_top_bs, mirror_top_bs->backing->bs, &error_abort); bdrv_replace_node(mirror_top_bs, mirror_top_bs->backing->bs, &error_abort);
bdrv_unref(mirror_top_bs); bdrv_unref(mirror_top_bs);

2
block/vmdk.c
View file

@ -1309,6 +1309,8 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
BDRVVmdkState *s = bs->opaque; BDRVVmdkState *s = bs->opaque;
uint32_t magic; uint32_t magic;
GRAPH_RDLOCK_GUARD_MAINLOOP();
ret = bdrv_open_file_child(NULL, options, "file", bs, errp); ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
if (ret < 0) { if (ret < 0) {
return ret; return ret;

6
include/block/block_int-common.h
View file

@ -311,7 +311,7 @@ struct BlockDriver {
*/ */
void (*bdrv_cancel_in_flight)(BlockDriverState *bs); void (*bdrv_cancel_in_flight)(BlockDriverState *bs);
int (*bdrv_inactivate)(BlockDriverState *bs); int GRAPH_RDLOCK_PTR (*bdrv_inactivate)(BlockDriverState *bs);
int (*bdrv_snapshot_create)(BlockDriverState *bs, int (*bdrv_snapshot_create)(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info); QEMUSnapshotInfo *sn_info);
@ -944,8 +944,8 @@ struct BdrvChildClass {
* when migration is completing) and it can start/stop requesting * when migration is completing) and it can start/stop requesting
* permissions and doing I/O on it. * permissions and doing I/O on it.
*/ */
void (*activate)(BdrvChild *child, Error **errp); void GRAPH_RDLOCK_PTR (*activate)(BdrvChild *child, Error **errp);
int (*inactivate)(BdrvChild *child); int GRAPH_RDLOCK_PTR (*inactivate)(BdrvChild *child);
void GRAPH_WRLOCK_PTR (*attach)(BdrvChild *child); void GRAPH_WRLOCK_PTR (*attach)(BdrvChild *child);
void GRAPH_WRLOCK_PTR (*detach)(BdrvChild *child); void GRAPH_WRLOCK_PTR (*detach)(BdrvChild *child);

8
include/block/block_int-global-state.h
View file

@ -212,8 +212,9 @@ void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
* bdrv_child_refresh_perms() instead and make the parent's * bdrv_child_refresh_perms() instead and make the parent's
* .bdrv_child_perm() implementation return the correct values. * .bdrv_child_perm() implementation return the correct values.
*/ */
int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, int GRAPH_RDLOCK
Error **errp); bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
Error **errp);
/** /**
* Calls bs->drv->bdrv_child_perm() and updates the child's permission * Calls bs->drv->bdrv_child_perm() and updates the child's permission
@ -223,7 +224,8 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
* values than before, but which will not result in the block layer * values than before, but which will not result in the block layer
* automatically refreshing the permissions. * automatically refreshing the permissions.
*/ */
int bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp); int GRAPH_RDLOCK
bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp);
bool GRAPH_RDLOCK bdrv_recurse_can_replace(BlockDriverState *bs, bool GRAPH_RDLOCK bdrv_recurse_can_replace(BlockDriverState *bs,
BlockDriverState *to_replace); BlockDriverState *to_replace);

4
include/sysemu/block-backend-global-state.h
View file

@ -61,8 +61,8 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp);
int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp); int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp);
bool bdrv_has_blk(BlockDriverState *bs); bool bdrv_has_blk(BlockDriverState *bs);
bool bdrv_is_root_node(BlockDriverState *bs); bool bdrv_is_root_node(BlockDriverState *bs);
int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm, int GRAPH_UNLOCKED blk_set_perm(BlockBackend *blk, uint64_t perm,
Error **errp); uint64_t shared_perm, Error **errp);
void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm); void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm);
void blk_iostatus_enable(BlockBackend *blk); void blk_iostatus_enable(BlockBackend *blk);

4
tests/unit/test-bdrv-graph-mod.c
View file

@ -372,6 +372,9 @@ static void test_parallel_perm_update(void)
/* Select fl1 as first child to be active */ /* Select fl1 as first child to be active */
s->selected = c_fl1; s->selected = c_fl1;
bdrv_graph_rdlock_main_loop();
bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort); bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort);
assert(c_fl1->perm & BLK_PERM_WRITE); assert(c_fl1->perm & BLK_PERM_WRITE);
@ -391,6 +394,7 @@ static void test_parallel_perm_update(void)
assert(c_fl1->perm & BLK_PERM_WRITE); assert(c_fl1->perm & BLK_PERM_WRITE);
assert(!(c_fl2->perm & BLK_PERM_WRITE)); assert(!(c_fl2->perm & BLK_PERM_WRITE));
bdrv_graph_rdunlock_main_loop();
bdrv_unref(top); bdrv_unref(top);
} }