Re: [PATCH 05/11] block: move drain outside of read-locked bdrv_inactivate_recurse()

[Kevin Wolf <kwolf@redhat.com>]

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.ebner@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