From 221caadcc5129d3ec5ad9ecfd7374de0f7050316 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:10 +0200 Subject: block: Mark bdrv_probe_blocksizes() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_probe_blocksizes() need to hold a reader lock for the graph because it calls bdrv_filter_bs(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-2-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 2 +- include/block/block_int-common.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 6bfafe7..fca0a40 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -281,7 +281,7 @@ bool bdrv_child_change_aio_context(BdrvChild *c, AioContext *ctx, int bdrv_try_change_aio_context(BlockDriverState *bs, AioContext *ctx, BdrvChild *ignore_child, Error **errp); -int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz); +int GRAPH_RDLOCK bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz); int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo); void GRAPH_WRLOCK diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index b8d9d24..8abdd27 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -386,7 +386,8 @@ struct BlockDriver { * On success, store them in @bsz and return zero. * On failure, return negative errno. */ - int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz); + int GRAPH_RDLOCK_PTR (*bdrv_probe_blocksizes)( + BlockDriverState *bs, BlockSizes *bsz); /** * Try to get @bs's geometry (cyls, heads, sectors) * On success, store them in @geo and return 0. -- cgit v1.1 From 067179868ec8cd467d9810143339e882cb60e388 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:11 +0200 Subject: block: Mark bdrv_has_zero_init() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_has_zero_init() need to hold a reader lock for the graph because it calls bdrv_filter_bs(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-3-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 2 +- include/block/block_int-common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index fca0a40..3ae468e 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -189,7 +189,7 @@ void bdrv_drain_all(void); void bdrv_aio_cancel(BlockAIOCB *acb); int bdrv_has_zero_init_1(BlockDriverState *bs); -int bdrv_has_zero_init(BlockDriverState *bs); +int coroutine_mixed_fn GRAPH_RDLOCK bdrv_has_zero_init(BlockDriverState *bs); BlockDriverState *bdrv_find_node(const char *node_name); BlockDeviceInfoList *bdrv_named_nodes_list(bool flat, Error **errp); XDbgBlockGraph * GRAPH_RDLOCK bdrv_get_xdbg_block_graph(Error **errp); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 8abdd27..c0db862 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -349,7 +349,7 @@ struct BlockDriver { * Returns 1 if newly created images are guaranteed to contain only * zeros, 0 otherwise. */ - int (*bdrv_has_zero_init)(BlockDriverState *bs); + int GRAPH_RDLOCK_PTR (*bdrv_has_zero_init)(BlockDriverState *bs); /* * Remove fd handlers, timers, and other event loop callbacks so the event -- cgit v1.1 From f5a3a270fe2b89d1be39f50ebdd15db80d59014b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:12 +0200 Subject: block: Mark bdrv_filter_bs() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_bs() 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 Message-ID: <20231027155333.420094-4-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-io.h | 2 +- include/block/block_int-io.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block-io.h b/include/block/block-io.h index ad270b6..58c4cf5 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -183,7 +183,7 @@ bdrv_co_eject(BlockDriverState *bs, bool eject_flag); const char *bdrv_get_format_name(BlockDriverState *bs); -bool bdrv_supports_compressed_writes(BlockDriverState *bs); +bool GRAPH_RDLOCK bdrv_supports_compressed_writes(BlockDriverState *bs); const char *bdrv_get_node_name(const BlockDriverState *bs); const char * GRAPH_RDLOCK diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 34eac72..26bff94 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -143,7 +143,8 @@ static inline BlockDriverState *bdrv_cow_bs(BlockDriverState *bs) return child_bs(bdrv_cow_child(bs)); } -static inline BlockDriverState *bdrv_filter_bs(BlockDriverState *bs) +static inline BlockDriverState * GRAPH_RDLOCK +bdrv_filter_bs(BlockDriverState *bs) { IO_CODE(); return child_bs(bdrv_filter_child(bs)); -- cgit v1.1 From 03b9eaca540322dd6dd4810aa57f3196ce971542 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:13 +0200 Subject: block: Mark bdrv_root_attach_child() GRAPH_WRLOCK Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_root_attach_child(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-5-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-global-state.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'include') diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h index 074b677..afce6c4 100644 --- a/include/block/block_int-global-state.h +++ b/include/block/block_int-global-state.h @@ -196,12 +196,13 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque, JobTxn *txn, Error **errp); -BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, - const char *child_name, - const BdrvChildClass *child_class, - BdrvChildRole child_role, - uint64_t perm, uint64_t shared_perm, - void *opaque, Error **errp); +BdrvChild * GRAPH_WRLOCK +bdrv_root_attach_child(BlockDriverState *child_bs, const char *child_name, + const BdrvChildClass *child_class, + BdrvChildRole child_role, + uint64_t perm, uint64_t shared_perm, + void *opaque, Error **errp); + void GRAPH_WRLOCK bdrv_root_unref_child(BdrvChild *child); void GRAPH_RDLOCK bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, -- cgit v1.1 From f3bbc53dc56c5d410f76442da6ad15ec8f9439fc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:14 +0200 Subject: block: Mark block_job_add_bdrv() GRAPH_WRLOCK Instead of taking the writer lock internally, require callers to already hold it when calling block_job_add_bdrv(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-6-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/blockjob.h | 5 +++-- include/block/blockjob_int.h | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'include') diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 95854f1..e594c10 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -138,8 +138,9 @@ BlockJob *block_job_get_locked(const char *id); * @job. This means that all operations will be blocked on @bs while * @job exists. */ -int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, - uint64_t perm, uint64_t shared_perm, Error **errp); +int GRAPH_WRLOCK +block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, + uint64_t perm, uint64_t shared_perm, Error **errp); /** * block_job_remove_all_bdrv: diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 18ee6f7..4c3d2e2 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -111,10 +111,11 @@ struct BlockJobDriver { * This function is not part of the public job interface; it should be * called from a wrapper that is specific to the job type. */ -void *block_job_create(const char *job_id, const BlockJobDriver *driver, - JobTxn *txn, BlockDriverState *bs, uint64_t perm, - uint64_t shared_perm, int64_t speed, int flags, - BlockCompletionFunc *cb, void *opaque, Error **errp); +void * GRAPH_UNLOCKED +block_job_create(const char *job_id, const BlockJobDriver *driver, + JobTxn *txn, BlockDriverState *bs, uint64_t perm, + uint64_t shared_perm, int64_t speed, int flags, + BlockCompletionFunc *cb, void *opaque, Error **errp); /** * block_job_free: -- cgit v1.1 From 372b69f503d47eb6619a98cac2ab5a6a569e3483 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:15 +0200 Subject: block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-7-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-io.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 26bff94..6800af7 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -150,7 +150,8 @@ bdrv_filter_bs(BlockDriverState *bs) return child_bs(bdrv_filter_child(bs)); } -static inline BlockDriverState *bdrv_filter_or_cow_bs(BlockDriverState *bs) +static inline BlockDriverState * GRAPH_RDLOCK +bdrv_filter_or_cow_bs(BlockDriverState *bs) { IO_CODE(); return child_bs(bdrv_filter_or_cow_child(bs)); -- cgit v1.1 From 430da832afb6a6eebb7c1726991c60fb06322d3e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:16 +0200 Subject: block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK 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 Message-ID: <20231027155333.420094-8-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-global-state.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h index afce6c4..ef31c58 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: -- cgit v1.1 From ad74751fc0ffdad7678224df0e752689ebb3f4b7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:17 +0200 Subject: block: Mark bdrv_skip_filters() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_skip_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 Message-ID: <20231027155333.420094-9-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 8 +++++--- include/block/block_int-io.h | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 3ae468e..b6860ae 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -144,9 +144,11 @@ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, void bdrv_register(BlockDriver *bdrv); int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, const char *backing_file_str); -BlockDriverState *bdrv_find_overlay(BlockDriverState *active, - BlockDriverState *bs); -BlockDriverState *bdrv_find_base(BlockDriverState *bs); + +BlockDriverState * GRAPH_RDLOCK +bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs); + +BlockDriverState * GRAPH_RDLOCK bdrv_find_base(BlockDriverState *bs); bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, Error **errp); int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 6800af7..4e7bf57 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -134,8 +134,8 @@ BdrvChild *bdrv_cow_child(BlockDriverState *bs); BdrvChild *bdrv_filter_child(BlockDriverState *bs); BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs); BdrvChild * GRAPH_RDLOCK bdrv_primary_child(BlockDriverState *bs); -BlockDriverState *bdrv_skip_filters(BlockDriverState *bs); -BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs); +BlockDriverState * GRAPH_RDLOCK bdrv_skip_filters(BlockDriverState *bs); +BlockDriverState * GRAPH_RDLOCK bdrv_backing_chain_next(BlockDriverState *bs); static inline BlockDriverState *bdrv_cow_bs(BlockDriverState *bs) { -- cgit v1.1 From 9275fc72bd0a1c35e915e4991c7d27209ecab923 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:18 +0200 Subject: block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_(un)freeze_backing_chain() need to hold a reader lock for the graph because it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing. Use the opportunity to make bdrv_is_backing_chain_frozen() static, it has no external callers. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-10-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index b6860ae..545708c 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -149,11 +149,12 @@ BlockDriverState * GRAPH_RDLOCK bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs); BlockDriverState * GRAPH_RDLOCK bdrv_find_base(BlockDriverState *bs); -bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, - Error **errp); -int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, - Error **errp); -void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); + +int GRAPH_RDLOCK +bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, + Error **errp); +void GRAPH_RDLOCK +bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); /* * The units of offset and total_work_size may be chosen arbitrarily by the -- cgit v1.1 From 79bb76272763c090f2b1dae9519c516257241cac Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:19 +0200 Subject: 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 Message-ID: <20231027155333.420094-11-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 545708c..9a33bd7 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -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); -- cgit v1.1 From ec82cc41a79ffa8b1a2dee76a420a34a59f117c6 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:20 +0200 Subject: block: Mark bdrv_filter_child() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_filter_child() need to hold a reader lock for the graph because it accesses bs->file/backing. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-12-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-io.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 4e7bf57..17547a2 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -131,8 +131,8 @@ int co_wrapper_mixed_bdrv_rdlock bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint); BdrvChild *bdrv_cow_child(BlockDriverState *bs); -BdrvChild *bdrv_filter_child(BlockDriverState *bs); -BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs); +BdrvChild * GRAPH_RDLOCK bdrv_filter_child(BlockDriverState *bs); +BdrvChild * GRAPH_RDLOCK bdrv_filter_or_cow_child(BlockDriverState *bs); BdrvChild * GRAPH_RDLOCK bdrv_primary_child(BlockDriverState *bs); BlockDriverState * GRAPH_RDLOCK bdrv_skip_filters(BlockDriverState *bs); BlockDriverState * GRAPH_RDLOCK bdrv_backing_chain_next(BlockDriverState *bs); -- cgit v1.1 From 78a9c76eefae877e63591728234604310c51d88f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:21 +0200 Subject: block: Mark bdrv_cow_child() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_cow_child() need to hold a reader lock for the graph because it accesses bs->backing. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-13-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-io.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 17547a2..4a7cf2b 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -130,14 +130,15 @@ bdrv_co_refresh_total_sectors(BlockDriverState *bs, int64_t hint); int co_wrapper_mixed_bdrv_rdlock bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint); -BdrvChild *bdrv_cow_child(BlockDriverState *bs); +BdrvChild * GRAPH_RDLOCK bdrv_cow_child(BlockDriverState *bs); BdrvChild * GRAPH_RDLOCK bdrv_filter_child(BlockDriverState *bs); BdrvChild * GRAPH_RDLOCK bdrv_filter_or_cow_child(BlockDriverState *bs); BdrvChild * GRAPH_RDLOCK bdrv_primary_child(BlockDriverState *bs); BlockDriverState * GRAPH_RDLOCK bdrv_skip_filters(BlockDriverState *bs); BlockDriverState * GRAPH_RDLOCK bdrv_backing_chain_next(BlockDriverState *bs); -static inline BlockDriverState *bdrv_cow_bs(BlockDriverState *bs) +static inline BlockDriverState * GRAPH_RDLOCK +bdrv_cow_bs(BlockDriverState *bs) { IO_CODE(); return child_bs(bdrv_cow_child(bs)); -- cgit v1.1 From d0f9fd94d92c15c6ab7f6b8855acd812b80dbbaa Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:22 +0200 Subject: block: Mark bdrv_set_backing_hd_drained() GRAPH_WRLOCK Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_set_backing_hd_drained(). Basically everthing in the function needs the lock and its callers may already want to hold the graph lock and so wouldn't be able to call functions that take it internally. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-14-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 9a33bd7..a1fd70e 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -101,9 +101,10 @@ bdrv_co_open_blockdev_ref(BlockdevRef *ref, Error **errp); int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, Error **errp); -int bdrv_set_backing_hd_drained(BlockDriverState *bs, - BlockDriverState *backing_hd, - Error **errp); +int GRAPH_WRLOCK +bdrv_set_backing_hd_drained(BlockDriverState *bs, BlockDriverState *backing_hd, + Error **errp); + int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, const char *bdref_key, Error **errp); -- cgit v1.1 From ccd6a37947574707613e826e2bf04d55f1d5f238 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:25 +0200 Subject: block: Mark bdrv_replace_node() GRAPH_WRLOCK Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_replace_node(). Its callers may already want to hold the graph lock and so wouldn't be able to call functions that take it internally. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-17-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index a1fd70e..9e0ccc1 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -71,8 +71,10 @@ bdrv_co_create_file(const char *filename, QemuOpts *opts, Error **errp); BlockDriverState *bdrv_new(void); int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, Error **errp); -int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, - Error **errp); + +int GRAPH_WRLOCK +bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, Error **errp); + int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, Error **errp); BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options, -- cgit v1.1 From 004915a96a7a40e942ac85e6d22518cbcd283506 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:26 +0200 Subject: block: Protect bs->backing with graph_lock Almost all functions that access bs->backing already take the graph lock now. Add locking to the remaining users and finally annotate the struct field itself as protected by the graph lock. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-18-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index c0db862..ed60669 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -1178,7 +1178,7 @@ struct BlockDriverState { * are connected with BdrvChildRole. */ QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) children; - BdrvChild *backing; + BdrvChild * GRAPH_RDLOCK_PTR backing; BdrvChild *file; QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) parents; -- cgit v1.1 From e2dd273754eb9a47c33660b4e14074e8e96ada4d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:28 +0200 Subject: block: Introduce bdrv_co_change_backing_file() bdrv_change_backing_file() is called both inside and outside coroutine context. This makes it difficult for it to take the graph lock internally. It also means that driver implementations need to be able to run outside of coroutines, too. Switch it to the usual model with a coroutine based implementation and a co_wrapper instead. The new function is marked GRAPH_RDLOCK. As the co_wrapper now runs the function in the AioContext of the node (as it should always have done), this is not GLOBAL_STATE_CODE() any more. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-20-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block-global-state.h | 3 +-- include/block/block-io.h | 8 ++++++++ include/block/block_int-common.h | 5 +++-- 3 files changed, 12 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 9e0ccc1..6b21fbc 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -142,8 +142,7 @@ bdrv_refresh_limits(BlockDriverState *bs, Transaction *tran, Error **errp); int bdrv_commit(BlockDriverState *bs); int GRAPH_RDLOCK bdrv_make_empty(BdrvChild *c, Error **errp); -int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, - const char *backing_fmt, bool warn); + void bdrv_register(BlockDriver *bdrv); int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, const char *backing_file_str); diff --git a/include/block/block-io.h b/include/block/block-io.h index 58c4cf5..f8729cc 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -210,6 +210,14 @@ void bdrv_round_to_subclusters(BlockDriverState *bs, void bdrv_get_backing_filename(BlockDriverState *bs, char *filename, int filename_size); +int coroutine_fn GRAPH_RDLOCK +bdrv_co_change_backing_file(BlockDriverState *bs, const char *backing_file, + const char *backing_fmt, bool warn); + +int co_wrapper_bdrv_rdlock +bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, + const char *backing_fmt, bool warn); + int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, int64_t pos, int size); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index ed60669..59f6d7f 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -331,8 +331,9 @@ struct BlockDriver { const char *name, Error **errp); - int (*bdrv_change_backing_file)(BlockDriverState *bs, - const char *backing_file, const char *backing_fmt); + int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_change_backing_file)( + BlockDriverState *bs, const char *backing_file, + const char *backing_fmt); /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */ int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event, -- cgit v1.1 From 79a558664840adf502fe94907b0a680836e3e98e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:29 +0200 Subject: block: Add missing GRAPH_RDLOCK annotations This adds GRAPH_RDLOCK to some driver callbacks that are already called with the graph lock held, and which will need the annotation because they access bs->file, but don't have it yet. This also covers a few callbacks that were not marked GRAPH_RDLOCK before, but where updating BlockDriver is trivially possible. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-21-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-common.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 59f6d7f..63bc523 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -310,7 +310,7 @@ struct BlockDriver { * One example usage is to avoid waiting for an nbd target node reconnect * timeout during job-cancel with force=true. */ - void (*bdrv_cancel_in_flight)(BlockDriverState *bs); + void GRAPH_RDLOCK_PTR (*bdrv_cancel_in_flight)(BlockDriverState *bs); int GRAPH_RDLOCK_PTR (*bdrv_inactivate)(BlockDriverState *bs); @@ -324,12 +324,12 @@ struct BlockDriver { BlockDriverState *bs, const char *snapshot_id, const char *name, Error **errp); - int (*bdrv_snapshot_list)(BlockDriverState *bs, - QEMUSnapshotInfo **psn_info); - int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs, - const char *snapshot_id, - const char *name, - Error **errp); + int GRAPH_RDLOCK_PTR (*bdrv_snapshot_list)( + BlockDriverState *bs, QEMUSnapshotInfo **psn_info); + + int GRAPH_RDLOCK_PTR (*bdrv_snapshot_load_tmp)( + BlockDriverState *bs, const char *snapshot_id, const char *name, + Error **errp); int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_change_backing_file)( BlockDriverState *bs, const char *backing_file, @@ -396,7 +396,8 @@ struct BlockDriver { * Only drivers that want to override guest geometry implement this * callback; see hd_geometry_guess(). */ - int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo); + int GRAPH_RDLOCK_PTR (*bdrv_probe_geometry)( + BlockDriverState *bs, HDGeometry *geo); void GRAPH_WRLOCK_PTR (*bdrv_add_child)( BlockDriverState *parent, BlockDriverState *child, Error **errp); -- cgit v1.1 From 1f051dcbdf2e4b6f518db731c84e304b2b9d15ce Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:33 +0200 Subject: block: Protect bs->file with graph_lock Almost all functions that access bs->file already take the graph lock now. Add locking to the remaining users and finally annotate the struct field itself as protected by the graph lock. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-25-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/block_int-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 63bc523..4e31d16 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -1181,7 +1181,7 @@ struct BlockDriverState { */ QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) children; BdrvChild * GRAPH_RDLOCK_PTR backing; - BdrvChild *file; + BdrvChild * GRAPH_RDLOCK_PTR file; QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) parents; -- cgit v1.1