Am 08.05.2025 um 16:09 hat Fiona Ebner geschrieben:
> This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
>
> More granular draining is not trivially possible, because
> bdrv_inactivate_recurse() can recursively call itself.
>
> Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
> ---
> block.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index c7c26533c9..10052027cd 100644
> --- a/block.c
> +++ b/block.c
> @@ -6991,6 +6991,8 @@ bdrv_inactivate_recurse(BlockDriverState *bs, bool
> top_level)
> int ret;
> uint64_t cumulative_perms, cumulative_shared_perms;
>
> + assert(qatomic_read(&bs->quiesce_counter) > 0);
> +
> GLOBAL_STATE_CODE();
>
> if (!bs->drv) {
> @@ -7036,9 +7038,7 @@ bdrv_inactivate_recurse(BlockDriverState *bs, bool
> top_level)
> return -EPERM;
> }
>
> - bdrv_drained_begin(bs);
> bs->open_flags |= BDRV_O_INACTIVE;
> - bdrv_drained_end(bs);
>
> /*
> * Update permissions, they may differ for inactive nodes.
> @@ -7063,14 +7063,20 @@ int bdrv_inactivate(BlockDriverState *bs, Error
> **errp)
> int ret;
>
> GLOBAL_STATE_CODE();
> - GRAPH_RDLOCK_GUARD_MAINLOOP();
> +
> + bdrv_drain_all_begin();
> + bdrv_graph_rdlock_main_loop();
>
> if (bdrv_has_bds_parent(bs, true)) {
> error_setg(errp, "Node has active parent node");
> + bdrv_graph_rdunlock_main_loop();
> + bdrv_drain_all_end();
> return -EPERM;
> }
>
> ret = bdrv_inactivate_recurse(bs, true);
> + bdrv_graph_rdunlock_main_loop();
> + bdrv_drain_all_end();
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Failed to inactivate node");
> return ret;
This might be another one that would become a bit clearer if you put the
unlock/drain_end at the end and use a goto out for the error cases.
Kevin
