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

[Rich Johnston <rjohnston@sgi.com>]

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.
>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs