On Fri, May 5, 2023 at 10:29 AM Zorro Lang <zlang@xxxxxxxxxx> wrote: > > On Fri, May 05, 2023 at 10:05:19PM +0800, Zorro Lang wrote: > > On Fri, May 05, 2023 at 09:11:52AM -0400, Anna Schumaker wrote: > > > Hi Zorro, > > > > > > On Fri, May 5, 2023 at 12:45 AM Zorro Lang <zlang@xxxxxxxxxx> wrote: > > > > > > > > On Thu, May 04, 2023 at 04:48:47PM -0400, Anna Schumaker wrote: > > > > > From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> > > > > > > > > > > The NFS client wasn't updating ctime after a setxattr request. This is a > > > > > test written while fixing the bug. > > > > > > > > > > Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> > > > > > --- > > > > > tests/nfs/002 | 39 +++++++++++++++++++++++++++++++++++++++ > > > > > tests/nfs/002.out | 2 ++ > > > > > 2 files changed, 41 insertions(+) > > > > > create mode 100755 tests/nfs/002 > > > > > create mode 100644 tests/nfs/002.out > > > > > > > > > > diff --git a/tests/nfs/002 b/tests/nfs/002 > > > > > new file mode 100755 > > > > > index 000000000000..5bfedef6c57d > > > > > --- /dev/null > > > > > +++ b/tests/nfs/002 > > > > > @@ -0,0 +1,39 @@ > > > > > +#! /bin/bash > > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > > > > > +# > > > > > +# FS QA Test 002 > > > > > +# > > > > > +# Test a bug whene the NFS client wasn't sending a post-op GETATTR to the > ^^ ? Whoops, typo! I'll fix that in v2 > > > > > > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > > If there's a known bug fix (git commit) in linux, you can use > _fixed_by_kernel_commit(), refer to other cases which use this function. > > > > > > +# > > > > > +. ./common/preamble > > > > > +_begin_fstest auto quick attr > > > > > + > > > > > +# Import common functions > > > > > +. ./common/filter > > I think the common/filter isn't needed. > Okay > > > > > +. ./common/attr > > > > > + > > > > > +# real QA test starts here > > > > > +_supported_fs nfs > > > > > > > > Great, a new nfs test case! > > > > > > > > > +_require_test_nfs_version 4.2 > > > > > > > > But I'm wondering if this case can be a generic test case, due to the operations > > > > in this case are common (need xattr and ctime support), don't depend on > > > > any nfs specific features/operations. > > > > > > This probably could be a generic test case. > > > > Great :) > > > > > > > > > > > > > Not sure why nfs4.2 is necessary, can it be removed or replaced ? > > > > > > That's because xattrs were added to the NFS protocol in NFS v4.2, so I > > > filtered out the other versions since they're not going to run anyway. > > > I think xattr support is already checked to properly skip this on > > > other versions, however, so changing this to a generic test shouldn't > > > create a new failure on earlier NFS versions. > > > > That makes sense. > > > > > > > > Should I send a v2 with those changes? And should I find an open test > > > > Sure, please send v2 to change this case to be a generic test case. Then we > > can check if more other filesystems has this issue :) > > > > > number, or choose something like "generic/999"? > > > > That depends on you. Due to there's only one test case in this patch, so you > > just need to base on latest for-next branch, then choose a number which has > > been taken, I'll deal with that if there's conflict when I merge. > > > > > Anna > > > > > > > > Thanks, > > > > Zorro > > > > > > _require_test > > > > > > +_require_attrs > > > > > + > > > > > +touch $TEST_DIR/testfile > > We can't be sure there's not a file (or even a directory or others) named > "testfile" in TEST_DIR when fstests is running. So better to remove it at > first. E.g > > rm -rf $TEST_DIR/testfile Okay > > > > > > + > > > > > +before_ctime=$(stat -c %z $TEST_DIR/testfile) > > > > > +$SETFATTR_PROG -n user.foobar -v 123 $TEST_DIR/testfile > > > > > +after_ctime=$(stat -c %z $TEST_DIR/testfile) > > Might "%Z" be better to be compared? (optional) No, I tried that at first but since it's reporting seconds since epoch it doesn't have enough granularity to detect the change. Anna > > Thanks, > Zorro > > > > > > + > > > > > +test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change." > > > > > + > > > > > + > > > > > +before_ctime=$after_ctime > > > > > +$SETFATTR_PROG -x user.foobar $TEST_DIR/testfile > > > > > +after_ctime=$(stat -c %z $TEST_DIR/testfile) > > > > > + > > > > > +test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change." > > > > > + > > > > > +echo "Silence is golden" > > > > > +status=0 > > > > > +exit > > > > > diff --git a/tests/nfs/002.out b/tests/nfs/002.out > > > > > new file mode 100644 > > > > > index 000000000000..61705c7cc203 > > > > > --- /dev/null > > > > > +++ b/tests/nfs/002.out > > > > > @@ -0,0 +1,2 @@ > > > > > +QA output created by 002 > > > > > +Silence is golden > > > > > -- > > > > > 2.40.1 > > > > > > > > > > > > >
