* [PATCH] xfstests: xfs directory unbalance assert test [not found] <20130917145946.124195107@sgi.com> @ 2013-09-17 14:59 ` Mark Tinguely 2013-09-17 15:28 ` Eryu Guan ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Mark Tinguely @ 2013-09-17 14:59 UTC (permalink / raw) To: xfs [-- Attachment #1: xfstests-fill-directory.patch --] [-- Type: text/plain, Size: 2907 bytes --] This tests triggers an assert in the XFS directory unbalance code. This test originally written by Brian Foster and suggestions from Micheal Semon. Signed-off-by: Mark Tinguely <tinguely@sgi.com> --- tests/generic/319 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/319.out | 2 + tests/generic/group | 1 3 files changed, 65 insertions(+) Index: b/tests/generic/319 =================================================================== --- /dev/null +++ b/tests/generic/319 @@ -0,0 +1,62 @@ +#! /bin/bash +# FS QA Test No. 319 +# +# Test directory code correctly handles fsstress filling the filesystem +# +#----------------------------------------------------------------------- +# Copyright (c) 2013 SGI. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#----------------------------------------------------------------------- +# + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter +_require_scratch + +# real QA test starts here + +_supported_fs generic +_supported_os IRIX Linux + +_scratch_unmount > /dev/null 2>&1 +_scratch_mkfs_sized 11g >> $seqres.full 2>&1 +_scratch_mount > /dev/null 2>&1 + +# Fill the filesystem. +FSSTRESS_ARGS="-z -s 1378390208 -fsymlink=1 -n7500000 -p4 -d $SCRATCH_MNT" +$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1 + +cd $SCRATCH_MNT >> $seqres.full 2>&1 +sync +# A debug XFS may assert in the remove due to a directory bug. +rm -rf * +echo "--- silence is golden ---" +status=0 +exit Index: b/tests/generic/319.out =================================================================== --- /dev/null +++ b/tests/generic/319.out @@ -0,0 +1,2 @@ +QA output created by 319 +--- silence is golden --- Index: b/tests/generic/group =================================================================== --- a/tests/generic/group +++ b/tests/generic/group @@ -121,3 +121,4 @@ 316 auto quick 317 auto metadata quick 318 acl attr auto quick +319 stress _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 14:59 ` [PATCH] xfstests: xfs directory unbalance assert test Mark Tinguely @ 2013-09-17 15:28 ` Eryu Guan 2013-09-17 15:48 ` Mark Tinguely 2013-09-17 15:41 ` Eric Sandeen 2013-09-17 21:29 ` Dave Chinner 2 siblings, 1 reply; 9+ messages in thread From: Eryu Guan @ 2013-09-17 15:28 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Tue, Sep 17, 2013 at 09:59:47AM -0500, Mark Tinguely wrote: > This tests triggers an assert in the XFS directory unbalance code. > This test originally written by Brian Foster and suggestions > from Micheal Semon. > > Signed-off-by: Mark Tinguely <tinguely@sgi.com> > --- > tests/generic/319 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/319.out | 2 + > tests/generic/group | 1 > 3 files changed, 65 insertions(+) > > Index: b/tests/generic/319 > =================================================================== > --- /dev/null > +++ b/tests/generic/319 > @@ -0,0 +1,62 @@ > +#! /bin/bash > +# FS QA Test No. 319 > +# > +# Test directory code correctly handles fsstress filling the filesystem > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2013 SGI. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > +_require_scratch > + > +# real QA test starts here > + > +_supported_fs generic > +_supported_os IRIX Linux > + > +_scratch_unmount > /dev/null 2>&1 This is not necessary, _require_scratch has done the unmount work. > +_scratch_mkfs_sized 11g >> $seqres.full 2>&1 _scratch_mkfs_sized expects fssize in bytes, 11g is not a valid value The comments in common/rc about _scratch_mkfs_sized say # _scratch_mkfs_sized <size in bytes> [optional blocksize] > +_scratch_mount > /dev/null 2>&1 > + > +# Fill the filesystem. > +FSSTRESS_ARGS="-z -s 1378390208 -fsymlink=1 -n7500000 -p4 -d $SCRATCH_MNT" > +$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1 > + > +cd $SCRATCH_MNT >> $seqres.full 2>&1 > +sync > +# A debug XFS may assert in the remove due to a directory bug. > +rm -rf * > +echo "--- silence is golden ---" > +status=0 > +exit > Index: b/tests/generic/319.out > =================================================================== > --- /dev/null > +++ b/tests/generic/319.out > @@ -0,0 +1,2 @@ > +QA output created by 319 > +--- silence is golden --- > Index: b/tests/generic/group > =================================================================== > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -121,3 +121,4 @@ > 316 auto quick > 317 auto metadata quick > 318 acl attr auto quick > +319 stress Should be in auto group too I guess. Thanks, Eryu Guan > > > _______________________________________________ > 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] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 15:28 ` Eryu Guan @ 2013-09-17 15:48 ` Mark Tinguely 2013-09-17 15:51 ` Eric Sandeen 0 siblings, 1 reply; 9+ messages in thread From: Mark Tinguely @ 2013-09-17 15:48 UTC (permalink / raw) To: Eryu Guan; +Cc: xfs On 09/17/13 10:28, Eryu Guan wrote: > On Tue, Sep 17, 2013 at 09:59:47AM -0500, Mark Tinguely wrote: ... >> +_scratch_unmount> /dev/null 2>&1 > > This is not necessary, _require_scratch has done the unmount work. okay, stole that from other tests. > >> +_scratch_mkfs_sized 11g>> $seqres.full 2>&1 > > _scratch_mkfs_sized expects fssize in bytes, 11g is not a valid value > The comments in common/rc about _scratch_mkfs_sized say > > # _scratch_mkfs_sized<size in bytes> [optional blocksize] That was a shortcut for xfs. Looking in common/rc. I see that it breaks the other filesystems that need the size in blocks. ... >> @@ -0,0 +1,2 @@ >> +QA output created by 319 >> +--- silence is golden --- >> Index: b/tests/generic/group >> =================================================================== >> --- a/tests/generic/group >> +++ b/tests/generic/group >> @@ -121,3 +121,4 @@ >> 316 auto quick >> 317 auto metadata quick >> 318 acl attr auto quick >> +319 stress > > Should be in auto group too I guess. It takes a very long time to run to completion, don't know if people want this in the auto run. > Thanks, > Eryu Guan Thanks for the feedback --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 15:48 ` Mark Tinguely @ 2013-09-17 15:51 ` Eric Sandeen 2013-09-17 16:00 ` Mark Tinguely 0 siblings, 1 reply; 9+ messages in thread From: Eric Sandeen @ 2013-09-17 15:51 UTC (permalink / raw) To: Mark Tinguely; +Cc: Eryu Guan, xfs On 9/17/13 10:48 AM, Mark Tinguely wrote: > On 09/17/13 10:28, Eryu Guan wrote: >> On Tue, Sep 17, 2013 at 09:59:47AM -0500, Mark Tinguely wrote: > ... > >>> +_scratch_unmount> /dev/null 2>&1 >> >> This is not necessary, _require_scratch has done the unmount work. > > okay, stole that from other tests. > >> >>> +_scratch_mkfs_sized 11g>> $seqres.full 2>&1 >> >> _scratch_mkfs_sized expects fssize in bytes, 11g is not a valid value >> The comments in common/rc about _scratch_mkfs_sized say >> >> # _scratch_mkfs_sized<size in bytes> [optional blocksize] > > That was a shortcut for xfs. Looking in common/rc. I see that it breaks the other filesystems that need the size in blocks. at least mkfs.extN also understands "11g" but the helper does not, because it causes a failure in the device size check, (for any fs): [ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small" > ... > >>> @@ -0,0 +1,2 @@ >>> +QA output created by 319 >>> +--- silence is golden --- >>> Index: b/tests/generic/group >>> =================================================================== >>> --- a/tests/generic/group >>> +++ b/tests/generic/group >>> @@ -121,3 +121,4 @@ >>> 316 auto quick >>> 317 auto metadata quick >>> 318 acl attr auto quick >>> +319 stress >> >> Should be in auto group too I guess. > > It takes a very long time to run to completion, don't know if people want this in the auto run. how long is long? We do have "quick" for people who want quick. I think auto is probably ok. Maybe we should add a "slow" group, and you can "-x slow" :) -Eric >> Thanks, >> Eryu Guan > > Thanks for the feedback > > --Mark. > > _______________________________________________ > 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] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 15:51 ` Eric Sandeen @ 2013-09-17 16:00 ` Mark Tinguely 0 siblings, 0 replies; 9+ messages in thread From: Mark Tinguely @ 2013-09-17 16:00 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eryu Guan, xfs On 09/17/13 10:51, Eric Sandeen wrote: > On 9/17/13 10:48 AM, Mark Tinguely wrote: >> On 09/17/13 10:28, Eryu Guan wrote: >>> On Tue, Sep 17, 2013 at 09:59:47AM -0500, Mark Tinguely wrote: >> ... >> >>>> +_scratch_unmount> /dev/null 2>&1 >>> >>> This is not necessary, _require_scratch has done the unmount work. >> >> okay, stole that from other tests. >> >>> >>>> +_scratch_mkfs_sized 11g>> $seqres.full 2>&1 >>> >>> _scratch_mkfs_sized expects fssize in bytes, 11g is not a valid value >>> The comments in common/rc about _scratch_mkfs_sized say >>> >>> # _scratch_mkfs_sized<size in bytes> [optional blocksize] >> >> That was a shortcut for xfs. Looking in common/rc. I see that it breaks the other filesystems that need the size in blocks. > > at least mkfs.extN also understands "11g" but the helper does not, because > it causes a failure in the device size check, (for any fs): > > [ "$fssize" -gt "$devsize" ]&& _notrun "Scratch device too small" >> ... >> >>>> @@ -0,0 +1,2 @@ >>>> +QA output created by 319 >>>> +--- silence is golden --- >>>> Index: b/tests/generic/group >>>> =================================================================== >>>> --- a/tests/generic/group >>>> +++ b/tests/generic/group >>>> @@ -121,3 +121,4 @@ >>>> 316 auto quick >>>> 317 auto metadata quick >>>> 318 acl attr auto quick >>>> +319 stress >>> >>> Should be in auto group too I guess. >> >> It takes a very long time to run to completion, don't know if people want this in the auto run. > > how long is long? We do have "quick" for people who want quick. I think auto is probably > ok. Maybe we should add a "slow" group, and you can "-x slow" :) > > -Eric > >>> Thanks, >>> Eryu Guan >> >> Thanks for the feedback >> >> --Mark. About 45 minutes. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 14:59 ` [PATCH] xfstests: xfs directory unbalance assert test Mark Tinguely 2013-09-17 15:28 ` Eryu Guan @ 2013-09-17 15:41 ` Eric Sandeen 2013-09-17 15:58 ` Mark Tinguely 2013-09-17 21:29 ` Dave Chinner 2 siblings, 1 reply; 9+ messages in thread From: Eric Sandeen @ 2013-09-17 15:41 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On 9/17/13 9:59 AM, Mark Tinguely wrote: > This tests triggers an assert in the XFS directory unbalance code. > This test originally written by Brian Foster and suggestions > from Micheal Semon. cool, thanks. Comments below. > Signed-off-by: Mark Tinguely <tinguely@sgi.com> > --- > tests/generic/319 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/319.out | 2 + > tests/generic/group | 1 > 3 files changed, 65 insertions(+) > > Index: b/tests/generic/319 > =================================================================== > --- /dev/null > +++ b/tests/generic/319 > @@ -0,0 +1,62 @@ > +#! /bin/bash > +# FS QA Test No. 319 > +# > +# Test directory code correctly handles fsstress filling the filesystem > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2013 SGI. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > +} That seems pointless; usually it's done w/ rm -f $tmp.* right after, but we have no tmpfile, so... > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > +_require_scratch > + > +# real QA test starts here > + > +_supported_fs generic > +_supported_os IRIX Linux > + > +_scratch_unmount > /dev/null 2>&1 Aside: I see this done both ways - is it required to unmount scratch at the beginning of a test? I don't think so (I know it's done in many tests, though, but again, C&P & cargo cult? Or not? I'm not sure :( ) I guess it doesn't hurt, but at some point I'd like to get it straight about who's required to umount scratch, and when (if at all). > +_scratch_mkfs_sized 11g >> $seqres.full 2>&1 _scratch_mkfs_sized doesn't take units like this ('g'), so the above fails to actually make an 11g fs: # Create fs of certain size on scratch device # _scratch_mkfs_sized <size in bytes> [optional blocksize] _scratch_mkfs_sized() so we get this in 319.full: expr: non-numeric argument ./common/rc: line 576: [: 11g: integer expression expected but then it seems like mkfs carries on anyway w/ defaults. :( Apparently the mkfs 11g part isn't actually critical? ;) maybe _scratch_mkfs_sized needs something like this at the top: re='^[0-9]+$' if ! [[ $fssize =~ $re ]] ; then _notrun "error: _scratch_mkfs_sized: $fssize not a number of bytes" fi > +_scratch_mount > /dev/null 2>&1 is ignore-all-output really the right thing to do? When does _scratch_mount emit anything? (more cargo cult)? :) > +# Fill the filesystem. > +FSSTRESS_ARGS="-z -s 1378390208 -fsymlink=1 -n7500000 -p4 -d $SCRATCH_MNT" > +$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1 > + > +cd $SCRATCH_MNT >> $seqres.full 2>&1 cd doesn't emit anything except on error, right, and if there's an error we'd better stop the test right here! > +sync > +# A debug XFS may assert in the remove due to a directory bug. > +rm -rf * I'd feel a whole lot better if you did rm -rf $SCRATCH_MNT/* just in case we somehow ended up in the wrong place here. Or even better if you pointed fsstress at $SCRATCH_MNT/$seq.dir, and then did rm -rf $SCRATCH_MNT/$seq.dir - just to avoid that nasty should-never-happen-still-scary "rm -rf /*" -Eric > +echo "--- silence is golden ---" > +status=0 > +exit > Index: b/tests/generic/319.out > =================================================================== > --- /dev/null > +++ b/tests/generic/319.out > @@ -0,0 +1,2 @@ > +QA output created by 319 > +--- silence is golden --- > Index: b/tests/generic/group > =================================================================== > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -121,3 +121,4 @@ > 316 auto quick > 317 auto metadata quick > 318 acl attr auto quick > +319 stress > > > _______________________________________________ > 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] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 15:41 ` Eric Sandeen @ 2013-09-17 15:58 ` Mark Tinguely 2013-09-17 16:06 ` Eric Sandeen 0 siblings, 1 reply; 9+ messages in thread From: Mark Tinguely @ 2013-09-17 15:58 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs On 09/17/13 10:41, Eric Sandeen wrote: > On 9/17/13 9:59 AM, Mark Tinguely wrote: >> This tests triggers an assert in the XFS directory unbalance code. >> This test originally written by Brian Foster and suggestions >> from Micheal Semon. > > cool, thanks. Comments below. > >> Signed-off-by: Mark Tinguely<tinguely@sgi.com> >> --- >> tests/generic/319 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/319.out | 2 + >> tests/generic/group | 1 >> 3 files changed, 65 insertions(+) >> >> Index: b/tests/generic/319 >> =================================================================== >> --- /dev/null >> +++ b/tests/generic/319 >> @@ -0,0 +1,62 @@ >> +#! /bin/bash >> +# FS QA Test No. 319 >> +# >> +# Test directory code correctly handles fsstress filling the filesystem >> +# >> +#----------------------------------------------------------------------- >> +# Copyright (c) 2013 SGI. All Rights Reserved. >> +# >> +# This program is free software; you can redistribute it and/or >> +# modify it under the terms of the GNU General Public License as >> +# published by the Free Software Foundation. >> +# >> +# This program is distributed in the hope that it would be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program; if not, write the Free Software Foundation, >> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA >> +#----------------------------------------------------------------------- >> +# >> + >> +seq=`basename $0` >> +seqres=$RESULT_DIR/$seq >> +echo "QA output created by $seq" >> + >> +here=`pwd` >> +tmp=/tmp/$$ >> +status=1 # failure is the default! >> +trap "_cleanup; exit \$status" 0 1 2 3 15 >> + >> +_cleanup() >> +{ >> + cd / >> +} > > That seems pointless; usually it's done w/ rm -f $tmp.* > right after, but we have no tmpfile, so... Yeah, no cleanup is needed. >> + >> +# get standard environment, filters and checks >> +. ./common/rc >> +. ./common/filter >> +_require_scratch >> + >> +# real QA test starts here >> + >> +_supported_fs generic >> +_supported_os IRIX Linux >> + >> +_scratch_unmount> /dev/null 2>&1 > > Aside: > > I see this done both ways - is it required to unmount scratch at the beginning > of a test? I don't think so (I know it's done in many tests, though, but > again, C&P& cargo cult? Or not? I'm not sure :( ) > > I guess it doesn't hurt, but at some point I'd like to get it straight > about who's required to umount scratch, and when (if at all). Have to unmount for the mkfs, as noted by Eryu, it is already done. I would rather manually unmount it than be surprised when someone changes the common files. > >> +_scratch_mkfs_sized 11g>> $seqres.full 2>&1 > > _scratch_mkfs_sized doesn't take units like this ('g'), so the above fails to > actually make an 11g fs: > > # Create fs of certain size on scratch device > # _scratch_mkfs_sized<size in bytes> [optional blocksize] > _scratch_mkfs_sized() > > so we get this in 319.full: > > expr: non-numeric argument > ./common/rc: line 576: [: 11g: integer expression expected > > but then it seems like mkfs carries on anyway w/ defaults. :( > > Apparently the mkfs 11g part isn't actually critical? ;) it works on xfs because mkfs.xfs size can take those values, but yes it breaks on other filesystem. my bad. > > maybe _scratch_mkfs_sized needs something like this at the top: > > re='^[0-9]+$' > if ! [[ $fssize =~ $re ]] ; then > _notrun "error: _scratch_mkfs_sized: $fssize not a number of bytes" > fi > >> +_scratch_mount> /dev/null 2>&1 > > is ignore-all-output really the right thing to do? When does _scratch_mount > emit anything? (more cargo cult)? :) > >> +# Fill the filesystem. >> +FSSTRESS_ARGS="-z -s 1378390208 -fsymlink=1 -n7500000 -p4 -d $SCRATCH_MNT" >> +$FSSTRESS_PROG $FSSTRESS_ARGS>> $seqres.full 2>&1 >> + >> +cd $SCRATCH_MNT>> $seqres.full 2>&1 > > cd doesn't emit anything except on error, right, and if there's an error we'd better > stop the test right here! > >> +sync >> +# A debug XFS may assert in the remove due to a directory bug. >> +rm -rf * > > I'd feel a whole lot better if you did rm -rf $SCRATCH_MNT/* just in case > we somehow ended up in the wrong place here. > > Or even better if you pointed fsstress at $SCRATCH_MNT/$seq.dir, and > then did rm -rf $SCRATCH_MNT/$seq.dir - just to avoid that nasty > should-never-happen-still-scary "rm -rf /*" > Makes a lot of sense. > -Eric Thanks Eric. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 15:58 ` Mark Tinguely @ 2013-09-17 16:06 ` Eric Sandeen 0 siblings, 0 replies; 9+ messages in thread From: Eric Sandeen @ 2013-09-17 16:06 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On 9/17/13 10:58 AM, Mark Tinguely wrote: > On 09/17/13 10:41, Eric Sandeen wrote: >> On 9/17/13 9:59 AM, Mark Tinguely wrote: >>> This tests triggers an assert in the XFS directory unbalance code. >>> This test originally written by Brian Foster and suggestions >>> from Micheal Semon. >> >> cool, thanks. Comments below. >> >>> Signed-off-by: Mark Tinguely<tinguely@sgi.com> >>> --- >>> tests/generic/319 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> tests/generic/319.out | 2 + >>> tests/generic/group | 1 >>> 3 files changed, 65 insertions(+) >>> >>> Index: b/tests/generic/319 >>> =================================================================== >>> --- /dev/null >>> +++ b/tests/generic/319 >>> @@ -0,0 +1,62 @@ >>> +#! /bin/bash >>> +# FS QA Test No. 319 >>> +# >>> +# Test directory code correctly handles fsstress filling the filesystem >>> +# >>> +#----------------------------------------------------------------------- >>> +# Copyright (c) 2013 SGI. All Rights Reserved. >>> +# >>> +# This program is free software; you can redistribute it and/or >>> +# modify it under the terms of the GNU General Public License as >>> +# published by the Free Software Foundation. >>> +# >>> +# This program is distributed in the hope that it would be useful, >>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> +# GNU General Public License for more details. >>> +# >>> +# You should have received a copy of the GNU General Public License >>> +# along with this program; if not, write the Free Software Foundation, >>> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA >>> +#----------------------------------------------------------------------- >>> +# >>> + >>> +seq=`basename $0` >>> +seqres=$RESULT_DIR/$seq >>> +echo "QA output created by $seq" >>> + >>> +here=`pwd` >>> +tmp=/tmp/$$ >>> +status=1 # failure is the default! >>> +trap "_cleanup; exit \$status" 0 1 2 3 15 >>> + >>> +_cleanup() >>> +{ >>> + cd / >>> +} >> >> That seems pointless; usually it's done w/ rm -f $tmp.* >> right after, but we have no tmpfile, so... > > Yeah, no cleanup is needed. > >>> + >>> +# get standard environment, filters and checks >>> +. ./common/rc >>> +. ./common/filter >>> +_require_scratch >>> + >>> +# real QA test starts here >>> + >>> +_supported_fs generic >>> +_supported_os IRIX Linux >>> + >>> +_scratch_unmount> /dev/null 2>&1 >> >> Aside: >> >> I see this done both ways - is it required to unmount scratch at the beginning >> of a test? I don't think so (I know it's done in many tests, though, but >> again, C&P& cargo cult? Or not? I'm not sure :( ) >> >> I guess it doesn't hurt, but at some point I'd like to get it straight >> about who's required to umount scratch, and when (if at all). > > Have to unmount for the mkfs, as noted by Eryu, it is already done. I > would rather manually unmount it than be surprised when someone > changes the common files. If that happens, tons of tests will break. I'd really just remove it for clarity, but *shrug* >> >>> +_scratch_mkfs_sized 11g>> $seqres.full 2>&1 >> >> _scratch_mkfs_sized doesn't take units like this ('g'), so the above fails to >> actually make an 11g fs: >> >> # Create fs of certain size on scratch device >> # _scratch_mkfs_sized<size in bytes> [optional blocksize] >> _scratch_mkfs_sized() >> >> so we get this in 319.full: >> >> expr: non-numeric argument >> ./common/rc: line 576: [: 11g: integer expression expected >> >> but then it seems like mkfs carries on anyway w/ defaults. :( >> >> Apparently the mkfs 11g part isn't actually critical? ;) > > it works on xfs because mkfs.xfs size can take those values, but yes it breaks on other filesystem. my bad. One other nitpick in this area, please remove $seqres.full before you start so it doesn't grow each time the test is run. (hm maybe we should add that to ./check or something; so many tests miss this) But - no, it doesn't work for xfs either, at least not in all cases, because it doesn't do the device size check. Here's xfs output on a < 11G device: expr: non-numeric argument ./common/rc: line 582: [: 11g: integer expression expected ** mkfs failed with extra mkfs options added to "-bsize=4096" by test 319 ** ** attempting to mkfs using only test 319 options: -d size=11g -b size=4096 ** size 11g specified for data subvolume is too large, maximum is 1048241 blocks Usage: mkfs.xfs /* blocksize */ [-b log=n|size=num] ... and w/o error checking (2>&1 and no || _fail) the test just carries on w/o a fresh mkfs, on whatever size it happens to be. -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfstests: xfs directory unbalance assert test 2013-09-17 14:59 ` [PATCH] xfstests: xfs directory unbalance assert test Mark Tinguely 2013-09-17 15:28 ` Eryu Guan 2013-09-17 15:41 ` Eric Sandeen @ 2013-09-17 21:29 ` Dave Chinner 2 siblings, 0 replies; 9+ messages in thread From: Dave Chinner @ 2013-09-17 21:29 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Tue, Sep 17, 2013 at 09:59:47AM -0500, Mark Tinguely wrote: > +# Fill the filesystem. > +FSSTRESS_ARGS="-z -s 1378390208 -fsymlink=1 -n7500000 -p4 -d $SCRATCH_MNT" > +$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1 > + > +cd $SCRATCH_MNT >> $seqres.full 2>&1 > +sync > +# A debug XFS may assert in the remove due to a directory bug. > +rm -rf * Wouldn't this be better: sync rm -rf $SCRATCH_MNT/* Remember, xfstests runs as root and so having a "rm -rf *" anywhere in a test is a disaster just waiting to happen.... 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] 9+ messages in thread
end of thread, other threads:[~2013-09-17 21:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130917145946.124195107@sgi.com>
2013-09-17 14:59 ` [PATCH] xfstests: xfs directory unbalance assert test Mark Tinguely
2013-09-17 15:28 ` Eryu Guan
2013-09-17 15:48 ` Mark Tinguely
2013-09-17 15:51 ` Eric Sandeen
2013-09-17 16:00 ` Mark Tinguely
2013-09-17 15:41 ` Eric Sandeen
2013-09-17 15:58 ` Mark Tinguely
2013-09-17 16:06 ` Eric Sandeen
2013-09-17 21:29 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox