Re: [PATCH] block: Add missing null checks during query-named-block-nodes

[Kevin Wolf <kwolf@redhat.com>]

Am 27.10.2025 um 20:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Anyway, starting from 3860c0201924d, permission restrictions become less aggressive, let's
> check, staying at 3860c0201924d, and with same hack to bdrv_drop_filter:
> 
> check -qcow2 028 055 056 124 129 141 185 219 222 256 257 264 281 283 294 304
> 
> - only 257 fails. (why?)
> 
> Double check with 3860c0201924d^: again, almost all fails:
> 
> Failures: 028 055 056 124 129 185 222 256 257 264 281 304
> 
> 
> 
> And same with current master: 257 fails, if hack bdrv_drop_filter() to
> pass detach_subchain=false to bdrv_replace_node_common().

I'm trying to understand the 257, but that seems to be a fairly
complicated test case.

Looking at the first failing part, this uses error injection with a
blkdebug configuration where one single I/O error is injected on the
second read after the first flush. This feels very specific, like it's
targeting a specific operation to fail, but it doesn't really document
what it is.

John, I'm sure that after six years, you remember the details! :-)

Anyway, the difference that we're seeing after changing
bdrv_drop_filter() to pass detach_subchain=false is that now the error
seems to be triggered earlier. The same error that we get in the bad
output happens in the reference output, too, but only during "Test
Backup #1" rather than "Reference Backup #1".

So taking a shot in the dark, I tried failing the second read after the
_second_ flush (patch see below), and that fixes the test apart from the
changed blockdev-add commands. So not detaching the cor node causes an
additional flush on the image. I suspect this might be the flush in
bdrv_close() when the cor node is deleted?

Anyway, an additional flush is harmless, so I think we should be good if
we just change the test case.

Kevin


diff --git a/block.c b/block.c
index cf08e64add..982371e735 100644
--- a/block.c
+++ b/block.c
@@ -5478,7 +5478,7 @@ int bdrv_drop_filter(BlockDriverState *bs, Error **errp)

     bdrv_drained_begin(child_bs);
     bdrv_graph_wrlock();
-    ret = bdrv_replace_node_common(bs, child_bs, true, true, errp);
+    ret = bdrv_replace_node_common(bs, child_bs, true, false, errp);
     bdrv_graph_wrunlock();
     bdrv_drained_end(child_bs);

diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index 7d3720b8e5..dffc3ba0a4 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -310,14 +310,18 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', failure=None):
                     'state': 1,
                     'new_state': 2
                 }, {
-                    'event': 'read_aio',
+                    'event': 'flush_to_disk',
                     'state': 2,
                     'new_state': 3
+                }, {
+                    'event': 'read_aio',
+                    'state': 3,
+                    'new_state': 4
                 }],
                 'inject-error': [{
                     'event': 'read_aio',
                     'errno': 5,
-                    'state': 3,
+                    'state': 4,
                     'immediately': False,
                     'once': True
                 }]