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

[Kevin Wolf]

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