From: "Denis V. Lunev" <[email protected]> it->bs is the cursor into monitor_bdrv_states for the second phase of bdrv_next(), so it has to be NULL when that phase starts. Commit f6d38c9f6d made the first phase store the node it returns there, to unreference the right one when the graph changes underneath. The cursor is now left pointing at the last BlockBackend root, so the second phase resumes from there instead of from the head of the list and never returns a node added before it.
A skipped node drops out of the vm_stop and migration handover paths: not flushed, not inactivated, not snapshotted. bdrv_inactivate_all() still reports success, so the node keeps its image lock and the migration target cannot open the image. Only detached nodes are hit in practice, as these callers also recurse into children. Reset the cursor when the second phase starts. old_bs is taken at the top of the function, so f6d38c9f6d keeps working. Cc: [email protected] Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes") Signed-off-by: Denis V. Lunev <[email protected]> Cc: Fiona Ebner <[email protected]> Cc: Kevin Wolf <[email protected]> Message-ID: <[email protected]> Reviewed-by: Fiona Ebner <[email protected]> Tested-by: Fiona Ebner <[email protected]> Reviewed-by: Kevin Wolf <[email protected]> Signed-off-by: Kevin Wolf <[email protected]> --- block/block-backend.c | 1 + tests/qemu-iotests/tests/inactive-node-nbd | 16 ++++++++++++++++ tests/qemu-iotests/tests/inactive-node-nbd.out | 8 ++++++++ 3 files changed, 25 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 5e59bac1a3c..ab837fac921 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -625,6 +625,7 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it) return bs; } it->phase = BDRV_NEXT_MONITOR_OWNED; + it->bs = NULL; } /* Then return the monitor-owned BDSes without a BB attached. Ignore all diff --git a/tests/qemu-iotests/tests/inactive-node-nbd b/tests/qemu-iotests/tests/inactive-node-nbd index 24242265b16..bb6ee5c75ab 100755 --- a/tests/qemu-iotests/tests/inactive-node-nbd +++ b/tests/qemu-iotests/tests/inactive-node-nbd @@ -48,6 +48,7 @@ def node_is_active(_vm, node_name): with iotests.FilePath('disk.img') as path, \ iotests.FilePath('ro.img') as ro_path, \ + iotests.FilePath('detached.img') as detached_path, \ iotests.FilePath('snap.qcow2') as snap_path, \ iotests.FilePath('snap2.qcow2') as snap2_path, \ iotests.FilePath('target.img') as target_path, \ @@ -60,6 +61,7 @@ with iotests.FilePath('disk.img') as path, \ iotests.qemu_img_create('-f', iotests.imgfmt, path, img_size) iotests.qemu_img_create('-f', iotests.imgfmt, target_path, img_size) iotests.qemu_img_create('-f', iotests.imgfmt, ro_path, img_size) + iotests.qemu_img_create('-f', iotests.imgfmt, detached_path, img_size) iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt, snap_path) @@ -70,6 +72,9 @@ with iotests.FilePath('disk.img') as path, \ vm.add_blockdev(f'file,node-name=disk-file,filename={path}') vm.add_blockdev(f'{iotests.imgfmt},file=disk-file,node-name=disk-fmt,' 'active=off') + vm.add_blockdev(f'file,node-name=detached-file,filename={detached_path}') + vm.add_blockdev(f'{iotests.imgfmt},file=detached-file,' + 'node-name=detached-fmt') vm.add_blockdev(f'file,node-name=target-file,filename={target_path}') vm.add_blockdev(f'{iotests.imgfmt},file=target-file,node-name=target-fmt') vm.add_blockdev(f'file,node-name=ro-file,filename={ro_path},read-only=on') @@ -311,6 +316,17 @@ with iotests.FilePath('disk.img') as path, \ iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt')) + iotests.log('\n=== Inactivating all nodes at once ===') + + # detached-fmt has no parent and no BlockBackend, so nothing can reach it + # by recursion. It is only inactivated if bdrv_next() actually returns it. + vm.qmp_log('stop') + vm.qmp_log('blockdev-set-active', active=False) + + iotests.log('detached-fmt active: %s' % node_is_active(vm, 'detached-fmt')) + iotests.log('detached-file active: %s' + % node_is_active(vm, 'detached-file')) + iotests.log('\nShutting down...') vm.shutdown() log = vm.get_log() diff --git a/tests/qemu-iotests/tests/inactive-node-nbd.out b/tests/qemu-iotests/tests/inactive-node-nbd.out index 96af7608ded..5ff0063ec46 100644 --- a/tests/qemu-iotests/tests/inactive-node-nbd.out +++ b/tests/qemu-iotests/tests/inactive-node-nbd.out @@ -243,5 +243,13 @@ snap-fmt active: True snap2-fmt active: True target-fmt active: True +=== Inactivating all nodes at once === +{"execute": "stop", "arguments": {}} +{"return": {}} +{"execute": "blockdev-set-active", "arguments": {"active": false}} +{"return": {}} +detached-fmt active: False +detached-file active: False + Shutting down... -- 2.56.0
