xfs
[Top] [All Lists]

Re: xfstests: MKFS_OPTIONS is not being reinitialized

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: xfstests: MKFS_OPTIONS is not being reinitialized
From: Boris Ranto <branto@xxxxxxxxxx>
Date: Thu, 18 Nov 2010 12:40:36 -0500 (EST)
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1979397072.1486461290101694923.JavaMail.root@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
----- "Dave Chinner" <david@xxxxxxxxxxxxx> 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....
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

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.

Cheers,
Boris Ranto
<branto@xxxxxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>