On 2/11/21 8:26 AM, Philippe Mathieu-Daudé wrote: > We are going to switch the 'null-co' default 'read-zeroes' value > from FALSE to TRUE in the next commit. First explicit the FALSE > value when it is not set.
Grammar suggestion, along with a suggestion for an additional sentence to make the intent of this commit clearer: As a first step, request an explicit FALSE value rather than relying on the defaults. This is intended to be a purely mechanical adjustment for no performance behavior in the tests; later patches may then flip or elide the explicit choice for tests where performance does not matter. > > Suggested-by: Eric Blake <ebl...@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > --- > - Missing: 056 & 155. I couldn't figure out the proper syntax, > any help welcomed... 056 - looks like just one line: self.vm = iotests.VM().add_drive_raw("file=blkdebug::null-co://,id=drive0,align=65536,driver=blkdebug") the best way to add it here would be rewriting that line to use blockdev syntax rather than blkdebug: URI syntax. The other question is whether it is a noticeable time difference when the default is flipped in 2/2. 155 - looks like several uses such as: class TestBlockdevMirrorForcedBacking(MirrorBaseClass): cmd = 'blockdev-mirror' existing = True target_backing = None target_blockdev_backing = { 'driver': 'null-co' } target_real_backing = 'null-co://' > - I'm unsure about 162, this doesn't seem to use the null-co > driver but rather testing global syntax. Concur; I don't see any reason to worry about this one (but mentioning it in the commit message can't hurt in case someone asks later) # blkdebug expects all of its arguments to be strings, but its # bdrv_refresh_filename() implementation should not assume that they have been # passed as strings in the original options QDict. # So this should emit blkdebug:42:null-co:// as the filename: touch 42 $QEMU_IMG info 'json:{"driver": "blkdebug", "config": 42, "image.driver": "null-co"}' \ > --- > docs/devel/testing.rst | 14 +++++++------- > tests/qtest/fuzz/generic_fuzz_configs.h | 11 ++++++----- > tests/test-bdrv-drain.c | 10 ++++++++-- > tests/acceptance/virtio_check_params.py | 2 +- > tests/perf/block/qcow2/convert-blockstatus | 6 +++--- > tests/qemu-iotests/040 | 2 +- You did a pretty good hunt for culprits! > tests/qemu-iotests/041 | 12 ++++++++---- > tests/qemu-iotests/051 | 2 +- > tests/qemu-iotests/051.out | 2 +- > tests/qemu-iotests/051.pc.out | 4 ++-- and for the fallout to the iotests. I did not audit for which tests are easy candidates for dropping the explicit read-zeroes=false (that is, where the extra time in allowing the flipped default doesn't penalize the test), but am okay giving this patch: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org