* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-08-28 22:20 [PATCH] block: fix bdrv_next() skipping monitor-owned nodes Denis V. Lunev
@ 2026-09-01 12:23 ` Fiona Ebner
2026-09-01 12:50 ` Denis V. Lunev
2026-09-07 10:45 ` Denis V. Lunev
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Fiona Ebner @ 2026-09-01 12:23 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel; +Cc: qemu-block, Kevin Wolf, qemu-stable
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-09-01 12:23 ` Fiona Ebner
@ 2026-09-01 12:50 ` Denis V. Lunev
2026-10-02 10:07 ` Erik Fastermann
0 siblings, 1 reply; 8+ messages in thread
From: Denis V. Lunev @ 2026-09-01 12:50 UTC (permalink / raw)
To: Fiona Ebner, Denis V. Lunev, qemu-devel
Cc: qemu-block, Kevin Wolf, qemu-stable
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-09-01 12:50 ` Denis V. Lunev
@ 2026-10-02 10:07 ` Erik Fastermann
0 siblings, 0 replies; 8+ messages in thread
From: Erik Fastermann @ 2026-10-02 10:07 UTC (permalink / raw)
To: Denis V. Lunev
Cc: Denis V. Lunev, Fiona Ebner, Kevin Wolf, qemu-devel, qemu-block,
qemu-stable
On 9/1/26 14:50, Denis V. Lunev wrote:
> On 9/1/26 14:23, Fiona Ebner wrote:
>> 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.
I came across the bug by accident while exploring the code.
It is easy to trigger with a plain -drive next to a detached -blockdev
node. If the last BlockBackend root is not monitor-owned, it is not on
monitor_bdrv_states, so bdrv_next_monitor_owned() returns NULL right
away and the second phase returns no node at all, not only those added
before it.
Reproducer:
qemu-img create -f qcow2 a.qcow2 64M
qemu-img create -f qcow2 b.qcow2 64M
qemu-system-x86_64 -S -nodefaults -display none -monitor stdio \
-drive file=a.qcow2,if=none,id=drv0 \
-blockdev file,filename=b.qcow2,node-name=bfile \
-blockdev qcow2,file=bfile,node-name=b0
(qemu) savevm s1
(qemu) info snapshots
(qemu) quit
qemu-img snapshot -l a.qcow2
qemu-img snapshot -l b.qcow2
Without the fix, only a.qcow2 has snapshot s1 and savevm does not
report an error. Since "info snapshots" uses the same iterator, it
even lists s1 as present on all disks. With the fix, both images have
it.
For savevm, only nodes without any parent are affected, since
snapshots only consider nodes that are attached to a BlockBackend or
have no parent at all. Such detached nodes are rare and often
short-lived in practice I think, which would explain the lack of
reports. Still, the failure is silent, it is a regression since 9.0,
and the fix is a single line, so I think it is a good candidate for
stable.
Best regards,
Erik
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-08-28 22:20 [PATCH] block: fix bdrv_next() skipping monitor-owned nodes Denis V. Lunev
2026-09-01 12:23 ` Fiona Ebner
@ 2026-09-07 10:45 ` Denis V. Lunev
2026-09-15 11:43 ` Denis V. Lunev
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Denis V. Lunev @ 2026-09-07 10:45 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel; +Cc: qemu-block, Fiona Ebner, Kevin Wolf
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-08-28 22:20 [PATCH] block: fix bdrv_next() skipping monitor-owned nodes Denis V. Lunev
2026-09-01 12:23 ` Fiona Ebner
2026-09-07 10:45 ` Denis V. Lunev
@ 2026-09-15 11:43 ` Denis V. Lunev
2026-09-21 19:40 ` Denis V. Lunev
2026-09-22 15:23 ` Kevin Wolf
4 siblings, 0 replies; 8+ messages in thread
From: Denis V. Lunev @ 2026-09-15 11:43 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel; +Cc: qemu-block, Fiona Ebner, Kevin Wolf
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-08-28 22:20 [PATCH] block: fix bdrv_next() skipping monitor-owned nodes Denis V. Lunev
` (2 preceding siblings ...)
2026-09-15 11:43 ` Denis V. Lunev
@ 2026-09-21 19:40 ` Denis V. Lunev
2026-09-22 15:23 ` Kevin Wolf
4 siblings, 0 replies; 8+ messages in thread
From: Denis V. Lunev @ 2026-09-21 19:40 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel; +Cc: qemu-block, Fiona Ebner, Kevin Wolf
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: fix bdrv_next() skipping monitor-owned nodes
2026-08-28 22:20 [PATCH] block: fix bdrv_next() skipping monitor-owned nodes Denis V. Lunev
` (3 preceding siblings ...)
2026-09-21 19:40 ` Denis V. Lunev
@ 2026-09-22 15:23 ` Kevin Wolf
4 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2026-09-22 15:23 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: qemu-devel, qemu-block, Fiona Ebner
Am 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
^ permalink raw reply [flat|nested] 8+ messages in thread