[PATCH fstests v3 0/2] fstests: add appropriate checks for fs features for some tests

[PATCH fstests v3 0/2] fstests: add appropriate checks for fs features for some tests

[Jeff Layton]

A number of fstests fail on NFS, primarily because it lacks certain
features that are widely supported on local Linux filesystems. Fix
up the test for POSIX ACLs and add a new requirement for fiemap support
in generic/578.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- add new checks for fiemap and setcap
- Link to v2: https://lore.kernel.org/r/20230824-fixes-v2-0-d60c2faf1057@kernel.org

Changes in v3:
- use _require_xfs_io_command "fiemap" in generic/578
- drop the _require_setcap patch (Zorro has similar that adds a _require_capabilities)

---
Jeff Layton (2):
      common/attr: fix the _require_acl test
      generic/578: add a check to ensure that fiemap is supported

 common/attr       | 9 ++++-----
 tests/generic/578 | 1 +
 2 files changed, 5 insertions(+), 5 deletions(-)
---
base-commit: 8de535c53887bb49adae74a1b2e83e77d7e8457d
change-id: 20230824-fixes-914cc3f9ef72

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH fstests v3 1/2] common/attr: fix the _require_acl test

[Jeff Layton]

_require_acl tests whether you're able to fetch the ACL from a file
using chacl, and then tests for an -EOPNOTSUPP error return.
Unfortunately, filesystems that don't support them (like NFSv4) just
return -ENODATA when someone calls getxattr for the POSIX ACL, so the
test doesn't work.

Fix the test to have chacl set an ACL on the file instead, which should
reliably fail on filesystems that don't support them.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 common/attr | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/common/attr b/common/attr
index cce4d1b201b2..3ebba682c894 100644
--- a/common/attr
+++ b/common/attr
@@ -163,13 +163,12 @@ _require_acls()
     [ -n "$CHACL_PROG" ] || _notrun "chacl command not found"
 
     #
-    # Test if chacl is able to list ACLs on the target filesystems.  On really
-    # old kernels the system calls might not be implemented at all, but the
-    # more common case is that the tested filesystem simply doesn't support
-    # ACLs.
+    # Test if chacl is able to set an ACL on a file.  On really old kernels
+    # the system calls might not be implemented at all, but the more common
+    # case is that the tested filesystem simply doesn't support ACLs.
     #
     touch $TEST_DIR/syscalltest
-    chacl -l $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
+    chacl 'u::rw-,g::---,o::---' $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
     cat $TEST_DIR/syscalltest.out >> $seqres.full
 
     if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then

-- 
2.41.0

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

[Jeff Layton]

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(+)

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
 

-- 
2.41.0

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

[Yongcheng Yang]

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

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

[Zorro Lang]

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
>