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

[Kevin Wolf <kwolf@redhat.com>]

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

Am 22.08.2023 um 21:35 hat Stefan Hajnoczi geschrieben:
> 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.

The only thing bdrv_graph_rdlock_main_loop() does is asserting that the
conditions for doing nothing are met (GLOBAL_STATE_CODE + non-coroutine
context) and providing the right TSA attributes to make the compiler
happy.

Kevin

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