Re: [PATCH v3 1/2] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()

[Kevin Wolf]

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 <kwolf@redhat.com>
> Signed-off-by: Sergio Lopez <slp@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).

Kevin