On Thu, Dec 14, 2017 at 1:44 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > On Wed, Dec 13, 2017 at 03:28:05PM -0800, Darrick J. Wong wrote: >> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> >> >> In this test we use a fixed sequence of operations in fsstress to create >> some number of files and dirs and then exercise xfsdump/xfsrestore on >> them. Since clonerange/deduperange are not supported on all xfs >> configurations, detect if they're in fsstress and disable them so that >> we always execute exactly the same sequence of operations no matter how >> the filesystem is configured. >> >> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> >> --- >> tests/xfs/068 | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tests/xfs/068 b/tests/xfs/068 >> index 7151e28..f95a539 100755 >> --- a/tests/xfs/068 >> +++ b/tests/xfs/068 >> @@ -43,6 +43,14 @@ trap "rm -rf $tmp.*; exit \$status" 0 1 2 3 15 >> _supported_fs xfs >> _supported_os Linux >> >> +# Remove fsstress commands that aren't supported on all xfs configs >> +if $FSSTRESS_PROG | grep -q clonerange; then >> + FSSTRESS_AVOID="-f clonerange=0 $FSSTRESS_AVOID" >> +fi >> +if $FSSTRESS_PROG | grep -q deduperange; then >> + FSSTRESS_AVOID="-f deduperange=0 $FSSTRESS_AVOID" >> +fi >> + > > I'd put this inside _create_dumpdir_stress_num as it's supposed to > DTRT for the dump/restore that follows. Otherwise looks fine. > Guys, Please take a look at the only 2 changes in the history of this test. I would like to make sure we are not in a loop: 5d36d85 xfs/068: update golden output due to new operations in fsstress 6e5194d fsstress: Add fallocate insert range operation The first change excludes the new insert op (by dchinner on commit) The second change re-includes insert op, does not exclude new mread/mwrite ops and updates golden output, following this discussion: https://marc.info/?l=fstests&m=149014697111838&w=2 (the referenced thread ends with a ? to Dave, but was followed by v6..v8 that were "silently acked" by Dave). I personally argued that the blacklist approach to xfs/068 is fragile and indeed this is the third time the test breaks in the history I know of, because of added fsstress ops. Fine. As long as we at least stay consistent with a decision about update golden output vs. exclude ops and document the decision in a comment with the reasoning, so we won't have to repeat this discussion next time. Darrick, IMO, we should follow the path of updating golden output and instead of dropping clone/dedupe from ops table in runtime, you should make them a noop or ignore the error, keeping the random sequence unchanged. This is more or less what happens with insert/collapse (error is ignored) already, so it would be weird to make exceptions. For reference, fsx does disable insert/collapse/zero/punch at runtime and that does change the random sequence of fsx. Cheers, Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
