On Thu, Mar 24, 2022 at 03:09:07PM +0100, Hanna Reitz wrote: > When the stream block job cuts out the nodes between top and base in > stream_prepare(), it does not drain the subtree manually; it fetches the > base node, and tries to insert it as the top node's backing node with > bdrv_set_backing_hd(). bdrv_set_backing_hd() however will drain, and so > the actual base node might change (because the base node is actually not > part of the stream job) before the old base node passed to > bdrv_set_backing_hd() is installed. > ... > > Therefore, we need to keep the whole subtree drained in > stream_prepare(), so that the graph modification it performs is > effectively atomic, i.e. that the base node it fetches is still the base > node when bdrv_set_backing_hd() sets it as the top node's backing node. > > Verify this by asserting in said 030's test case that the base node is > always the top node's immediate backing child when both jobs are done. > > Signed-off-by: Hanna Reitz <hre...@redhat.com> > --- > v2: > - Oops, the base can be NULL. Would have noticed if I had ran all test > cases from 030, and not just test_overlapping_5()... > That means that keeping a strong reference to the base node must be > conditional, based on whether there even is a base node or not. > (I mean, technically we do not even need to keep a strong reference to > that node, given that we are in a drained section, but I believe it is > better style to do it anyway.)
I agree with the conclusion that we don't need a strong reference once you fix the bigger problem of the lock-by-drain, but that it is better style to include it anyway. > --- > block/stream.c | 15 ++++++++++++++- > tests/qemu-iotests/030 | 5 +++++ > 2 files changed, 19 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
