* [PATCH v2] generic/728: Add a test for xattr ctime updates @ 2023-05-05 17:24 Anna Schumaker 2023-05-05 22:39 ` Darrick J. Wong 2023-05-10 12:45 ` Zorro Lang 0 siblings, 2 replies; 6+ messages in thread From: Anna Schumaker @ 2023-05-05 17:24 UTC (permalink / raw) To: linux-nfs, fstests; +Cc: anna From: Anna Schumaker <Anna.Schumaker@Netapp.com> 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@Netapp.com> --- v2: - Move test to generic/ - Address comments from the mailing list --- tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/728.out | 2 ++ 2 files changed, 44 insertions(+) create mode 100755 tests/generic/728 create mode 100644 tests/generic/728.out diff --git a/tests/generic/728 b/tests/generic/728 new file mode 100755 index 000000000000..ab2414c151db --- /dev/null +++ b/tests/generic/728 @@ -0,0 +1,42 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. +# +# FS QA Test 728 +# +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the +# server after setting an xattr, resulting in `stat` reporting a stale ctime. +# +. ./common/preamble +_begin_fstest auto quick attr + +# Import common functions +. ./common/attr + +# real QA test starts here +_supported_fs generic +_require_test +_require_attrs + +rm -rf $TEST_DIR/testfile +touch $TEST_DIR/testfile + + +_check_xattr_op() +{ + what=$1 + shift 1 + + before_ctime=$(stat -c %z $TEST_DIR/testfile) + $SETFATTR_PROG $* $TEST_DIR/testfile + after_ctime=$(stat -c %z $TEST_DIR/testfile) + + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." +} + +_check_xattr_op setxattr -n user.foobar -v 123 +_check_xattr_op removexattr -x user.foobar + +echo "Silence is golden" +status=0 +exit diff --git a/tests/generic/728.out b/tests/generic/728.out new file mode 100644 index 000000000000..ab39f45fe5da --- /dev/null +++ b/tests/generic/728.out @@ -0,0 +1,2 @@ +QA output created by 728 +Silence is golden -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] generic/728: Add a test for xattr ctime updates 2023-05-05 17:24 [PATCH v2] generic/728: Add a test for xattr ctime updates Anna Schumaker @ 2023-05-05 22:39 ` Darrick J. Wong 2023-05-07 8:19 ` Zorro Lang 2023-05-10 12:45 ` Zorro Lang 1 sibling, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2023-05-05 22:39 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, fstests On Fri, May 05, 2023 at 01:24:27PM -0400, Anna Schumaker wrote: > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > 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@Netapp.com> > > --- > v2: > - Move test to generic/ > - Address comments from the mailing list > --- > tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ > tests/generic/728.out | 2 ++ > 2 files changed, 44 insertions(+) > create mode 100755 tests/generic/728 > create mode 100644 tests/generic/728.out > > diff --git a/tests/generic/728 b/tests/generic/728 > new file mode 100755 > index 000000000000..ab2414c151db > --- /dev/null > +++ b/tests/generic/728 > @@ -0,0 +1,42 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > +# > +# FS QA Test 728 > +# > +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > +# > +. ./common/preamble > +_begin_fstest auto quick attr > + > +# Import common functions > +. ./common/attr > + > +# real QA test starts here > +_supported_fs generic > +_require_test > +_require_attrs > + > +rm -rf $TEST_DIR/testfile > +touch $TEST_DIR/testfile > + > + > +_check_xattr_op() > +{ > + what=$1 > + shift 1 > + > + before_ctime=$(stat -c %z $TEST_DIR/testfile) > + $SETFATTR_PROG $* $TEST_DIR/testfile > + after_ctime=$(stat -c %z $TEST_DIR/testfile) What happens if the filesystem's timestamp granularity isn't sufficient to capture a difference across a setxattr operation? Most of the time we only update the (fast) timestamps at every clock tick, right? --D > + > + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." > +} > + > +_check_xattr_op setxattr -n user.foobar -v 123 > +_check_xattr_op removexattr -x user.foobar > + > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/generic/728.out b/tests/generic/728.out > new file mode 100644 > index 000000000000..ab39f45fe5da > --- /dev/null > +++ b/tests/generic/728.out > @@ -0,0 +1,2 @@ > +QA output created by 728 > +Silence is golden > -- > 2.40.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] generic/728: Add a test for xattr ctime updates 2023-05-05 22:39 ` Darrick J. Wong @ 2023-05-07 8:19 ` Zorro Lang 0 siblings, 0 replies; 6+ messages in thread From: Zorro Lang @ 2023-05-07 8:19 UTC (permalink / raw) To: Darrick J. Wong, Anna Schumaker; +Cc: linux-nfs, fstests On Fri, May 05, 2023 at 03:39:29PM -0700, Darrick J. Wong wrote: > On Fri, May 05, 2023 at 01:24:27PM -0400, Anna Schumaker wrote: > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > > > 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@Netapp.com> > > > > --- > > v2: > > - Move test to generic/ > > - Address comments from the mailing list > > --- > > tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/728.out | 2 ++ > > 2 files changed, 44 insertions(+) > > create mode 100755 tests/generic/728 > > create mode 100644 tests/generic/728.out > > > > diff --git a/tests/generic/728 b/tests/generic/728 > > new file mode 100755 > > index 000000000000..ab2414c151db > > --- /dev/null > > +++ b/tests/generic/728 > > @@ -0,0 +1,42 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > > +# > > +# FS QA Test 728 > > +# > > +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the > > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > > +# > > +. ./common/preamble > > +_begin_fstest auto quick attr > > + > > +# Import common functions > > +. ./common/attr > > + > > +# real QA test starts here > > +_supported_fs generic > > +_require_test > > +_require_attrs > > + > > +rm -rf $TEST_DIR/testfile > > +touch $TEST_DIR/testfile > > + > > + > > +_check_xattr_op() > > +{ > > + what=$1 > > + shift 1 > > + > > + before_ctime=$(stat -c %z $TEST_DIR/testfile) > > + $SETFATTR_PROG $* $TEST_DIR/testfile > > + after_ctime=$(stat -c %z $TEST_DIR/testfile) > > What happens if the filesystem's timestamp granularity isn't sufficient > to capture a difference across a setxattr operation? Most of the time > we only update the (fast) timestamps at every clock tick, right? OK, by talking with Darrick, I think "sleep 2s" might avoid most problem of timestamp granularity at here. (If this change don't affect your original test target). Any thoughts? > > --D > > > + > > + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." > > +} > > + > > +_check_xattr_op setxattr -n user.foobar -v 123 > > +_check_xattr_op removexattr -x user.foobar > > + > > +echo "Silence is golden" > > +status=0 > > +exit > > diff --git a/tests/generic/728.out b/tests/generic/728.out > > new file mode 100644 > > index 000000000000..ab39f45fe5da > > --- /dev/null > > +++ b/tests/generic/728.out > > @@ -0,0 +1,2 @@ > > +QA output created by 728 > > +Silence is golden > > -- > > 2.40.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] generic/728: Add a test for xattr ctime updates 2023-05-05 17:24 [PATCH v2] generic/728: Add a test for xattr ctime updates Anna Schumaker 2023-05-05 22:39 ` Darrick J. Wong @ 2023-05-10 12:45 ` Zorro Lang 2023-05-10 16:13 ` Anna Schumaker 1 sibling, 1 reply; 6+ messages in thread From: Zorro Lang @ 2023-05-10 12:45 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, fstests On Fri, May 05, 2023 at 01:24:27PM -0400, Anna Schumaker wrote: > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > 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@Netapp.com> > > --- > v2: > - Move test to generic/ > - Address comments from the mailing list > --- > tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ > tests/generic/728.out | 2 ++ > 2 files changed, 44 insertions(+) > create mode 100755 tests/generic/728 > create mode 100644 tests/generic/728.out > > diff --git a/tests/generic/728 b/tests/generic/728 > new file mode 100755 > index 000000000000..ab2414c151db > --- /dev/null > +++ b/tests/generic/728 > @@ -0,0 +1,42 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > +# > +# FS QA Test 728 > +# > +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > +# > +. ./common/preamble > +_begin_fstest auto quick attr > + > +# Import common functions > +. ./common/attr > + > +# real QA test starts here > +_supported_fs generic > +_require_test > +_require_attrs > + > +rm -rf $TEST_DIR/testfile > +touch $TEST_DIR/testfile > + > + > +_check_xattr_op() > +{ > + what=$1 > + shift 1 > + > + before_ctime=$(stat -c %z $TEST_DIR/testfile) > + $SETFATTR_PROG $* $TEST_DIR/testfile Hi Anna, If there's not objection from you, I'll add below codes at here, and merge this case in next fstests release: + # There might be filesystem's timestamp granularity, so sleep 2 seconds + # for most filesystems can capture a different timestamp across a + # setxattr operation + sleep 2s BTW, is there a known git commit which fix the issue of this case trying to test? Thanks, Zorro > + after_ctime=$(stat -c %z $TEST_DIR/testfile) > + > + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." > +} > + > +_check_xattr_op setxattr -n user.foobar -v 123 > +_check_xattr_op removexattr -x user.foobar > + > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/generic/728.out b/tests/generic/728.out > new file mode 100644 > index 000000000000..ab39f45fe5da > --- /dev/null > +++ b/tests/generic/728.out > @@ -0,0 +1,2 @@ > +QA output created by 728 > +Silence is golden > -- > 2.40.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] generic/728: Add a test for xattr ctime updates 2023-05-10 12:45 ` Zorro Lang @ 2023-05-10 16:13 ` Anna Schumaker 2023-05-11 13:40 ` Zorro Lang 0 siblings, 1 reply; 6+ messages in thread From: Anna Schumaker @ 2023-05-10 16:13 UTC (permalink / raw) To: Zorro Lang; +Cc: linux-nfs, fstests On Wed, May 10, 2023 at 8:45 AM Zorro Lang <zlang@redhat.com> wrote: > > On Fri, May 05, 2023 at 01:24:27PM -0400, Anna Schumaker wrote: > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > > > 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@Netapp.com> > > > > --- > > v2: > > - Move test to generic/ > > - Address comments from the mailing list > > --- > > tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/728.out | 2 ++ > > 2 files changed, 44 insertions(+) > > create mode 100755 tests/generic/728 > > create mode 100644 tests/generic/728.out > > > > diff --git a/tests/generic/728 b/tests/generic/728 > > new file mode 100755 > > index 000000000000..ab2414c151db > > --- /dev/null > > +++ b/tests/generic/728 > > @@ -0,0 +1,42 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > > +# > > +# FS QA Test 728 > > +# > > +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the > > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > > +# > > +. ./common/preamble > > +_begin_fstest auto quick attr > > + > > +# Import common functions > > +. ./common/attr > > + > > +# real QA test starts here > > +_supported_fs generic > > +_require_test > > +_require_attrs > > + > > +rm -rf $TEST_DIR/testfile > > +touch $TEST_DIR/testfile > > + > > + > > +_check_xattr_op() > > +{ > > + what=$1 > > + shift 1 > > + > > + before_ctime=$(stat -c %z $TEST_DIR/testfile) > > + $SETFATTR_PROG $* $TEST_DIR/testfile > > Hi Anna, > > If there's not objection from you, I'll add below codes at here, and merge > this case in next fstests release: Yeah, no objection from me. Sorry I haven't had a chance to get around to the change this week, I've been doing LSF. > > + # There might be filesystem's timestamp granularity, so sleep 2 seconds > + # for most filesystems can capture a different timestamp across a > + # setxattr operation > + sleep 2s > > BTW, is there a known git commit which fix the issue of this case trying to > test? Not yet, but there will be soon. Those patches are still in review and might need minor fixups. Can the git commit be added once they're ready? Anna > > Thanks, > Zorro > > > + after_ctime=$(stat -c %z $TEST_DIR/testfile) > > + > > + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." > > +} > > + > > +_check_xattr_op setxattr -n user.foobar -v 123 > > +_check_xattr_op removexattr -x user.foobar > > + > > +echo "Silence is golden" > > +status=0 > > +exit > > diff --git a/tests/generic/728.out b/tests/generic/728.out > > new file mode 100644 > > index 000000000000..ab39f45fe5da > > --- /dev/null > > +++ b/tests/generic/728.out > > @@ -0,0 +1,2 @@ > > +QA output created by 728 > > +Silence is golden > > -- > > 2.40.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] generic/728: Add a test for xattr ctime updates 2023-05-10 16:13 ` Anna Schumaker @ 2023-05-11 13:40 ` Zorro Lang 0 siblings, 0 replies; 6+ messages in thread From: Zorro Lang @ 2023-05-11 13:40 UTC (permalink / raw) To: Anna Schumaker; +Cc: linux-nfs, fstests On Wed, May 10, 2023 at 12:13:59PM -0400, Anna Schumaker wrote: > On Wed, May 10, 2023 at 8:45 AM Zorro Lang <zlang@redhat.com> wrote: > > > > On Fri, May 05, 2023 at 01:24:27PM -0400, Anna Schumaker wrote: > > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > > > > > 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@Netapp.com> > > > > > > --- > > > v2: > > > - Move test to generic/ > > > - Address comments from the mailing list > > > --- > > > tests/generic/728 | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > tests/generic/728.out | 2 ++ > > > 2 files changed, 44 insertions(+) > > > create mode 100755 tests/generic/728 > > > create mode 100644 tests/generic/728.out > > > > > > diff --git a/tests/generic/728 b/tests/generic/728 > > > new file mode 100755 > > > index 000000000000..ab2414c151db > > > --- /dev/null > > > +++ b/tests/generic/728 > > > @@ -0,0 +1,42 @@ > > > +#! /bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# Copyright (c) 2023 Netapp Inc., All Rights Reserved. > > > +# > > > +# FS QA Test 728 > > > +# > > > +# Test a bug where the NFS client wasn't sending a post-op GETATTR to the > > > +# server after setting an xattr, resulting in `stat` reporting a stale ctime. > > > +# > > > +. ./common/preamble > > > +_begin_fstest auto quick attr > > > + > > > +# Import common functions > > > +. ./common/attr > > > + > > > +# real QA test starts here > > > +_supported_fs generic > > > +_require_test > > > +_require_attrs > > > + > > > +rm -rf $TEST_DIR/testfile > > > +touch $TEST_DIR/testfile > > > + > > > + > > > +_check_xattr_op() > > > +{ > > > + what=$1 > > > + shift 1 > > > + > > > + before_ctime=$(stat -c %z $TEST_DIR/testfile) > > > + $SETFATTR_PROG $* $TEST_DIR/testfile > > > > Hi Anna, > > > > If there's not objection from you, I'll add below codes at here, and merge > > this case in next fstests release: > > Yeah, no objection from me. Sorry I haven't had a chance to get around > to the change this week, I've been doing LSF. Oh, no push, hope you enjoy that! > > > > > + # There might be filesystem's timestamp granularity, so sleep 2 seconds > > + # for most filesystems can capture a different timestamp across a > > + # setxattr operation > > + sleep 2s > > > > BTW, is there a known git commit which fix the issue of this case trying to > > test? > > Not yet, but there will be soon. Those patches are still in review and > might need minor fixups. Can the git commit be added once they're > ready? Sure. Someone would like to add that later (don't forget:), someone else would like to write the commit id as "xxxxxxxxx", then replace it after the patch get merged. Both ways are good to me. If you prefer the former, I can merge this V2 with above change. Or I can wait the V3 from you. Thanks, Zorro > > Anna > > > > > Thanks, > > Zorro > > > > > + after_ctime=$(stat -c %z $TEST_DIR/testfile) > > > + > > > + test "$before_ctime" != "$after_ctime" || echo "Expected ctime to change after $what." > > > +} > > > + > > > +_check_xattr_op setxattr -n user.foobar -v 123 > > > +_check_xattr_op removexattr -x user.foobar > > > + > > > +echo "Silence is golden" > > > +status=0 > > > +exit > > > diff --git a/tests/generic/728.out b/tests/generic/728.out > > > new file mode 100644 > > > index 000000000000..ab39f45fe5da > > > --- /dev/null > > > +++ b/tests/generic/728.out > > > @@ -0,0 +1,2 @@ > > > +QA output created by 728 > > > +Silence is golden > > > -- > > > 2.40.1 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-11 13:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-05 17:24 [PATCH v2] generic/728: Add a test for xattr ctime updates Anna Schumaker 2023-05-05 22:39 ` Darrick J. Wong 2023-05-07 8:19 ` Zorro Lang 2023-05-10 12:45 ` Zorro Lang 2023-05-10 16:13 ` Anna Schumaker 2023-05-11 13:40 ` Zorro Lang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.