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

[PATCH fstests 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>
---
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 | 3 +++
 tests/generic/294 | 4 ++++
 tests/generic/357 | 5 +++++
 3 files changed, 12 insertions(+)
---
base-commit: 0ca1d4fbb2e9a492968f2951df101f24477f7991
change-id: 20230831-nfs-skip-7625a54f9e67

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

[PATCH fstests 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/generic/294 b/tests/generic/294
index 406b1b3954b9..777b62aec9ad 100755
--- a/tests/generic/294
+++ b/tests/generic/294
@@ -15,6 +15,10 @@ _begin_fstest auto quick
 
 # real QA test starts here
 
+# NFS will optimize away the on-the-wire lookup before attempting to
+# create a new file (since that means an extra round trip).
+test $FSTYP = "nfs"  && _notrun "NFS optmizes away lookups on exclusive creates"
+
 # Modify as appropriate.
 _supported_fs generic
 _require_scratch

-- 
2.41.0

[PATCH fstests 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..909f0c8c6762 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.
+test $FSTYP = "nfs"  && _notrun "NFS allows reflinked swapfiles"
+
 _require_scratch_swapfile
 _require_scratch_reflink
 _require_cp_reflink

-- 
2.41.0

[PATCH fstests 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.

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

diff --git a/tests/generic/187 b/tests/generic/187
index 0653b92f12f4..05e609964a9d 100755
--- a/tests/generic/187
+++ b/tests/generic/187
@@ -36,6 +36,9 @@ _require_xfs_io_command "fpunch"
 test $FSTYP = "btrfs" && _notrun "Can't fragment free space on btrfs."
 _require_odirect
 
+# This test is unreliable on NFS, as it depends on the exported filesystem.
+test $FSTYP = "nfs" && _notrun "This test is unreliable on NFS"
+
 _fragment_freesp()
 {
 	file=$1

-- 
2.41.0

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

[Zorro Lang]

On Thu, Aug 31, 2023 at 02:40:28PM -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 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/294 b/tests/generic/294
> index 406b1b3954b9..777b62aec9ad 100755
> --- a/tests/generic/294
> +++ b/tests/generic/294
> @@ -15,6 +15,10 @@ _begin_fstest auto quick
>  
>  # real QA test starts here
>  
> +# NFS will optimize away the on-the-wire lookup before attempting to
> +# create a new file (since that means an extra round trip).
> +test $FSTYP = "nfs"  && _notrun "NFS optmizes away lookups on exclusive creates"
> +
>  # Modify as appropriate.
>  _supported_fs generic

I don't know if nfs-list wants to skip these test cases on nfs. Anyway, if
there's not an objection from nfs team, the _supported_fs helper can use
a black list, likes:

  _supported_fs ^nfs

If a test case doesn't support nfs totally, you can use this and give it a
proper comment.

Thanks,
Zorro

>  _require_scratch
> 
> -- 
> 2.41.0
> 

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

[Jeff Layton]

On Fri, 2023-09-01 at 13:14 +0800, Zorro Lang wrote:
> On Thu, Aug 31, 2023 at 02:40:28PM -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 | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/generic/294 b/tests/generic/294
> > index 406b1b3954b9..777b62aec9ad 100755
> > --- a/tests/generic/294
> > +++ b/tests/generic/294
> > @@ -15,6 +15,10 @@ _begin_fstest auto quick
> >  
> >  # real QA test starts here
> >  
> > +# NFS will optimize away the on-the-wire lookup before attempting to
> > +# create a new file (since that means an extra round trip).
> > +test $FSTYP = "nfs"  && _notrun "NFS optmizes away lookups on exclusive creates"
> > +
> >  # Modify as appropriate.
> >  _supported_fs generic
> 
> I don't know if nfs-list wants to skip these test cases on nfs. Anyway, if
> there's not an objection from nfs team, the _supported_fs helper can use
> a black list, likes:
> 
>   _supported_fs ^nfs
> 
> If a test case doesn't support nfs totally, you can use this and give it a
> proper comment.
> 

I think we do want to skip it. This one consistently fails on NFS and is
testing very specific and subtle behavior that is not required by POSIX.

I can respin these using the _supported_fs syntax if that's preferred
though. 
-- 
Jeff Layton <jlayton@kernel.org>