On Mon, Sep 11, 2023 at 11:46:04AM +0200, Kevin Wolf wrote: > bdrv_unref() is called by a lot of places that need to hold the graph > lock (it naturally happens in the context of operations that change the > graph). However, bdrv_unref() takes the graph writer lock internally, so > it can't actually be called while already holding a graph lock without > causing a deadlock. > > bdrv_unref() also can't just become GRAPH_WRLOCK because it drains the > node before closing it, and draining requires that the graph is > unlocked. > > The solution is to defer deleting the node until we don't hold the lock > any more and draining is possible again. > > Note that keeping images open for longer than necessary can create > problems, too: You can't open an image again before it is really closed > (if image locking didn't prevent it, it would cause corruption). > Reopening an image immediately happens at least during bdrv_open() and > bdrv_co_create(). > > In order to solve this problem, make sure to run the deferred unref in > bdrv_graph_wrunlock(), i.e. the first possible place where we can drain > again. This is also why bdrv_schedule_unref() is marked GRAPH_WRLOCK. > > The output of iotest 051 is updated because the additional polling > changes the order of HMP output, resulting in a new "(qemu)" prompt in > the test output that was previously on a separate line and filtered out. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > include/block/block-global-state.h | 1 + > block.c | 17 +++++++++++++++++ > block/graph-lock.c | 26 +++++++++++++++++++------- > tests/qemu-iotests/051.pc.out | 6 +++--- > 4 files changed, 40 insertions(+), 10 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature
