Am 26.05.2025 um 15:21 hat Fiona Ebner geschrieben:
> This is part of resolving the deadlock mentioned in commit "block:
> move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK".
>
> The function bdrv_set_backing_hd_drained() holds the graph lock, so it
> is not allowed to drain. It is called by:
> 1. bdrv_set_backing_hd(), where a drained section is introduced,
> replacing the previously present bs-specific drains.
> 2. stream_prepare(), where a drained section is introduced replacing
> the previously present bs-specific drains.
>
> Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
> ---
>
> No changes in v3.
>
> block.c | 6 ++----
> block/stream.c | 6 ++----
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/block.c b/block.c
> index 64db71e38d..75322789b5 100644
> --- a/block.c
> +++ b/block.c
> @@ -3569,7 +3569,6 @@ int bdrv_set_backing_hd_drained(BlockDriverState *bs,
> assert(bs->backing->bs->quiesce_counter > 0);
> }
>
> - bdrv_drain_all_begin();
> ret = bdrv_set_file_or_backing_noperm(bs, backing_hd, true, tran, errp);
> if (ret < 0) {
> goto out;
> @@ -3578,7 +3577,6 @@ int bdrv_set_backing_hd_drained(BlockDriverState *bs,
> ret = bdrv_refresh_perms(bs, tran, errp);
> out:
> tran_finalize(tran, ret);
> - bdrv_drain_all_end();
> return ret;
> }
Do we need to update the comment for bdrv_set_backing_hd_drained()?
* If a backing child is already present (i.e. we're detaching a node), that
* child node must be drained.
Same as in the previous patch, this is now probably all nodes.
> @@ -3594,11 +3592,11 @@ int bdrv_set_backing_hd(BlockDriverState *bs,
> BlockDriverState *backing_hd,
> bdrv_graph_rdunlock_main_loop();
>
> bdrv_ref(drain_bs);
> - bdrv_drained_begin(drain_bs);
> + bdrv_drain_all_begin();
> bdrv_graph_wrlock();
> ret = bdrv_set_backing_hd_drained(bs, backing_hd, errp);
> bdrv_graph_wrunlock();
> - bdrv_drained_end(drain_bs);
> + bdrv_drain_all_end();
> bdrv_unref(drain_bs);
The only thing we do with drain_bs now is finding it, bdrv_ref() and
immediately bdrv_unref(). I don't think it should exist any more after
the change to drain_all.
Kevin
