On Wed, Sep 30, 2026 at 03:19:24PM +0200, Fiona Ebner wrote:
> Flush during bdrv_close() was introduced back in 2012 in commit
> 80ccf93b88 ("qemu-img: let 'qemu-img convert' flush data"). However, a
> failing flush was quietly ignored [0]. Report the error instead, since
> this could mean that the image contents are not as expected.
> 
> Adapt the iotests accordingly. For all tests except '080' and
> 'qsd-migrate' the failures come from blkdebug configurations. For
> '080', the failures are also directly expected, because of invalid L1
> table size and offset. For the 'qsd-migrate' test, the error happens,
> because the QSD node which is attached via NBD to the source VM was
> inactivated.
> 
> [0]: Note that the errors reported for qcow2 come from
> qcow2_inactivate() which is only called as part of
> bs->drv->bdrv_close() after bdrv_flush(). In particular, adding
> > 'once': 'on'
> to the blkdebug driver in the relevant test case in the 122 iotest
> shows that the failure can be silent in case of qcow2 too.
> 
> Signed-off-by: Fiona Ebner <[email protected]>
> ---

> +++ b/tests/qemu-iotests/026
> @@ -103,13 +103,13 @@ if [ "$event" == "l2_update" ]; then
>      $QEMU_IO -c "write $vmstate 0 512" "$TEST_IMG" > /dev/null 2>&1
>  fi
>  
> -$QEMU_IO -c "write $vmstate 0 128k " "$BLKDBG_TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "write $vmstate 0 128k " "$BLKDBG_TEST_IMG" 2>&1 | 
> _filter_qemu_io | _filter_testdir

This creates lines longer than 80 columns. Adding \-newline
continuations is an option. But there are plenty of existing long
lines in the iotests, so I don't see that as holding up this patch.

> +++ b/tests/qemu-iotests/122
> @@ -357,15 +357,15 @@ target_fails_flush="json:{
>      }
>  }"
>  
> -$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG".orig 
> "$target_fails_flush"
> -echo "qemu-img convert exited with status $?"
> +$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG".orig 
> "$target_fails_flush" 2>&1 | _filter_json_filename
> +echo "qemu-img convert exited with status ${PIPESTATUS[0]}"

Interesting churn from $? to ${PIPESTATUS[0]} on a line added earlier
in the same series. But using PIPESTATUS when there is no |, while
valid in bash, looks odd, so I don't mind.

Reviewed-by: Eric Blake <[email protected]>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org


Reply via email to