Re: [PATCH v2 29/36] blockdev: qmp_x_blockdev_reopen: acquire all contexts

[Vladimir Sementsov-Ogievskiy]

05.02.2021 19:01, Kevin Wolf wrote:
> Am 27.11.2020 um 15:45 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > During reopen we may add backing bs from other aio context, which may
> > lead to changing original context of top bs.
> >
> > We are going to move graph modification to prepare stage. So, it will
> > be possible that bdrv_flush() in bdrv_reopen_prepare called on bs in
> > non-original aio context, which we didn't aquire which leads to crash.
> >
> > More correct would be to acquire all aio context we are going to work
> > with. And the simplest ways is to just acquire all of them. It may be
> > optimized later if needed.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> I'm afraid it's not as easy. Holding the lock of more than one
> AioContext is always a bit risky with respect to deadlocks.
>
> For example, changing the AioContext of a node with
> bdrv_set_aio_context_ignore() has explicit rules that are now violated:
>
>   * The caller must own the AioContext lock for the old AioContext of bs, but it
>   * must not own the AioContext lock for new_context (unless new_context is the
>   * same as the current context of bs).
>
> Draining while holding all AioContext locks is suspicious, too. I think
> I have seen deadlocks before, which is why bdrv_drain_all_*() are
> careful to only ever lock a single AioContext at a time.
>
> Kevin

That's not good :\ Hmm, probably we just should flush everything before all 
graph modifications.

--
Best regards,
Vladimir