Re: [PATCH v2 8/8] xfs/068: fix clonerange problems in file/dir count output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 14, 2017 at 9:49 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> On Thu, Dec 14, 2017 at 08:52:32AM +0200, Amir Goldstein 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.
>
> I think the fundamental problem of xfs/068 is the hardcoded file numbers
> in .out file, perhaps we should calculate the expected number of
> files/dirs to be dumped/restored before the dump test and extract the
> actual restored number of files/dirs from xfsrestore output and do a
> comparison. (or save the whole tree structure for comparison? I haven't
> done any test yet, just some random thoughts for now.)
>
> Currently, xfs/068 will easily break if there's user-defined
> FSSTRESS_AVOID, e.g. FSSTRESS_AVOID="-ffallocate=0", and that's totally
> legal test configuration.
>
> IHMO we really should fix xfs/068 first to avoid hitting the same
> problem again and again.
>

Agreed.
And while at it, need to redirect:

echo "fsstress : $_param"

to $seq.full and remove it from golden output and print the
actual parameters in the log including  $FSSTRESS_AVOID, not the
make believe params.

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



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux