* xfstests: MKFS_OPTIONS is not being reinitialized
@ 2010-11-17 15:49 Boris Ranto
2010-11-17 16:30 ` Eric Sandeen
2010-11-18 4:44 ` Dave Chinner
0 siblings, 2 replies; 5+ messages in thread
From: Boris Ranto @ 2010-11-17 15:49 UTC (permalink / raw)
To: xfs
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
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"
Signed-off-by: Boris Ranto <branto@redhat.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: xfstests: MKFS_OPTIONS is not being reinitialized
2010-11-17 15:49 xfstests: MKFS_OPTIONS is not being reinitialized Boris Ranto
@ 2010-11-17 16:30 ` Eric Sandeen
2010-11-18 4:44 ` Dave Chinner
1 sibling, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2010-11-17 16:30 UTC (permalink / raw)
To: Boris Ranto; +Cc: xfs
On 11/17/10 9:49 AM, 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
>
> 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"
>
>
> Signed-off-by: Boris Ranto <branto@redhat.com>
Thanks!
I wonder though ... should we just invoke _scratch_mkfs with the geom options,
and leave MKFS_OPTIONS intact? That way we could still specify custom options
in the environment.
OTOH they might clash; as long as resetting MKFS_OPTIONS here only affects
this test, it's probably ok.
Thanks,
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: xfstests: MKFS_OPTIONS is not being reinitialized
2010-11-17 15:49 xfstests: MKFS_OPTIONS is not being reinitialized Boris Ranto
2010-11-17 16:30 ` Eric Sandeen
@ 2010-11-18 4:44 ` Dave Chinner
1 sibling, 0 replies; 5+ messages in thread
From: Dave Chinner @ 2010-11-18 4:44 UTC (permalink / raw)
To: Boris Ranto; +Cc: xfs
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@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: xfstests: MKFS_OPTIONS is not being reinitialized
[not found] <1979397072.1486461290101694923.JavaMail.root@zmail01.collab.prod.int.phx2.redhat.com>
@ 2010-11-18 17:40 ` Boris Ranto
2010-11-18 20:00 ` Eric Sandeen
0 siblings, 1 reply; 5+ messages in thread
From: Boris Ranto @ 2010-11-18 17:40 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
----- "Dave Chinner" <david@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....
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@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.
Cheers,
Boris Ranto
<branto@redhat.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: xfstests: MKFS_OPTIONS is not being reinitialized
2010-11-18 17:40 ` Boris Ranto
@ 2010-11-18 20:00 ` Eric Sandeen
0 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2010-11-18 20:00 UTC (permalink / raw)
To: Boris Ranto; +Cc: xfs
On 11/18/10 11:40 AM, Boris Ranto wrote:
>
> ----- "Dave Chinner" <david@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@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@redhat.com>
>
> _______________________________________________ xfs mailing list
> xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-18 19:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-17 15:49 xfstests: MKFS_OPTIONS is not being reinitialized Boris Ranto
2010-11-17 16:30 ` Eric Sandeen
2010-11-18 4:44 ` Dave Chinner
[not found] <1979397072.1486461290101694923.JavaMail.root@zmail01.collab.prod.int.phx2.redhat.com>
2010-11-18 17:40 ` Boris Ranto
2010-11-18 20:00 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox