On Fri, Oct 25, 2019 at 05:09:10PM +0800, Ian Kent wrote: > On Fri, 2019-10-25 at 16:48 +0800, Ian Kent wrote: > > On Fri, 2019-10-25 at 16:20 +0800, Ian Kent wrote: > > > On Fri, 2019-10-25 at 16:05 +0800, Zorro Lang wrote: > > > > On Thu, Oct 24, 2019 at 12:47:17PM +0800, Ian Kent wrote: > > > > > On Tue, 2019-10-22 at 18:01 +0800, Zorro Lang wrote: > > > > > > XFS is changing to suit the new mount API, so add this case > > > > > > to > > > > > > make > > > > > > sure the changing won't bring in regression issue on xfs > > > > > > mount > > > > > > option > > > > > > parse phase, and won't change some default behaviors either. > > > > > > > > > > I think this isn't quite right: > > > > > > > > > > [root@f30 xfstests-dev]# diff -u /home/raven/xfstests- > > > > > dev/tests/xfs/148.out /home/raven/xfstests- > > > > > dev/results//xfs/148.out.bad > > > > > --- /home/raven/xfstests-dev/tests/xfs/148.out 2019-10-24 > > > > > 09:27:27.304929313 +0800 > > > > > +++ /home/raven/xfstests-dev/results//xfs/148.out.bad 2019- > > > > > 10-24 10:42:40.739436223 +0800 > > > > > @@ -3,4 +3,10 @@ > > > > > ** create loop log device > > > > > ** create loop mount point > > > > > ** start xfs mount testing ... > > > > > +[FAILED]: mount /dev/loop0 /mnt/test/148.mnt > > > > > +ERROR: expect there's logbufs in > > > > > rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquo > > > > > ta > > > > > , > > > > > but not found > > > > > +[FAILED]: mount /dev/loop0 /mnt/test/148.mnt > > > > > +ERROR: expect there's logbsize in > > > > > rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquo > > > > > ta > > > > > , > > > > > but not found > > > > > +[FAILED]: mount /dev/loop0 /mnt/test/148.mnt > > > > > +ERROR: expect there's logbsize in > > > > > rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquo > > > > > ta > > > > > , > > > > > but not found > > > > > ** end of testing > > > > > > > > > > Above logbufs and logbsize are present in the options string > > > > > but > > > > > an error is reported. > > > > > > > > Oh, There're two reasons to cause this failure: > > > > 1) I made a mistake at here: > > > > if [ $rc -eq 0 ];then > > > > if [ "$found" != "true" ];then > > > > echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT > > > > $opts" > > > > echo "ERROR: expect there's $key in > > > > $info, > > > > but not found" > > > > return 1 > > > > fi > > > > else > > > > if [ "$found" != "false" ];then > > > > echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT > > > > $opts" > > > > echo "ERROR: expect there's not $key in > > > > $info, but found" > > > > return 1 > > > > fi > > > > fi > > > > > > > > I should exchange the "there's" line with "there's not" line. > > > > > > > > 2) The case expect there's not "logbufs=N" or "logbsize=N" output > > > > as > > > > default. > > > > But your system has these two options in /proc/mounts if mount > > > > without any > > > > options. > > > > > > > > Hmm... strange, I've tested this case on rhel8 and rhel7, all > > > > passed. > > > > Where > > > > can I reproduce this error? > > > > > > There's every chance this has changed along the way but the current > > > upstream source sets these two fields to default values at some > > > point (looks like via xfs_mountfs() from xfs_fs_fill_super() to > > > me). > > > > > > So the super block op ->show_options() sees them as > 0 and prints > > > the option in the proc output. > > > > > > But I'm not so familiar with xfs so the subtleties of these options > > > eludes me. > > > > Oh, I see what's going on. > > > > In, for example, RHEL-8 xlog_get_iclog_buffer_size() on entry we > > have: > > > > if (mp->m_logbufs <= 0) > > log->l_iclog_bufs = XLOG_MAX_ICLOGS; > > else > > log->l_iclog_bufs = mp->m_logbufs; > > > > which sets a struct xlog field but leaves the struct xfs_mount > > alone. > > > > But upstream on entry we have: > > > > if (mp->m_logbufs <= 0) > > mp->m_logbufs = XLOG_MAX_ICLOGS; > > if (mp->m_logbsize <= 0) > > mp->m_logbsize = XLOG_BIG_RECORD_BSIZE; > > > > which sets the struct xfs_mount fields before continuing. > > > > There's a lot more going on in xlog_get_iclog_buffer_size() so this > > is a very simple minded comparison on my part but the point is > > RHEL-8 (and I'm assuming RHEL-7 too) is different to the current > > xfs upstream. > > > > > Dave, can you help out please wrt. current RHEL xfs or what I'm > > > missing here? > > > > So I'd still like more info around this difference please Dave. > > Apologies for the post beer-clock intrusion, ;) > > Ok, more info., first see commit 1cb51258758d7: > > Author: Eric Sandeen <sandeen@xxxxxxxxxxx> > Date: Thu Aug 16 16:24:43 2007 +1000 > > [XFS] choose single default logbuf count & size > > and then commit 4f62282a36964: > > Author: Christoph Hellwig <hch@xxxxxx> > Date: Fri Jun 28 19:27:20 2019 -0700 > > xfs: cleanup xlog_get_iclog_buffer_size > > which is where the change occurred. > > I think this last change is what's causing the difference in > behaviour. Hmm... so the 'default behavior' is changed, should I tend to the lastest upstream behavior, then cause old kernel testing fail (report a bug to merge the upstream change)? Or skip this test, make it as an 'undifined' behavior? Thanks, Zorro > > > > > > > Thanks, > > > > Zorro > > > > > > > > > Ian > > > > > > > > > > > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx> > > > > > > --- > > > > > > tests/xfs/148 | 315 > > > > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > tests/xfs/148.out | 6 + > > > > > > tests/xfs/group | 1 + > > > > > > 3 files changed, 322 insertions(+) > > > > > > create mode 100755 tests/xfs/148 > > > > > > create mode 100644 tests/xfs/148.out > > > > > > > > > > > > diff --git a/tests/xfs/148 b/tests/xfs/148 > > > > > > new file mode 100755 > > > > > > index 00000000..5c268f18 > > > > > > --- /dev/null > > > > > > +++ b/tests/xfs/148 > > > > > > @@ -0,0 +1,315 @@ > > > > > > +#! /bin/bash > > > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > > > +# Copyright (c) 2019 Red Hat, Inc. All Rights Reserved. > > > > > > +# > > > > > > +# FS QA Test 148 > > > > > > +# > > > > > > +# XFS mount options sanity check, refer to 'man 5 xfs'. > > > > > > +# > > > > > > +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 / > > > > > > + rm -f $tmp.* > > > > > > + $UMOUNT_PROG $LOOP_MNT 2>/dev/null > > > > > > + if [ -n "$LOOP_DEV" ];then > > > > > > + _destroy_loop_device $LOOP_DEV 2>/dev/null > > > > > > + fi > > > > > > + if [ -n "$LOOP_SPARE_DEV" ];then > > > > > > + _destroy_loop_device $LOOP_SPARE_DEV > > > > > > 2>/dev/null > > > > > > + fi > > > > > > + rm -f $LOOP_IMG > > > > > > + rm -f $LOOP_SPARE_IMG > > > > > > + rmdir $LOOP_MNT > > > > > > +} > > > > > > + > > > > > > +# get standard environment, filters and checks > > > > > > +. ./common/rc > > > > > > +. ./common/filter > > > > > > + > > > > > > +# remove previous $seqres.full before test > > > > > > +rm -f $seqres.full > > > > > > + > > > > > > +# real QA test starts here > > > > > > +_supported_fs xfs > > > > > > +_supported_os Linux > > > > > > +_require_test > > > > > > +_require_loop > > > > > > +_require_xfs_io_command "falloc" > > > > > > + > > > > > > +LOOP_IMG=$TEST_DIR/$seq.dev > > > > > > +LOOP_SPARE_IMG=$TEST_DIR/$seq.logdev > > > > > > +LOOP_MNT=$TEST_DIR/$seq.mnt > > > > > > + > > > > > > +echo "** create loop device" > > > > > > +$XFS_IO_PROG -f -c "falloc 0 1g" $LOOP_IMG > > > > > > +LOOP_DEV=`_create_loop_device $LOOP_IMG` > > > > > > + > > > > > > +echo "** create loop log device" > > > > > > +$XFS_IO_PROG -f -c "falloc 0 512m" $LOOP_SPARE_IMG > > > > > > +LOOP_SPARE_DEV=`_create_loop_device $LOOP_SPARE_IMG` > > > > > > + > > > > > > +echo "** create loop mount point" > > > > > > +rmdir $LOOP_MNT 2>/dev/null > > > > > > +mkdir -p $LOOP_MNT || _fail "cannot create loopback mount > > > > > > point" > > > > > > + > > > > > > +# avoid the effection from MKFS_OPTIONS > > > > > > +MKFS_OPTIONS="" > > > > > > +do_mkfs() > > > > > > +{ > > > > > > + $MKFS_XFS_PROG -f $* $LOOP_DEV | _filter_mkfs > > > > > > > $seqres.full > > > > > > 2>$tmp.mkfs > > > > > > + if [ "${PIPESTATUS[0]}" -ne 0 ]; then > > > > > > + _fail "Fails on _mkfs_dev $* $LOOP_DEV" > > > > > > + fi > > > > > > + . $tmp.mkfs > > > > > > +} > > > > > > + > > > > > > +is_dev_mounted() > > > > > > +{ > > > > > > + findmnt --source $LOOP_DEV >/dev/null > > > > > > + return $? > > > > > > +} > > > > > > + > > > > > > +get_mount_info() > > > > > > +{ > > > > > > + findmnt --source $LOOP_DEV -o OPTIONS -n > > > > > > +} > > > > > > + > > > > > > +force_unmount() > > > > > > +{ > > > > > > + $UMOUNT_PROG $LOOP_MNT >/dev/null 2>&1 > > > > > > +} > > > > > > + > > > > > > +# _do_test <mount options> <should be mounted?> [<key > > > > > > string> > > > > > > <key > > > > > > should be found?>] > > > > > > +_do_test() > > > > > > +{ > > > > > > + local opts="$1" > > > > > > + local mounted="$2" # pass or fail > > > > > > + local key="$3" > > > > > > + local found="$4" # true or false > > > > > > + local rc > > > > > > + local info > > > > > > + > > > > > > + # mount test > > > > > > + _mount $LOOP_DEV $LOOP_MNT $opts 2>/dev/null > > > > > > + rc=$? > > > > > > + if [ $rc -eq 0 ];then > > > > > > + if [ "${mounted}" = "fail" ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: expect ${mounted}, but > > > > > > pass" > > > > > > + return 1 > > > > > > + fi > > > > > > + is_dev_mounted > > > > > > + if [ $? -ne 0 ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: fs not mounted even mount > > > > > > return > > > > > > 0" > > > > > > + return 1 > > > > > > + fi > > > > > > + else > > > > > > + if [ "${mount_ret}" = "pass" ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: expect ${mounted}, but > > > > > > fail" > > > > > > + return 1 > > > > > > + fi > > > > > > + is_dev_mounted > > > > > > + if [ $? -eq 0 ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: fs is mounted even mount > > > > > > return > > > > > > non-zero" > > > > > > + return 1 > > > > > > + fi > > > > > > + fi > > > > > > + > > > > > > + # Skip below checking if "$key" argument isn't > > > > > > specified > > > > > > + if [ -z "$key" ];then > > > > > > + return 0 > > > > > > + fi > > > > > > + # Check the mount options after fs mounted. > > > > > > + info=`get_mount_info` > > > > > > + echo $info | grep -q "${key}" > > > > > > + rc=$? > > > > > > + if [ $rc -eq 0 ];then > > > > > > + if [ "$found" != "true" ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: expect there's $key in > > > > > > $info, but > > > > > > not found" > > > > > > + return 1 > > > > > > + fi > > > > > > + else > > > > > > + if [ "$found" != "false" ];then > > > > > > + echo "[FAILED]: mount $LOOP_DEV > > > > > > $LOOP_MNT > > > > > > $opts" > > > > > > + echo "ERROR: expect there's not $key in > > > > > > $info, > > > > > > but found" > > > > > > + return 1 > > > > > > + fi > > > > > > + fi > > > > > > + > > > > > > + return 0 > > > > > > +} > > > > > > + > > > > > > +do_test() > > > > > > +{ > > > > > > + # force unmount before testing > > > > > > + force_unmount > > > > > > + _do_test "$@" > > > > > > + # force unmount after testing > > > > > > + force_unmount > > > > > > +} > > > > > > + > > > > > > +echo "** start xfs mount testing ..." > > > > > > +# Test allocsize=size > > > > > > +# Valid values for this option are page size (typically > > > > > > 4KiB) > > > > > > through to 1GiB > > > > > > +do_mkfs > > > > > > +if [ $dbsize -ge 1024 ];then > > > > > > + blsize="$((dbsize / 1024))k" > > > > > > +fi > > > > > > +do_test "" pass "allocsize" "false" > > > > > > +do_test "-o allocsize=$blsize" pass "allocsize=$blsize" > > > > > > "true" > > > > > > +do_test "-o allocsize=1048576k" pass "allocsize=1048576k" > > > > > > "true" > > > > > > +do_test "-o allocsize=$((dbsize / 2))" fail > > > > > > +do_test "-o allocsize=2g" fail > > > > > > + > > > > > > +# Test attr2 > > > > > > +do_mkfs -m crc=1 > > > > > > +do_test "" pass "attr2" "true" > > > > > > +do_test "-o attr2" pass "attr2" "true" > > > > > > +do_test "-o noattr2" fail > > > > > > +do_mkfs -m crc=0 > > > > > > +do_test "" pass "attr2" "true" > > > > > > +do_test "-o attr2" pass "attr2" "true" > > > > > > +do_test "-o noattr2" pass "attr2" "false" > > > > > > + > > > > > > +# Test discard > > > > > > +do_mkfs > > > > > > +do_test "" pass "discard" "false" > > > > > > +do_test "-o discard" pass "discard" "true" > > > > > > +do_test "-o nodiscard" pass "discard" "false" > > > > > > + > > > > > > +# Test grpid|bsdgroups|nogrpid|sysvgroups > > > > > > +do_test "" pass "grpid" "false" > > > > > > +do_test "-o grpid" pass "grpid" "true" > > > > > > +do_test "-o bsdgroups" pass "grpid" "true" > > > > > > +do_test "-o nogrpid" pass "grpid" "false" > > > > > > +do_test "-o sysvgroups" pass "grpid" "false" > > > > > > + > > > > > > +# Test filestreams > > > > > > +do_test "" pass "filestreams" "false" > > > > > > +do_test "-o filestreams" pass "filestreams" "true" > > > > > > + > > > > > > +# Test ikeep > > > > > > +do_test "" pass "ikeep" "false" > > > > > > +do_test "-o ikeep" pass "ikeep" "true" > > > > > > +do_test "-o noikeep" pass "ikeep" "false" > > > > > > + > > > > > > +# Test inode32|inode64 > > > > > > +do_test "" pass "inode64" "true" > > > > > > +do_test "-o inode32" pass "inode32" "true" > > > > > > +do_test "-o inode64" pass "inode64" "true" > > > > > > + > > > > > > +# Test largeio > > > > > > +do_test "" pass "largeio" "false" > > > > > > +do_test "-o largeio" pass "largeio" "true" > > > > > > +do_test "-o nolargeio" pass "largeio" "false" > > > > > > + > > > > > > +# Test logbufs=value. Valid numbers range from 2–8 > > > > > > inclusive. > > > > > > +do_test "" pass "logbufs" "false" > > > > > > +do_test "-o logbufs=8" pass "logbufs=8" "true" > > > > > > +do_test "-o logbufs=2" pass "logbufs=2" "true" > > > > > > +do_test "-o logbufs=1" fail > > > > > > +###### but it gets logbufs=8 now, why? bug? ####### > > > > > > +# do_test "-o logbufs=0" fail > > > > > > +do_test "-o logbufs=9" fail > > > > > > +do_test "-o logbufs=99999999999999" fail > > > > > > + > > > > > > +# Test logbsize=value. > > > > > > +do_mkfs -m crc=1 -l version=2 > > > > > > +do_test "" pass "logbsize" "false" > > > > > > +do_test "-o logbsize=16384" pass "logbsize=16k" "true" > > > > > > +do_test "-o logbsize=16k" pass "logbsize=16k" "true" > > > > > > +do_test "-o logbsize=32k" pass "logbsize=32k" "true" > > > > > > +do_test "-o logbsize=64k" pass "logbsize=64k" "true" > > > > > > +do_test "-o logbsize=128k" pass "logbsize=128k" "true" > > > > > > +do_test "-o logbsize=256k" pass "logbsize=256k" "true" > > > > > > +do_test "-o logbsize=8k" fail > > > > > > +do_test "-o logbsize=512k" fail > > > > > > +####### it's invalid, but it set to default size 32k > > > > > > +# do_test "-o logbsize=0" false > > > > > > +do_mkfs -m crc=0 -l version=1 > > > > > > +do_test "" pass "logbsize" "false" > > > > > > +do_test "-o logbsize=16384" pass "logbsize=16k" "true" > > > > > > +do_test "-o logbsize=16k" pass "logbsize=16k" "true" > > > > > > +do_test "-o logbsize=32k" pass "logbsize=32k" "true" > > > > > > +do_test "-o logbsize=64k" fail > > > > > > + > > > > > > +# Test logdev > > > > > > +do_mkfs > > > > > > +do_test "" pass "logdev" "false" > > > > > > +do_test "-o logdev=$LOOP_SPARE_DEV" fail > > > > > > +do_mkfs -l logdev=$LOOP_SPARE_DEV > > > > > > +do_test "-o logdev=$LOOP_SPARE_DEV" pass > > > > > > "logdev=$LOOP_SPARE_DEV" > > > > > > "true" > > > > > > +do_test "" fail > > > > > > + > > > > > > +# Test noalign > > > > > > +do_mkfs > > > > > > +do_test "" pass "noalign" "false" > > > > > > +do_test "-o noalign" pass "noalign" "true" > > > > > > + > > > > > > +# Test norecovery > > > > > > +do_test "" pass "norecovery" "false" > > > > > > +do_test "-o norecovery,ro" pass "norecovery" "true" > > > > > > +do_test "-o norecovery" fail > > > > > > + > > > > > > +# Test nouuid > > > > > > +do_test "" pass "nouuid" "false" > > > > > > +do_test "-o nouuid" pass "nouuid" "true" > > > > > > + > > > > > > +# Test noquota > > > > > > +do_test "" pass "noquota" "true" > > > > > > +do_test "-o noquota" pass "noquota" "true" > > > > > > + > > > > > > +# Test uquota/usrquota/quota/uqnoenforce/qnoenforce > > > > > > +do_test "" pass "usrquota" "false" > > > > > > +do_test "-o uquota" pass "usrquota" "true" > > > > > > +do_test "-o usrquota" pass "usrquota" "true" > > > > > > +do_test "-o quota" pass "usrquota" "true" > > > > > > +do_test "-o uqnoenforce" pass "usrquota" "true" > > > > > > +do_test "-o qnoenforce" pass "usrquota" "true" > > > > > > + > > > > > > +# Test gquota/grpquota/gqnoenforce > > > > > > +do_test "" pass "grpquota" "false" > > > > > > +do_test "-o gquota" pass "grpquota" "true" > > > > > > +do_test "-o grpquota" pass "grpquota" "true" > > > > > > +do_test "-o gqnoenforce" pass "gqnoenforce" "true" > > > > > > + > > > > > > +# Test pquota/prjquota/pqnoenforce > > > > > > +do_test "" pass "prjquota" "false" > > > > > > +do_test "-o pquota" pass "prjquota" "true" > > > > > > +do_test "-o prjquota" pass "prjquota" "true" > > > > > > +do_test "-o pqnoenforce" pass "pqnoenforce" "true" > > > > > > + > > > > > > +# Test sunit=value and swidth=value > > > > > > +do_mkfs -d sunit=128,swidth=128 > > > > > > +do_test "-o sunit=8,swidth=8" pass "sunit=8,swidth=8" "true" > > > > > > +do_test "-o sunit=8,swidth=64" pass "sunit=8,swidth=64" > > > > > > "true" > > > > > > +do_test "-o sunit=128,swidth=128" pass > > > > > > "sunit=128,swidth=128" > > > > > > "true" > > > > > > +do_test "-o sunit=256,swidth=256" pass > > > > > > "sunit=256,swidth=256" > > > > > > "true" > > > > > > +do_test "-o sunit=2,swidth=2" fail > > > > > > + > > > > > > +# Test swalloc > > > > > > +do_mkfs > > > > > > +do_test "" pass "swalloc" "false" > > > > > > +do_test "-o swalloc" pass "swalloc" "true" > > > > > > + > > > > > > +# Test wsync > > > > > > +do_test "" pass "wsync" "false" > > > > > > +do_test "-o wsync" pass "wsync" "true" > > > > > > + > > > > > > +echo "** end of testing" > > > > > > +# success, all done > > > > > > +status=0 > > > > > > +exit > > > > > > diff --git a/tests/xfs/148.out b/tests/xfs/148.out > > > > > > new file mode 100644 > > > > > > index 00000000..a71d9231 > > > > > > --- /dev/null > > > > > > +++ b/tests/xfs/148.out > > > > > > @@ -0,0 +1,6 @@ > > > > > > +QA output created by 148 > > > > > > +** create loop device > > > > > > +** create loop log device > > > > > > +** create loop mount point > > > > > > +** start xfs mount testing ... > > > > > > +** end of testing > > > > > > diff --git a/tests/xfs/group b/tests/xfs/group > > > > > > index f4ebcd8c..019aebad 100644 > > > > > > --- a/tests/xfs/group > > > > > > +++ b/tests/xfs/group > > > > > > @@ -145,6 +145,7 @@ > > > > > > 145 dmapi > > > > > > 146 dmapi > > > > > > 147 dmapi > > > > > > +148 auto quick mount > > > > > > 150 dmapi > > > > > > 151 dmapi > > > > > > 152 dmapi >