On Tue, 2017-06-13 at 16:32 +0800, Eryu Guan wrote: > On Mon, Jun 12, 2017 at 08:42:08AM -0400, Jeff Layton wrote: > > v4: respin set based on Eryu's comments > > > > These tests are companion tests to the patchset I recently posted with > > the cover letter: > > > > [PATCH v6 00/20] fs: enhanced writeback error reporting with errseq_t (pile #1) > > > > This set just adds 3 new xfstests to test writeback behavior. One generic > > filesystem test, one test for raw block devices, and one test for btrfs. > > The tests work with dmerror to ensure that writeback fails, and then > > tests how the kernel reports errors afterward. > > > > xfs, ext2/3/4 and btrfs all pass on a kernel with the patchset above. > > xfs, ext3/4 passed generic/999, btrfs passed btrfs/999 (with some local > modifications, see reply to btrfs/999), but ext2 (using ext4 driver) and > btrfs failed generic/999 on my host. (See test log at the end of mail.) > > In the ext2 case, this test requires an external log device to run, but > ext2 has no journal at all, I wonder if we should _notrun on ext2. > Technically it doesn't _require_ an external log device, it just won't pass on ext3/4 and xfs without one. Not sure how to convey that in a _require directive here. If you remove _require_logdev, it should pass on ext2 under the ext4.ko driver as well. ext2.ko still has issues though as the block device ends up getting flagged with errors twice. > btrfs doesn't support external log device either, it should not run this > generic test either. > Agreed. We just want it to run btrfs/999 > I think _require_logdev() should be updated too, to do a check on > $FSTYP, only allows filesystems that have external log device support to > continue to run. > Ok. > > > > The one comment I couldn't really address from earlier review is that > > we don't have a great way for xfstests to tell what sort of error > > reporting behavior it should expect from the running kernel. That > > makes it difficult to tell whether failure is expected during a given > > run. > > > > Maybe that's OK though and we should just let unconverted filesystems > > fail this test? > > If there's really no good way to tell if current fs supports this new > behavior, I think this is fine, strictly speaking, it's not a new > feature anyway. > > Thanks, > Eryu > > P.S. ext2 and btrfs failure in generic/999 run (I renumbered it to > generic/441) > > [root@ibm-x3550m3-05 xfstests]# ./check generic/441 generic/442 > FSTYP -- ext2 > PLATFORM -- Linux/x86_64 ibm-x3550m3-05 4.12.0-rc4.jlayton+ > MKFS_OPTIONS -- /dev/sdc2 > MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:root_t:s0 /dev/sdc2 /mnt/testarea/scratch > > generic/441 4s ... - output mismatch (see /root/xfstests/results//generic/441.out.bad) > --- tests/generic/441.out 2017-06-13 15:52:09.928413126 +0800 > +++ /root/xfstests/results//generic/441.out.bad 2017-06-13 16:21:41.414112226 +0800 > @@ -1,3 +1,3 @@ > QA output created by 441 > Format and mount > -Test passed! > +Third fsync on fd[0] failed: Input/output error > ... > (Run 'diff -u tests/generic/441.out /root/xfstests/results//generic/441.out.bad' to see the entire diff) > That means that it returned EIO on fsync when the error should have been cleared. I'll see if I can reproduce it on my setup. > [root@ibm-x3550m3-05 xfstests]# ./check generic/441 generic/442 > FSTYP -- btrfs > PLATFORM -- Linux/x86_64 ibm-x3550m3-05 4.12.0-rc4.jlayton+ > MKFS_OPTIONS -- /dev/sdc2 > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sdc2 /mnt/testarea/scratch > > generic/441 4s ... - output mismatch (see /root/xfstests/results//generic/441.out.bad) > --- tests/generic/441.out 2017-06-13 15:52:09.928413126 +0800 > +++ /root/xfstests/results//generic/441.out.bad 2017-06-13 16:25:17.483273992 +0800 > @@ -1,3 +1,3 @@ > QA output created by 441 > Format and mount > -Test passed! > +Third fsync on fd[0] failed: Read-only file system > ... > (Run 'diff -u tests/generic/441.out /root/xfstests/results//generic/441.out.bad' to see the entire diff) That's expected on btrfs. It needs btrfs/999 test to do this correctly. > > > > Jeff Layton (5): > > generic: add a writeback error handling test > > ext4: allow ext4 to use $SCRATCH_LOGDEV > > generic: test writeback error handling on dmerror devices > > ext3: allow it to put journal on a separate device when doing > > scratch_mkfs > > btrfs: make a btrfs version of writeback error reporting test > > > > .gitignore | 1 + > > common/dmerror | 13 ++- > > common/rc | 14 ++- > > doc/auxiliary-programs.txt | 16 ++++ > > src/Makefile | 2 +- > > src/dmerror | 44 +++++++++ > > src/fsync-err.c | 223 +++++++++++++++++++++++++++++++++++++++++++++ > > tests/btrfs/999 | 93 +++++++++++++++++++ > > tests/btrfs/group | 1 + > > tests/generic/998 | 64 +++++++++++++ > > tests/generic/998.out | 2 + > > tests/generic/999 | 77 ++++++++++++++++ > > tests/generic/999.out | 3 + > > tests/generic/group | 2 + > > 14 files changed, 548 insertions(+), 7 deletions(-) > > create mode 100755 src/dmerror > > create mode 100644 src/fsync-err.c > > create mode 100755 tests/btrfs/999 > > create mode 100755 tests/generic/998 > > create mode 100644 tests/generic/998.out > > create mode 100755 tests/generic/999 > > create mode 100644 tests/generic/999.out > > > > -- > > 2.13.0 > > -- Jeff Layton <jlayton@xxxxxxxxxx>