On Thu, Dec 14, 2017 at 8:52 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > 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. > Looking again, I think this test is currently broken if Fallocate is disabled at build time and will also be Broken if we fix golden output for clonerange and it Will be disabled in build time, because rand() on the Operation params won't happen. Amir. -- 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
