diff mbox series

[v3,2/2] generic/578: add a check to ensure that fiemap is supported

Message ID 20230825-fixes-v3-2-6484c098f8e8@kernel.org (mailing list archive)
State New, archived
Headers show
Series fstests: add appropriate checks for fs features for some tests | expand

Commit Message

Jeff Layton Aug. 25, 2023, 5:56 p.m. UTC
This test requires FIEMAP support.

Suggested-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tests/generic/578 | 1 +
 1 file changed, 1 insertion(+)

Comments

Yongcheng Yang Aug. 30, 2023, 2:55 a.m. UTC | #1
Hi Zorro,

Can we assume all the FIEMAP tests need this check first?
If so, there are some others need the same patch.

I.e.
[yoyang@yoyang-vm xfstests-dev]$ grep url .git/config
        url = git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
[yoyang@yoyang-vm xfstests-dev]$ git pl
Already up to date.
[yoyang@yoyang-vm xfstests-dev]$ git grep _begin_fstest tests/ | grep fiemap | wc -l
101
[yoyang@yoyang-vm xfstests-dev]$ git grep _require_xfs_io_command tests/ | grep fiemap | wc -l
86
[yoyang@yoyang-vm xfstests-dev]$

Best Regards,
Yongcheng
Zorro Lang Aug. 30, 2023, 12:55 p.m. UTC | #2
On Wed, Aug 30, 2023 at 10:55:03AM +0800, Yongcheng Yang wrote:
> Hi Zorro,
> 
> Can we assume all the FIEMAP tests need this check first?
> If so, there are some others need the same patch.
> 
> I.e.
> [yoyang@yoyang-vm xfstests-dev]$ grep url .git/config
>         url = git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> [yoyang@yoyang-vm xfstests-dev]$ git pl
> Already up to date.
> [yoyang@yoyang-vm xfstests-dev]$ git grep _begin_fstest tests/ | grep fiemap | wc -l
> 101
> [yoyang@yoyang-vm xfstests-dev]$ git grep _require_xfs_io_command tests/ | grep fiemap | wc -l
> 86
> [yoyang@yoyang-vm xfstests-dev]$

Hi Yongcheng,

Thanks for taking attention on it. 101 - 86 = 15, let's check these 15 cases
one by one:

[zorro@zlang-laptop xfstests-dev]$ for i in `egrep -rsnl _begin_fstest.*fiemap tests`;do grep -q $i < <(egrep -rsnl _require_xfs_io_command.*fiemap tests) || echo $i;done
tests/btrfs/079
tests/btrfs/140
tests/btrfs/004
tests/ext4/001
tests/ext4/308
tests/generic/655
tests/generic/654
tests/generic/578
tests/generic/541
tests/generic/542
tests/generic/516
tests/generic/519
tests/generic/540
tests/generic/543
tests/overlay/066

btrfs/079: It doesn't use fiemap direclty, it use filefrag command to trigger
           fiemap (if support). If FIEMAP is not supported then filefrag will
	   fall back to using FIBMAP. So it's not necessary to _notrun this case
	   if FIEMAP isn't supported I think.
btrfs/140: Similar as above
btrfs/004: Similar as above
ext4/001:  It use fiemap through _test_generic_punch helper, so I think it should
	   has "_require_xfs_io_command fiemap"
ext4/308:  I think it missed the `_require_xfs_io_command fiemap`
g/655:     It doesn't use fiemap, but use filefrag. And filefrag will fall back to
           FIBMAP, if FIEMAP isn't supported.
g/654:     Similar as above
g/578:     Similar as above
g/541:	   Similar as above
g/542:	   Similar as above
g/516:	   Similar as above
g/519:	   Similar as above
g/540:	   Similar as above
g/543:	   Similar as above
overlay/066: Similar as above

(If anything I said above is wrong, feel free to tell me:)

So I think ext4/001 and ext4/308 can have the `_require_xfs_io_command fiemap`.
But as they're ext4 specific test cases (not generic), so they won't affect
other fs (which doesn't support fiemap) testing. If you'd like, you can add
`_require_xfs_io_command fiemap` to these two cases.

Thanks,
Zorro

> 
> Best Regards,
> Yongcheng
>
diff mbox series

Patch

diff --git a/tests/generic/578 b/tests/generic/578
index b024f6ff90b4..e8bb97f7b96c 100755
--- a/tests/generic/578
+++ b/tests/generic/578
@@ -24,6 +24,7 @@  _cleanup()
 _supported_fs generic
 _require_test_program "mmap-write-concurrent"
 _require_command "$FILEFRAG_PROG" filefrag
+_require_xfs_io_command "fiemap"
 _require_test_reflink
 _require_cp_reflink