| Message ID | 165038953550.1677711.12931148474635467556.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: new tests for kernel 5.18 | expand |
On Tue, Apr 19, 2022 at 8:32 PM Darrick J. Wong <djwong@kernel.org> wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > This is a regression test to make sure that nonzero error returns from > a filesystem's ->sync_fs implementation are actually passed back to > userspace when the call stack involves syncfs(2). > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Zorro Lang <zlang@redhat.com> > --- > tests/xfs/839 | 42 ++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/839.out | 2 ++ > 2 files changed, 44 insertions(+) > create mode 100755 tests/xfs/839 > create mode 100644 tests/xfs/839.out > > > diff --git a/tests/xfs/839 b/tests/xfs/839 > new file mode 100755 > index 00000000..9bfe93ef > --- /dev/null > +++ b/tests/xfs/839 > @@ -0,0 +1,42 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 Oracle. All Rights Reserved. > +# > +# FS QA Test No. 839 > +# > +# Regression test for kernel commits: > +# > +# 5679897eb104 ("vfs: make sync_filesystem return errors from ->sync_fs") > +# 2d86293c7075 ("xfs: return errors in xfs_fs_sync_fs") > +# > +# During a code inspection, I noticed that sync_filesystem ignores the return > +# value of the ->sync_fs calls that it makes. sync_filesystem, in turn is used > +# by the syncfs(2) syscall to persist filesystem changes to disk. This means > +# that syncfs(2) does not capture internal filesystem errors that are neither > +# visible from the block device (e.g. media error) nor recorded in s_wb_err. > +# XFS historically returned 0 from ->sync_fs even if there were log failures, > +# so that had to be corrected as well. > +# > +# The kernel commits above fix this problem, so this test tries to trigger the > +# bug by using the shutdown ioctl on a clean, freshly mounted filesystem in the > +# hope that the EIO generated as a result of the filesystem being shut down is > +# only visible via ->sync_fs. > +# > +. ./common/preamble > +_begin_fstest auto quick shutdown > + > +# real QA test starts here > + > +# Modify as appropriate. > +_require_xfs_io_command syncfs > +_require_scratch_nocheck > +_require_scratch_shutdown > + From all the commotion around generic vs. xfs, you forgot: _supported_fs xfs Otherwise, Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir.
On Tue, Apr 19, 2022 at 10:32:15AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > This is a regression test to make sure that nonzero error returns from > a filesystem's ->sync_fs implementation are actually passed back to > userspace when the call stack involves syncfs(2). > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Zorro Lang <zlang@redhat.com> > --- > tests/xfs/839 | 42 ++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/839.out | 2 ++ > 2 files changed, 44 insertions(+) > create mode 100755 tests/xfs/839 > create mode 100644 tests/xfs/839.out > > > diff --git a/tests/xfs/839 b/tests/xfs/839 > new file mode 100755 > index 00000000..9bfe93ef > --- /dev/null > +++ b/tests/xfs/839 > @@ -0,0 +1,42 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 Oracle. All Rights Reserved. > +# > +# FS QA Test No. 839 > +# > +# Regression test for kernel commits: > +# > +# 5679897eb104 ("vfs: make sync_filesystem return errors from ->sync_fs") > +# 2d86293c7075 ("xfs: return errors in xfs_fs_sync_fs") > +# > +# During a code inspection, I noticed that sync_filesystem ignores the return > +# value of the ->sync_fs calls that it makes. sync_filesystem, in turn is used > +# by the syncfs(2) syscall to persist filesystem changes to disk. This means > +# that syncfs(2) does not capture internal filesystem errors that are neither > +# visible from the block device (e.g. media error) nor recorded in s_wb_err. > +# XFS historically returned 0 from ->sync_fs even if there were log failures, > +# so that had to be corrected as well. > +# > +# The kernel commits above fix this problem, so this test tries to trigger the > +# bug by using the shutdown ioctl on a clean, freshly mounted filesystem in the > +# hope that the EIO generated as a result of the filesystem being shut down is > +# only visible via ->sync_fs. > +# > +. ./common/preamble > +_begin_fstest auto quick shutdown > + > +# real QA test starts here > + > +# Modify as appropriate. Good to me, we can help to add the missing "_supported_fs xfs", if you don't like to send a new version for that again. Reviewed-by: Zorro Lang <zlang@redhat.com> > +_require_xfs_io_command syncfs > +_require_scratch_nocheck > +_require_scratch_shutdown > + > +# Reuse the fs formatted when we checked for the shutdown ioctl, and don't > +# bother checking the filesystem afterwards since we never wrote anything. > +_scratch_mount > +$XFS_IO_PROG -x -c 'shutdown -f ' -c syncfs $SCRATCH_MNT > + > +# success, all done > +status=0 > +exit > diff --git a/tests/xfs/839.out b/tests/xfs/839.out > new file mode 100644 > index 00000000..f275cdcc > --- /dev/null > +++ b/tests/xfs/839.out > @@ -0,0 +1,2 @@ > +QA output created by 839 > +syncfs: Input/output error >
diff --git a/tests/xfs/839 b/tests/xfs/839 new file mode 100755 index 00000000..9bfe93ef --- /dev/null +++ b/tests/xfs/839 @@ -0,0 +1,42 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 Oracle. All Rights Reserved. +# +# FS QA Test No. 839 +# +# Regression test for kernel commits: +# +# 5679897eb104 ("vfs: make sync_filesystem return errors from ->sync_fs") +# 2d86293c7075 ("xfs: return errors in xfs_fs_sync_fs") +# +# During a code inspection, I noticed that sync_filesystem ignores the return +# value of the ->sync_fs calls that it makes. sync_filesystem, in turn is used +# by the syncfs(2) syscall to persist filesystem changes to disk. This means +# that syncfs(2) does not capture internal filesystem errors that are neither +# visible from the block device (e.g. media error) nor recorded in s_wb_err. +# XFS historically returned 0 from ->sync_fs even if there were log failures, +# so that had to be corrected as well. +# +# The kernel commits above fix this problem, so this test tries to trigger the +# bug by using the shutdown ioctl on a clean, freshly mounted filesystem in the +# hope that the EIO generated as a result of the filesystem being shut down is +# only visible via ->sync_fs. +# +. ./common/preamble +_begin_fstest auto quick shutdown + +# real QA test starts here + +# Modify as appropriate. +_require_xfs_io_command syncfs +_require_scratch_nocheck +_require_scratch_shutdown + +# Reuse the fs formatted when we checked for the shutdown ioctl, and don't +# bother checking the filesystem afterwards since we never wrote anything. +_scratch_mount +$XFS_IO_PROG -x -c 'shutdown -f ' -c syncfs $SCRATCH_MNT + +# success, all done +status=0 +exit diff --git a/tests/xfs/839.out b/tests/xfs/839.out new file mode 100644 index 00000000..f275cdcc --- /dev/null +++ b/tests/xfs/839.out @@ -0,0 +1,2 @@ +QA output created by 839 +syncfs: Input/output error