On Wed, Aug 29, 2018 at 10:30:29PM -0500, Eric Sandeen wrote:
> On 8/29/18 10:19 PM, Eryu Guan wrote:
> > On Wed, Aug 29, 2018 at 04:43:38PM -0500, Eric Sandeen wrote:
> >> There are a few tests which fail on large filesytems because
> >> we run into mkfs limits.
> >>
> >> xfs/010 and xfs/013 hardcode 2 AGs, but if the device is larger
> >> than 2T this will fail. Check the device size and restrict it
> >> to just under 2T so that a 2-AG mkfs is possible.
> >>
> >> xfs/178 tries to decrease the agcount and re-mkfs, but if the
> >> default AG size was chosen to be 1T, decreasing the AG count
> >> results in too-large AGs and mkfs fails. The intention here
> >> AFAICT is to simply re-mkfs with non-overlapping AG headers,
> >> so increasing the AG count should achieve the same purpose,
> >> and cause mkfs to choose a smaller-than-default AG size which
> >> should pass.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> >> ---
> >>
> >> diff --git a/tests/xfs/010 b/tests/xfs/010
> >> index ee1595c8..2b30c867 100755
> >> --- a/tests/xfs/010
> >> +++ b/tests/xfs/010
> >> @@ -96,7 +96,14 @@ _require_xfs_finobt
> >>
> >> rm -f $seqres.full
> >>
> >> -_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2" | _filter_mkfs 2>$seqres.full
> >> +# If 2 AGs would result in too-large AG size, restrict the size
> >> +DSIZEOPT=""
> >> +DEV_SZ=$(blockdev --getsize64 $SCRATCH_DEV)
> >> +if [ "$DEV_SZ" -ge "$((2*(2**40)))" ]; then
> >> + DSIZEOPT="-d size=2047g"
> >> +fi
> >> +
> >> +_scratch_mkfs_xfs "-m crc=1,finobt=1 -d agcount=2 $DSIZEOPT" | _filter_mkfs 2>$seqres.full
> >
> > I think this kind of problem will happen to all tests that specify a
> > custom "agcount" in mkfs but without an "agsize" nor "size". For
> > example, xfs/062 would fail if $SCRATCH_DEV is larger than 8T. I guess
> > we need a more general way to fix the problem.
>
> Whoops, yes, I missed xfs/062
>
> > I'm thinking to introduce a new helper to require a given agcount will
> > fit the device size, and if the device is bigger than ($agcount * 1T)
> > then _notrun the test, something like (may need a better helper name):
> >
> > _require_xfs_support_agcount()
> > {
> > local dev=$1
> > local agcount=$2
> > local max_sz=$((agcount*(2**40)))
> > local dev_sz=$(blockdev --getsize64 $dev)
> >
> > if [ $dev_sz -gt $max_sz ]; then
> > _notrun "agcount $agcount is too small to hold $dev_sz device"
> > fi
> > }
>
> I'm not sure we should _notrun, though - if what we really want to check
> is a 2-AG filesystem on the scratch dev, there are times when we may simply
> want to make a smaller filesystem and proceed. For the cases I sent
> in my patch this should be perfectly fine...
>
> > Then add this _require rule to all tests that only specify a custom
> > agsize, e.g. for xfs/010 we could do
> >
> > _require_xfs_support_agcount $SCRATCH_DEV 2
> >
> > I roughly went through all xfs tests and found that the following ones
> > may need this new _require rule
> >
> > xfs/010
> > xfs/013
> > xfs/062
> > xfs/178
> > xfs/179
> > xfs/310
> >
> > Perhaps there're better ways to solve the problem, any suggestions are
> > welcomed!
>
> Rather than a _require and a _notrun, what about a mkfs_scratch_agcount()
> that does something like
Yeah, I know there're better ways :)
>
> mkfs_scratch_agcount()
Or _scratch_mkfs_agcount()? Following the naming as in _scratch_mkfs_sized().
> {
> agcount=$1
> opts=$2
And make sure local variables are declared as "local".
>
> # If $agcount AGs would result in too-large AG size, restrict the size
> # to create $agcount roughly 1T AGs.
> dsizeopt=""
> dev_sz=$(blockdev --getsize64 $SCRATCH_DEV)
> if [ "$dev_sz" -ge "$(($agcount*(2**40)))" ]; then
> dsizeopt="-d size=$(($agcount*((2**40)-1)))"
> fi
> _scratch_mkfs_xfs "$opts -d agcount=$agcount $dsizeopt" | _filter_mkfs 2>$seqres.full
> }
>
> or something like that?
Thanks!
Eryu
