On Thu, Sep 19, 2019 at 09:02:06AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 19, 2019 at 11:00:24PM +0800, Zorro Lang wrote:
> > xfs/030 always fails after d0e484ac699f ("check: wipe scratch devices
> > between tests") get merged.
> >
> > Due to xfs/030 does a sized(100m) mkfs. Before we merge above commit,
> > mkfs.xfs detects an old primary superblock, it will write zeroes to
> > all superblocks before formatting the new filesystem. But this won't
> > be done if we wipe the first superblock(by merging above commit).
> >
> > That means if we make a (smaller) sized xfs after wipefs, those *old*
> > superblocks which created by last time mkfs.xfs will be left on disk.
>
> One thing missing from this patch -- if the test formatted the scratch
> device with non-default geometry, the backup superblocks from that
Make sense, I didn't think about non-default geometry.
> filesystem will not be erased. Going back to my example from the email
> thread, if the scratch disk has:
>
> SB0 [16M zeroes] SB1 [16M zeroes] <4 more AGs> <zeroes from 100M to 1G> \
> SB'1 [1G space] SB'2 [1G space] SB'3 [1G space]
>
> Where SB[0-5] are the ones written by xfs/030 and SB'[1-3] were written
> by a previous test that did the default scratch device mkfs, then this
> patch will wipe out SB'[1-3] and SB0:
>
> 000 [16M zeroes] SB1 [16M zeroes] <4 more AGs> <zeroes from 100M to 1G> \
> 0000 [1G space] 0000 [1G space] 0000 [1G space]
>
> But that still leaves SB[1-5] which xfs_repair could stumble over later.
> For example, if the next test to be run formats a filesystem with 24MB
> AGs (instead of 16) and zaps the superblock, then repair will eventually
> try a linear scan looking for superblocks and find the ones from the
> 16MB filesystem first.
>
> There isn't a sequence of tests that do this, but so long as we're
> fixing this we might as well zap as much as we can. So I propose adding
> to try_wipe_scratch_xfs() the following:
>
> dbsize=
> _scratch_xfs_db -c 'sb 0' -c 'p blocksize agblocks agcount' 2>&1 | \
> sed -e 's/ = /=/g' -e 's/blocksize/dbsize/g' \
> -e 's/agblocks/agsize/g' > $tmp.mkfs
> . $tmp.mkfs
>
> and then repeat the for loop. If there isn't a filesystem then
> $tmp.mkfs will be an empty file and the loop won't run.
Sure, although I don't know why we must change the variable's name :)
>
> > Then when we do xfs_repair, if xfs_repair can't find the first SB, it
> > will go to find those *old* SB at first. When it finds them,
> > everyting goes wrong.
> >
> > So I try to get XFS AG geometry(by default) and then try to erase all
> > superblocks. Thanks Darrick J. Wong helped to analyze this issue.
> >
> > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > ---
> > common/rc | 4 ++++
> > common/xfs | 23 +++++++++++++++++++++++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/common/rc b/common/rc
> > index 66c7fd4d..fe13f659 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -4048,6 +4048,10 @@ _try_wipe_scratch_devs()
> > for dev in $SCRATCH_DEV_POOL $SCRATCH_DEV $SCRATCH_LOGDEV $SCRATCH_RTDEV; do
> > test -b $dev && $WIPEFS_PROG -a $dev
> > done
> > +
> > + if [ "$FSTYP" = "xfs" ];then
> > + try_wipe_scratch_xfs
> > + fi
>
> We probably ought to delegate all wiping to try_wipe_scratch_xfs, i.e.:
>
> test -b $dev || continue
> case "$FSTYP" in
> "xfs")
> _try_wipe_scratch_xfs
> ;;
> *)
> $WIPEFS_PROG -a $dev
> ;;
> esac
>
> and add the WIPEFS_PROG call to _try_wipe_scratch_xfs.
Sure,
Thanks!
Zorro
>
> > }
> >
> > # Only run this on xfs if xfs_scrub is available and has the unicode checker
> > diff --git a/common/xfs b/common/xfs
> > index 1bce3c18..34516f82 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -884,3 +884,26 @@ _xfs_mount_agcount()
> > {
> > $XFS_INFO_PROG "$1" | grep agcount= | sed -e 's/^.*agcount=\([0-9]*\),.*$/\1/g'
> > }
> > +
> > +# wipe the superblock of each XFS AGs
> > +try_wipe_scratch_xfs()
>
> Common helper functions should start with a '_'
>
> > +{
> > + local tmp=`mktemp -u`
> > +
> > + _scratch_mkfs_xfs -N 2>/dev/null | perl -ne '
> > + if (/^meta-data=.*\s+agcount=(\d+), agsize=(\d+) blks/) {
> > + print STDOUT "agcount=$1\nagsize=$2\n";
> > + }
> > + if (/^data\s+=\s+bsize=(\d+)\s/) {
> > + print STDOUT "dbsize=$1\n";
> > + }' > $tmp.mkfs
> > +
> > + . $tmp.mkfs
> > + if [ -n "$agcount" -a -n "$agsize" -a -n "$dbsize" ];then
> > + for ((i = 0; i < agcount; i++)); do
> > + $XFS_IO_PROG -c "pwrite $((i * dbsize * agsize)) $dbsize" \
> > + $SCRATCH_DEV >/dev/null;
> > + done
> > + fi
> > + rm -f $tmp.mkfs
>
> Add code as discussed above.
>
> --D
>
> > +}
> > --
> > 2.20.1
> >
