Re: [PATCH 17/21] block: Take graph rdlock in bdrv_drop_intermediate()

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On Thu, Aug 17, 2023 at 02:50:16PM +0200, Kevin Wolf wrote:
> The function reads the parents list, so it needs to hold the graph lock.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 7df8780d6e..a82389f742 100644
> --- a/block.c
> +++ b/block.c
> @@ -5934,9 +5934,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
>          backing_file_str = base->filename;
>      }
>  
> +    bdrv_graph_rdlock_main_loop();
>      QLIST_FOREACH(c, &top->parents, next_parent) {
>          updated_children = g_slist_prepend(updated_children, c);
>      }
> +    bdrv_graph_rdunlock_main_loop();

This is GLOBAL_STATE_CODE, so why take the read lock? I thought nothing
can modify the graph here. If it could, then stashing the parents in the
updated_children probably wouldn't be safe anyway.

>  
>      /*
>       * It seems correct to pass detach_subchain=true here, but it triggers
> -- 
> 2.41.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]