[PATCH fstests v2 0/3] generic: skip a few tests on NFS

[PATCH fstests v2 0/3] generic: skip a few tests on NFS

[Jeff Layton]

There are some tests in xfstests generic section that fail or are
unreliable for reasons that are known but are difficult to detect via
feature tests.

Check the FSTYP on these tests and _notrun if it's "nfs". Also add some
documenting comments as to why we skip them on NFS.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- switch to using _supported_fs()
- Link to v1: https://lore.kernel.org/r/20230831-nfs-skip-v1-0-d54c1c6a9af2@kernel.org

---
Jeff Layton (3):
      generic/294: don't run this test on NFS
      generic/357: don't run this test on NFS
      generic/187: don't run this test on NFS

 tests/generic/187 | 6 +++++-
 tests/generic/294 | 6 ++++--
 tests/generic/357 | 5 +++++
 3 files changed, 14 insertions(+), 3 deletions(-)
---
base-commit: 0ca1d4fbb2e9a492968f2951df101f24477f7991
change-id: 20230831-nfs-skip-7625a54f9e67

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

[PATCH fstests v2 1/3] generic/294: don't run this test on NFS

[Jeff Layton]

When creating a new dentry (of any type), NFS will optimize away any
on-the-wire lookups prior to the create since that means an extra
round trip to the server. Because of that, it consistently fails this
test.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tests/generic/294 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/generic/294 b/tests/generic/294
index 406b1b3954b9..46c7001234a5 100755
--- a/tests/generic/294
+++ b/tests/generic/294
@@ -15,8 +15,10 @@ _begin_fstest auto quick
 
 # real QA test starts here
 
-# Modify as appropriate.
-_supported_fs generic
+# NFS will optimize away the on-the-wire lookup before attempting to
+# create a new file (since that means an extra round trip).
+_supported_fs ^nfs generic
+
 _require_scratch
 _require_symlinks
 _require_mknod

-- 
2.41.0

[PATCH fstests v2 2/3] generic/357: don't run this test on NFS

[Jeff Layton]

NFS doesn't keep track of whether a file is reflinked or not, so it
doesn't prevent this behavior. It shouldn't be a problem for NFS anyway,
so just skip this test there.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tests/generic/357 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/generic/357 b/tests/generic/357
index ce748f854327..a0299b32bc90 100755
--- a/tests/generic/357
+++ b/tests/generic/357
@@ -24,6 +24,11 @@ _cleanup()
 . ./common/reflink
 
 # real QA test starts here
+
+# For NFS, a reflink is just a CLONE operation, and after that
+# point it's dealt with by the server.
+_supported_fs ^nfs generic
+
 _require_scratch_swapfile
 _require_scratch_reflink
 _require_cp_reflink

-- 
2.41.0

[PATCH fstests v2 3/3] generic/187: don't run this test on NFS

[Jeff Layton]

This test is unreliable on NFS. It fails consistently when run vs. a
server exporting btrfs, but passes when the server exports xfs. Since we
don't have any sort of attribute that we can require to test this, just
skip this one on NFS.

Also, subsume the check for btrfs into the _supported_fs check, and add
a comment for it.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tests/generic/187 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/generic/187 b/tests/generic/187
index 0653b92f12f4..1b3509424462 100755
--- a/tests/generic/187
+++ b/tests/generic/187
@@ -29,13 +29,17 @@ _cleanup()
 . ./common/reflink
 
 # real QA test starts here
+
+# btrfs can't fragment free space. This test is unreliable on NFS, as it
+# depends on the exported filesystem.
+_supported_fs ^btrfs ^nfs generic
 _require_scratch_reflink
 _require_cp_reflink
 _require_xfs_io_command "falloc"
 _require_xfs_io_command "fpunch"
-test $FSTYP = "btrfs" && _notrun "Can't fragment free space on btrfs."
 _require_odirect
 
+
 _fragment_freesp()
 {
 	file=$1

-- 
2.41.0

Re: [PATCH fstests v2 1/3] generic/294: don't run this test on NFS

[Zorro Lang]

On Fri, Sep 01, 2023 at 01:39:55PM -0400, Jeff Layton wrote:
> When creating a new dentry (of any type), NFS will optimize away any
> on-the-wire lookups prior to the create since that means an extra
> round trip to the server. Because of that, it consistently fails this
> test.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  tests/generic/294 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/294 b/tests/generic/294
> index 406b1b3954b9..46c7001234a5 100755
> --- a/tests/generic/294
> +++ b/tests/generic/294
> @@ -15,8 +15,10 @@ _begin_fstest auto quick
>  
>  # real QA test starts here
>  
> -# Modify as appropriate.
> -_supported_fs generic
> +# NFS will optimize away the on-the-wire lookup before attempting to
> +# create a new file (since that means an extra round trip).
> +_supported_fs ^nfs generic

If we use black list, don't need to use "generic" to specify white list. E.g.

  $ grep -rsn _supported_fs tests/generic/|grep \\^
  tests/generic/699:25:_supported_fs ^overlay
  tests/generic/631:41:_supported_fs ^overlay
  tests/generic/679:27:_supported_fs ^xfs
  tests/generic/500:45:_supported_fs ^btrfs

Anyway, others look good to me, if no objection from nfs list, I can help
to merge this patchset without the "generic" :)

Thanks,
Zorro

> +
>  _require_scratch
>  _require_symlinks
>  _require_mknod
> 
> -- 
> 2.41.0
> 

Re: [PATCH fstests v2 1/3] generic/294: don't run this test on NFS

[Jeff Layton]

On Sat, 2023-09-02 at 03:19 +0800, Zorro Lang wrote:
> On Fri, Sep 01, 2023 at 01:39:55PM -0400, Jeff Layton wrote:
> > When creating a new dentry (of any type), NFS will optimize away any
> > on-the-wire lookups prior to the create since that means an extra
> > round trip to the server. Because of that, it consistently fails this
> > test.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  tests/generic/294 | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/generic/294 b/tests/generic/294
> > index 406b1b3954b9..46c7001234a5 100755
> > --- a/tests/generic/294
> > +++ b/tests/generic/294
> > @@ -15,8 +15,10 @@ _begin_fstest auto quick
> >  
> >  # real QA test starts here
> >  
> > -# Modify as appropriate.
> > -_supported_fs generic
> > +# NFS will optimize away the on-the-wire lookup before attempting to
> > +# create a new file (since that means an extra round trip).
> > +_supported_fs ^nfs generic
> 
> If we use black list, don't need to use "generic" to specify white list. E.g.
> 
>   $ grep -rsn _supported_fs tests/generic/|grep \\^
>   tests/generic/699:25:_supported_fs ^overlay
>   tests/generic/631:41:_supported_fs ^overlay
>   tests/generic/679:27:_supported_fs ^xfs
>   tests/generic/500:45:_supported_fs ^btrfs
> 
> Anyway, others look good to me, if no objection from nfs list, I can help
> to merge this patchset without the "generic" :)
> 

Ok, I was looking at the _support_fs implementation and it looked like
you needed a "generic" entry on the end or it wouldn't pass for other
fs, but if you're sure, that sounds good to me.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>