xfstests: MKFS_OPTIONS is not being reinitialized

[Eric Sandeen]

On 11/18/10 11:40 AM, Boris Ranto wrote:
> 
> ----- "Dave Chinner" <david at fromorbit.com> wrote:
> 
>> On Wed, Nov 17, 2010 at 04:49:58PM +0100, Boris Ranto wrote:
>>> Test case 223 constantly fails because the variable carrying
>>> mkfs options is not being reinitialized.
>>> 
>>> Test calls function _scratch_mkfs_geom repeatedly in for loop
>> without
>>> cleaning the MKFS_OPTIONS variable. Since _scratch_mkfs_geom
>>> only appends options to the variable, MKFS_OPTIONS looks like
>>> this in
>> 5th
>>> iteration: MKFS_OPTIONS="-bsize=4096-b size=4096 -d
>>> su=8192,sw=4-b size=4096
>> -d
>>> su=16384,sw=4-b size=4096 -d su=32768,sw=4-b size=4096 -d 
>>> su=65536,sw=4-b size=4096 -d su=131072,sw=4"
>>> 
>>> It is also easy to see that _scratch_mkfs_geom does not append
>> leading
>>> space when it appends the variable.
>>> 
>>> Following patch fixes the issue for me and based on my testing
>>> does
>> not
>>> break any other test case:
>>> 
>>> diff -uprN xfstests-dev/223 xfstests-dev-new/223 ---
>>> xfstests-dev/223	2010-11-09 08:53:39.000000000 -0500 +++
>>> xfstests-dev-new/223	2010-11-17 08:05:56.745068628 -0500 @@ -58,6
>>> +58,7 @@ for SUNIT_K in 8 16 32 64 128; do let
>>> SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
>>> 
>>> echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ===" +	export
>>> MKFS_OPTIONS="" _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >>
>>> $seq.full 2>&1 _scratch_mount
>> 
>> That'll drop any custom mkfs options on the floor for that test.
>> 
>>> 
>>> diff -uprN xfstests-dev/common.rc xfstests-dev-new/common.rc ---
>>> xfstests-dev/common.rc	2010-11-09 08:53:39.000000000 -0500 +++
>>> xfstests-dev-new/common.rc	2010-11-17 08:07:06.972132647 -0500 @@
>>> -349,10 +349,10 @@ _scratch_mkfs_geom()
>>> 
>>> case $FSTYP in xfs) -	MKFS_OPTIONS+="-b size=$blocksize, -d
>> su=$sunit_bytes,sw=$swidth_mult"
>>> +	MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw= 
>>> $swidth_mult" ;; ext4) -	MKFS_OPTIONS+="-b $blocksize -E
>> stride=$sunit_blocks,stripe_width=
>>> $swidth_blocks" +	MKFS_OPTIONS+=" -b $blocksize -E
>> stride=$sunit_blocks,stripe_width=
>>> $swidth_blocks" ;; *) _notrun "can't mkfs $FSTYP with geometry"
>> 
>> Perhaps rather than using MKFS_OPTIONS, this should call 
>> scratch_mkfs directly with these as extra options, similar to the 
>> way _scratch_mkfs_sized() does. That would leave custom options
>> set, and only pass the test specific options once to mkfs....

I thought about that too, but I'm worried about some options
clashing.

>> Cheers,
>> 
>> Dave. -- Dave Chinner david at fromorbit.com
> 
> If I remember it right this would not work because MKFS_OPTIONS
> already contained -bsize=4096 at the beginning of the test and if
> another -b size=$blocksize would be added to the options mkfs would
> fail again.

*nod*

I think I'm ok with it the way it is.  FWIW, I committed it yesterday ...
We can change further if needed though, of course.

Thanks,
-Eric

> Cheers, Boris Ranto <branto at redhat.com>
> 
> _______________________________________________ xfs mailing list 
> xfs at oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
>