[PATCH trivial] tests/qemu-iotests/tests/mirror-sparse: actually require O_DIRECT

[PATCH trivial] tests/qemu-iotests/tests/mirror-sparse: actually require O_DIRECT

[Michael Tokarev]

Commit c0ddcb2cbc146e introduced the test which uses cache=direct
mode, without checking if the scratch filesystem supports O_DIRECT.
A subsequent commit, afeb002e0ad49d, tried to fix that issue, but
instead of checking for o_direct, it checked for
`_supported_cache_modes none directsync`, which is not what the
original mirror-sparse test uses.  Fix both by actually checking
for o_direct.

Fixes: c0ddcb2cbc146e "tests: Add iotest mirror-sparse for recent patches"
Fixes: afeb002e0ad49d "tests/qemu-iotests/tests/mirror-sparse: skip if O_DIRECT is not supported"
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
This is what happens when rushing for last-minute
pre-release fixes..  I'm sorry for this.

 tests/qemu-iotests/tests/mirror-sparse | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/tests/mirror-sparse b/tests/qemu-iotests/tests/mirror-sparse
index 3b183eea88..ee7101bd50 100755
--- a/tests/qemu-iotests/tests/mirror-sparse
+++ b/tests/qemu-iotests/tests/mirror-sparse
@@ -40,7 +40,7 @@ cd ..
 _supported_fmt qcow2 raw  # Format of the source. dst is always raw file
 _supported_proto file
 _supported_os Linux
-_supported_cache_modes none directsync
+_require_o_direct
 _require_disk_usage
 
 echo
-- 
2.47.2

Re: [PATCH trivial] tests/qemu-iotests/tests/mirror-sparse: actually require O_DIRECT

[Philippe Mathieu-Daudé]

On 6/8/25 09:56, Michael Tokarev wrote:
> Commit c0ddcb2cbc146e introduced the test which uses cache=direct
> mode, without checking if the scratch filesystem supports O_DIRECT.
> A subsequent commit, afeb002e0ad49d, tried to fix that issue, but
> instead of checking for o_direct, it checked for
> `_supported_cache_modes none directsync`, which is not what the
> original mirror-sparse test uses.  Fix both by actually checking
> for o_direct.
> 
> Fixes: c0ddcb2cbc146e "tests: Add iotest mirror-sparse for recent patches"
> Fixes: afeb002e0ad49d "tests/qemu-iotests/tests/mirror-sparse: skip if O_DIRECT is not supported"
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> This is what happens when rushing for last-minute
> pre-release fixes..  I'm sorry for this.

My bad actually :/

> 
>   tests/qemu-iotests/tests/mirror-sparse | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/tests/mirror-sparse b/tests/qemu-iotests/tests/mirror-sparse
> index 3b183eea88..ee7101bd50 100755
> --- a/tests/qemu-iotests/tests/mirror-sparse
> +++ b/tests/qemu-iotests/tests/mirror-sparse
> @@ -40,7 +40,7 @@ cd ..
>   _supported_fmt qcow2 raw  # Format of the source. dst is always raw file
>   _supported_proto file
>   _supported_os Linux
> -_supported_cache_modes none directsync
> +_require_o_direct
>   _require_disk_usage
>   
>   echo

Re: [PATCH trivial] tests/qemu-iotests/tests/mirror-sparse: actually require O_DIRECT

[Michael Tokarev]

On 06.08.2025 13:01, Philippe Mathieu-Daudé wrote:
> On 6/8/25 09:56, Michael Tokarev wrote:

>> This is what happens when rushing for last-minute
>> pre-release fixes..  I'm sorry for this.
> 
> My bad actually :/

Well, we both are guilty here :)

And I'm probably more guilty because my initial fix was testing the
same as the actual code is using, and I felt that your suggestion is
wrong - I had everything to at least *check* the damn thing closely.
But I didn't do that.

It is more, - I felt that with your condition, the test might be skipped
even on o_direct-supporting filesystem.  But I didn't verify that *too*,
which is even worse.

Instead, I blindly took your advice, without verifying my own doubts.
I think this is more wrong than, when one have serious doubts but does
not bother even verifying them.

Now, back to work..

Philippe, R-b? :)

Thanks,

/mjt

Re: [PATCH trivial] tests/qemu-iotests/tests/mirror-sparse: actually require O_DIRECT

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

On Wed, Aug 06, 2025 at 10:56:31AM +0300, Michael Tokarev wrote:
> Commit c0ddcb2cbc146e introduced the test which uses cache=direct
> mode, without checking if the scratch filesystem supports O_DIRECT.
> A subsequent commit, afeb002e0ad49d, tried to fix that issue, but
> instead of checking for o_direct, it checked for
> `_supported_cache_modes none directsync`, which is not what the
> original mirror-sparse test uses.  Fix both by actually checking
> for o_direct.
> 
> Fixes: c0ddcb2cbc146e "tests: Add iotest mirror-sparse for recent patches"
> Fixes: afeb002e0ad49d "tests/qemu-iotests/tests/mirror-sparse: skip if O_DIRECT is not supported"
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> This is what happens when rushing for last-minute
> pre-release fixes..  I'm sorry for this.
> 
>  tests/qemu-iotests/tests/mirror-sparse | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please include this in a pull request for QEMU v10.1.0-rc3. Thanks!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]