Re: [PATCH v1 1/2] generic/054,055: add _require_v2log

From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Cc: guaneryu@gmail.com, fstests@vger.kernel.org
Subject: Re: [PATCH v1 1/2] generic/054,055: add _require_v2log
Date: Wed, 19 Feb 2020 18:17:09 -0800	[thread overview]
Message-ID: <20200220021709.GF9504@magnolia> (raw)
In-Reply-To: <8b833401-4ffa-e8b5-48e2-2ce63a2690ba@cn.fujitsu.com>

On Thu, Feb 20, 2020 at 10:01:30AM +0800, Yang Xu wrote:
> 
> 
> on 2020/02/20 6:11, Darrick J. Wong wrote:
> > On Wed, Feb 19, 2020 at 04:48:07PM +0800, Yang Xu wrote:
> > > 
> > > 
> > > on 2020/02/19 13:06, Darrick J. Wong wrote:
> > > > On Tue, Feb 18, 2020 at 03:24:10PM +0800, Yang Xu wrote:
> > > > > This two case fail on my server machine with GPT disklabe type,
> > > > > info as below:
> > > > > mount failed: -o logbsize=32k
> > > > > dmesg info as below:
> > > > > [92633.871954] XFS (sda7): logbuf size must be greater than or equal to log stripe size
> > > > > 
> > > > > The min logbuf size is 256k on my server machine. It needs to use _require_v2log to check.
> > > > 
> > > > Works fine here.  What is sda7's storage and raid alignment
> > > > configuration?
> > > /dev/sda is a SAS disk, raid aligment is 256k.  When I mkfs.xfs -f
> > > /dev/sda7(20G), its lsunit value is 64blks(block size is 4k).
> > 
> > _require_v2log probably ought to be smarter about that.
> maybe _require_v2log to specify logbszie for mount options?
> > 
> > But, uh, seeing as generic/05[45] don't currently call _require_v2log,
> > how did logbsize=32k end up in the mount options to fail those two
> > tests?
> > 
> But generic/05[45] call _get_log_configs. It will use _xfs_log_config
> and _xfs_log_config calls logbszie=32k/64k/128k/256k for mount options.
> So xfs/xfs_super.c xfs_finish_flag will report EINVAL error if logbsize <
> logsunit.  So on my machine this case failed without calling _require_v2log.

logbsize=32k is a perfectly valid option on a fs with a v2 log, provided
the log sunit isn't larger than 32k.  _require_v2log conveniently
_notruns the test when mounting with logbsize=32k fails, but
maybe the problem here is that generic/05[45] need to format the fs with
-l su=0 so that we can exercise full control over the mount options?

--D

> Best Regards
> Yang Xu
> > --D
> > 
> > > Best Regards
> > > Yang Xu
> > > > 
> > > > --D
> > > > 
> > > > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> > > > > ---
> > > > >    common/log        | 1 +
> > > > >    tests/generic/054 | 1 +
> > > > >    tests/generic/055 | 1 +
> > > > >    3 files changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/common/log b/common/log
> > > > > index c7921f50..f82919a6 100644
> > > > > --- a/common/log
> > > > > +++ b/common/log
> > > > > @@ -494,6 +494,7 @@ _log_version()
> > > > >    _require_v2log()
> > > > >    {
> > > > > +    [ "$FSTYP" != xfs ] && return 0
> > > > >        # test out mkfs to see if it supports "-l version=2"
> > > > >        export MKFS_OPTIONS="-l version=2"
> > > > >        if ! _scratch_mkfs_xfs >>$seqres.full 2>&1; then
> > > > > diff --git a/tests/generic/054 b/tests/generic/054
> > > > > index 95d40c3c..b4b74615 100755
> > > > > --- a/tests/generic/054
> > > > > +++ b/tests/generic/054
> > > > > @@ -30,6 +30,7 @@ rm -f $seqres.full $tmp.*
> > > > >    _require_scratch
> > > > >    _require_scratch_shutdown
> > > > >    _require_logstate
> > > > > +_require_v2log
> > > > >    echo "*** init FS"
> > > > >    _scratch_unmount >/dev/null 2>&1
> > > > > diff --git a/tests/generic/055 b/tests/generic/055
> > > > > index 9715f27b..071c4eaf 100755
> > > > > --- a/tests/generic/055
> > > > > +++ b/tests/generic/055
> > > > > @@ -57,6 +57,7 @@ rm -f $seqres.full $tmp.*
> > > > >    _require_scratch
> > > > >    _require_scratch_shutdown
> > > > >    _require_logstate
> > > > > +_require_v2log
> > > > >    QUOTA_OPTION=`_get_quota_option`
> > > > > -- 
> > > > > 2.18.0
> > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
> 