block: Mark bdrv_get_parent_name() and callers GRAPH_RDLOCK

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_get_parent_name() need to hold a reader lock for the graph
because it accesses the parents list of a node.

For some places, we know that they will hold the lock, but we don't have
the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock()
with a FIXME comment. These places will be removed once everything is
properly annotated.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20230929145157.45443-13-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Kevin Wolf 2023-09-29 16:51:47 +02:00
parent c0fc5123ad
commit 4026f1c4f3
17 changed files with 55 additions and 14 deletions

14
block.c
View File

@ -279,8 +279,9 @@ bool bdrv_is_read_only(BlockDriverState *bs)
return !(bs->open_flags & BDRV_O_RDWR); return !(bs->open_flags & BDRV_O_RDWR);
} }
static int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, static int GRAPH_RDLOCK
bool ignore_allow_rdw, Error **errp) bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,
bool ignore_allow_rdw, Error **errp)
{ {
IO_CODE(); IO_CODE();
@ -319,6 +320,8 @@ int bdrv_apply_auto_read_only(BlockDriverState *bs, const char *errmsg,
int ret = 0; int ret = 0;
IO_CODE(); IO_CODE();
assume_graph_lock(); /* FIXME */
if (!(bs->open_flags & BDRV_O_RDWR)) { if (!(bs->open_flags & BDRV_O_RDWR)) {
return 0; return 0;
} }
@ -4950,7 +4953,10 @@ bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
* to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR is * to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR is
* not set, or if the BDS still has copy_on_read enabled */ * not set, or if the BDS still has copy_on_read enabled */
read_only = !(reopen_state->flags & BDRV_O_RDWR); read_only = !(reopen_state->flags & BDRV_O_RDWR);
bdrv_graph_rdlock_main_loop();
ret = bdrv_can_set_read_only(reopen_state->bs, read_only, true, &local_err); ret = bdrv_can_set_read_only(reopen_state->bs, read_only, true, &local_err);
bdrv_graph_rdunlock_main_loop();
if (local_err) { if (local_err) {
error_propagate(errp, local_err); error_propagate(errp, local_err);
goto error; goto error;
@ -4984,9 +4990,11 @@ bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
} else { } else {
/* It is currently mandatory to have a bdrv_reopen_prepare() /* It is currently mandatory to have a bdrv_reopen_prepare()
* handler for each supported drv. */ * handler for each supported drv. */
bdrv_graph_rdlock_main_loop();
error_setg(errp, "Block format '%s' used by node '%s' " error_setg(errp, "Block format '%s' used by node '%s' "
"does not support reopening files", drv->format_name, "does not support reopening files", drv->format_name,
bdrv_get_device_or_node_name(reopen_state->bs)); bdrv_get_device_or_node_name(reopen_state->bs));
bdrv_graph_rdunlock_main_loop();
ret = -1; ret = -1;
goto error; goto error;
} }
@ -7242,6 +7250,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
{ {
BdrvOpBlocker *blocker; BdrvOpBlocker *blocker;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
if (!QLIST_EMPTY(&bs->op_blockers[op])) { if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]); blocker = QLIST_FIRST(&bs->op_blockers[op]);

View File

@ -374,6 +374,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
assert(bs); assert(bs);
assert(target); assert(target);
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
/* QMP interface protects us from these cases */ /* QMP interface protects us from these cases */
assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL); assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL);

View File

@ -1363,9 +1363,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
bitmap_new(DIV_ROUND_UP(s->header_size, s->bat_dirty_block)); bitmap_new(DIV_ROUND_UP(s->header_size, s->bat_dirty_block));
/* Disable migration until bdrv_activate method is added */ /* Disable migration until bdrv_activate method is added */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The Parallels format used by node '%s' " error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
"does not support live migration", "does not support live migration",
bdrv_get_device_or_node_name(bs)); bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();
ret = migrate_add_blocker(s->migration_blocker, errp); ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) { if (ret < 0) {
error_setg(errp, "Migration blocker error"); error_setg(errp, "Migration blocker error");

View File

@ -301,9 +301,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
} }
/* Disable migration when qcow images are used */ /* Disable migration when qcow images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The qcow format used by node '%s' " error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
"does not support live migration", "does not support live migration",
bdrv_get_device_or_node_name(bs)); bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();
ret = migrate_add_blocker(s->migration_blocker, errp); ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) { if (ret < 0) {
error_free(s->migration_blocker); error_free(s->migration_blocker);

View File

@ -166,6 +166,8 @@ static int check_constraints_on_bitmap(BlockDriverState *bs,
int64_t len = bdrv_getlength(bs); int64_t len = bdrv_getlength(bs);
int64_t bitmap_bytes; int64_t bitmap_bytes;
assume_graph_lock(); /* FIXME */
assert(granularity > 0); assert(granularity > 0);
assert((granularity & (granularity - 1)) == 0); assert((granularity & (granularity - 1)) == 0);

View File

@ -2737,6 +2737,8 @@ static int qcow2_inactivate(BlockDriverState *bs)
int ret, result = 0; int ret, result = 0;
Error *local_err = NULL; Error *local_err = NULL;
assume_graph_lock(); /* FIXME */
qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err); qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err);
if (local_err != NULL) { if (local_err != NULL) {
result = -EINVAL; result = -EINVAL;
@ -5945,6 +5947,8 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
char *message; char *message;
va_list ap; va_list ap;
assume_graph_lock(); /* FIXME */
fatal = fatal && bdrv_is_writable(bs); fatal = fatal && bdrv_is_writable(bs);
if (s->signaled_corruption && if (s->signaled_corruption &&

View File

@ -1003,10 +1003,9 @@ int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp
bool qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, bool qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
bool release_stored, Error **errp); bool release_stored, Error **errp);
int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
bool coroutine_fn qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, bool coroutine_fn GRAPH_RDLOCK
const char *name, qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
uint32_t granularity, uint32_t granularity, Error **errp);
Error **errp);
int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
const char *name, const char *name,
Error **errp); Error **errp);

View File

@ -206,7 +206,7 @@ static void quorum_report_bad(QuorumOpType type, uint64_t offset,
end_sector - start_sector); end_sector - start_sector);
} }
static void quorum_report_failure(QuorumAIOCB *acb) static void GRAPH_RDLOCK quorum_report_failure(QuorumAIOCB *acb)
{ {
const char *reference = bdrv_get_device_or_node_name(acb->bs); const char *reference = bdrv_get_device_or_node_name(acb->bs);
int64_t start_sector = acb->offset / BDRV_SECTOR_SIZE; int64_t start_sector = acb->offset / BDRV_SECTOR_SIZE;
@ -219,7 +219,7 @@ static void quorum_report_failure(QuorumAIOCB *acb)
static int quorum_vote_error(QuorumAIOCB *acb); static int quorum_vote_error(QuorumAIOCB *acb);
static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb) static bool GRAPH_RDLOCK quorum_has_too_much_io_failed(QuorumAIOCB *acb)
{ {
BDRVQuorumState *s = acb->bs->opaque; BDRVQuorumState *s = acb->bs->opaque;

View File

@ -1208,6 +1208,8 @@ static int qemu_rbd_reopen_prepare(BDRVReopenState *state,
BDRVRBDState *s = state->bs->opaque; BDRVRBDState *s = state->bs->opaque;
int ret = 0; int ret = 0;
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (s->snap && state->flags & BDRV_O_RDWR) { if (s->snap && state->flags & BDRV_O_RDWR) {
error_setg(errp, error_setg(errp,
"Cannot change node '%s' to r/w when using RBD snapshot", "Cannot change node '%s' to r/w when using RBD snapshot",

View File

@ -432,6 +432,7 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
BlockDriver *drv = bs->drv; BlockDriver *drv = bs->drv;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!drv) { if (!drv) {
error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs)); error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
@ -641,8 +642,10 @@ int bdrv_all_goto_snapshot(const char *name,
} }
aio_context_release(ctx); aio_context_release(ctx);
if (ret < 0) { if (ret < 0) {
bdrv_graph_rdlock_main_loop();
error_prepend(errp, "Could not load snapshot '%s' on '%s': ", error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
name, bdrv_get_device_or_node_name(bs)); name, bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();
return -1; return -1;
} }

View File

@ -492,9 +492,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
} }
/* Disable migration when vdi images are used */ /* Disable migration when vdi images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The vdi format used by node '%s' " error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
"does not support live migration", "does not support live migration",
bdrv_get_device_or_node_name(bs)); bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();
ret = migrate_add_blocker(s->migration_blocker, errp); ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) { if (ret < 0) {
error_free(s->migration_blocker); error_free(s->migration_blocker);

View File

@ -446,9 +446,12 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
} }
/* Disable migration when VHD images are used */ /* Disable migration when VHD images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The vpc format used by node '%s' " error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
"does not support live migration", "does not support live migration",
bdrv_get_device_or_node_name(bs)); bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();
ret = migrate_add_blocker(s->migration_blocker, errp); ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) { if (ret < 0) {
error_free(s->migration_blocker); error_free(s->migration_blocker);

View File

@ -1144,6 +1144,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
QemuOpts *opts; QemuOpts *opts;
int ret; int ret;
GRAPH_RDLOCK_GUARD_MAINLOOP();
#ifdef DEBUG #ifdef DEBUG
vvv = s; vvv = s;
#endif #endif

View File

@ -485,6 +485,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
BlockJob *job; BlockJob *job;
int ret; int ret;
GLOBAL_STATE_CODE(); GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (job_id == NULL && !(flags & JOB_INTERNAL)) { if (job_id == NULL && !(flags & JOB_INTERNAL)) {
job_id = bdrv_get_device_name(bs); job_id = bdrv_get_device_name(bs);

View File

@ -183,8 +183,12 @@ const char *bdrv_get_format_name(BlockDriverState *bs);
bool bdrv_supports_compressed_writes(BlockDriverState *bs); bool bdrv_supports_compressed_writes(BlockDriverState *bs);
const char *bdrv_get_node_name(const BlockDriverState *bs); const char *bdrv_get_node_name(const BlockDriverState *bs);
const char *bdrv_get_device_name(const BlockDriverState *bs);
const char *bdrv_get_device_or_node_name(const BlockDriverState *bs); const char * GRAPH_RDLOCK
bdrv_get_device_name(const BlockDriverState *bs);
const char * GRAPH_RDLOCK
bdrv_get_device_or_node_name(const BlockDriverState *bs);
int coroutine_fn GRAPH_RDLOCK int coroutine_fn GRAPH_RDLOCK
bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);

View File

@ -99,7 +99,7 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
*/ */
void bdrv_wakeup(BlockDriverState *bs); void bdrv_wakeup(BlockDriverState *bs);
const char *bdrv_get_parent_name(const BlockDriverState *bs); const char * GRAPH_RDLOCK bdrv_get_parent_name(const BlockDriverState *bs);
bool blk_dev_has_tray(BlockBackend *blk); bool blk_dev_has_tray(BlockBackend *blk);
bool blk_dev_is_tray_open(BlockBackend *blk); bool blk_dev_is_tray_open(BlockBackend *blk);

View File

@ -33,9 +33,10 @@ BlockDeviceInfo * GRAPH_RDLOCK
bdrv_block_device_info(BlockBackend *blk, BlockDriverState *bs, bdrv_block_device_info(BlockBackend *blk, BlockDriverState *bs,
bool flat, Error **errp); bool flat, Error **errp);
int bdrv_query_snapshot_info_list(BlockDriverState *bs, int GRAPH_RDLOCK
SnapshotInfoList **p_list, bdrv_query_snapshot_info_list(BlockDriverState *bs,
Error **errp); SnapshotInfoList **p_list,
Error **errp);
void GRAPH_RDLOCK void GRAPH_RDLOCK
bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, bool flat, bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, bool flat,
bool skip_implicit_filters, Error **errp); bool skip_implicit_filters, Error **errp);