Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] xfstests: xfs mount option sanity test
@ 2019-10-30 10:34 Zorro Lang
  2019-10-30 16:39 ` Darrick J. Wong
  2020-01-13 18:00 ` Darrick J. Wong
  0 siblings, 2 replies; 14+ messages in thread
From: Zorro Lang @ 2019-10-30 10:34 UTC (permalink / raw)
  To: fstests; +Cc: linux-xfs

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.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

V2 did below changes:
1) Fix wrong output messages in _do_test function
2) Remove logbufs=N and logbsize=N default display test. Lastest upstream
   kernel displays these options in /proc/mounts by default, but old kernel
   doesn't show them except user indicate these options when mount xfs.
   Refer to https://marc.info/?l=fstests&m=157199699615477&w=2

Thanks,
Zorro

 tests/xfs/148     | 320 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/148.out |   6 +
 tests/xfs/group   |   1 +
 3 files changed, 327 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..a662f6f7
--- /dev/null
+++ b/tests/xfs/148
@@ -0,0 +1,320 @@
+#! /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 not $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 $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.
+# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
+# prints "logbufs=N" in /proc/mounts, but old kernel not. So the default
+# 'display' about logbufs can't be expected, disable this test.
+#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
+do_test "-o logbufs=9" fail
+do_test "-o logbufs=99999999999999" fail
+
+# Test logbsize=value.
+do_mkfs -m crc=1 -l version=2
+# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
+# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
+# 'display' about logbsize can't be expected, disable this test.
+#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
+do_mkfs -m crc=0 -l version=1
+# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
+# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
+# 'display' about logbsize can't be expected, disable this test.
+#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
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-10-30 10:34 [PATCH v2] xfstests: xfs mount option sanity test Zorro Lang
@ 2019-10-30 16:39 ` Darrick J. Wong
  2019-10-30 23:24   ` Zorro Lang
  2020-01-13 18:00 ` Darrick J. Wong
  1 sibling, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2019-10-30 16:39 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, linux-xfs

On Wed, Oct 30, 2019 at 06:34:10PM +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.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V2 did below changes:
> 1) Fix wrong output messages in _do_test function

Hmm, I still see this on 5.4-rc4:

+[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
+ERROR: expect there's logbsize=16k in , but found
+[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
+ERROR: expect there's logbsize=16k in , but found

Oh, right, you're stripping out MKFS_OPTIONS and formatting a loop
device, which on my system means you get rmapbt=1 by default and
whatnot.

I think the larger problem here might be that now we have to figure out
the special-casing of some of these options.

--D

> 2) Remove logbufs=N and logbsize=N default display test. Lastest upstream
>    kernel displays these options in /proc/mounts by default, but old kernel
>    doesn't show them except user indicate these options when mount xfs.
>    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> 
> Thanks,
> Zorro
> 
>  tests/xfs/148     | 320 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/148.out |   6 +
>  tests/xfs/group   |   1 +
>  3 files changed, 327 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..a662f6f7
> --- /dev/null
> +++ b/tests/xfs/148
> @@ -0,0 +1,320 @@
> +#! /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 not $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 $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.
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbufs=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbufs can't be expected, disable this test.
> +#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
> +do_test "-o logbufs=9" fail
> +do_test "-o logbufs=99999999999999" fail
> +
> +# Test logbsize=value.
> +do_mkfs -m crc=1 -l version=2
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbsize can't be expected, disable this test.
> +#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
> +do_mkfs -m crc=0 -l version=1
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbsize can't be expected, disable this test.
> +#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
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-10-30 16:39 ` Darrick J. Wong
@ 2019-10-30 23:24   ` Zorro Lang
  2019-12-11  6:42     ` Zorro Lang
  0 siblings, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2019-10-30 23:24 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> > 
> > Hi,
> > 
> > V2 did below changes:
> > 1) Fix wrong output messages in _do_test function
> 
> Hmm, I still see this on 5.4-rc4:
> 
> +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> +ERROR: expect there's logbsize=16k in , but found
                                             ^^^
                                             not

> +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> +ERROR: expect there's logbsize=16k in , but found
                                             ^^^
                                             not

Sorry for this typo, I'll fix it.

> 
> Oh, right, you're stripping out MKFS_OPTIONS and formatting a loop
> device, which on my system means you get rmapbt=1 by default and
> whatnot.

Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in /proc/mounts?

Actually set MKFS_OPTIONS="" is not helpful for this case, due to I run
"$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out MKFS_OPTIONS,
because I use SCRACH_DEV at first :)

> 
> I think the larger problem here might be that now we have to figure out
> the special-casing of some of these options.

Maybe we should avoid the testing about those behaviors can't be sure, if we
can't make it have a fixed output.

Thanks,
Zorro

> 
> --D
> 
> > 2) Remove logbufs=N and logbsize=N default display test. Lastest upstream
> >    kernel displays these options in /proc/mounts by default, but old kernel
> >    doesn't show them except user indicate these options when mount xfs.
> >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > 
> > Thanks,
> > Zorro
> > 
> >  tests/xfs/148     | 320 ++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/148.out |   6 +
> >  tests/xfs/group   |   1 +
> >  3 files changed, 327 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..a662f6f7
> > --- /dev/null
> > +++ b/tests/xfs/148
> > @@ -0,0 +1,320 @@
> > +#! /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 not $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 $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.
> > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So the default
> > +# 'display' about logbufs can't be expected, disable this test.
> > +#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
> > +do_test "-o logbufs=9" fail
> > +do_test "-o logbufs=99999999999999" fail
> > +
> > +# Test logbsize=value.
> > +do_mkfs -m crc=1 -l version=2
> > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> > +# 'display' about logbsize can't be expected, disable this test.
> > +#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
> > +do_mkfs -m crc=0 -l version=1
> > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> > +# 'display' about logbsize can't be expected, disable this test.
> > +#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
> > -- 
> > 2.20.1
> > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-10-30 23:24   ` Zorro Lang
@ 2019-12-11  6:42     ` Zorro Lang
  2019-12-11  8:33       ` Ian Kent
  0 siblings, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2019-12-11  6:42 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > > 
> > > Hi,
> > > 
> > > V2 did below changes:
> > > 1) Fix wrong output messages in _do_test function
> > 
> > Hmm, I still see this on 5.4-rc4:
> > 
> > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > +ERROR: expect there's logbsize=16k in , but found
>                                              ^^^
>                                              not
> 
> > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > +ERROR: expect there's logbsize=16k in , but found
>                                              ^^^
>                                              not
> 
> Sorry for this typo, I'll fix it.
> 
> > 
> > Oh, right, you're stripping out MKFS_OPTIONS and formatting a loop
> > device, which on my system means you get rmapbt=1 by default and
> > whatnot.
> 
> Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in /proc/mounts?
> 
> Actually set MKFS_OPTIONS="" is not helpful for this case, due to I run
> "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out MKFS_OPTIONS,
> because I use SCRACH_DEV at first :)
> 
> > 
> > I think the larger problem here might be that now we have to figure out
> > the special-casing of some of these options.
> 
> Maybe we should avoid the testing about those behaviors can't be sure, if we
> can't make it have a fixed output.

It's been long time passed. I still don't have a proper way to reproduce and
avoid this failure you hit. Does anyone has a better idea for this case?

Thanks,
Zorro

> 
> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > 2) Remove logbufs=N and logbsize=N default display test. Lastest upstream
> > >    kernel displays these options in /proc/mounts by default, but old kernel
> > >    doesn't show them except user indicate these options when mount xfs.
> > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > 
> > > Thanks,
> > > Zorro
> > > 
> > >  tests/xfs/148     | 320 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/xfs/148.out |   6 +
> > >  tests/xfs/group   |   1 +
> > >  3 files changed, 327 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..a662f6f7
> > > --- /dev/null
> > > +++ b/tests/xfs/148
> > > @@ -0,0 +1,320 @@
> > > +#! /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 not $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 $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.
> > > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So the default
> > > +# 'display' about logbufs can't be expected, disable this test.
> > > +#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
> > > +do_test "-o logbufs=9" fail
> > > +do_test "-o logbufs=99999999999999" fail
> > > +
> > > +# Test logbsize=value.
> > > +do_mkfs -m crc=1 -l version=2
> > > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> > > +# 'display' about logbsize can't be expected, disable this test.
> > > +#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
> > > +do_mkfs -m crc=0 -l version=1
> > > +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> > > +# 'display' about logbsize can't be expected, disable this test.
> > > +#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
> > > -- 
> > > 2.20.1
> > > 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-12-11  6:42     ` Zorro Lang
@ 2019-12-11  8:33       ` Ian Kent
  2019-12-16  9:17         ` Zorro Lang
  0 siblings, 1 reply; 14+ messages in thread
From: Ian Kent @ 2019-12-11  8:33 UTC (permalink / raw)
  To: Zorro Lang, Darrick J. Wong; +Cc: fstests, linux-xfs

On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > 
> > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > ---
> > > > 
> > > > Hi,
> > > > 
> > > > V2 did below changes:
> > > > 1) Fix wrong output messages in _do_test function
> > > 
> > > Hmm, I still see this on 5.4-rc4:
> > > 
> > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > +ERROR: expect there's logbsize=16k in , but found
> >                                              ^^^
> >                                              not
> > 
> > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > +ERROR: expect there's logbsize=16k in , but found
> >                                              ^^^
> >                                              not
> > 
> > Sorry for this typo, I'll fix it.
> > 
> > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > loop
> > > device, which on my system means you get rmapbt=1 by default and
> > > whatnot.
> > 
> > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > /proc/mounts?
> > 
> > Actually set MKFS_OPTIONS="" is not helpful for this case, due to I
> > run
> > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > MKFS_OPTIONS,
> > because I use SCRACH_DEV at first :)
> > 
> > > I think the larger problem here might be that now we have to
> > > figure out
> > > the special-casing of some of these options.
> > 
> > Maybe we should avoid the testing about those behaviors can't be
> > sure, if we
> > can't make it have a fixed output.
> 
> It's been long time passed. I still don't have a proper way to
> reproduce and
> avoid this failure you hit. Does anyone has a better idea for this
> case?

I think I understand the problem but correct me if I'm wrong.

Couldn't you cover both cases, pass an additional parameter that
says what the default is if it isn't specified as an option and
don't fail if it is present in the options.

You could check kernel versions to decide whether to pass the
third parameter and so decide if you need to allow for a default
option to account for the differing behaviour.

Ian
> 
> Thanks,
> Zorro
> 
> > Thanks,
> > Zorro
> > 
> > > --D
> > > 
> > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > Lastest upstream
> > > >    kernel displays these options in /proc/mounts by default,
> > > > but old kernel
> > > >    doesn't show them except user indicate these options when
> > > > mount xfs.
> > > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > >  tests/xfs/148     | 320
> > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/xfs/148.out |   6 +
> > > >  tests/xfs/group   |   1 +
> > > >  3 files changed, 327 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..a662f6f7
> > > > --- /dev/null
> > > > +++ b/tests/xfs/148
> > > > @@ -0,0 +1,320 @@
> > > > +#! /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 not $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 $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.
> > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > xlog_get_iclog_buffer_size)
> > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So
> > > > the default
> > > > +# 'display' about logbufs can't be expected, disable this
> > > > test.
> > > > +#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
> > > > +do_test "-o logbufs=9" fail
> > > > +do_test "-o logbufs=99999999999999" fail
> > > > +
> > > > +# Test logbsize=value.
> > > > +do_mkfs -m crc=1 -l version=2
> > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > xlog_get_iclog_buffer_size)
> > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > the default
> > > > +# 'display' about logbsize can't be expected, disable this
> > > > test.
> > > > +#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
> > > > +do_mkfs -m crc=0 -l version=1
> > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > xlog_get_iclog_buffer_size)
> > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > the default
> > > > +# 'display' about logbsize can't be expected, disable this
> > > > test.
> > > > +#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
> > > > -- 
> > > > 2.20.1
> > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-12-11  8:33       ` Ian Kent
@ 2019-12-16  9:17         ` Zorro Lang
  2019-12-16  9:36           ` Zorro Lang
  2019-12-16 10:42           ` Ian Kent
  0 siblings, 2 replies; 14+ messages in thread
From: Zorro Lang @ 2019-12-16  9:17 UTC (permalink / raw)
  To: Ian Kent; +Cc: Darrick J. Wong, fstests, linux-xfs

On Wed, Dec 11, 2019 at 04:33:20PM +0800, Ian Kent wrote:
> On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> > On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > > 
> > > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > > ---
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > V2 did below changes:
> > > > > 1) Fix wrong output messages in _do_test function
> > > > 
> > > > Hmm, I still see this on 5.4-rc4:
> > > > 
> > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > > +ERROR: expect there's logbsize=16k in , but found
> > >                                              ^^^
> > >                                              not
> > > 
> > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > > +ERROR: expect there's logbsize=16k in , but found
> > >                                              ^^^
> > >                                              not
> > > 
> > > Sorry for this typo, I'll fix it.
> > > 
> > > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > > loop
> > > > device, which on my system means you get rmapbt=1 by default and
> > > > whatnot.
> > > 
> > > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > > /proc/mounts?
> > > 
> > > Actually set MKFS_OPTIONS="" is not helpful for this case, due to I
> > > run
> > > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > > MKFS_OPTIONS,
> > > because I use SCRACH_DEV at first :)
> > > 
> > > > I think the larger problem here might be that now we have to
> > > > figure out
> > > > the special-casing of some of these options.
> > > 
> > > Maybe we should avoid the testing about those behaviors can't be
> > > sure, if we
> > > can't make it have a fixed output.
> > 
> > It's been long time passed. I still don't have a proper way to
> > reproduce and
> > avoid this failure you hit. Does anyone has a better idea for this
> > case?
> 
> I think I understand the problem but correct me if I'm wrong.
> 
> Couldn't you cover both cases, pass an additional parameter that
> says what the default is if it isn't specified as an option and
> don't fail if it is present in the options.
> 
> You could check kernel versions to decide whether to pass the
> third parameter and so decide if you need to allow for a default
> option to account for the differing behaviour.

Hi Ian,

It's not a issue about displaying default mount options. It's:

# mount $dev_xfs $mnt -o logbsize=16k
# findmnt --source $dev_xfs -o OPTIONS -n |grep "logbsize=16k"
<nothing output>

The case expects there's "logbsize=16k", but it can't find that option.

I don't know how to reproduce it.

Thanks,
Zorro

> 
> Ian
> > 
> > Thanks,
> > Zorro
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > > --D
> > > > 
> > > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > > Lastest upstream
> > > > >    kernel displays these options in /proc/mounts by default,
> > > > > but old kernel
> > > > >    doesn't show them except user indicate these options when
> > > > > mount xfs.
> > > > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > > 
> > > > > Thanks,
> > > > > Zorro
> > > > > 
> > > > >  tests/xfs/148     | 320
> > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  tests/xfs/148.out |   6 +
> > > > >  tests/xfs/group   |   1 +
> > > > >  3 files changed, 327 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..a662f6f7
> > > > > --- /dev/null
> > > > > +++ b/tests/xfs/148
> > > > > @@ -0,0 +1,320 @@
> > > > > +#! /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 not $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 $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.
> > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > xlog_get_iclog_buffer_size)
> > > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So
> > > > > the default
> > > > > +# 'display' about logbufs can't be expected, disable this
> > > > > test.
> > > > > +#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
> > > > > +do_test "-o logbufs=9" fail
> > > > > +do_test "-o logbufs=99999999999999" fail
> > > > > +
> > > > > +# Test logbsize=value.
> > > > > +do_mkfs -m crc=1 -l version=2
> > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > xlog_get_iclog_buffer_size)
> > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > the default
> > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > test.
> > > > > +#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
> > > > > +do_mkfs -m crc=0 -l version=1
> > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > xlog_get_iclog_buffer_size)
> > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > the default
> > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > test.
> > > > > +#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
> > > > > -- 
> > > > > 2.20.1
> > > > > 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-12-16  9:17         ` Zorro Lang
@ 2019-12-16  9:36           ` Zorro Lang
  2020-01-13 15:40             ` Zorro Lang
  2019-12-16 10:42           ` Ian Kent
  1 sibling, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2019-12-16  9:36 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On Mon, Dec 16, 2019 at 05:17:07PM +0800, Zorro Lang wrote:
> On Wed, Dec 11, 2019 at 04:33:20PM +0800, Ian Kent wrote:
> > On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> > > On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > > > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > > > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > > > 
> > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > V2 did below changes:
> > > > > > 1) Fix wrong output messages in _do_test function
> > > > > 
> > > > > Hmm, I still see this on 5.4-rc4:
> > > > > 
> > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > > > +ERROR: expect there's logbsize=16k in , but found
> > > >                                              ^^^
> > > >                                              not
> > > > 
> > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > > > +ERROR: expect there's logbsize=16k in , but found
> > > >                                              ^^^
> > > >                                              not
> > > > 
> > > > Sorry for this typo, I'll fix it.
> > > > 
> > > > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > > > loop
> > > > > device, which on my system means you get rmapbt=1 by default and
> > > > > whatnot.
> > > > 
> > > > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > > > /proc/mounts?
> > > > 
> > > > Actually set MKFS_OPTIONS="" is not helpful for this case, due to I
> > > > run
> > > > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > > > MKFS_OPTIONS,
> > > > because I use SCRACH_DEV at first :)
> > > > 
> > > > > I think the larger problem here might be that now we have to
> > > > > figure out
> > > > > the special-casing of some of these options.
> > > > 
> > > > Maybe we should avoid the testing about those behaviors can't be
> > > > sure, if we
> > > > can't make it have a fixed output.
> > > 
> > > It's been long time passed. I still don't have a proper way to
> > > reproduce and
> > > avoid this failure you hit. Does anyone has a better idea for this
> > > case?
> > 
> > I think I understand the problem but correct me if I'm wrong.
> > 
> > Couldn't you cover both cases, pass an additional parameter that
> > says what the default is if it isn't specified as an option and
> > don't fail if it is present in the options.
> > 
> > You could check kernel versions to decide whether to pass the
> > third parameter and so decide if you need to allow for a default
> > option to account for the differing behaviour.
> 
> Hi Ian,
> 
> It's not a issue about displaying default mount options. It's:
> 
> # mount $dev_xfs $mnt -o logbsize=16k
> # findmnt --source $dev_xfs -o OPTIONS -n |grep "logbsize=16k"
> <nothing output>
> 
> The case expects there's "logbsize=16k", but it can't find that option.
> 
> I don't know how to reproduce it.

Hi Darrick,

Can you still reproduce this failure? Would you mind telling me how to reproduce
it or how to correct my case :-P

The xfs_fs_show_options() function shows:

        if (mp->m_logbsize > 0)
                seq_printf(m, ",logbsize=%dk", mp->m_logbsize >> 10);

I think the logbsize should be printed if we set '-o logbsize=16k'.

Thanks,
Zorro

> 
> Thanks,
> Zorro
> 
> > 
> > Ian
> > > 
> > > Thanks,
> > > Zorro
> > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > > > --D
> > > > > 
> > > > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > > > Lastest upstream
> > > > > >    kernel displays these options in /proc/mounts by default,
> > > > > > but old kernel
> > > > > >    doesn't show them except user indicate these options when
> > > > > > mount xfs.
> > > > > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > > > 
> > > > > > Thanks,
> > > > > > Zorro
> > > > > > 
> > > > > >  tests/xfs/148     | 320
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  tests/xfs/148.out |   6 +
> > > > > >  tests/xfs/group   |   1 +
> > > > > >  3 files changed, 327 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..a662f6f7
> > > > > > --- /dev/null
> > > > > > +++ b/tests/xfs/148
> > > > > > @@ -0,0 +1,320 @@
> > > > > > +#! /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 not $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 $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.
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So
> > > > > > the default
> > > > > > +# 'display' about logbufs can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > +do_test "-o logbufs=9" fail
> > > > > > +do_test "-o logbufs=99999999999999" fail
> > > > > > +
> > > > > > +# Test logbsize=value.
> > > > > > +do_mkfs -m crc=1 -l version=2
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > the default
> > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > +do_mkfs -m crc=0 -l version=1
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > the default
> > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > -- 
> > > > > > 2.20.1
> > > > > > 
> > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-12-16  9:17         ` Zorro Lang
  2019-12-16  9:36           ` Zorro Lang
@ 2019-12-16 10:42           ` Ian Kent
  1 sibling, 0 replies; 14+ messages in thread
From: Ian Kent @ 2019-12-16 10:42 UTC (permalink / raw)
  To: Zorro Lang; +Cc: Darrick J. Wong, fstests, linux-xfs

On Mon, 2019-12-16 at 17:17 +0800, Zorro Lang wrote:
> On Wed, Dec 11, 2019 at 04:33:20PM +0800, Ian Kent wrote:
> > On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> > > On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > > > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong
> > > > wrote:
> > > > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > > > 
> > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > V2 did below changes:
> > > > > > 1) Fix wrong output messages in _do_test function
> > > > > 
> > > > > Hmm, I still see this on 5.4-rc4:
> > > > > 
> > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > > > +ERROR: expect there's logbsize=16k in , but found
> > > >                                              ^^^
> > > >                                              not
> > > > 
> > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > > > +ERROR: expect there's logbsize=16k in , but found
> > > >                                              ^^^
> > > >                                              not
> > > > 
> > > > Sorry for this typo, I'll fix it.
> > > > 
> > > > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > > > loop
> > > > > device, which on my system means you get rmapbt=1 by default
> > > > > and
> > > > > whatnot.
> > > > 
> > > > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > > > /proc/mounts?
> > > > 
> > > > Actually set MKFS_OPTIONS="" is not helpful for this case, due
> > > > to I
> > > > run
> > > > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > > > MKFS_OPTIONS,
> > > > because I use SCRACH_DEV at first :)
> > > > 
> > > > > I think the larger problem here might be that now we have to
> > > > > figure out
> > > > > the special-casing of some of these options.
> > > > 
> > > > Maybe we should avoid the testing about those behaviors can't
> > > > be
> > > > sure, if we
> > > > can't make it have a fixed output.
> > > 
> > > It's been long time passed. I still don't have a proper way to
> > > reproduce and
> > > avoid this failure you hit. Does anyone has a better idea for
> > > this
> > > case?
> > 
> > I think I understand the problem but correct me if I'm wrong.
> > 
> > Couldn't you cover both cases, pass an additional parameter that
> > says what the default is if it isn't specified as an option and
> > don't fail if it is present in the options.
> > 
> > You could check kernel versions to decide whether to pass the
> > third parameter and so decide if you need to allow for a default
> > option to account for the differing behaviour.
> 
> Hi Ian,
> 
> It's not a issue about displaying default mount options. It's:
> 
> # mount $dev_xfs $mnt -o logbsize=16k
> # findmnt --source $dev_xfs -o OPTIONS -n |grep "logbsize=16k"
> <nothing output>
> 
> The case expects there's "logbsize=16k", but it can't find that
> option.
> 
> I don't know how to reproduce it.

Yes, my mistake.

And the only way I can see the option not being printed is if the
super block info. field m_logbsize is zero and I can't see anywhere
that can happen, assuming it is set properly from the options (and
it should be AFAICS).

> 
> Thanks,
> Zorro
> 
> > Ian
> > > Thanks,
> > > Zorro
> > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > > > --D
> > > > > 
> > > > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > > > Lastest upstream
> > > > > >    kernel displays these options in /proc/mounts by
> > > > > > default,
> > > > > > but old kernel
> > > > > >    doesn't show them except user indicate these options
> > > > > > when
> > > > > > mount xfs.
> > > > > >    Refer to 
> > > > > > https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > > > 
> > > > > > Thanks,
> > > > > > Zorro
> > > > > > 
> > > > > >  tests/xfs/148     | 320
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  tests/xfs/148.out |   6 +
> > > > > >  tests/xfs/group   |   1 +
> > > > > >  3 files changed, 327 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..a662f6f7
> > > > > > --- /dev/null
> > > > > > +++ b/tests/xfs/148
> > > > > > @@ -0,0 +1,320 @@
> > > > > > +#! /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 not $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 $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.
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not.
> > > > > > So
> > > > > > the default
> > > > > > +# 'display' about logbufs can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > +do_test "-o logbufs=9" fail
> > > > > > +do_test "-o logbufs=99999999999999" fail
> > > > > > +
> > > > > > +# Test logbsize=value.
> > > > > > +do_mkfs -m crc=1 -l version=2
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not.
> > > > > > So
> > > > > > the default
> > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > +do_mkfs -m crc=0 -l version=1
> > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > xlog_get_iclog_buffer_size)
> > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not.
> > > > > > So
> > > > > > the default
> > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > test.
> > > > > > +#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
> > > > > > -- 
> > > > > > 2.20.1
> > > > > > 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-12-16  9:36           ` Zorro Lang
@ 2020-01-13 15:40             ` Zorro Lang
  2020-01-13 17:33               ` Darrick J. Wong
  0 siblings, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2020-01-13 15:40 UTC (permalink / raw)
  To: fstests; +Cc: Darrick J. Wong, linux-xfs

On Mon, Dec 16, 2019 at 05:36:08PM +0800, Zorro Lang wrote:
> On Mon, Dec 16, 2019 at 05:17:07PM +0800, Zorro Lang wrote:
> > On Wed, Dec 11, 2019 at 04:33:20PM +0800, Ian Kent wrote:
> > > On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> > > > On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > > > > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > > > > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > > > > 
> > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > > > > ---
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > V2 did below changes:
> > > > > > > 1) Fix wrong output messages in _do_test function
> > > > > > 
> > > > > > Hmm, I still see this on 5.4-rc4:
> > > > > > 
> > > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > > > > +ERROR: expect there's logbsize=16k in , but found
> > > > >                                              ^^^
> > > > >                                              not
> > > > > 
> > > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > > > > +ERROR: expect there's logbsize=16k in , but found
> > > > >                                              ^^^
> > > > >                                              not
> > > > > 
> > > > > Sorry for this typo, I'll fix it.
> > > > > 
> > > > > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > > > > loop
> > > > > > device, which on my system means you get rmapbt=1 by default and
> > > > > > whatnot.
> > > > > 
> > > > > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > > > > /proc/mounts?
> > > > > 
> > > > > Actually set MKFS_OPTIONS="" is not helpful for this case, due to I
> > > > > run
> > > > > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > > > > MKFS_OPTIONS,
> > > > > because I use SCRACH_DEV at first :)
> > > > > 
> > > > > > I think the larger problem here might be that now we have to
> > > > > > figure out
> > > > > > the special-casing of some of these options.
> > > > > 
> > > > > Maybe we should avoid the testing about those behaviors can't be
> > > > > sure, if we
> > > > > can't make it have a fixed output.
> > > > 
> > > > It's been long time passed. I still don't have a proper way to
> > > > reproduce and
> > > > avoid this failure you hit. Does anyone has a better idea for this
> > > > case?
> > > 
> > > I think I understand the problem but correct me if I'm wrong.
> > > 
> > > Couldn't you cover both cases, pass an additional parameter that
> > > says what the default is if it isn't specified as an option and
> > > don't fail if it is present in the options.
> > > 
> > > You could check kernel versions to decide whether to pass the
> > > third parameter and so decide if you need to allow for a default
> > > option to account for the differing behaviour.
> > 
> > Hi Ian,
> > 
> > It's not a issue about displaying default mount options. It's:
> > 
> > # mount $dev_xfs $mnt -o logbsize=16k
> > # findmnt --source $dev_xfs -o OPTIONS -n |grep "logbsize=16k"
> > <nothing output>
> > 
> > The case expects there's "logbsize=16k", but it can't find that option.
> > 
> > I don't know how to reproduce it.
> 
> Hi Darrick,
> 
> Can you still reproduce this failure? Would you mind telling me how to reproduce
> it or how to correct my case :-P
> 
> The xfs_fs_show_options() function shows:
> 
>         if (mp->m_logbsize > 0)
>                 seq_printf(m, ",logbsize=%dk", mp->m_logbsize >> 10);
> 
> I think the logbsize should be printed if we set '-o logbsize=16k'.

Ping. This patch has been blocked for long time, may I ask how to make
it to be merged.

Thanks,
Zorro

> 
> Thanks,
> Zorro
> 
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Ian
> > > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > > > Thanks,
> > > > > Zorro
> > > > > 
> > > > > > --D
> > > > > > 
> > > > > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > > > > Lastest upstream
> > > > > > >    kernel displays these options in /proc/mounts by default,
> > > > > > > but old kernel
> > > > > > >    doesn't show them except user indicate these options when
> > > > > > > mount xfs.
> > > > > > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > Zorro
> > > > > > > 
> > > > > > >  tests/xfs/148     | 320
> > > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  tests/xfs/148.out |   6 +
> > > > > > >  tests/xfs/group   |   1 +
> > > > > > >  3 files changed, 327 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..a662f6f7
> > > > > > > --- /dev/null
> > > > > > > +++ b/tests/xfs/148
> > > > > > > @@ -0,0 +1,320 @@
> > > > > > > +#! /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 not $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 $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.
> > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So
> > > > > > > the default
> > > > > > > +# 'display' about logbufs can't be expected, disable this
> > > > > > > test.
> > > > > > > +#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
> > > > > > > +do_test "-o logbufs=9" fail
> > > > > > > +do_test "-o logbufs=99999999999999" fail
> > > > > > > +
> > > > > > > +# Test logbsize=value.
> > > > > > > +do_mkfs -m crc=1 -l version=2
> > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > > the default
> > > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > > test.
> > > > > > > +#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
> > > > > > > +do_mkfs -m crc=0 -l version=1
> > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > > the default
> > > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > > test.
> > > > > > > +#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
> > > > > > > -- 
> > > > > > > 2.20.1
> > > > > > > 
> > > 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2020-01-13 15:40             ` Zorro Lang
@ 2020-01-13 17:33               ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2020-01-13 17:33 UTC (permalink / raw)
  To: fstests, linux-xfs

On Mon, Jan 13, 2020 at 11:40:53PM +0800, Zorro Lang wrote:
> On Mon, Dec 16, 2019 at 05:36:08PM +0800, Zorro Lang wrote:
> > On Mon, Dec 16, 2019 at 05:17:07PM +0800, Zorro Lang wrote:
> > > On Wed, Dec 11, 2019 at 04:33:20PM +0800, Ian Kent wrote:
> > > > On Wed, 2019-12-11 at 14:42 +0800, Zorro Lang wrote:
> > > > > On Thu, Oct 31, 2019 at 07:24:53AM +0800, Zorro Lang wrote:
> > > > > > On Wed, Oct 30, 2019 at 09:39:22AM -0700, Darrick J. Wong wrote:
> > > > > > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > > > > > > 
> > > > > > > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > V2 did below changes:
> > > > > > > > 1) Fix wrong output messages in _do_test function
> > > > > > > 
> > > > > > > Hmm, I still see this on 5.4-rc4:
> > > > > > > 
> > > > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16384
> > > > > > > +ERROR: expect there's logbsize=16k in , but found
> > > > > >                                              ^^^
> > > > > >                                              not
> > > > > > 
> > > > > > > +[FAILED]: mount /dev/loop0 /mnt/148.mnt -o logbsize=16k
> > > > > > > +ERROR: expect there's logbsize=16k in , but found
> > > > > >                                              ^^^
> > > > > >                                              not
> > > > > > 
> > > > > > Sorry for this typo, I'll fix it.
> > > > > > 
> > > > > > > Oh, right, you're stripping out MKFS_OPTIONS and formatting a
> > > > > > > loop
> > > > > > > device, which on my system means you get rmapbt=1 by default and
> > > > > > > whatnot.
> > > > > > 
> > > > > > Hmm...  why rmapbt=1 cause logbsize=16k can't be displayed in
> > > > > > /proc/mounts?
> > > > > > 
> > > > > > Actually set MKFS_OPTIONS="" is not helpful for this case, due to I
> > > > > > run
> > > > > > "$MKFS_XFS_PROG -f $* $LOOP_DEV" directly. I strip out
> > > > > > MKFS_OPTIONS,
> > > > > > because I use SCRACH_DEV at first :)
> > > > > > 
> > > > > > > I think the larger problem here might be that now we have to
> > > > > > > figure out
> > > > > > > the special-casing of some of these options.
> > > > > > 
> > > > > > Maybe we should avoid the testing about those behaviors can't be
> > > > > > sure, if we
> > > > > > can't make it have a fixed output.
> > > > > 
> > > > > It's been long time passed. I still don't have a proper way to
> > > > > reproduce and
> > > > > avoid this failure you hit. Does anyone has a better idea for this
> > > > > case?
> > > > 
> > > > I think I understand the problem but correct me if I'm wrong.
> > > > 
> > > > Couldn't you cover both cases, pass an additional parameter that
> > > > says what the default is if it isn't specified as an option and
> > > > don't fail if it is present in the options.
> > > > 
> > > > You could check kernel versions to decide whether to pass the
> > > > third parameter and so decide if you need to allow for a default
> > > > option to account for the differing behaviour.
> > > 
> > > Hi Ian,
> > > 
> > > It's not a issue about displaying default mount options. It's:
> > > 
> > > # mount $dev_xfs $mnt -o logbsize=16k
> > > # findmnt --source $dev_xfs -o OPTIONS -n |grep "logbsize=16k"
> > > <nothing output>
> > > 
> > > The case expects there's "logbsize=16k", but it can't find that option.
> > > 
> > > I don't know how to reproduce it.
> > 
> > Hi Darrick,
> > 
> > Can you still reproduce this failure? Would you mind telling me how to reproduce
> > it or how to correct my case :-P
> > 
> > The xfs_fs_show_options() function shows:
> > 
> >         if (mp->m_logbsize > 0)
> >                 seq_printf(m, ",logbsize=%dk", mp->m_logbsize >> 10);
> > 
> > I think the logbsize should be printed if we set '-o logbsize=16k'.
> 
> Ping. This patch has been blocked for long time, may I ask how to make
> it to be merged.

Ahhhh, this one.  Um, I figured out what was causing all the problems at
the beginning of this thread and will reply to it.

--D

> Thanks,
> Zorro
> 
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Thanks,
> > > Zorro
> > > 
> > > > 
> > > > Ian
> > > > > 
> > > > > Thanks,
> > > > > Zorro
> > > > > 
> > > > > > Thanks,
> > > > > > Zorro
> > > > > > 
> > > > > > > --D
> > > > > > > 
> > > > > > > > 2) Remove logbufs=N and logbsize=N default display test.
> > > > > > > > Lastest upstream
> > > > > > > >    kernel displays these options in /proc/mounts by default,
> > > > > > > > but old kernel
> > > > > > > >    doesn't show them except user indicate these options when
> > > > > > > > mount xfs.
> > > > > > > >    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Zorro
> > > > > > > > 
> > > > > > > >  tests/xfs/148     | 320
> > > > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  tests/xfs/148.out |   6 +
> > > > > > > >  tests/xfs/group   |   1 +
> > > > > > > >  3 files changed, 327 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..a662f6f7
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/tests/xfs/148
> > > > > > > > @@ -0,0 +1,320 @@
> > > > > > > > +#! /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 not $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 $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.
> > > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > > +# prints "logbufs=N" in /proc/mounts, but old kernel not. So
> > > > > > > > the default
> > > > > > > > +# 'display' about logbufs can't be expected, disable this
> > > > > > > > test.
> > > > > > > > +#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
> > > > > > > > +do_test "-o logbufs=9" fail
> > > > > > > > +do_test "-o logbufs=99999999999999" fail
> > > > > > > > +
> > > > > > > > +# Test logbsize=value.
> > > > > > > > +do_mkfs -m crc=1 -l version=2
> > > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > > > the default
> > > > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > > > test.
> > > > > > > > +#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
> > > > > > > > +do_mkfs -m crc=0 -l version=1
> > > > > > > > +# New kernel (refer to 4f62282a3696 xfs: cleanup
> > > > > > > > xlog_get_iclog_buffer_size)
> > > > > > > > +# prints "logbsize=N" in /proc/mounts, but old kernel not. So
> > > > > > > > the default
> > > > > > > > +# 'display' about logbsize can't be expected, disable this
> > > > > > > > test.
> > > > > > > > +#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
> > > > > > > > -- 
> > > > > > > > 2.20.1
> > > > > > > > 
> > > > 
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2019-10-30 10:34 [PATCH v2] xfstests: xfs mount option sanity test Zorro Lang
  2019-10-30 16:39 ` Darrick J. Wong
@ 2020-01-13 18:00 ` Darrick J. Wong
  2020-01-13 18:23   ` Darrick J. Wong
  1 sibling, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2020-01-13 18:00 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, linux-xfs, Eric Sandeen

On Wed, Oct 30, 2019 at 06:34:10PM +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.

This testcase examines the intersection of some mkfs options and a large
number of mount options, but it doesn't log any breadcrumbs of which
scenario it's testing at any given time.  When something goes wrong,
it's /very/ difficult to map that back to the do_mkfs/do_test call.

(FWIW I added this test as xfs/997 and attached the diff I needed to
debug the problem I complained about a month ago.)

> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V2 did below changes:
> 1) Fix wrong output messages in _do_test function
> 2) Remove logbufs=N and logbsize=N default display test. Lastest upstream
>    kernel displays these options in /proc/mounts by default, but old kernel
>    doesn't show them except user indicate these options when mount xfs.
>    Refer to https://marc.info/?l=fstests&m=157199699615477&w=2
> 
> Thanks,
> Zorro
> 
>  tests/xfs/148     | 320 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/148.out |   6 +
>  tests/xfs/group   |   1 +
>  3 files changed, 327 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..a662f6f7
> --- /dev/null
> +++ b/tests/xfs/148
> @@ -0,0 +1,320 @@
> +#! /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()
> +{

Please log what options we're passing.

> +	$MKFS_XFS_PROG -f $* $LOOP_DEV | _filter_mkfs >$seqres.full 2>$tmp.mkfs

>>$seqres.full, because this otherwise obliterates seqres.full

> +	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

Please capture the stderr output to seqres.full

> +	rc=$?
> +	if [ $rc -eq 0 ];then
> +		if [ "${mounted}" = "fail" ];then
> +			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> +			echo "ERROR: expect ${mounted}, but pass"

The wording of these error messages is a little weird, I'll fix that....

> +			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

$mount_ret is not defined here, did you mean $mounted?

> +			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}"

echo "$info" so we can at least pretend to have proper string handling
;)

> +	rc=$?
> +	if [ $rc -eq 0 ];then
> +		if [ "$found" != "true" ];then
> +			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> +			echo "ERROR: expect there's not $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 $key in $info, but found"

So earlier, I complained that I was seeing this error in the case where
the fs was formatted with rmapbt=1 and the mount options were "-o
logbsize=16k".  It turns out that formatting a 1G fs with an internal
log results in a log that is the minimum allowed size with the default
mount options.

When the test adds in the logbsize=16k argument, this increases the
kernel's expectation of minimum log size from 3273 to 3299 blocks, so
the mount fails.  The stderr output from mount is discarded, which
means one has to refer to dmesg to figure out that the mount failed.
The test leaves no mkfs/mount option breadcrumb trail, so it is
difficult even to figure out where we were in the test.

The erroneous $mount_ret logic allows the test to escape to the mount
info testing, which of course fails because the fs didn't mount, which
produced the broken-looking error message about "expect there's
logbsize=16k in  , but not found".

So ... I've fixed this test up enough to be (mostly) passable, but that
leaves the problem that a filesystem formatted with the minimum size log
fails to mount when one starts running with non-default mount options.
That alone might not be a big deal since we always advise to accept the
defaults unless a customer's workload proves that a non-default option
is better, however...

A filesystem formatted with the mininum log size is at high risk of
encountering mount failures in the future if we are not /exceedingly/
careful about changing things that cause perturbations in the log
reservation calculations.  I observed that at some point between 4.15
and 4.18 the minimum log size calculation decreased (when formatting
with reflink=1), though's only likely to affect people moving to an
older kernel, so we're probably ok for now.

I /think/ a solution here is to change mkfs to detect when the log size
it proposes to format is the same as the minimum log size and increase
it by (say) 10% to buffer the fs against these kinds of fluctuations.
Obviously if the user forces the log size to minimum then we'll continue
to respect that, but we don't have to format that way by default.

--D

> +			return 1
> +		fi
> +	fi
> +
> +	return 0
> +}
> +
> +do_test()
> +{

Please log what mount options we're testing here.

--D

> +	# 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.
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbufs=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbufs can't be expected, disable this test.
> +#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
> +do_test "-o logbufs=9" fail
> +do_test "-o logbufs=99999999999999" fail
> +
> +# Test logbsize=value.
> +do_mkfs -m crc=1 -l version=2
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbsize can't be expected, disable this test.
> +#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
> +do_mkfs -m crc=0 -l version=1
> +# New kernel (refer to 4f62282a3696 xfs: cleanup xlog_get_iclog_buffer_size)
> +# prints "logbsize=N" in /proc/mounts, but old kernel not. So the default
> +# 'display' about logbsize can't be expected, disable this test.
> +#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
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2020-01-13 18:00 ` Darrick J. Wong
@ 2020-01-13 18:23   ` Darrick J. Wong
  2020-01-14  6:44     ` Zorro Lang
  0 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2020-01-13 18:23 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests, linux-xfs, Eric Sandeen

On Mon, Jan 13, 2020 at 10:00:36AM -0800, Darrick J. Wong wrote:
> On Wed, Oct 30, 2019 at 06:34:10PM +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.
> 
> This testcase examines the intersection of some mkfs options and a large
> number of mount options, but it doesn't log any breadcrumbs of which
> scenario it's testing at any given time.  When something goes wrong,
> it's /very/ difficult to map that back to the do_mkfs/do_test call.
> 
> (FWIW I added this test as xfs/997 and attached the diff I needed to

HAH I lied.  Here's the patch.

--D

From: Darrick J. Wong <darrick.wong@oracle.com>
Subject: [PATCH] xfs/997: fix problems with test

Fix problems with Zorro Lang's new mount options test.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/997     |   19 ++++++-----
 tests/xfs/997.out |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 8 deletions(-)

diff --git a/tests/xfs/997 b/tests/xfs/997
index a662f6f7..9d59b4f4 100755
--- a/tests/xfs/997
+++ b/tests/xfs/997
@@ -65,7 +65,8 @@ mkdir -p $LOOP_MNT || _fail "cannot create loopback mount point"
 MKFS_OPTIONS=""
 do_mkfs()
 {
-	$MKFS_XFS_PROG -f $* $LOOP_DEV | _filter_mkfs >$seqres.full 2>$tmp.mkfs
+	echo "FORMAT: $@" | tee -a $seqres.full
+	$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
@@ -99,12 +100,12 @@ _do_test()
 	local info
 
 	# mount test
-	_mount $LOOP_DEV $LOOP_MNT $opts 2>/dev/null
+	_mount $LOOP_DEV $LOOP_MNT $opts 2>>$seqres.full
 	rc=$?
 	if [ $rc -eq 0 ];then
 		if [ "${mounted}" = "fail" ];then
 			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
-			echo "ERROR: expect ${mounted}, but pass"
+			echo "ERROR: expect mount to fail, but it succeeded"
 			return 1
 		fi
 		is_dev_mounted
@@ -114,9 +115,9 @@ _do_test()
 			return 1
 		fi
 	else
-		if [ "${mount_ret}" = "pass" ];then
+		if [ "${mounted}" = "pass" ];then
 			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
-			echo "ERROR: expect ${mounted}, but fail"
+			echo "ERROR: expect mount to succeed, but it failed"
 			return 1
 		fi
 		is_dev_mounted
@@ -133,18 +134,18 @@ _do_test()
 	fi
 	# Check the mount options after fs mounted.
 	info=`get_mount_info`
-	echo $info | grep -q "${key}"
+	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 not $key in $info, but not found"
+			echo "ERROR: expected to find \"$key\" in mount info \"$info\""
 			return 1
 		fi
 	else
 		if [ "$found" != "false" ];then
 			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
-			echo "ERROR: expect there's $key in $info, but found"
+			echo "ERROR: did not expect to find \"$key\" in \"$info\""
 			return 1
 		fi
 	fi
@@ -154,6 +155,8 @@ _do_test()
 
 do_test()
 {
+	echo "TEST: $@" | tee -a $seqres.full
+
 	# force unmount before testing
 	force_unmount
 	_do_test "$@"
diff --git a/tests/xfs/997.out b/tests/xfs/997.out
index f2fc684f..c6624925 100644
--- a/tests/xfs/997.out
+++ b/tests/xfs/997.out
@@ -3,4 +3,98 @@ QA output created by 997
 ** create loop log device
 ** create loop mount point
 ** start xfs mount testing ...
+FORMAT: 
+TEST:  pass allocsize false
+TEST: -o allocsize=4k pass allocsize=4k true
+TEST: -o allocsize=1048576k pass allocsize=1048576k true
+TEST: -o allocsize=2048 fail
+TEST: -o allocsize=2g fail
+FORMAT: -m crc=1
+TEST:  pass attr2 true
+TEST: -o attr2 pass attr2 true
+TEST: -o noattr2 fail
+FORMAT: -m crc=0
+TEST:  pass attr2 true
+TEST: -o attr2 pass attr2 true
+TEST: -o noattr2 pass attr2 false
+FORMAT: 
+TEST:  pass discard false
+TEST: -o discard pass discard true
+TEST: -o nodiscard pass discard false
+TEST:  pass grpid false
+TEST: -o grpid pass grpid true
+TEST: -o bsdgroups pass grpid true
+TEST: -o nogrpid pass grpid false
+TEST: -o sysvgroups pass grpid false
+TEST:  pass filestreams false
+TEST: -o filestreams pass filestreams true
+TEST:  pass ikeep false
+TEST: -o ikeep pass ikeep true
+TEST: -o noikeep pass ikeep false
+TEST:  pass inode64 true
+TEST: -o inode32 pass inode32 true
+TEST: -o inode64 pass inode64 true
+TEST:  pass largeio false
+TEST: -o largeio pass largeio true
+TEST: -o nolargeio pass largeio false
+TEST: -o logbufs=8 pass logbufs=8 true
+TEST: -o logbufs=2 pass logbufs=2 true
+TEST: -o logbufs=1 fail
+TEST: -o logbufs=9 fail
+TEST: -o logbufs=99999999999999 fail
+FORMAT: -m crc=1 -l version=2
+TEST: -o logbsize=16384 pass logbsize=16k true
+TEST: -o logbsize=16k pass logbsize=16k true
+TEST: -o logbsize=32k pass logbsize=32k true
+TEST: -o logbsize=64k pass logbsize=64k true
+TEST: -o logbsize=128k pass logbsize=128k true
+TEST: -o logbsize=256k pass logbsize=256k true
+TEST: -o logbsize=8k fail
+TEST: -o logbsize=512k fail
+FORMAT: -m crc=0 -l version=1
+TEST: -o logbsize=16384 pass logbsize=16k true
+TEST: -o logbsize=16k pass logbsize=16k true
+TEST: -o logbsize=32k pass logbsize=32k true
+TEST: -o logbsize=64k fail
+FORMAT: 
+TEST:  pass logdev false
+TEST: -o logdev=/dev/loop1 fail
+FORMAT: -l logdev=/dev/loop1
+TEST: -o logdev=/dev/loop1 pass logdev=/dev/loop1 true
+TEST:  fail
+FORMAT: 
+TEST:  pass noalign false
+TEST: -o noalign pass noalign true
+TEST:  pass norecovery false
+TEST: -o norecovery,ro pass norecovery true
+TEST: -o norecovery fail
+TEST:  pass nouuid false
+TEST: -o nouuid pass nouuid true
+TEST:  pass noquota true
+TEST: -o noquota pass noquota true
+TEST:  pass usrquota false
+TEST: -o uquota pass usrquota true
+TEST: -o usrquota pass usrquota true
+TEST: -o quota pass usrquota true
+TEST: -o uqnoenforce pass usrquota true
+TEST: -o qnoenforce pass usrquota true
+TEST:  pass grpquota false
+TEST: -o gquota pass grpquota true
+TEST: -o grpquota pass grpquota true
+TEST: -o gqnoenforce pass gqnoenforce true
+TEST:  pass prjquota false
+TEST: -o pquota pass prjquota true
+TEST: -o prjquota pass prjquota true
+TEST: -o pqnoenforce pass pqnoenforce true
+FORMAT: -d sunit=128,swidth=128
+TEST: -o sunit=8,swidth=8 pass sunit=8,swidth=8 true
+TEST: -o sunit=8,swidth=64 pass sunit=8,swidth=64 true
+TEST: -o sunit=128,swidth=128 pass sunit=128,swidth=128 true
+TEST: -o sunit=256,swidth=256 pass sunit=256,swidth=256 true
+TEST: -o sunit=2,swidth=2 fail
+FORMAT: 
+TEST:  pass swalloc false
+TEST: -o swalloc pass swalloc true
+TEST:  pass wsync false
+TEST: -o wsync pass wsync true
 ** end of testing

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2020-01-14  6:44     ` Zorro Lang
@ 2020-01-14  6:41       ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2020-01-14  6:41 UTC (permalink / raw)
  To: fstests, linux-xfs, Eric Sandeen

On Tue, Jan 14, 2020 at 02:44:33PM +0800, Zorro Lang wrote:
> On Mon, Jan 13, 2020 at 10:23:56AM -0800, Darrick J. Wong wrote:
> > On Mon, Jan 13, 2020 at 10:00:36AM -0800, Darrick J. Wong wrote:
> > > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > > 
> > > This testcase examines the intersection of some mkfs options and a large
> > > number of mount options, but it doesn't log any breadcrumbs of which
> > > scenario it's testing at any given time.  When something goes wrong,
> > > it's /very/ difficult to map that back to the do_mkfs/do_test call.
> > > 
> > > (FWIW I added this test as xfs/997 and attached the diff I needed to
> > 
> > HAH I lied.  Here's the patch.
> 
> Thanks Darrick. I'd like to output these informations.
> 
> But your original problem which blocked this case is this:
> https://marc.info/?l=fstests&m=157247745012095&w=2
> 
> I don't know how to deal with it, and don't know how to reproduce either.
> Is it still a issue for you?

It is still an issue, but with the diff applied I can now shift the
focus (at least for the moment) to mkfs' formatting of filesystems with
the minimum log sizes. ;)

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > Subject: [PATCH] xfs/997: fix problems with test
> > 
> > Fix problems with Zorro Lang's new mount options test.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/997     |   19 ++++++-----
> >  tests/xfs/997.out |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 105 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/xfs/997 b/tests/xfs/997
> > index a662f6f7..9d59b4f4 100755
> > --- a/tests/xfs/997
> > +++ b/tests/xfs/997
> > @@ -65,7 +65,8 @@ mkdir -p $LOOP_MNT || _fail "cannot create loopback mount point"
> >  MKFS_OPTIONS=""
> >  do_mkfs()
> >  {
> > -	$MKFS_XFS_PROG -f $* $LOOP_DEV | _filter_mkfs >$seqres.full 2>$tmp.mkfs
> > +	echo "FORMAT: $@" | tee -a $seqres.full
> > +	$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
> > @@ -99,12 +100,12 @@ _do_test()
> >  	local info
> >  
> >  	# mount test
> > -	_mount $LOOP_DEV $LOOP_MNT $opts 2>/dev/null
> > +	_mount $LOOP_DEV $LOOP_MNT $opts 2>>$seqres.full
> >  	rc=$?
> >  	if [ $rc -eq 0 ];then
> >  		if [ "${mounted}" = "fail" ];then
> >  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> > -			echo "ERROR: expect ${mounted}, but pass"
> > +			echo "ERROR: expect mount to fail, but it succeeded"
> >  			return 1
> >  		fi
> >  		is_dev_mounted
> > @@ -114,9 +115,9 @@ _do_test()
> >  			return 1
> >  		fi
> >  	else
> > -		if [ "${mount_ret}" = "pass" ];then
> > +		if [ "${mounted}" = "pass" ];then
> >  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> > -			echo "ERROR: expect ${mounted}, but fail"
> > +			echo "ERROR: expect mount to succeed, but it failed"
> >  			return 1
> >  		fi
> >  		is_dev_mounted
> > @@ -133,18 +134,18 @@ _do_test()
> >  	fi
> >  	# Check the mount options after fs mounted.
> >  	info=`get_mount_info`
> > -	echo $info | grep -q "${key}"
> > +	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 not $key in $info, but not found"
> > +			echo "ERROR: expected to find \"$key\" in mount info \"$info\""
> >  			return 1
> >  		fi
> >  	else
> >  		if [ "$found" != "false" ];then
> >  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> > -			echo "ERROR: expect there's $key in $info, but found"
> > +			echo "ERROR: did not expect to find \"$key\" in \"$info\""
> >  			return 1
> >  		fi
> >  	fi
> > @@ -154,6 +155,8 @@ _do_test()
> >  
> >  do_test()
> >  {
> > +	echo "TEST: $@" | tee -a $seqres.full
> > +
> >  	# force unmount before testing
> >  	force_unmount
> >  	_do_test "$@"
> > diff --git a/tests/xfs/997.out b/tests/xfs/997.out
> > index f2fc684f..c6624925 100644
> > --- a/tests/xfs/997.out
> > +++ b/tests/xfs/997.out
> > @@ -3,4 +3,98 @@ QA output created by 997
> >  ** create loop log device
> >  ** create loop mount point
> >  ** start xfs mount testing ...
> > +FORMAT: 
> > +TEST:  pass allocsize false
> > +TEST: -o allocsize=4k pass allocsize=4k true
> > +TEST: -o allocsize=1048576k pass allocsize=1048576k true
> > +TEST: -o allocsize=2048 fail
> > +TEST: -o allocsize=2g fail
> > +FORMAT: -m crc=1
> > +TEST:  pass attr2 true
> > +TEST: -o attr2 pass attr2 true
> > +TEST: -o noattr2 fail
> > +FORMAT: -m crc=0
> > +TEST:  pass attr2 true
> > +TEST: -o attr2 pass attr2 true
> > +TEST: -o noattr2 pass attr2 false
> > +FORMAT: 
> > +TEST:  pass discard false
> > +TEST: -o discard pass discard true
> > +TEST: -o nodiscard pass discard false
> > +TEST:  pass grpid false
> > +TEST: -o grpid pass grpid true
> > +TEST: -o bsdgroups pass grpid true
> > +TEST: -o nogrpid pass grpid false
> > +TEST: -o sysvgroups pass grpid false
> > +TEST:  pass filestreams false
> > +TEST: -o filestreams pass filestreams true
> > +TEST:  pass ikeep false
> > +TEST: -o ikeep pass ikeep true
> > +TEST: -o noikeep pass ikeep false
> > +TEST:  pass inode64 true
> > +TEST: -o inode32 pass inode32 true
> > +TEST: -o inode64 pass inode64 true
> > +TEST:  pass largeio false
> > +TEST: -o largeio pass largeio true
> > +TEST: -o nolargeio pass largeio false
> > +TEST: -o logbufs=8 pass logbufs=8 true
> > +TEST: -o logbufs=2 pass logbufs=2 true
> > +TEST: -o logbufs=1 fail
> > +TEST: -o logbufs=9 fail
> > +TEST: -o logbufs=99999999999999 fail
> > +FORMAT: -m crc=1 -l version=2
> > +TEST: -o logbsize=16384 pass logbsize=16k true
> > +TEST: -o logbsize=16k pass logbsize=16k true
> > +TEST: -o logbsize=32k pass logbsize=32k true
> > +TEST: -o logbsize=64k pass logbsize=64k true
> > +TEST: -o logbsize=128k pass logbsize=128k true
> > +TEST: -o logbsize=256k pass logbsize=256k true
> > +TEST: -o logbsize=8k fail
> > +TEST: -o logbsize=512k fail
> > +FORMAT: -m crc=0 -l version=1
> > +TEST: -o logbsize=16384 pass logbsize=16k true
> > +TEST: -o logbsize=16k pass logbsize=16k true
> > +TEST: -o logbsize=32k pass logbsize=32k true
> > +TEST: -o logbsize=64k fail
> > +FORMAT: 
> > +TEST:  pass logdev false
> > +TEST: -o logdev=/dev/loop1 fail
> > +FORMAT: -l logdev=/dev/loop1
> > +TEST: -o logdev=/dev/loop1 pass logdev=/dev/loop1 true
> > +TEST:  fail
> > +FORMAT: 
> > +TEST:  pass noalign false
> > +TEST: -o noalign pass noalign true
> > +TEST:  pass norecovery false
> > +TEST: -o norecovery,ro pass norecovery true
> > +TEST: -o norecovery fail
> > +TEST:  pass nouuid false
> > +TEST: -o nouuid pass nouuid true
> > +TEST:  pass noquota true
> > +TEST: -o noquota pass noquota true
> > +TEST:  pass usrquota false
> > +TEST: -o uquota pass usrquota true
> > +TEST: -o usrquota pass usrquota true
> > +TEST: -o quota pass usrquota true
> > +TEST: -o uqnoenforce pass usrquota true
> > +TEST: -o qnoenforce pass usrquota true
> > +TEST:  pass grpquota false
> > +TEST: -o gquota pass grpquota true
> > +TEST: -o grpquota pass grpquota true
> > +TEST: -o gqnoenforce pass gqnoenforce true
> > +TEST:  pass prjquota false
> > +TEST: -o pquota pass prjquota true
> > +TEST: -o prjquota pass prjquota true
> > +TEST: -o pqnoenforce pass pqnoenforce true
> > +FORMAT: -d sunit=128,swidth=128
> > +TEST: -o sunit=8,swidth=8 pass sunit=8,swidth=8 true
> > +TEST: -o sunit=8,swidth=64 pass sunit=8,swidth=64 true
> > +TEST: -o sunit=128,swidth=128 pass sunit=128,swidth=128 true
> > +TEST: -o sunit=256,swidth=256 pass sunit=256,swidth=256 true
> > +TEST: -o sunit=2,swidth=2 fail
> > +FORMAT: 
> > +TEST:  pass swalloc false
> > +TEST: -o swalloc pass swalloc true
> > +TEST:  pass wsync false
> > +TEST: -o wsync pass wsync true
> >  ** end of testing
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xfstests: xfs mount option sanity test
  2020-01-13 18:23   ` Darrick J. Wong
@ 2020-01-14  6:44     ` Zorro Lang
  2020-01-14  6:41       ` Darrick J. Wong
  0 siblings, 1 reply; 14+ messages in thread
From: Zorro Lang @ 2020-01-14  6:44 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs, Eric Sandeen

On Mon, Jan 13, 2020 at 10:23:56AM -0800, Darrick J. Wong wrote:
> On Mon, Jan 13, 2020 at 10:00:36AM -0800, Darrick J. Wong wrote:
> > On Wed, Oct 30, 2019 at 06:34:10PM +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.
> > 
> > This testcase examines the intersection of some mkfs options and a large
> > number of mount options, but it doesn't log any breadcrumbs of which
> > scenario it's testing at any given time.  When something goes wrong,
> > it's /very/ difficult to map that back to the do_mkfs/do_test call.
> > 
> > (FWIW I added this test as xfs/997 and attached the diff I needed to
> 
> HAH I lied.  Here's the patch.

Thanks Darrick. I'd like to output these informations.

But your original problem which blocked this case is this:
https://marc.info/?l=fstests&m=157247745012095&w=2

I don't know how to deal with it, and don't know how to reproduce either.
Is it still a issue for you?

Thanks,
Zorro

> 
> --D
> 
> From: Darrick J. Wong <darrick.wong@oracle.com>
> Subject: [PATCH] xfs/997: fix problems with test
> 
> Fix problems with Zorro Lang's new mount options test.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/997     |   19 ++++++-----
>  tests/xfs/997.out |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 105 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/xfs/997 b/tests/xfs/997
> index a662f6f7..9d59b4f4 100755
> --- a/tests/xfs/997
> +++ b/tests/xfs/997
> @@ -65,7 +65,8 @@ mkdir -p $LOOP_MNT || _fail "cannot create loopback mount point"
>  MKFS_OPTIONS=""
>  do_mkfs()
>  {
> -	$MKFS_XFS_PROG -f $* $LOOP_DEV | _filter_mkfs >$seqres.full 2>$tmp.mkfs
> +	echo "FORMAT: $@" | tee -a $seqres.full
> +	$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
> @@ -99,12 +100,12 @@ _do_test()
>  	local info
>  
>  	# mount test
> -	_mount $LOOP_DEV $LOOP_MNT $opts 2>/dev/null
> +	_mount $LOOP_DEV $LOOP_MNT $opts 2>>$seqres.full
>  	rc=$?
>  	if [ $rc -eq 0 ];then
>  		if [ "${mounted}" = "fail" ];then
>  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> -			echo "ERROR: expect ${mounted}, but pass"
> +			echo "ERROR: expect mount to fail, but it succeeded"
>  			return 1
>  		fi
>  		is_dev_mounted
> @@ -114,9 +115,9 @@ _do_test()
>  			return 1
>  		fi
>  	else
> -		if [ "${mount_ret}" = "pass" ];then
> +		if [ "${mounted}" = "pass" ];then
>  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> -			echo "ERROR: expect ${mounted}, but fail"
> +			echo "ERROR: expect mount to succeed, but it failed"
>  			return 1
>  		fi
>  		is_dev_mounted
> @@ -133,18 +134,18 @@ _do_test()
>  	fi
>  	# Check the mount options after fs mounted.
>  	info=`get_mount_info`
> -	echo $info | grep -q "${key}"
> +	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 not $key in $info, but not found"
> +			echo "ERROR: expected to find \"$key\" in mount info \"$info\""
>  			return 1
>  		fi
>  	else
>  		if [ "$found" != "false" ];then
>  			echo "[FAILED]: mount $LOOP_DEV $LOOP_MNT $opts"
> -			echo "ERROR: expect there's $key in $info, but found"
> +			echo "ERROR: did not expect to find \"$key\" in \"$info\""
>  			return 1
>  		fi
>  	fi
> @@ -154,6 +155,8 @@ _do_test()
>  
>  do_test()
>  {
> +	echo "TEST: $@" | tee -a $seqres.full
> +
>  	# force unmount before testing
>  	force_unmount
>  	_do_test "$@"
> diff --git a/tests/xfs/997.out b/tests/xfs/997.out
> index f2fc684f..c6624925 100644
> --- a/tests/xfs/997.out
> +++ b/tests/xfs/997.out
> @@ -3,4 +3,98 @@ QA output created by 997
>  ** create loop log device
>  ** create loop mount point
>  ** start xfs mount testing ...
> +FORMAT: 
> +TEST:  pass allocsize false
> +TEST: -o allocsize=4k pass allocsize=4k true
> +TEST: -o allocsize=1048576k pass allocsize=1048576k true
> +TEST: -o allocsize=2048 fail
> +TEST: -o allocsize=2g fail
> +FORMAT: -m crc=1
> +TEST:  pass attr2 true
> +TEST: -o attr2 pass attr2 true
> +TEST: -o noattr2 fail
> +FORMAT: -m crc=0
> +TEST:  pass attr2 true
> +TEST: -o attr2 pass attr2 true
> +TEST: -o noattr2 pass attr2 false
> +FORMAT: 
> +TEST:  pass discard false
> +TEST: -o discard pass discard true
> +TEST: -o nodiscard pass discard false
> +TEST:  pass grpid false
> +TEST: -o grpid pass grpid true
> +TEST: -o bsdgroups pass grpid true
> +TEST: -o nogrpid pass grpid false
> +TEST: -o sysvgroups pass grpid false
> +TEST:  pass filestreams false
> +TEST: -o filestreams pass filestreams true
> +TEST:  pass ikeep false
> +TEST: -o ikeep pass ikeep true
> +TEST: -o noikeep pass ikeep false
> +TEST:  pass inode64 true
> +TEST: -o inode32 pass inode32 true
> +TEST: -o inode64 pass inode64 true
> +TEST:  pass largeio false
> +TEST: -o largeio pass largeio true
> +TEST: -o nolargeio pass largeio false
> +TEST: -o logbufs=8 pass logbufs=8 true
> +TEST: -o logbufs=2 pass logbufs=2 true
> +TEST: -o logbufs=1 fail
> +TEST: -o logbufs=9 fail
> +TEST: -o logbufs=99999999999999 fail
> +FORMAT: -m crc=1 -l version=2
> +TEST: -o logbsize=16384 pass logbsize=16k true
> +TEST: -o logbsize=16k pass logbsize=16k true
> +TEST: -o logbsize=32k pass logbsize=32k true
> +TEST: -o logbsize=64k pass logbsize=64k true
> +TEST: -o logbsize=128k pass logbsize=128k true
> +TEST: -o logbsize=256k pass logbsize=256k true
> +TEST: -o logbsize=8k fail
> +TEST: -o logbsize=512k fail
> +FORMAT: -m crc=0 -l version=1
> +TEST: -o logbsize=16384 pass logbsize=16k true
> +TEST: -o logbsize=16k pass logbsize=16k true
> +TEST: -o logbsize=32k pass logbsize=32k true
> +TEST: -o logbsize=64k fail
> +FORMAT: 
> +TEST:  pass logdev false
> +TEST: -o logdev=/dev/loop1 fail
> +FORMAT: -l logdev=/dev/loop1
> +TEST: -o logdev=/dev/loop1 pass logdev=/dev/loop1 true
> +TEST:  fail
> +FORMAT: 
> +TEST:  pass noalign false
> +TEST: -o noalign pass noalign true
> +TEST:  pass norecovery false
> +TEST: -o norecovery,ro pass norecovery true
> +TEST: -o norecovery fail
> +TEST:  pass nouuid false
> +TEST: -o nouuid pass nouuid true
> +TEST:  pass noquota true
> +TEST: -o noquota pass noquota true
> +TEST:  pass usrquota false
> +TEST: -o uquota pass usrquota true
> +TEST: -o usrquota pass usrquota true
> +TEST: -o quota pass usrquota true
> +TEST: -o uqnoenforce pass usrquota true
> +TEST: -o qnoenforce pass usrquota true
> +TEST:  pass grpquota false
> +TEST: -o gquota pass grpquota true
> +TEST: -o grpquota pass grpquota true
> +TEST: -o gqnoenforce pass gqnoenforce true
> +TEST:  pass prjquota false
> +TEST: -o pquota pass prjquota true
> +TEST: -o prjquota pass prjquota true
> +TEST: -o pqnoenforce pass pqnoenforce true
> +FORMAT: -d sunit=128,swidth=128
> +TEST: -o sunit=8,swidth=8 pass sunit=8,swidth=8 true
> +TEST: -o sunit=8,swidth=64 pass sunit=8,swidth=64 true
> +TEST: -o sunit=128,swidth=128 pass sunit=128,swidth=128 true
> +TEST: -o sunit=256,swidth=256 pass sunit=256,swidth=256 true
> +TEST: -o sunit=2,swidth=2 fail
> +FORMAT: 
> +TEST:  pass swalloc false
> +TEST: -o swalloc pass swalloc true
> +TEST:  pass wsync false
> +TEST: -o wsync pass wsync true
>  ** end of testing
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 10:34 [PATCH v2] xfstests: xfs mount option sanity test Zorro Lang
2019-10-30 16:39 ` Darrick J. Wong
2019-10-30 23:24   ` Zorro Lang
2019-12-11  6:42     ` Zorro Lang
2019-12-11  8:33       ` Ian Kent
2019-12-16  9:17         ` Zorro Lang
2019-12-16  9:36           ` Zorro Lang
2020-01-13 15:40             ` Zorro Lang
2020-01-13 17:33               ` Darrick J. Wong
2019-12-16 10:42           ` Ian Kent
2020-01-13 18:00 ` Darrick J. Wong
2020-01-13 18:23   ` Darrick J. Wong
2020-01-14  6:44     ` Zorro Lang
2020-01-14  6:41       ` Darrick J. Wong

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \
		linux-xfs@vger.kernel.org
	public-inbox-index linux-xfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git