On Tue, Jul 11, 2017 at 07:13:41AM -0400, Brian Foster wrote: > On Tue, Jul 11, 2017 at 10:39:55AM +0200, Carlos Maiolino wrote: > > Hi Brian, > > > > > > > > > > > > > > The test is still under xfs (rather than generic). > > > > > > > > > > I'm still not really convinced this test should be added to generic, this tests > > a very specific behavior of XFS journal, the journal behavior on XFS and ext4 > > for example, are very different, > > > > Sure, but this generally isn't how we determine whether a test is > generic or fs-specific. IME, a test is made specific iff it depends on > some fs-specific feature/option to run, regardless of whether the > underlying problem is generic or not. > Sounds reasonable > Granted, if there is a major behavior discrepency between filesystems > such that making this test generic would complicate the test or require > a different implementation, I think that is reasonable enough for an > exception. > > > either ext4 or btrfs for example, will remount the filesystem as RO during the > > xfs_io process to fill the filesystem, so, even though the filesystem can keep > > consistency, the test shows it as a failure. > > > > What do you think? I honestly believe it would be better to keep this test as a > > XFS test, the behavior here is XFS specific. > > > > Ok, so to me the question here is: can we make this test function > normally on both XFS and other fs' that exhibit the behavior above > without major changes and disrupting the original intent? For example, > can the test be updated to accommodate the fact that some filesystems > may go inactive after the overprovisioned write? > > It seems to me that it can with something as simple as the appended > diff. Of course, this probably should be enhanced further to verify that > the fs went read-only or to simply not ignore a freeze failure if [ > $FSTYP == xfs ], etc. > Yup, I think it is doable, I don't 100% agree with keeping it as a generic test, but whatever, I'll make it generic and send it > Brian > > --- 8< --- > > diff --git a/tests/xfs/999 b/tests/xfs/999 > old mode 100644 > new mode 100755 > index b46f1cc8..5ee7521b > --- a/tests/xfs/999 > +++ b/tests/xfs/999 > @@ -55,7 +55,6 @@ _cleanup() > # real QA test starts here > > # Modify as appropriate. > -_supported_fs xfs > _supported_os Linux > _require_scratch_nocheck > _require_dm_target thin-pool > @@ -102,7 +101,8 @@ xfs_io -f -d -c 'pwrite -b 1m 0 120m' $SCRATCH_MNT/f1 >>$seqres.full 2>&1 > > # This freeze will never complete until the dm-thin POOL device is extended. > # This is expected, it is only used so xfsaild is triggered to flush AIL items. > -fsfreeze -f $SCRATCH_MNT & > +fsfreeze -f $SCRATCH_MNT >>$seqres.full 2>&1 & > +freezepid=$! > > # Wait enough so xfsaild can run > sleep 10 > @@ -110,9 +110,12 @@ sleep 10 > # Make some extra space available so the freeze above can proceed > lvextend -L $newpsize $vgname/$poolname >>$seqres.full 2>&1 > > +wait -n $freezepid > +ret=$? > + > # Try to thaw the filesystem, and complete test if if succeed. > # NOTE: This will hang on affected XFS filesystems. > -fsfreeze -u $SCRATCH_MNT > +[ $ret == 0 ] && fsfreeze -u $SCRATCH_MNT > echo "Test OK" > > status=0 > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Carlos -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
