Re: [PATCH 11/10] xfstests: rework large filesystem testing - add golden output

Rich Johnston
2012-09-06T12:56:11Z

Thanks for the comments.

On 09/05/2012 05:26 PM, Dave Chinner wrote:
> On Fri, Aug 31, 2012 at 02:43:27PM -0500, rjohnston@sgi.com wrote:
>> Patch "rework large filesystem testing" introduces a new option --large-fs
>> which creates a new file $SCRATCH_MNT/.use_space. If this 10 part patchset is
>> applied, the following tests will fail:
>> 019 026 027 028 046 047 050 056 059 060 062 063 064 065 066
>
> That's a lot more tests than I see failing.

It is very repeatable for me.

>
>> This patch accounts for the following new output when testing xfs filesystems with
>> the --large-fs option by creating new output file to compare against
>> ($seq.largefs.out):
>
> Creating new output files is the absolute last resort. Indeed, what
> happens when you get different output for tests that already select
> an output file based on, say, platform or some other criteria? We
> get a combinatorial explosion of golden output files, and that is
> simply not manageable.
>
> The usual thing to do is update the necessary filters or change the
> way the tests run to avoid trivial output file differences e.g. use
> a subdir rather than SCRATCH_MNT directly. Or, for example the
> filters that munge different standard error messages from different
> platforms to be the same...
>

OK good to know.

>> 1. The following four lines appear in test 019.
>> File: "./.use_space"
>> Size: 6312890368 Filetype: Regular File
>> Mode: (0600/-rw-------) Uid: (0) Gid: (0)
>> Device: <DEVICE> Inode: <INODE> Links: 1
>
> This test doesn't really need to be run for large filesystems -
> running it on large filesystems doesn't improve the coverage of or
> our confidence in the code it is testing, so I'd just add a
> _require_no_large_scratch_dev to it.
>

Works for me.

>> 2. When the nodump attribute is set, the xfsdump -e option will cause the
>> following additional lines to appear.
>> xfsdump: NOTE: pruned 1 files: skip attribute set
>> Only in SCRATCH_MNT: .use_space
>> SCRATCH_MNT/.use_space
>
> Ok, those are the errors I haven't seen - not sure why. I'll have to
> look into that.
>
> However, this is definitely a case of updating the dump output
> filter to remove these messages from the output stream. The
> alternative is to change the common dump code to use a subdirectory
> rather than the root directory so it doesn't see these files at all.
>

Good suggestion

>> 3. Number of files off by one.
>> xfsrestore: # directories and (off by 1) entries processed
>
> That would be fixed by using a subdir for the dump tests. I don't
> recommend that the number should be filtered, as having dump report
> the correct number of files scanned is important.

I agree.

>
>> [ROOT] 0 0 0 00 [--------] (off by 1) 0 0 00 [--------] 0 0 0 00 [--------]
>
> Perhaps the usre/group of the use_space file needs to be changed so
> it doesn't impact on the test results. Alternatively, a filter could
> be written/modified to fix the number appropriately.

Sounds reasonable.

>
>> This patch also modifies check and common.quota to use the new output file
>> $seq.largefs.out when the --large-fs option is used (LARGE_SCRATCH_DEV = yes)
>> or $seq.out when the --large-fs option is NOT used (LARGE_SCRATCH_DEV != yes).
>>
>> Signed-off-by: Rich Johnston <rjohnston@sgi.com>
>>
>> ---
>> 019.largefs.out | 5 +++
>> 026.largefs.out | 4 ++-
>> 027.largefs.out | 2 -
>> 028.largefs.out | 5 +++
>> 046.largefs.out | 3 +-
>> 047.largefs.out | 5 +++
>> 050.largefs.out | 72 ++++++++++++++++++++++++++++----------------------------
>> 056.largefs.out | 3 +-
>> 059.largefs.out | 2 +
>> 060.largefs.out | 4 ++-
>> 062.largefs.out | 2 +
>> 063.largefs.out | 3 +-
>> 064.largefs.out | 41 ++++++++++++++++---------------
>> 065.largefs.out | 29 +++++++++++-----------
>> 066.largefs.out | 3 +-
>> check | 12 +++++++--
>> common.quota | 20 ++++++++++-----
>> 17 files changed, 128 insertions(+), 87 deletions(-)
>
> FWIW, this patch is supposed to add these *.largefs.out files, right? The
> patch, however:
>
>> Index: b/019.largefs.out
>> ===================================================================
>> --- a/019.largefs.out
>> +++ b/019.largefs.out
>> @@ -9,6 +9,11 @@ Wrote 2048.00Kb (value 0x2c)
>> Mode: (0777/drwxrwxrwx) Uid: (3) Gid: (1)
>> Device: <DEVICE> Inode: <INODE> Links: 3
>>
>> + File: "./.use_space"
>> + Size: 6312890368 Filetype: Regular File
>> + Mode: (0600/-rw-------) Uid: (0) Gid: (0)
>> +Device: <DEVICE> Inode: <INODE> Links: 1
>> +
>> File: "./bigfile"
>> Size: 2097152 Filetype: Regular File
>> Mode: (0666/-rw-rw-rw-) Uid: (3) Gid: (0)
>
> ... assumes they already exist...
>

Yup my bad, I only posted the differences from the original *.out files.

May I make the suggested changes, or as this is your patchset do you
want to make them?

Regards,
--Rich

> Cheers,
>
> Dave.