On Mon, Feb 01, 2021 at 01:06:31PM +0100, Kevin Wolf wrote:
> Am 21.01.2021 um 18:06 hat Sergio Lopez geschrieben:
> > Some graphs may contain an indirect reference to the first BDS in the
> > chain that can be reached while walking it bottom->up from one its
> > children.
> >
> > Doubling-processing of a BDS is especially problematic for the
> > aio_notifiers, as they might attempt to work on both the old and the
> > new AIO contexts.
> >
> > To avoid this problem, add every child and parent to the ignore list
> > before actually processing them.
> >
> > Suggested-by: Kevin Wolf <kw...@redhat.com>
> > Signed-off-by: Sergio Lopez <s...@redhat.com>
> > ---
> > block.c | 34 +++++++++++++++++++++++++++-------
> > 1 file changed, 27 insertions(+), 7 deletions(-)
>
> The patch looks correct to me, I'm just wondering about one thing:
>
> > diff --git a/block.c b/block.c
> > index 8b9d457546..3da99312db 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -6414,7 +6414,10 @@ void bdrv_set_aio_context_ignore(BlockDriverState
> > *bs,
> > AioContext *new_context, GSList **ignore)
> > {
> > AioContext *old_context = bdrv_get_aio_context(bs);
> > - BdrvChild *child;
> > + GSList *children_to_process = NULL;
> > + GSList *parents_to_process = NULL;
>
> Why do we need these separate lists? Can't we just iterate over
> bs->parents/children a second time? I don't think the graph can change
> between the first and the second loop (and if it could, the result would
> be broken anyway).It's not strictly needed, but this makes the code more readable by making our intentions clearer. To my eyes, at least. Sergio.
signature.asc
Description: PGP signature
