Re: [PATCH v2] fstests: remove dio related target file to respect mount options that only affecst new inodes

Zorro Lang <[email protected]>
Newsgroups org.kernel.vger.fstests,org.kernel.vger.linux-btrfs
Message-ID <ahlXBkOh3TF-3mRG@zlang-mailbox>
On Thu, May 28, 2026 at 08:46:59PM +0930, Qu Wenruo wrote:
> [BUG]
> Test case generic/362 and generic/365 will fail with nodatasum, but
> that's only when TEST_DEV is newly formatted.
> 
>  FSTYP         -- btrfs
>  PLATFORM      -- Linux/x86_64 btrfs-vm 7.1.0-rc4-custom+ #381 SMP PREEMPT_DYNAMIC Tue May 26 10:47:14 ACST 2026
>  MKFS_OPTIONS  -- -O bgt -K /dev/mapper/test-scratch1
>  MOUNT_OPTIONS -- -o nodatasum /dev/mapper/test-scratch1 /mnt/scratch
> 
>   generic/362  0s ... - output mismatch (see /home/adam/xfstests/results//generic/362.out.bad)
>     --- tests/generic/362.out	2024-08-24 15:31:37.200000000 +0930
>     +++ /home/adam/xfstests/results//generic/362.out.bad	2026-05-28 16:18:33.866141979 +0930
>     @@ -1,2 +1,3 @@
>      QA output created by 362
>     +First write failed: Input/output error
>      Silence is golden
>     ...
>     (Run 'diff -u /home/adam/xfstests/tests/generic/362.out /home/adam/xfstests/results//generic/362.out.bad'  to see the entire diff)
> 
>   generic/364  11s ... - output mismatch (see /home/adam/xfstests/results//generic/364.out.bad)
>     --- tests/generic/364.out	2024-09-30 09:09:51.216666681 +0930
>     +++ /home/adam/xfstests/results//generic/364.out.bad	2026-05-28 16:18:34.318532257 +0930
>     @@ -1,2 +1,3 @@
>      QA output created by 364
>     +Fsync failed: Input/output error
>      Silence is golden
>     ...
>     (Run 'diff -u /home/adam/xfstests/tests/generic/364.out /home/adam/xfstests/results//generic/364.out.bad'  to see the entire diff)
> 
> But if one has formated TEST_DEV, run test with default mount option,
> then change the mount option to "nodatasum", the test will not fail
> anymore:
> 
>  FSTYP         -- btrfs
>  PLATFORM      -- Linux/x86_64 btrfs-vm 7.1.0-rc4-custom+ #381 SMP PREEMPT_DYNAMIC Tue May 26 10:47:14 ACST 2026
>  MKFS_OPTIONS  -- -O bgt -K /dev/mapper/test-scratch1
>  MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
> 
>  generic/362  0s ...  1s
>  generic/364  11s ...  10s
>  Ran: generic/362 generic/364
>  Passed all 2 tests
> 
>  FSTYP         -- btrfs
>  PLATFORM      -- Linux/x86_64 btrfs-vm 7.1.0-rc4-custom+ #381 SMP PREEMPT_DYNAMIC Tue May 26 10:47:14 ACST 2026
>  MKFS_OPTIONS  -- -O bgt -K /dev/mapper/test-scratch1
>  MOUNT_OPTIONS -- -o nodatasum /dev/mapper/test-scratch1 /mnt/scratch
> 
>  generic/362  1s ...  0s
>  generic/364  10s ...  11s
>  Ran: generic/362 generic/364
>  Passed all 2 tests
> 
> [CAUSE]
> Btrfs' nodatasum mount option only affects new files, but the test cases
> are using TEST_DEV, and never delete the target files
> "$TEST_DIR/dio-append-buf-fault" for generic/362 and
> "$TEST_DIR/dio-write-fsync-same-fd" for generic/364.
> 
> So if the files are created with default mount options, then all later
> "nodatasum" mount options will not affect those files, thus hide the test
> failure.
> 
> [FIX]
> For all test cases utilizing "$here/src/dio*", add a _cleanup()
> function, to remove target files, so that for mount options that only
> affect new inodes, the mount options will be respected, and expose
> failures.
> 
> Now generic/36[24] will properly fail for "nodatasum" mount option on
> btrfs.

This patch is good to me,

Reviewed-by: Zorro Lang <[email protected]>

If you still hope to have a specific test case to uncover this known btrfs
bug, feel free to add a new one (to use SCRATCH_DEV and "nodatasum").

Thanks,
Zorro

> 
> Reviewed-by: Filipe Manana <[email protected]>
> Signed-off-by: Qu Wenruo <[email protected]>
> ---
> Changelog:
> v2:
> - Fix several grammar errors
> - Remove temporary files in _cleanup()
> ---
>  tests/generic/362 | 7 +++++++
>  tests/generic/364 | 8 ++++++++
>  tests/generic/418 | 7 +++++++
>  tests/generic/708 | 7 +++++++
>  4 files changed, 29 insertions(+)
> 
> diff --git a/tests/generic/362 b/tests/generic/362
> index 0cfaa726..e54d44b0 100755
> --- a/tests/generic/362
> +++ b/tests/generic/362
> @@ -10,6 +10,13 @@
>  . ./common/preamble
>  _begin_fstest auto quick
>  
> +_cleanup()
> +{
> +	cd /
> +	rm -f "$tmp.*"
> +	rm -f "$TEST_DIR/dio-append-buf-fault"
> +}
> +
>  # NFS forbade open with O_APPEND|O_DIRECT
>  _exclude_fs nfs
>  
> diff --git a/tests/generic/364 b/tests/generic/364
> index f7fb002f..8f13061f 100755
> --- a/tests/generic/364
> +++ b/tests/generic/364
> @@ -11,6 +11,14 @@
>  . ./common/preamble
>  _begin_fstest auto quick
>  
> +_cleanup()
> +{
> +	cd /
> +	rm -f "$tmp.*"
> +	rm -f "$TEST_DIR/dio-write-fsync-same-fd"
> +}
> +
> +
>  _require_test
>  _require_odirect
>  _require_test_program dio-write-fsync-same-fd
> diff --git a/tests/generic/418 b/tests/generic/418
> index 36789198..7ef1ef9f 100755
> --- a/tests/generic/418
> +++ b/tests/generic/418
> @@ -26,6 +26,13 @@ _require_block_device $TEST_DEV
>  _require_test_program "dio-invalidate-cache"
>  _require_test_program "feature"
>  
> +_cleanup()
> +{
> +	cd /
> +	rm -f "$tmp.*"
> +	rm -f "$testfile"
> +}
> +
>  diotest=$here/src/dio-invalidate-cache
>  testfile=$TEST_DIR/$seq-diotest
>  sectorsize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
> diff --git a/tests/generic/708 b/tests/generic/708
> index 827dac13..f504ca57 100755
> --- a/tests/generic/708
> +++ b/tests/generic/708
> @@ -14,6 +14,13 @@
>  . ./common/preamble
>  _begin_fstest quick auto mmap
>  
> +_cleanup()
> +{
> +	cd /
> +	rm -f "$tmp.*"
> +	rm -f "$src" "$dst"
> +}
> +
>  _fixed_by_fs_commit btrfs b73a6fd1b1ef \
>  		"btrfs: split partial dio bios before submit"
>  
> -- 
> 2.51.2
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.