tests/qemu-iotests/tests/mirror-sparse | 1 + 1 file changed, 1 insertion(+)
This test uses cache.direct=true, but does not check if O_DIRECT
is supported by the underlying filesystem, and fails, for example,
on a tmpfs (which is rather common on various auto-builders, in CI,
etc).
Fix this by using _require_o_direct.
This example shows where our testing framework is significantly
lacking. In this test, qemu produces an error message on stderr
at startup, because it can't use O_DIRECT mode. But this error
message is not shown anywhere at all, even when running this test
separately outside of meson framework, - stderr is completely
hidden, and the only error we're getting is
+Timeout waiting for capabilities on handle 0
so it's rather painful to find what the actual error is. I think
that besides this change, we should also change the testing framework
to show stderr at least in case of test failure, and especially when
the failure occurs at the very beginning when we're checking for
sanity.
Fixes: c0ddcb2cbc146e "tests: Add iotest mirror-sparse for recent patches"
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
tests/qemu-iotests/tests/mirror-sparse | 1 +
1 file changed, 1 insertion(+)
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
echo
echo "=== Initial image setup ==="
--
2.47.2On 1/8/25 14:28, Michael Tokarev wrote: > This test uses cache.direct=true, but does not check if O_DIRECT > is supported by the underlying filesystem, and fails, for example, > on a tmpfs (which is rather common on various auto-builders, in CI, > etc). > > Fix this by using _require_o_direct. > > This example shows where our testing framework is significantly > lacking. In this test, qemu produces an error message on stderr > at startup, because it can't use O_DIRECT mode. But this error > message is not shown anywhere at all, even when running this test > separately outside of meson framework, - stderr is completely > hidden, and the only error we're getting is > > +Timeout waiting for capabilities on handle 0 > > so it's rather painful to find what the actual error is. I think > that besides this change, we should also change the testing framework > to show stderr at least in case of test failure, and especially when > the failure occurs at the very beginning when we're checking for > sanity. > > Fixes: c0ddcb2cbc146e "tests: Add iotest mirror-sparse for recent patches" > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > --- > tests/qemu-iotests/tests/mirror-sparse | 1 + > 1 file changed, 1 insertion(+) > > 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 ? > > echo > echo "=== Initial image setup ==="
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. Thanks, /mjt
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
On 6/8/25 09:30, Kevin Wolf wrote:
> 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.
Oops, sorry for the bad suggestion :/
© 2016 - 2025 Red Hat, Inc.