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(+)
From: Denis V. Lunev <den@openvz.org>
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.
Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Fiona Ebner <f.ebner@proxmox.com>
Cc: Kevin Wolf <kwolf@redhat.com>
---
Notes:
The iterator was correct from its introduction in 2016 until 2024:
it->bs was written only by the second phase, and the node to unreference
came from the BlockBackend instead.
f6d38c9f6d's own reproducer does not fire for me, 240 runs across 8
timings, even with its fix reverted. Tried hard.
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 37ba7e9fc4..55497c4551 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 664157bfd0..ef876ae6be 100755
--- a/tests/qemu-iotests/tests/inactive-node-nbd
+++ b/tests/qemu-iotests/tests/inactive-node-nbd
@@ -47,6 +47,7 @@ def node_is_active(_vm, node_name):
return node['active']
with iotests.FilePath('disk.img') as 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, \
@@ -58,6 +59,7 @@ with iotests.FilePath('disk.img') as path, \
iotests.log('Preparing disk...')
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, detached_path, img_size)
iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt,
snap_path)
@@ -68,6 +70,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=snap-file,filename={snap_path}')
@@ -297,6 +302,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 a458b4fc05..48451dc0c5 100644
--- a/tests/qemu-iotests/tests/inactive-node-nbd.out
+++ b/tests/qemu-iotests/tests/inactive-node-nbd.out
@@ -235,5 +235,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.53.0Am 29.08.2026 um 00:20 hat Denis V. Lunev geschrieben:
> From: Denis V. Lunev <den@openvz.org>
>
> 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.
>
> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Fiona Ebner <f.ebner@proxmox.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
Thanks, applied to the block branch.
Kevin
On 8/29/26 00:20, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> From: Denis V. Lunev <den@openvz.org>
>
> 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.
>
> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Fiona Ebner <f.ebner@proxmox.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> ---
>
> Notes:
> The iterator was correct from its introduction in 2016 until 2024:
> it->bs was written only by the second phase, and the node to unreference
> came from the BlockBackend instead.
>
> f6d38c9f6d's own reproducer does not fire for me, 240 runs across 8
> timings, even with its fix reverted. Tried hard.
>
> 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 37ba7e9fc4..55497c4551 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 664157bfd0..ef876ae6be 100755
> --- a/tests/qemu-iotests/tests/inactive-node-nbd
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd
> @@ -47,6 +47,7 @@ def node_is_active(_vm, node_name):
> return node['active']
>
> with iotests.FilePath('disk.img') as 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, \
> @@ -58,6 +59,7 @@ with iotests.FilePath('disk.img') as path, \
> iotests.log('Preparing disk...')
> 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, detached_path, img_size)
>
> iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt,
> snap_path)
> @@ -68,6 +70,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=snap-file,filename={snap_path}')
> @@ -297,6 +302,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 a458b4fc05..48451dc0c5 100644
> --- a/tests/qemu-iotests/tests/inactive-node-nbd.out
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd.out
> @@ -235,5 +235,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...
>
ping
On 8/29/26 00:20, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> From: Denis V. Lunev <den@openvz.org>
>
> 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.
>
> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Fiona Ebner <f.ebner@proxmox.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> ---
>
> Notes:
> The iterator was correct from its introduction in 2016 until 2024:
> it->bs was written only by the second phase, and the node to unreference
> came from the BlockBackend instead.
>
> f6d38c9f6d's own reproducer does not fire for me, 240 runs across 8
> timings, even with its fix reverted. Tried hard.
>
> 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 37ba7e9fc4..55497c4551 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 664157bfd0..ef876ae6be 100755
> --- a/tests/qemu-iotests/tests/inactive-node-nbd
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd
> @@ -47,6 +47,7 @@ def node_is_active(_vm, node_name):
> return node['active']
>
> with iotests.FilePath('disk.img') as 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, \
> @@ -58,6 +59,7 @@ with iotests.FilePath('disk.img') as path, \
> iotests.log('Preparing disk...')
> 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, detached_path, img_size)
>
> iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt,
> snap_path)
> @@ -68,6 +70,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=snap-file,filename={snap_path}')
> @@ -297,6 +302,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 a458b4fc05..48451dc0c5 100644
> --- a/tests/qemu-iotests/tests/inactive-node-nbd.out
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd.out
> @@ -235,5 +235,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...
>
ping
On 8/29/26 00:20, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> From: Denis V. Lunev <den@openvz.org>
>
> 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.
>
> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Fiona Ebner <f.ebner@proxmox.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> ---
>
> Notes:
> The iterator was correct from its introduction in 2016 until 2024:
> it->bs was written only by the second phase, and the node to unreference
> came from the BlockBackend instead.
>
> f6d38c9f6d's own reproducer does not fire for me, 240 runs across 8
> timings, even with its fix reverted. Tried hard.
>
> 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 37ba7e9fc4..55497c4551 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 664157bfd0..ef876ae6be 100755
> --- a/tests/qemu-iotests/tests/inactive-node-nbd
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd
> @@ -47,6 +47,7 @@ def node_is_active(_vm, node_name):
> return node['active']
>
> with iotests.FilePath('disk.img') as 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, \
> @@ -58,6 +59,7 @@ with iotests.FilePath('disk.img') as path, \
> iotests.log('Preparing disk...')
> 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, detached_path, img_size)
>
> iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt,
> snap_path)
> @@ -68,6 +70,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=snap-file,filename={snap_path}')
> @@ -297,6 +302,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 a458b4fc05..48451dc0c5 100644
> --- a/tests/qemu-iotests/tests/inactive-node-nbd.out
> +++ b/tests/qemu-iotests/tests/inactive-node-nbd.out
> @@ -235,5 +235,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...
>
Kevin,
have you had a chance to take a look?
Den
Hi Denis,
Am 29.08.26 um 12:20 AM schrieb Denis V. Lunev:
> From: Denis V. Lunev <den@openvz.org>
>
> 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.
>
> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Fiona Ebner <f.ebner@proxmox.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
thank you for the fix!
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Should also go into stable I suppose?
> ---
>
> Notes:
> The iterator was correct from its introduction in 2016 until 2024:
> it->bs was written only by the second phase, and the node to unreference
> came from the BlockBackend instead.
>
> f6d38c9f6d's own reproducer does not fire for me, 240 runs across 8
> timings, even with its fix reverted. Tried hard.
The reproducer has a very short time window. I'm still able to trigger
an assertion failure with commit f6d38c9f6d reverted, by putting in some
'query-version' commands in between the 'block-stream' and 'quit'
commands, and running the script in a loop, e.g.:
[I] root@pve9a1 ~# cat ~/block-stream.sh
#!/bin/bash
rm -f /tmp/backing.qcow2
rm -f /tmp/top.qcow2
./qemu-img create /tmp/backing.qcow2 -f qcow2 64M
./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2
./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2
./qemu-system-x86_64 --qmp stdio \
--blockdev
qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \
<<EOF
{"execute": "qmp_capabilities"}
{"execute": "block-stream", "arguments": { "job-id": "stream0",
"device": "node0" } }
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "query-version"}
{"execute": "quit"}
EOF
Best Regards,
Fiona
On 9/1/26 14:23, Fiona Ebner wrote:
> Hi Denis,
>
> Am 29.08.26 um 12:20 AM schrieb Denis V. Lunev:
>> From: Denis V. Lunev <den@openvz.org>
>>
>> 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.
>>
>> Fixes: f6d38c9f6d ("block-backend: fix edge case in bdrv_next() where BDS associated to BB changes")
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Cc: Fiona Ebner <f.ebner@proxmox.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
> thank you for the fix!
>
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
>
> Should also go into stable I suppose?
Unsure. We do not know real consequences, there are no reports.
I found this problem by accident only testing migration with
big trees a lot.
Den
© 2016 - 2026 Red Hat, Inc.