Re: [PATCH-for-10.1] tests/qemu-iotests/tests/mirror-sparse: skip if O_DIRECT is not supported

[Kevin Wolf <kwolf@redhat.com>]

Am 05.08.2025 um 19:56 hat Michael Tokarev geschrieben:
> On 05.08.2025 20:23, Philippe Mathieu-Daudé wrote:
> 
> > > diff --git a/tests/qemu-iotests/tests/mirror-sparse b/tests/qemu-
> > > iotests/tests/mirror-sparse
> > > index cfcaa600ab..19843a622c 100755
> > > --- a/tests/qemu-iotests/tests/mirror-sparse
> > > +++ b/tests/qemu-iotests/tests/mirror-sparse
> > > @@ -41,6 +41,7 @@ _supported_fmt qcow2 raw  # Format of the source.
> > > dst is always raw file
> > >   _supported_proto file
> > >   _supported_os Linux
> > >   _require_disk_usage
> > > +_require_o_direct
> > 
> > Could the correct use be:
> > 
> >    _supported_cache_modes none directsync
> 
> Yes that works too.  I've no idea which is "better" - we've
> a bit too many options here, I think.  I'll change it to your
> suggestion.

No, _require_o_direct is better because it directly checks if files in
the scratch directory support O_DIRECT, which is what we need here
because the test unconditionally opens the file this way:

    -blockdev '{"driver":"file", "cache":{"direct":true, "no-flush":false},
                "filename":"'"$TEST_IMG.base"'", "node-name":"src-file"}' \

_supported_cache_modes is about the cache mode requested on the command
line when running qemu-iotests, which is not what we're interested in.
The relevant call doesn't even consider the command line option. It
still "fixes" the failure because requesting "none" or "directsync"
makes it do the O_DIRECT check, too.

But the effect is different: With "_supported_cache_modes none
directsync", the test will always be skipped if on the command line
"writeback" was requested (it's the default, so we'll skip the test by
default - that's a bad idea). With _require_o_direct it will only be
skipped if the filesystem really doesn't support O_DIRECT.

Kevin