[PATCH 0/5] miscellaneous tests

[PATCH 0/5] miscellaneous tests

[Darrick J. Wong]

I decided to roll everything up into a patch series, though these
patches mostly address different things.

So... I've reposted the ext4 fsmap test, refactored the xfs error
injection helpers to work with the new interface in 4.13, and added
support for fuzzing xfs quota record fields.

--D

[PATCH 1/5] xfs: only run scrub in dry run mode

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When checking a filesystem, explicitly run xfs_scrub in dry run mode
so that it will not ever try to preen, fix, or optimize anything.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/common/xfs b/common/xfs
index cfdbfff..9224499 100644
--- a/common/xfs
+++ b/common/xfs
@@ -331,7 +331,7 @@ _check_xfs_filesystem()
 
 	if [ "$type" = "xfs" ]; then
 		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
-			"$XFS_SCRUB_PROG" $scrubflag -vd $device >>$seqres.full
+			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
 			if [ $? -ne 0 ]; then
 				_log_err "filesystem on $device failed scrub"
 				ok=0

[PATCH 2/5] ext4: fsmap tests

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Test the GETFSMAP ioctl against ext4.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/ext4/700     |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/700.out |    7 +++++
 tests/ext4/701     |   65 ++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/701.out |    3 ++
 tests/ext4/702     |   66 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/702.out |    4 +++
 tests/ext4/group   |    3 ++
 7 files changed, 224 insertions(+)
 create mode 100755 tests/ext4/700
 create mode 100644 tests/ext4/700.out
 create mode 100755 tests/ext4/701
 create mode 100644 tests/ext4/701.out
 create mode 100755 tests/ext4/702
 create mode 100644 tests/ext4/702.out


diff --git a/tests/ext4/700 b/tests/ext4/700
new file mode 100755
index 0000000..a4a1b83
--- /dev/null
+++ b/tests/ext4/700
@@ -0,0 +1,76 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that getfsmap reports the BG metadata we're expecting.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+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".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs ext4
+_require_scratch
+_require_xfs_io_command "fsmap"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Get fsmap" | tee -a $seqres.full
+$XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT > $TEST_DIR/fsmap
+cat $TEST_DIR/fsmap >> $seqres.full
+
+echo "Check fs metadata" | tee -a $seqres.full
+x=$(grep -c 'static fs metadata' $TEST_DIR/fsmap)
+test $x -gt 0 || echo "No fs metadata?"
+
+echo "Check block bitmap" | tee -a $seqres.full
+x=$(grep -c 'special 102:1' $TEST_DIR/fsmap)
+test $x -gt 0 || echo "No block bitmaps?"
+
+echo "Check inode bitmap" | tee -a $seqres.full
+x=$(grep -c 'special 102:2' $TEST_DIR/fsmap)
+test $x -gt 0 || echo "No inode bitmaps?"
+
+echo "Check inodes" | tee -a $seqres.full
+x=$(grep -c 'inodes' $TEST_DIR/fsmap)
+test $x -gt 0 || echo "No inodes?"
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/700.out b/tests/ext4/700.out
new file mode 100644
index 0000000..3984319
--- /dev/null
+++ b/tests/ext4/700.out
@@ -0,0 +1,7 @@
+QA output created by 700
+Format and mount
+Get fsmap
+Check fs metadata
+Check block bitmap
+Check inode bitmap
+Check inodes
diff --git a/tests/ext4/701 b/tests/ext4/701
new file mode 100755
index 0000000..6ae93d7
--- /dev/null
+++ b/tests/ext4/701
@@ -0,0 +1,65 @@
+#! /bin/bash
+# FS QA Test No. 701
+#
+# Populate filesystem, check that fsmap -n10000 matches fsmap -n1.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+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".* $TEST_DIR/a $TEST_DIR/b
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs ext4
+_require_scratch
+_require_populate_commands
+_require_xfs_io_command "fsmap"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+
+echo "Compare fsmap" | tee -a $seqres.full
+_scratch_mount
+$XFS_IO_PROG -c 'fsmap -n 65536' $SCRATCH_MNT | grep -v 'EXT:' > $TEST_DIR/a
+$XFS_IO_PROG -c 'fsmap -n 1' $SCRATCH_MNT | grep -v 'EXT:' > $TEST_DIR/b
+cat $TEST_DIR/a $TEST_DIR/b >> $seqres.full
+
+diff -uw $TEST_DIR/a $TEST_DIR/b
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/701.out b/tests/ext4/701.out
new file mode 100644
index 0000000..41ef1c6
--- /dev/null
+++ b/tests/ext4/701.out
@@ -0,0 +1,3 @@
+QA output created by 701
+Format and mount
+Compare fsmap
diff --git a/tests/ext4/702 b/tests/ext4/702
new file mode 100755
index 0000000..ce3ae11
--- /dev/null
+++ b/tests/ext4/702
@@ -0,0 +1,66 @@
+#! /bin/bash
+# FS QA Test No. 702
+#
+# Check that getfsmap reports external log devices
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+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".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs ext4
+_require_logdev
+_require_scratch
+_require_xfs_io_command "fsmap"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Get fsmap" | tee -a $seqres.full
+$XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
+$XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT | tr '[]()' '    ' > $TEST_DIR/fsmap
+
+echo "Check device field of FS metadata and journalling log"
+data_dev=$(grep 'static fs metadata' $TEST_DIR/fsmap | head -n 1 | awk '{print $2}')
+journal_dev=$(grep 'journalling log' $TEST_DIR/fsmap | head -n 1 | awk '{print $2}')
+test "${data_dev}" != "${journal_dev}" || echo "data ${data_dev} journal ${journal_dev}?"
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/702.out b/tests/ext4/702.out
new file mode 100644
index 0000000..1d3b7ef
--- /dev/null
+++ b/tests/ext4/702.out
@@ -0,0 +1,4 @@
+QA output created by 702
+Format and mount
+Get fsmap
+Check device field of FS metadata and journalling log
diff --git a/tests/ext4/group b/tests/ext4/group
index 664c059..9007adf 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -37,3 +37,6 @@
 306 auto rw resize quick
 307 auto ioctl rw defrag
 308 auto ioctl rw prealloc quick defrag
+700 auto quick fsmap
+701 auto quick fsmap
+702 auto quick fsmap

[PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the XFS error injection helpers to use the new errortag
interface to configure error injection.  If that isn't present, fall
back either to the xfs_io/ioctl based injection or the older sysfs
knobs.  Refactor existing testcases to use the new helpers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/xfs/141 |    5 +++--
 tests/xfs/196 |   17 ++++++-----------
 3 files changed, 65 insertions(+), 15 deletions(-)


diff --git a/common/inject b/common/inject
index 8ecc290..9aa24de 100644
--- a/common/inject
+++ b/common/inject
@@ -35,10 +35,46 @@ _require_error_injection()
 	esac
 }
 
+# Find a given xfs mount's errortag injection knob in sysfs
+_find_xfs_mount_errortag_knob()
+{
+	dev="$1"
+	knob="$2"
+	shortdev="$(_short_dev "${dev}")"
+	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
+
+	# Some of the new sysfs errortag knobs were previously available via
+	# another sysfs path.
+	case "${knob}" in
+	"log_bad_crc")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
+		fi
+		;;
+	"drop_writes")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
+		fi
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
+		fi
+		;;
+	*)
+		;;
+	esac
+
+	echo "${tagfile}"
+}
+
 # Requires that xfs_io inject command knows about this error type
 _require_xfs_io_error_injection()
 {
 	type="$1"
+
+	# Can we find the error injection knobs via the new errortag
+	# configuration mechanism?
+	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
+
 	_require_error_injection
 
 	# NOTE: We can't actually test error injection here because xfs
@@ -54,16 +90,34 @@ _require_xfs_io_error_injection()
 _test_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	knob="$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	else
+		_notrun "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Inject an error into the scratch fs
 _scratch_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	knob="$(_find_xfs_mount_errortag_knob "${SCRATCH_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	else
+		_notrun "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Unmount and remount the scratch device, dumping the log
diff --git a/tests/xfs/141 b/tests/xfs/141
index 56ff14e..f61e524 100755
--- a/tests/xfs/141
+++ b/tests/xfs/141
@@ -47,13 +47,14 @@ rm -f $seqres.full
 
 # get standard environment, filters and checks
 . ./common/rc
+. ./common/inject
 
 # real QA test starts here
 
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
+_require_xfs_io_error_injection "log_bad_crc"
 _require_scratch
 _require_command "$KILLALL_PROG" killall
 
@@ -69,7 +70,7 @@ for i in $(seq 1 5); do
 	# (increase this value to run fsstress longer).
 	factor=$((RANDOM % 100 + 1))
 	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
-	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
+	_scratch_inject_error "log_bad_crc" "$factor"
 
 	# Run fsstress until the filesystem shuts down. It will shut down
 	# automatically when error injection triggers.
diff --git a/tests/xfs/196 b/tests/xfs/196
index e9b0649..fe3f570 100755
--- a/tests/xfs/196
+++ b/tests/xfs/196
@@ -45,6 +45,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/punch
+. ./common/inject
 
 # real QA test starts here
 rm -f $seqres.full
@@ -53,13 +54,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-
-DROP_WRITES="drop_writes"
-# replace "drop_writes" with "fail_writes" for old kernel
-if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
-	DROP_WRITES="fail_writes"
-fi
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
+_require_xfs_io_error_injection "drop_writes"
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
@@ -72,7 +67,7 @@ bytes=$((64 * 1024))
 $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
 
 # Enable write drops. All buffered writes are dropped from this point on.
-echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 1
 
 # Write every other 4k range to split the larger delalloc extent into many more
 # smaller extents. Use pwrite because with write failures enabled, all
@@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
 	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
 done
 
-echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 0
 
 _scratch_cycle_mount
 $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
@@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
 	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
 
 	punchoffset=$((offset + 75))
-	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes"
 	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
-	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes" 0
 done
 
 echo "Silence is golden."

Re: [PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Eryu Guan]

On Fri, Jul 21, 2017 at 03:04:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the XFS error injection helpers to use the new errortag
> interface to configure error injection.  If that isn't present, fall
> back either to the xfs_io/ioctl based injection or the older sysfs
> knobs.  Refactor existing testcases to use the new helpers.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

This looks good to me overall, but I still perfer let other people
who're more familar with XFS errortag and error injection to review too.
While I do have some questions/comments :)

> ---
>  common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/xfs/141 |    5 +++--
>  tests/xfs/196 |   17 ++++++-----------
>  3 files changed, 65 insertions(+), 15 deletions(-)
> 
> 
> diff --git a/common/inject b/common/inject
> index 8ecc290..9aa24de 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -35,10 +35,46 @@ _require_error_injection()
>  	esac
>  }
>  
> +# Find a given xfs mount's errortag injection knob in sysfs
> +_find_xfs_mount_errortag_knob()

While The function name and comment both indicate it needs a mounted
XFS, it seems weird that the first argument is expected to be a block
device. And do we need to check if the given device is really mounted?

> +{
> +	dev="$1"
> +	knob="$2"
> +	shortdev="$(_short_dev "${dev}")"
> +	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
> +
> +	# Some of the new sysfs errortag knobs were previously available via
> +	# another sysfs path.
> +	case "${knob}" in
> +	"log_bad_crc")
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
> +		fi
> +		;;
> +	"drop_writes")
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
> +		fi
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
> +		fi
> +		;;
> +	*)
> +		;;
> +	esac
> +
> +	echo "${tagfile}"
> +}
> +
>  # Requires that xfs_io inject command knows about this error type
>  _require_xfs_io_error_injection()
>  {
>  	type="$1"
> +
> +	# Can we find the error injection knobs via the new errortag
> +	# configuration mechanism?
> +	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
> +

As this check goes prior to the _require_error_injection check, so I
assume this new errortag framework doesn't depend on a debug built XFS,
can I?

>  	_require_error_injection
>  
>  	# NOTE: We can't actually test error injection here because xfs
> @@ -54,16 +90,34 @@ _require_xfs_io_error_injection()
>  _test_inject_error()
>  {
>  	type="$1"
> +	value="$2"
>  
> -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> +	knob="$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")"
> +	if [ -w "${knob}" ]; then
> +		test -z "${value}" && value="default"
> +		echo -n "${value}" > "${knob}"
> +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> +	else
> +		_notrun "Cannot inject error ${type} value ${value}."

_require_xfs_io_error_injection already made sure we can do error
jection, it looks like a bug somewhere to me if we can't inject error
here, either wanted to inject error without checking the support status
first or an implementation bug in the error injection framework in
xfstests. So "_fail" might be the right choice.

> +	fi
>  }
>  
>  # Inject an error into the scratch fs
>  _scratch_inject_error()
>  {
>  	type="$1"
> +	value="$2"
>  
> -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> +	knob="$(_find_xfs_mount_errortag_knob "${SCRATCH_DEV}" "${type}")"
> +	if [ -w "${knob}" ]; then
> +		test -z "${value}" && value="default"
> +		echo -n "${value}" > "${knob}"
> +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> +	else
> +		_notrun "Cannot inject error ${type} value ${value}."

Same here.

Thanks,
Eryu

> +	fi
>  }
>  
>  # Unmount and remount the scratch device, dumping the log
> diff --git a/tests/xfs/141 b/tests/xfs/141
> index 56ff14e..f61e524 100755
> --- a/tests/xfs/141
> +++ b/tests/xfs/141
> @@ -47,13 +47,14 @@ rm -f $seqres.full
>  
>  # get standard environment, filters and checks
>  . ./common/rc
> +. ./common/inject
>  
>  # real QA test starts here
>  
>  # Modify as appropriate.
>  _supported_fs xfs
>  _supported_os Linux
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> +_require_xfs_io_error_injection "log_bad_crc"
>  _require_scratch
>  _require_command "$KILLALL_PROG" killall
>  
> @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
>  	# (increase this value to run fsstress longer).
>  	factor=$((RANDOM % 100 + 1))
>  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> +	_scratch_inject_error "log_bad_crc" "$factor"
>  
>  	# Run fsstress until the filesystem shuts down. It will shut down
>  	# automatically when error injection triggers.
> diff --git a/tests/xfs/196 b/tests/xfs/196
> index e9b0649..fe3f570 100755
> --- a/tests/xfs/196
> +++ b/tests/xfs/196
> @@ -45,6 +45,7 @@ _cleanup()
>  # get standard environment, filters and checks
>  . ./common/rc
>  . ./common/punch
> +. ./common/inject
>  
>  # real QA test starts here
>  rm -f $seqres.full
> @@ -53,13 +54,7 @@ rm -f $seqres.full
>  _supported_fs generic
>  _supported_os Linux
>  _require_scratch
> -
> -DROP_WRITES="drop_writes"
> -# replace "drop_writes" with "fail_writes" for old kernel
> -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> -	DROP_WRITES="fail_writes"
> -fi
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> +_require_xfs_io_error_injection "drop_writes"
>  
>  _scratch_mkfs >/dev/null 2>&1
>  _scratch_mount
> @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
>  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
>  
>  # Enable write drops. All buffered writes are dropped from this point on.
> -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +_scratch_inject_error "drop_writes" 1
>  
>  # Write every other 4k range to split the larger delalloc extent into many more
>  # smaller extents. Use pwrite because with write failures enabled, all
> @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
>  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
>  done
>  
> -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +_scratch_inject_error "drop_writes" 0
>  
>  _scratch_cycle_mount
>  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
>  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
>  
>  	punchoffset=$((offset + 75))
> -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +	_scratch_inject_error "drop_writes"
>  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +	_scratch_inject_error "drop_writes" 0
>  done
>  
>  echo "Silence is golden."
> 

Re: [PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Brian Foster]

On Wed, Aug 02, 2017 at 04:37:42PM +0800, Eryu Guan wrote:
> On Fri, Jul 21, 2017 at 03:04:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Refactor the XFS error injection helpers to use the new errortag
> > interface to configure error injection.  If that isn't present, fall
> > back either to the xfs_io/ioctl based injection or the older sysfs
> > knobs.  Refactor existing testcases to use the new helpers.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This looks good to me overall, but I still perfer let other people
> who're more familar with XFS errortag and error injection to review too.
> While I do have some questions/comments :)
> 
> > ---
> >  common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  tests/xfs/141 |    5 +++--
> >  tests/xfs/196 |   17 ++++++-----------
> >  3 files changed, 65 insertions(+), 15 deletions(-)
> > 
> > 
> > diff --git a/common/inject b/common/inject
> > index 8ecc290..9aa24de 100644
> > --- a/common/inject
> > +++ b/common/inject
> > @@ -35,10 +35,46 @@ _require_error_injection()
> >  	esac
> >  }
> >  
> > +# Find a given xfs mount's errortag injection knob in sysfs
> > +_find_xfs_mount_errortag_knob()
> 
> While The function name and comment both indicate it needs a mounted
> XFS, it seems weird that the first argument is expected to be a block
> device. And do we need to check if the given device is really mounted?
> 

The xfs per-mount sysfs knobs distinguish between mounts via the
block device name.

...
> >  # Requires that xfs_io inject command knows about this error type
> >  _require_xfs_io_error_injection()
> >  {
> >  	type="$1"
> > +
> > +	# Can we find the error injection knobs via the new errortag
> > +	# configuration mechanism?
> > +	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
> > +
> 
> As this check goes prior to the _require_error_injection check, so I
> assume this new errortag framework doesn't depend on a debug built XFS,
> can I?
> 

It does depend on debug mode so it probably makes sense to push this
after the _require_error_injection check. That way the DEBUG=0 message
has precedent over a message regarding lack of support for a particular
knob.

Outside of Eryu's further comments, the rest looks good to me. This
reminds me that I still need to post the latest log tail overwrite test
that depends on this, now that the kernel bits have been reviewed...

Brian

> >  	_require_error_injection
> >  
> >  	# NOTE: We can't actually test error injection here because xfs
> > @@ -54,16 +90,34 @@ _require_xfs_io_error_injection()
> >  _test_inject_error()
> >  {
> >  	type="$1"
> > +	value="$2"
> >  
> > -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > +	knob="$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")"
> > +	if [ -w "${knob}" ]; then
> > +		test -z "${value}" && value="default"
> > +		echo -n "${value}" > "${knob}"
> > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > +	else
> > +		_notrun "Cannot inject error ${type} value ${value}."
> 
> _require_xfs_io_error_injection already made sure we can do error
> jection, it looks like a bug somewhere to me if we can't inject error
> here, either wanted to inject error without checking the support status
> first or an implementation bug in the error injection framework in
> xfstests. So "_fail" might be the right choice.
> 
> > +	fi
> >  }
> >  
> >  # Inject an error into the scratch fs
> >  _scratch_inject_error()
> >  {
> >  	type="$1"
> > +	value="$2"
> >  
> > -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > +	knob="$(_find_xfs_mount_errortag_knob "${SCRATCH_DEV}" "${type}")"
> > +	if [ -w "${knob}" ]; then
> > +		test -z "${value}" && value="default"
> > +		echo -n "${value}" > "${knob}"
> > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > +	else
> > +		_notrun "Cannot inject error ${type} value ${value}."
> 
> Same here.
> 
> Thanks,
> Eryu
> 
> > +	fi
> >  }
> >  
> >  # Unmount and remount the scratch device, dumping the log
> > diff --git a/tests/xfs/141 b/tests/xfs/141
> > index 56ff14e..f61e524 100755
> > --- a/tests/xfs/141
> > +++ b/tests/xfs/141
> > @@ -47,13 +47,14 @@ rm -f $seqres.full
> >  
> >  # get standard environment, filters and checks
> >  . ./common/rc
> > +. ./common/inject
> >  
> >  # real QA test starts here
> >  
> >  # Modify as appropriate.
> >  _supported_fs xfs
> >  _supported_os Linux
> > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> > +_require_xfs_io_error_injection "log_bad_crc"
> >  _require_scratch
> >  _require_command "$KILLALL_PROG" killall
> >  
> > @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
> >  	# (increase this value to run fsstress longer).
> >  	factor=$((RANDOM % 100 + 1))
> >  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> > -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> > +	_scratch_inject_error "log_bad_crc" "$factor"
> >  
> >  	# Run fsstress until the filesystem shuts down. It will shut down
> >  	# automatically when error injection triggers.
> > diff --git a/tests/xfs/196 b/tests/xfs/196
> > index e9b0649..fe3f570 100755
> > --- a/tests/xfs/196
> > +++ b/tests/xfs/196
> > @@ -45,6 +45,7 @@ _cleanup()
> >  # get standard environment, filters and checks
> >  . ./common/rc
> >  . ./common/punch
> > +. ./common/inject
> >  
> >  # real QA test starts here
> >  rm -f $seqres.full
> > @@ -53,13 +54,7 @@ rm -f $seqres.full
> >  _supported_fs generic
> >  _supported_os Linux
> >  _require_scratch
> > -
> > -DROP_WRITES="drop_writes"
> > -# replace "drop_writes" with "fail_writes" for old kernel
> > -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> > -	DROP_WRITES="fail_writes"
> > -fi
> > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> > +_require_xfs_io_error_injection "drop_writes"
> >  
> >  _scratch_mkfs >/dev/null 2>&1
> >  _scratch_mount
> > @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
> >  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
> >  
> >  # Enable write drops. All buffered writes are dropped from this point on.
> > -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +_scratch_inject_error "drop_writes" 1
> >  
> >  # Write every other 4k range to split the larger delalloc extent into many more
> >  # smaller extents. Use pwrite because with write failures enabled, all
> > @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
> >  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
> >  done
> >  
> > -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +_scratch_inject_error "drop_writes" 0
> >  
> >  _scratch_cycle_mount
> >  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> > @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
> >  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
> >  
> >  	punchoffset=$((offset + 75))
> > -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +	_scratch_inject_error "drop_writes"
> >  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> > -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +	_scratch_inject_error "drop_writes" 0
> >  done
> >  
> >  echo "Silence is golden."
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Darrick J. Wong]

On Wed, Aug 02, 2017 at 10:30:10AM -0400, Brian Foster wrote:
> On Wed, Aug 02, 2017 at 04:37:42PM +0800, Eryu Guan wrote:
> > On Fri, Jul 21, 2017 at 03:04:45PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Refactor the XFS error injection helpers to use the new errortag
> > > interface to configure error injection.  If that isn't present, fall
> > > back either to the xfs_io/ioctl based injection or the older sysfs
> > > knobs.  Refactor existing testcases to use the new helpers.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > This looks good to me overall, but I still perfer let other people
> > who're more familar with XFS errortag and error injection to review too.
> > While I do have some questions/comments :)
> > 
> > > ---
> > >  common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  tests/xfs/141 |    5 +++--
> > >  tests/xfs/196 |   17 ++++++-----------
> > >  3 files changed, 65 insertions(+), 15 deletions(-)
> > > 
> > > 
> > > diff --git a/common/inject b/common/inject
> > > index 8ecc290..9aa24de 100644
> > > --- a/common/inject
> > > +++ b/common/inject
> > > @@ -35,10 +35,46 @@ _require_error_injection()
> > >  	esac
> > >  }
> > >  
> > > +# Find a given xfs mount's errortag injection knob in sysfs
> > > +_find_xfs_mount_errortag_knob()
> > 
> > While The function name and comment both indicate it needs a mounted
> > XFS, it seems weird that the first argument is expected to be a block
> > device. And do we need to check if the given device is really mounted?
> > 
> 
> The xfs per-mount sysfs knobs distinguish between mounts via the
> block device name.

What if we rename the helper and change its documentation as such?

# Find the errortag injection knob in sysfs for a given xfs mount's
# block device.
_find_xfs_mountdev_errortag_knob()
{
	...
}

> ...
> > >  # Requires that xfs_io inject command knows about this error type
> > >  _require_xfs_io_error_injection()
> > >  {
> > >  	type="$1"
> > > +
> > > +	# Can we find the error injection knobs via the new errortag
> > > +	# configuration mechanism?
> > > +	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
> > > +
> > 
> > As this check goes prior to the _require_error_injection check, so I
> > assume this new errortag framework doesn't depend on a debug built XFS,
> > can I?

It depends on the sysfs knobs, and the sysfs knobs in turn depend on
XFS_DEBUG=y.

> It does depend on debug mode so it probably makes sense to push this
> after the _require_error_injection check. That way the DEBUG=0 message
> has precedent over a message regarding lack of support for a particular
> knob.

Hm?  This is what _require_xfs_io_error_injection does:

First we compute the path to the knob if it exists
(_find_xfs_mount_errortag_knob).

Then we check if that path is writable.  If it is, error injection is
enabled (which presumably means XFS_DEBUG=y) and we can continue.

If not, then we use _require_error_injection to bail out if XFS_DEBUG=n.

If we don't bail out, XFS_DEBUG=y and so we check if the xfs_io inject
help page knows about the error type, and bail out if it doesn't.


In the end it doesn't really matter if we look for XFS_DEBUG=y before or
after we look for the new sysfs knob.  I figured it was simple enough to
assume that if the knob is present and writable, then our preconditions
are satisfied and it's ok to proceed with the injection test.

That said, I also interpreted the "look for evidence of XFS_DEBUG=y" and
"see if xfs_io inject knows about the specific error tag" sequence as a
best effort work around for the fact that the ioctl-based injection
mechanism isn't directly discoverable at all, so instead we assume it's
safe to proceed if we can detect a debug kernel and xfs_io is new enough
to issue the request.

--D

> Outside of Eryu's further comments, the rest looks good to me. This
> reminds me that I still need to post the latest log tail overwrite test
> that depends on this, now that the kernel bits have been reviewed...
> 
> Brian
> 
> > >  	_require_error_injection
> > >  
> > >  	# NOTE: We can't actually test error injection here because xfs
> > > @@ -54,16 +90,34 @@ _require_xfs_io_error_injection()
> > >  _test_inject_error()
> > >  {
> > >  	type="$1"
> > > +	value="$2"
> > >  
> > > -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > > +	knob="$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")"
> > > +	if [ -w "${knob}" ]; then
> > > +		test -z "${value}" && value="default"
> > > +		echo -n "${value}" > "${knob}"
> > > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > > +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > > +	else
> > > +		_notrun "Cannot inject error ${type} value ${value}."
> > 
> > _require_xfs_io_error_injection already made sure we can do error
> > jection, it looks like a bug somewhere to me if we can't inject error
> > here, either wanted to inject error without checking the support status
> > first or an implementation bug in the error injection framework in
> > xfstests. So "_fail" might be the right choice.
> > 
> > > +	fi
> > >  }
> > >  
> > >  # Inject an error into the scratch fs
> > >  _scratch_inject_error()
> > >  {
> > >  	type="$1"
> > > +	value="$2"
> > >  
> > > -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > > +	knob="$(_find_xfs_mount_errortag_knob "${SCRATCH_DEV}" "${type}")"
> > > +	if [ -w "${knob}" ]; then
> > > +		test -z "${value}" && value="default"
> > > +		echo -n "${value}" > "${knob}"
> > > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > > +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > > +	else
> > > +		_notrun "Cannot inject error ${type} value ${value}."
> > 
> > Same here.
> > 
> > Thanks,
> > Eryu
> > 
> > > +	fi
> > >  }
> > >  
> > >  # Unmount and remount the scratch device, dumping the log
> > > diff --git a/tests/xfs/141 b/tests/xfs/141
> > > index 56ff14e..f61e524 100755
> > > --- a/tests/xfs/141
> > > +++ b/tests/xfs/141
> > > @@ -47,13 +47,14 @@ rm -f $seqres.full
> > >  
> > >  # get standard environment, filters and checks
> > >  . ./common/rc
> > > +. ./common/inject
> > >  
> > >  # real QA test starts here
> > >  
> > >  # Modify as appropriate.
> > >  _supported_fs xfs
> > >  _supported_os Linux
> > > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> > > +_require_xfs_io_error_injection "log_bad_crc"
> > >  _require_scratch
> > >  _require_command "$KILLALL_PROG" killall
> > >  
> > > @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
> > >  	# (increase this value to run fsstress longer).
> > >  	factor=$((RANDOM % 100 + 1))
> > >  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> > > -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> > > +	_scratch_inject_error "log_bad_crc" "$factor"
> > >  
> > >  	# Run fsstress until the filesystem shuts down. It will shut down
> > >  	# automatically when error injection triggers.
> > > diff --git a/tests/xfs/196 b/tests/xfs/196
> > > index e9b0649..fe3f570 100755
> > > --- a/tests/xfs/196
> > > +++ b/tests/xfs/196
> > > @@ -45,6 +45,7 @@ _cleanup()
> > >  # get standard environment, filters and checks
> > >  . ./common/rc
> > >  . ./common/punch
> > > +. ./common/inject
> > >  
> > >  # real QA test starts here
> > >  rm -f $seqres.full
> > > @@ -53,13 +54,7 @@ rm -f $seqres.full
> > >  _supported_fs generic
> > >  _supported_os Linux
> > >  _require_scratch
> > > -
> > > -DROP_WRITES="drop_writes"
> > > -# replace "drop_writes" with "fail_writes" for old kernel
> > > -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> > > -	DROP_WRITES="fail_writes"
> > > -fi
> > > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> > > +_require_xfs_io_error_injection "drop_writes"
> > >  
> > >  _scratch_mkfs >/dev/null 2>&1
> > >  _scratch_mount
> > > @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
> > >  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
> > >  
> > >  # Enable write drops. All buffered writes are dropped from this point on.
> > > -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > +_scratch_inject_error "drop_writes" 1
> > >  
> > >  # Write every other 4k range to split the larger delalloc extent into many more
> > >  # smaller extents. Use pwrite because with write failures enabled, all
> > > @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
> > >  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
> > >  done
> > >  
> > > -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > +_scratch_inject_error "drop_writes" 0
> > >  
> > >  _scratch_cycle_mount
> > >  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> > > @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
> > >  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
> > >  
> > >  	punchoffset=$((offset + 75))
> > > -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > +	_scratch_inject_error "drop_writes"
> > >  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> > > -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > +	_scratch_inject_error "drop_writes" 0
> > >  done
> > >  
> > >  echo "Silence is golden."
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Brian Foster]

On Wed, Aug 02, 2017 at 08:52:57AM -0700, Darrick J. Wong wrote:
> On Wed, Aug 02, 2017 at 10:30:10AM -0400, Brian Foster wrote:
> > On Wed, Aug 02, 2017 at 04:37:42PM +0800, Eryu Guan wrote:
> > > On Fri, Jul 21, 2017 at 03:04:45PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Refactor the XFS error injection helpers to use the new errortag
> > > > interface to configure error injection.  If that isn't present, fall
> > > > back either to the xfs_io/ioctl based injection or the older sysfs
> > > > knobs.  Refactor existing testcases to use the new helpers.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > This looks good to me overall, but I still perfer let other people
> > > who're more familar with XFS errortag and error injection to review too.
> > > While I do have some questions/comments :)
> > > 
> > > > ---
> > > >  common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  tests/xfs/141 |    5 +++--
> > > >  tests/xfs/196 |   17 ++++++-----------
> > > >  3 files changed, 65 insertions(+), 15 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/common/inject b/common/inject
> > > > index 8ecc290..9aa24de 100644
> > > > --- a/common/inject
> > > > +++ b/common/inject
> > > > @@ -35,10 +35,46 @@ _require_error_injection()
> > > >  	esac
> > > >  }
> > > >  
> > > > +# Find a given xfs mount's errortag injection knob in sysfs
> > > > +_find_xfs_mount_errortag_knob()
> > > 
> > > While The function name and comment both indicate it needs a mounted
> > > XFS, it seems weird that the first argument is expected to be a block
> > > device. And do we need to check if the given device is really mounted?
> > > 
> > 
> > The xfs per-mount sysfs knobs distinguish between mounts via the
> > block device name.
> 
> What if we rename the helper and change its documentation as such?
> 
> # Find the errortag injection knob in sysfs for a given xfs mount's
> # block device.
> _find_xfs_mountdev_errortag_knob()
> {
> 	...
> }
> 
> > ...
> > > >  # Requires that xfs_io inject command knows about this error type
> > > >  _require_xfs_io_error_injection()
> > > >  {
> > > >  	type="$1"
> > > > +
> > > > +	# Can we find the error injection knobs via the new errortag
> > > > +	# configuration mechanism?
> > > > +	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
> > > > +
> > > 
> > > As this check goes prior to the _require_error_injection check, so I
> > > assume this new errortag framework doesn't depend on a debug built XFS,
> > > can I?
> 
> It depends on the sysfs knobs, and the sysfs knobs in turn depend on
> XFS_DEBUG=y.
> 
> > It does depend on debug mode so it probably makes sense to push this
> > after the _require_error_injection check. That way the DEBUG=0 message
> > has precedent over a message regarding lack of support for a particular
> > knob.
> 
> Hm?  This is what _require_xfs_io_error_injection does:
> 
> First we compute the path to the knob if it exists
> (_find_xfs_mount_errortag_knob).
> 
> Then we check if that path is writable.  If it is, error injection is
> enabled (which presumably means XFS_DEBUG=y) and we can continue.
> 
> If not, then we use _require_error_injection to bail out if XFS_DEBUG=n.
> 

Ah, right. I misinterpreted that the lack of the knob would fall through
to the subsequent checks, so there's no difference with respect to the
resulting message if DEBUG=0. It still looks cleaner to me to check
debug mode first, fwiw, but not a big deal either way.

Brian

> If we don't bail out, XFS_DEBUG=y and so we check if the xfs_io inject
> help page knows about the error type, and bail out if it doesn't.
> 
> 
> In the end it doesn't really matter if we look for XFS_DEBUG=y before or
> after we look for the new sysfs knob.  I figured it was simple enough to
> assume that if the knob is present and writable, then our preconditions
> are satisfied and it's ok to proceed with the injection test.
> 
> That said, I also interpreted the "look for evidence of XFS_DEBUG=y" and
> "see if xfs_io inject knows about the specific error tag" sequence as a
> best effort work around for the fact that the ioctl-based injection
> mechanism isn't directly discoverable at all, so instead we assume it's
> safe to proceed if we can detect a debug kernel and xfs_io is new enough
> to issue the request.
> 
> --D
> 
> > Outside of Eryu's further comments, the rest looks good to me. This
> > reminds me that I still need to post the latest log tail overwrite test
> > that depends on this, now that the kernel bits have been reviewed...
> > 
> > Brian
> > 
> > > >  	_require_error_injection
> > > >  
> > > >  	# NOTE: We can't actually test error injection here because xfs
> > > > @@ -54,16 +90,34 @@ _require_xfs_io_error_injection()
> > > >  _test_inject_error()
> > > >  {
> > > >  	type="$1"
> > > > +	value="$2"
> > > >  
> > > > -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > > > +	knob="$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")"
> > > > +	if [ -w "${knob}" ]; then
> > > > +		test -z "${value}" && value="default"
> > > > +		echo -n "${value}" > "${knob}"
> > > > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > > > +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > > > +	else
> > > > +		_notrun "Cannot inject error ${type} value ${value}."
> > > 
> > > _require_xfs_io_error_injection already made sure we can do error
> > > jection, it looks like a bug somewhere to me if we can't inject error
> > > here, either wanted to inject error without checking the support status
> > > first or an implementation bug in the error injection framework in
> > > xfstests. So "_fail" might be the right choice.
> > > 
> > > > +	fi
> > > >  }
> > > >  
> > > >  # Inject an error into the scratch fs
> > > >  _scratch_inject_error()
> > > >  {
> > > >  	type="$1"
> > > > +	value="$2"
> > > >  
> > > > -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > > > +	knob="$(_find_xfs_mount_errortag_knob "${SCRATCH_DEV}" "${type}")"
> > > > +	if [ -w "${knob}" ]; then
> > > > +		test -z "${value}" && value="default"
> > > > +		echo -n "${value}" > "${knob}"
> > > > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > > > +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > > > +	else
> > > > +		_notrun "Cannot inject error ${type} value ${value}."
> > > 
> > > Same here.
> > > 
> > > Thanks,
> > > Eryu
> > > 
> > > > +	fi
> > > >  }
> > > >  
> > > >  # Unmount and remount the scratch device, dumping the log
> > > > diff --git a/tests/xfs/141 b/tests/xfs/141
> > > > index 56ff14e..f61e524 100755
> > > > --- a/tests/xfs/141
> > > > +++ b/tests/xfs/141
> > > > @@ -47,13 +47,14 @@ rm -f $seqres.full
> > > >  
> > > >  # get standard environment, filters and checks
> > > >  . ./common/rc
> > > > +. ./common/inject
> > > >  
> > > >  # real QA test starts here
> > > >  
> > > >  # Modify as appropriate.
> > > >  _supported_fs xfs
> > > >  _supported_os Linux
> > > > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> > > > +_require_xfs_io_error_injection "log_bad_crc"
> > > >  _require_scratch
> > > >  _require_command "$KILLALL_PROG" killall
> > > >  
> > > > @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
> > > >  	# (increase this value to run fsstress longer).
> > > >  	factor=$((RANDOM % 100 + 1))
> > > >  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> > > > -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> > > > +	_scratch_inject_error "log_bad_crc" "$factor"
> > > >  
> > > >  	# Run fsstress until the filesystem shuts down. It will shut down
> > > >  	# automatically when error injection triggers.
> > > > diff --git a/tests/xfs/196 b/tests/xfs/196
> > > > index e9b0649..fe3f570 100755
> > > > --- a/tests/xfs/196
> > > > +++ b/tests/xfs/196
> > > > @@ -45,6 +45,7 @@ _cleanup()
> > > >  # get standard environment, filters and checks
> > > >  . ./common/rc
> > > >  . ./common/punch
> > > > +. ./common/inject
> > > >  
> > > >  # real QA test starts here
> > > >  rm -f $seqres.full
> > > > @@ -53,13 +54,7 @@ rm -f $seqres.full
> > > >  _supported_fs generic
> > > >  _supported_os Linux
> > > >  _require_scratch
> > > > -
> > > > -DROP_WRITES="drop_writes"
> > > > -# replace "drop_writes" with "fail_writes" for old kernel
> > > > -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> > > > -	DROP_WRITES="fail_writes"
> > > > -fi
> > > > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> > > > +_require_xfs_io_error_injection "drop_writes"
> > > >  
> > > >  _scratch_mkfs >/dev/null 2>&1
> > > >  _scratch_mount
> > > > @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
> > > >  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
> > > >  
> > > >  # Enable write drops. All buffered writes are dropped from this point on.
> > > > -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > > +_scratch_inject_error "drop_writes" 1
> > > >  
> > > >  # Write every other 4k range to split the larger delalloc extent into many more
> > > >  # smaller extents. Use pwrite because with write failures enabled, all
> > > > @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
> > > >  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
> > > >  done
> > > >  
> > > > -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > > +_scratch_inject_error "drop_writes" 0
> > > >  
> > > >  _scratch_cycle_mount
> > > >  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> > > > @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
> > > >  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
> > > >  
> > > >  	punchoffset=$((offset + 75))
> > > > -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > > +	_scratch_inject_error "drop_writes"
> > > >  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> > > > -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > > > +	_scratch_inject_error "drop_writes" 0
> > > >  done
> > > >  
> > > >  echo "Silence is golden."
> > > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] common/inject: refactor helpers to use new errortag interface

[Eryu Guan]

On Wed, Aug 02, 2017 at 08:52:57AM -0700, Darrick J. Wong wrote:
> On Wed, Aug 02, 2017 at 10:30:10AM -0400, Brian Foster wrote:
> > On Wed, Aug 02, 2017 at 04:37:42PM +0800, Eryu Guan wrote:
> > > On Fri, Jul 21, 2017 at 03:04:45PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Refactor the XFS error injection helpers to use the new errortag
> > > > interface to configure error injection.  If that isn't present, fall
> > > > back either to the xfs_io/ioctl based injection or the older sysfs
> > > > knobs.  Refactor existing testcases to use the new helpers.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > This looks good to me overall, but I still perfer let other people
> > > who're more familar with XFS errortag and error injection to review too.
> > > While I do have some questions/comments :)
> > > 
> > > > ---
> > > >  common/inject |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  tests/xfs/141 |    5 +++--
> > > >  tests/xfs/196 |   17 ++++++-----------
> > > >  3 files changed, 65 insertions(+), 15 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/common/inject b/common/inject
> > > > index 8ecc290..9aa24de 100644
> > > > --- a/common/inject
> > > > +++ b/common/inject
> > > > @@ -35,10 +35,46 @@ _require_error_injection()
> > > >  	esac
> > > >  }
> > > >  
> > > > +# Find a given xfs mount's errortag injection knob in sysfs
> > > > +_find_xfs_mount_errortag_knob()
> > > 
> > > While The function name and comment both indicate it needs a mounted
> > > XFS, it seems weird that the first argument is expected to be a block
> > > device. And do we need to check if the given device is really mounted?
> > > 
> > 
> > The xfs per-mount sysfs knobs distinguish between mounts via the
> > block device name.
> 
> What if we rename the helper and change its documentation as such?
> 
> # Find the errortag injection knob in sysfs for a given xfs mount's
> # block device.
> _find_xfs_mountdev_errortag_knob()

This looks better to me, thanks!

> {
> 	...
> }
> 
> > ...
> > > >  # Requires that xfs_io inject command knows about this error type
> > > >  _require_xfs_io_error_injection()
> > > >  {
> > > >  	type="$1"
> > > > +
> > > > +	# Can we find the error injection knobs via the new errortag
> > > > +	# configuration mechanism?
> > > > +	test -w "$(_find_xfs_mount_errortag_knob "${TEST_DEV}" "${type}")" && return
> > > > +
> > > 
> > > As this check goes prior to the _require_error_injection check, so I
> > > assume this new errortag framework doesn't depend on a debug built XFS,
> > > can I?
> 
> It depends on the sysfs knobs, and the sysfs knobs in turn depend on
> XFS_DEBUG=y.
> 
> > It does depend on debug mode so it probably makes sense to push this
> > after the _require_error_injection check. That way the DEBUG=0 message
> > has precedent over a message regarding lack of support for a particular
> > knob.
> 
> Hm?  This is what _require_xfs_io_error_injection does:
> 
> First we compute the path to the knob if it exists
> (_find_xfs_mount_errortag_knob).
> 
> Then we check if that path is writable.  If it is, error injection is
> enabled (which presumably means XFS_DEBUG=y) and we can continue.
> 
> If not, then we use _require_error_injection to bail out if XFS_DEBUG=n.
> 
> If we don't bail out, XFS_DEBUG=y and so we check if the xfs_io inject
> help page knows about the error type, and bail out if it doesn't.
> 
> 
> In the end it doesn't really matter if we look for XFS_DEBUG=y before or
> after we look for the new sysfs knob.  I figured it was simple enough to
> assume that if the knob is present and writable, then our preconditions
> are satisfied and it's ok to proceed with the injection test.

Agreed, the order doesn't really matter here, I'm fine with either way.

Thanks,
Eryu

[PATCH v2 3/5] common/inject: refactor helpers to use new errortag interface

[Darrick J. Wong]

Refactor the XFS error injection helpers to use the new errortag
interface to configure error injection.  If that isn't present, fall
back either to the xfs_io/ioctl based injection or the older sysfs
knobs.  Refactor existing testcases to use the new helpers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: fix name of the 'find knob' function to communicate that it takes a
mountpoint's device, not the mountpoint itself; check for debug before
we check for knobs
---
 common/inject |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/xfs/141 |    5 +++--
 tests/xfs/196 |   17 ++++++----------
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/common/inject b/common/inject
index 8ecc290..c1b7869 100644
--- a/common/inject
+++ b/common/inject
@@ -35,12 +35,49 @@ _require_error_injection()
 	esac
 }
 
+# Find the errortag injection knob in sysfs for a given xfs mount's
+# block device.
+_find_xfs_mountdev_errortag_knob()
+{
+	dev="$1"
+	knob="$2"
+	shortdev="$(_short_dev "${dev}")"
+	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
+
+	# Some of the new sysfs errortag knobs were previously available via
+	# another sysfs path.
+	case "${knob}" in
+	"log_bad_crc")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
+		fi
+		;;
+	"drop_writes")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
+		fi
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
+		fi
+		;;
+	*)
+		;;
+	esac
+
+	echo "${tagfile}"
+}
+
 # Requires that xfs_io inject command knows about this error type
 _require_xfs_io_error_injection()
 {
 	type="$1"
 	_require_error_injection
 
+	# Can we find the error injection knobs via the new errortag
+	# configuration mechanism?
+	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
+	test -w "${knob}" && return
+
 	# NOTE: We can't actually test error injection here because xfs
 	# hasn't always range checked the argument to xfs_errortag_add.
 	# We also don't want to trip an error before we're ready to deal
@@ -54,16 +91,34 @@ _require_xfs_io_error_injection()
 _test_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	else
+		_notrun "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Inject an error into the scratch fs
 _scratch_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	knob="$(_find_xfs_mountdev_errortag_knob "${SCRATCH_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	else
+		_notrun "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Unmount and remount the scratch device, dumping the log
diff --git a/tests/xfs/141 b/tests/xfs/141
index 56ff14e..f61e524 100755
--- a/tests/xfs/141
+++ b/tests/xfs/141
@@ -47,13 +47,14 @@ rm -f $seqres.full
 
 # get standard environment, filters and checks
 . ./common/rc
+. ./common/inject
 
 # real QA test starts here
 
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
+_require_xfs_io_error_injection "log_bad_crc"
 _require_scratch
 _require_command "$KILLALL_PROG" killall
 
@@ -69,7 +70,7 @@ for i in $(seq 1 5); do
 	# (increase this value to run fsstress longer).
 	factor=$((RANDOM % 100 + 1))
 	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
-	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
+	_scratch_inject_error "log_bad_crc" "$factor"
 
 	# Run fsstress until the filesystem shuts down. It will shut down
 	# automatically when error injection triggers.
diff --git a/tests/xfs/196 b/tests/xfs/196
index e9b0649..fe3f570 100755
--- a/tests/xfs/196
+++ b/tests/xfs/196
@@ -45,6 +45,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/punch
+. ./common/inject
 
 # real QA test starts here
 rm -f $seqres.full
@@ -53,13 +54,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-
-DROP_WRITES="drop_writes"
-# replace "drop_writes" with "fail_writes" for old kernel
-if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
-	DROP_WRITES="fail_writes"
-fi
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
+_require_xfs_io_error_injection "drop_writes"
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
@@ -72,7 +67,7 @@ bytes=$((64 * 1024))
 $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
 
 # Enable write drops. All buffered writes are dropped from this point on.
-echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 1
 
 # Write every other 4k range to split the larger delalloc extent into many more
 # smaller extents. Use pwrite because with write failures enabled, all
@@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
 	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
 done
 
-echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 0
 
 _scratch_cycle_mount
 $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
@@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
 	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
 
 	punchoffset=$((offset + 75))
-	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes"
 	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
-	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes" 0
 done
 
 echo "Silence is golden."

Re: [PATCH v2 3/5] common/inject: refactor helpers to use new errortag interface

[Eryu Guan]

On Thu, Aug 03, 2017 at 08:49:00AM -0700, Darrick J. Wong wrote:
> Refactor the XFS error injection helpers to use the new errortag
> interface to configure error injection.  If that isn't present, fall
> back either to the xfs_io/ioctl based injection or the older sysfs
> knobs.  Refactor existing testcases to use the new helpers.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v2: fix name of the 'find knob' function to communicate that it takes a
> mountpoint's device, not the mountpoint itself; check for debug before
> we check for knobs
> ---
>  common/inject |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/xfs/141 |    5 +++--
>  tests/xfs/196 |   17 ++++++----------
>  3 files changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/common/inject b/common/inject
> index 8ecc290..c1b7869 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -35,12 +35,49 @@ _require_error_injection()
>  	esac
>  }
>  
> +# Find the errortag injection knob in sysfs for a given xfs mount's
> +# block device.
> +_find_xfs_mountdev_errortag_knob()
> +{
> +	dev="$1"
> +	knob="$2"
> +	shortdev="$(_short_dev "${dev}")"
> +	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
> +
> +	# Some of the new sysfs errortag knobs were previously available via
> +	# another sysfs path.
> +	case "${knob}" in
> +	"log_bad_crc")
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
> +		fi
> +		;;
> +	"drop_writes")
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
> +		fi
> +		if [ ! -w "${tagfile}" ]; then
> +			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
> +		fi
> +		;;
> +	*)
> +		;;
> +	esac
> +
> +	echo "${tagfile}"
> +}
> +
>  # Requires that xfs_io inject command knows about this error type
>  _require_xfs_io_error_injection()
>  {
>  	type="$1"
>  	_require_error_injection
>  
> +	# Can we find the error injection knobs via the new errortag
> +	# configuration mechanism?
> +	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
> +	test -w "${knob}" && return
> +
>  	# NOTE: We can't actually test error injection here because xfs
>  	# hasn't always range checked the argument to xfs_errortag_add.
>  	# We also don't want to trip an error before we're ready to deal
> @@ -54,16 +91,34 @@ _require_xfs_io_error_injection()
>  _test_inject_error()
>  {
>  	type="$1"
> +	value="$2"
>  
> -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> +	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
> +	if [ -w "${knob}" ]; then
> +		test -z "${value}" && value="default"
> +		echo -n "${value}" > "${knob}"
> +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> +	else
> +		_notrun "Cannot inject error ${type} value ${value}."

(Seems you missed the comments on this "_notrun" in my first reply :)

I don't think _notrun belongs here, because at this point we should have
already made sure, by _require_xfs_io_error_injection, that XFS does
support the error injection we want. Something went wrong If we failed
to inject the error. So I think "_fail" is more appropriate.

> +	fi
>  }
>  
>  # Inject an error into the scratch fs
>  _scratch_inject_error()
>  {
>  	type="$1"
> +	value="$2"
>  
> -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> +	knob="$(_find_xfs_mountdev_errortag_knob "${SCRATCH_DEV}" "${type}")"
> +	if [ -w "${knob}" ]; then
> +		test -z "${value}" && value="default"
> +		echo -n "${value}" > "${knob}"
> +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> +	else
> +		_notrun "Cannot inject error ${type} value ${value}."

Same here.

Thanks,
Eryu

> +	fi
>  }
>  
>  # Unmount and remount the scratch device, dumping the log
> diff --git a/tests/xfs/141 b/tests/xfs/141
> index 56ff14e..f61e524 100755
> --- a/tests/xfs/141
> +++ b/tests/xfs/141
> @@ -47,13 +47,14 @@ rm -f $seqres.full
>  
>  # get standard environment, filters and checks
>  . ./common/rc
> +. ./common/inject
>  
>  # real QA test starts here
>  
>  # Modify as appropriate.
>  _supported_fs xfs
>  _supported_os Linux
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> +_require_xfs_io_error_injection "log_bad_crc"
>  _require_scratch
>  _require_command "$KILLALL_PROG" killall
>  
> @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
>  	# (increase this value to run fsstress longer).
>  	factor=$((RANDOM % 100 + 1))
>  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> +	_scratch_inject_error "log_bad_crc" "$factor"
>  
>  	# Run fsstress until the filesystem shuts down. It will shut down
>  	# automatically when error injection triggers.
> diff --git a/tests/xfs/196 b/tests/xfs/196
> index e9b0649..fe3f570 100755
> --- a/tests/xfs/196
> +++ b/tests/xfs/196
> @@ -45,6 +45,7 @@ _cleanup()
>  # get standard environment, filters and checks
>  . ./common/rc
>  . ./common/punch
> +. ./common/inject
>  
>  # real QA test starts here
>  rm -f $seqres.full
> @@ -53,13 +54,7 @@ rm -f $seqres.full
>  _supported_fs generic
>  _supported_os Linux
>  _require_scratch
> -
> -DROP_WRITES="drop_writes"
> -# replace "drop_writes" with "fail_writes" for old kernel
> -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> -	DROP_WRITES="fail_writes"
> -fi
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> +_require_xfs_io_error_injection "drop_writes"
>  
>  _scratch_mkfs >/dev/null 2>&1
>  _scratch_mount
> @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
>  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
>  
>  # Enable write drops. All buffered writes are dropped from this point on.
> -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +_scratch_inject_error "drop_writes" 1
>  
>  # Write every other 4k range to split the larger delalloc extent into many more
>  # smaller extents. Use pwrite because with write failures enabled, all
> @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
>  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
>  done
>  
> -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +_scratch_inject_error "drop_writes" 0
>  
>  _scratch_cycle_mount
>  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
>  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
>  
>  	punchoffset=$((offset + 75))
> -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +	_scratch_inject_error "drop_writes"
>  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> +	_scratch_inject_error "drop_writes" 0
>  done
>  
>  echo "Silence is golden."
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/5] common/inject: refactor helpers to use new errortag interface

[Darrick J. Wong]

On Fri, Aug 04, 2017 at 08:45:02AM +0800, Eryu Guan wrote:
> On Thu, Aug 03, 2017 at 08:49:00AM -0700, Darrick J. Wong wrote:
> > Refactor the XFS error injection helpers to use the new errortag
> > interface to configure error injection.  If that isn't present, fall
> > back either to the xfs_io/ioctl based injection or the older sysfs
> > knobs.  Refactor existing testcases to use the new helpers.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > v2: fix name of the 'find knob' function to communicate that it takes a
> > mountpoint's device, not the mountpoint itself; check for debug before
> > we check for knobs
> > ---
> >  common/inject |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  tests/xfs/141 |    5 +++--
> >  tests/xfs/196 |   17 ++++++----------
> >  3 files changed, 66 insertions(+), 15 deletions(-)
> > 
> > diff --git a/common/inject b/common/inject
> > index 8ecc290..c1b7869 100644
> > --- a/common/inject
> > +++ b/common/inject
> > @@ -35,12 +35,49 @@ _require_error_injection()
> >  	esac
> >  }
> >  
> > +# Find the errortag injection knob in sysfs for a given xfs mount's
> > +# block device.
> > +_find_xfs_mountdev_errortag_knob()
> > +{
> > +	dev="$1"
> > +	knob="$2"
> > +	shortdev="$(_short_dev "${dev}")"
> > +	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
> > +
> > +	# Some of the new sysfs errortag knobs were previously available via
> > +	# another sysfs path.
> > +	case "${knob}" in
> > +	"log_bad_crc")
> > +		if [ ! -w "${tagfile}" ]; then
> > +			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
> > +		fi
> > +		;;
> > +	"drop_writes")
> > +		if [ ! -w "${tagfile}" ]; then
> > +			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
> > +		fi
> > +		if [ ! -w "${tagfile}" ]; then
> > +			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
> > +		fi
> > +		;;
> > +	*)
> > +		;;
> > +	esac
> > +
> > +	echo "${tagfile}"
> > +}
> > +
> >  # Requires that xfs_io inject command knows about this error type
> >  _require_xfs_io_error_injection()
> >  {
> >  	type="$1"
> >  	_require_error_injection
> >  
> > +	# Can we find the error injection knobs via the new errortag
> > +	# configuration mechanism?
> > +	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
> > +	test -w "${knob}" && return
> > +
> >  	# NOTE: We can't actually test error injection here because xfs
> >  	# hasn't always range checked the argument to xfs_errortag_add.
> >  	# We also don't want to trip an error before we're ready to deal
> > @@ -54,16 +91,34 @@ _require_xfs_io_error_injection()
> >  _test_inject_error()
> >  {
> >  	type="$1"
> > +	value="$2"
> >  
> > -	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > +	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
> > +	if [ -w "${knob}" ]; then
> > +		test -z "${value}" && value="default"
> > +		echo -n "${value}" > "${knob}"
> > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > +		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
> > +	else
> > +		_notrun "Cannot inject error ${type} value ${value}."
> 
> (Seems you missed the comments on this "_notrun" in my first reply :)
> 
> I don't think _notrun belongs here, because at this point we should have
> already made sure, by _require_xfs_io_error_injection, that XFS does
> support the error injection we want. Something went wrong If we failed
> to inject the error. So I think "_fail" is more appropriate.

Oops. Will send v3 fixing both.

> > +	fi
> >  }
> >  
> >  # Inject an error into the scratch fs
> >  _scratch_inject_error()
> >  {
> >  	type="$1"
> > +	value="$2"
> >  
> > -	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > +	knob="$(_find_xfs_mountdev_errortag_knob "${SCRATCH_DEV}" "${type}")"
> > +	if [ -w "${knob}" ]; then
> > +		test -z "${value}" && value="default"
> > +		echo -n "${value}" > "${knob}"
> > +	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
> > +		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > +	else
> > +		_notrun "Cannot inject error ${type} value ${value}."
> 
> Same here.
> 
> Thanks,
> Eryu
> 
> > +	fi
> >  }
> >  
> >  # Unmount and remount the scratch device, dumping the log
> > diff --git a/tests/xfs/141 b/tests/xfs/141
> > index 56ff14e..f61e524 100755
> > --- a/tests/xfs/141
> > +++ b/tests/xfs/141
> > @@ -47,13 +47,14 @@ rm -f $seqres.full
> >  
> >  # get standard environment, filters and checks
> >  . ./common/rc
> > +. ./common/inject
> >  
> >  # real QA test starts here
> >  
> >  # Modify as appropriate.
> >  _supported_fs xfs
> >  _supported_os Linux
> > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> > +_require_xfs_io_error_injection "log_bad_crc"
> >  _require_scratch
> >  _require_command "$KILLALL_PROG" killall
> >  
> > @@ -69,7 +70,7 @@ for i in $(seq 1 5); do
> >  	# (increase this value to run fsstress longer).
> >  	factor=$((RANDOM % 100 + 1))
> >  	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> > -	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> > +	_scratch_inject_error "log_bad_crc" "$factor"
> >  
> >  	# Run fsstress until the filesystem shuts down. It will shut down
> >  	# automatically when error injection triggers.
> > diff --git a/tests/xfs/196 b/tests/xfs/196
> > index e9b0649..fe3f570 100755
> > --- a/tests/xfs/196
> > +++ b/tests/xfs/196
> > @@ -45,6 +45,7 @@ _cleanup()
> >  # get standard environment, filters and checks
> >  . ./common/rc
> >  . ./common/punch
> > +. ./common/inject
> >  
> >  # real QA test starts here
> >  rm -f $seqres.full
> > @@ -53,13 +54,7 @@ rm -f $seqres.full
> >  _supported_fs generic
> >  _supported_os Linux
> >  _require_scratch
> > -
> > -DROP_WRITES="drop_writes"
> > -# replace "drop_writes" with "fail_writes" for old kernel
> > -if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
> > -	DROP_WRITES="fail_writes"
> > -fi
> > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
> > +_require_xfs_io_error_injection "drop_writes"
> >  
> >  _scratch_mkfs >/dev/null 2>&1
> >  _scratch_mount
> > @@ -72,7 +67,7 @@ bytes=$((64 * 1024))
> >  $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
> >  
> >  # Enable write drops. All buffered writes are dropped from this point on.
> > -echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +_scratch_inject_error "drop_writes" 1
> >  
> >  # Write every other 4k range to split the larger delalloc extent into many more
> >  # smaller extents. Use pwrite because with write failures enabled, all
> > @@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
> >  	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
> >  done
> >  
> > -echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +_scratch_inject_error "drop_writes" 0
> >  
> >  _scratch_cycle_mount
> >  $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
> > @@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
> >  	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
> >  
> >  	punchoffset=$((offset + 75))
> > -	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +	_scratch_inject_error "drop_writes"
> >  	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
> > -	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
> > +	_scratch_inject_error "drop_writes" 0
> >  done
> >  
> >  echo "Silence is golden."
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/5] common/inject: refactor helpers to use new errortag interface

[Darrick J. Wong]

Refactor the XFS error injection helpers to use the new errortag
interface to configure error injection.  If that isn't present, fall
back either to the xfs_io/ioctl based injection or the older sysfs
knobs.  Refactor existing testcases to use the new helpers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v3: fix _notrun to _fail when we can't set a knob to a particular value
v2: fix name of the 'find knob' function to communicate that it takes a
mountpoint's device, not the mountpoint itself; check for debug before
we check for knobs
---
 common/inject |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/xfs/141 |    5 +++--
 tests/xfs/196 |   17 ++++++----------
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/common/inject b/common/inject
index 8ecc290..f30d0ba 100644
--- a/common/inject
+++ b/common/inject
@@ -35,12 +35,49 @@ _require_error_injection()
 	esac
 }
 
+# Find the errortag injection knob in sysfs for a given xfs mount's
+# block device.
+_find_xfs_mountdev_errortag_knob()
+{
+	dev="$1"
+	knob="$2"
+	shortdev="$(_short_dev "${dev}")"
+	tagfile="/sys/fs/xfs/${shortdev}/errortag/${knob}"
+
+	# Some of the new sysfs errortag knobs were previously available via
+	# another sysfs path.
+	case "${knob}" in
+	"log_bad_crc")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/log/log_badcrc_factor"
+		fi
+		;;
+	"drop_writes")
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/drop_writes"
+		fi
+		if [ ! -w "${tagfile}" ]; then
+			tagfile="/sys/fs/xfs/${shortdev}/fail_writes"
+		fi
+		;;
+	*)
+		;;
+	esac
+
+	echo "${tagfile}"
+}
+
 # Requires that xfs_io inject command knows about this error type
 _require_xfs_io_error_injection()
 {
 	type="$1"
 	_require_error_injection
 
+	# Can we find the error injection knobs via the new errortag
+	# configuration mechanism?
+	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
+	test -w "${knob}" && return
+
 	# NOTE: We can't actually test error injection here because xfs
 	# hasn't always range checked the argument to xfs_errortag_add.
 	# We also don't want to trip an error before we're ready to deal
@@ -54,16 +91,34 @@ _require_xfs_io_error_injection()
 _test_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	knob="$(_find_xfs_mountdev_errortag_knob "${TEST_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $TEST_DIR
+	else
+		_fail "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Inject an error into the scratch fs
 _scratch_inject_error()
 {
 	type="$1"
+	value="$2"
 
-	$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	knob="$(_find_xfs_mountdev_errortag_knob "${SCRATCH_DEV}" "${type}")"
+	if [ -w "${knob}" ]; then
+		test -z "${value}" && value="default"
+		echo -n "${value}" > "${knob}"
+	elif [ -z "${value}" ] || [ "${value}" = "default" ]; then
+		$XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
+	else
+		_fail "Cannot inject error ${type} value ${value}."
+	fi
 }
 
 # Unmount and remount the scratch device, dumping the log
diff --git a/tests/xfs/141 b/tests/xfs/141
index 56ff14e..f61e524 100755
--- a/tests/xfs/141
+++ b/tests/xfs/141
@@ -47,13 +47,14 @@ rm -f $seqres.full
 
 # get standard environment, filters and checks
 . ./common/rc
+. ./common/inject
 
 # real QA test starts here
 
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
+_require_xfs_io_error_injection "log_bad_crc"
 _require_scratch
 _require_command "$KILLALL_PROG" killall
 
@@ -69,7 +70,7 @@ for i in $(seq 1 5); do
 	# (increase this value to run fsstress longer).
 	factor=$((RANDOM % 100 + 1))
 	echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
-	echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
+	_scratch_inject_error "log_bad_crc" "$factor"
 
 	# Run fsstress until the filesystem shuts down. It will shut down
 	# automatically when error injection triggers.
diff --git a/tests/xfs/196 b/tests/xfs/196
index e9b0649..fe3f570 100755
--- a/tests/xfs/196
+++ b/tests/xfs/196
@@ -45,6 +45,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/punch
+. ./common/inject
 
 # real QA test starts here
 rm -f $seqres.full
@@ -53,13 +54,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-
-DROP_WRITES="drop_writes"
-# replace "drop_writes" with "fail_writes" for old kernel
-if [ -f /sys/fs/xfs/$(_short_dev $TEST_DEV)/fail_writes ];then
-	DROP_WRITES="fail_writes"
-fi
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/${DROP_WRITES}
+_require_xfs_io_error_injection "drop_writes"
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
@@ -72,7 +67,7 @@ bytes=$((64 * 1024))
 $XFS_IO_PROG -f -c "pwrite 0 $bytes" $file >> $seqres.full 2>&1
 
 # Enable write drops. All buffered writes are dropped from this point on.
-echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 1
 
 # Write every other 4k range to split the larger delalloc extent into many more
 # smaller extents. Use pwrite because with write failures enabled, all
@@ -89,7 +84,7 @@ for i in $(seq 4096 8192 $endoff); do
 	$XFS_IO_PROG -c "pwrite $i 4k" $file >> $seqres.full 2>&1
 done
 
-echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+_scratch_inject_error "drop_writes" 0
 
 _scratch_cycle_mount
 $XFS_IO_PROG -c 'bmap -vp' $file | _filter_bmap
@@ -104,9 +99,9 @@ for offset in $(seq 0 100 500); do
 	$XFS_IO_PROG -fc "pwrite ${offset}m 100m" $file >> $seqres.full 2>&1
 
 	punchoffset=$((offset + 75))
-	echo 1 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes"
 	$XFS_IO_PROG -c "pwrite ${punchoffset}m 4k" $file >> $seqres.full 2>&1
-	echo 0 > /sys/fs/xfs/$sdev/$DROP_WRITES
+	_scratch_inject_error "drop_writes" 0
 done
 
 echo "Silence is golden."

[PATCH 4/5] common/populate: enable xfs quota accounting

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When we're creating a populated xfs image, turn on quotas so that we can
fuzz those fields too.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)


diff --git a/common/populate b/common/populate
index 498151f..b77c508 100644
--- a/common/populate
+++ b/common/populate
@@ -21,6 +21,7 @@
 #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
 #  Mountain View, CA 94043, USA, or: http://www.sgi.com
 #-----------------------------------------------------------------------
+. ./common/quota
 
 _require_populate_commands() {
 	_require_xfs_io_command "falloc"
@@ -94,6 +95,47 @@ __populate_fill_fs() {
 	done
 }
 
+# For XFS, force on all the quota options if quota is enabled
+# and the user didn't feed us noquota.
+_populate_xfs_qmount_option()
+{
+	# User explicitly told us not to quota
+	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
+		return
+	fi
+
+	# Don't bother if we can't turn on quotas
+	if [ ! -f /proc/fs/xfs/xqmstat ]; then
+		# No quota support
+		return
+	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
+		# Quotas not supported on rt filesystems
+		return
+	elif [ -z "${XFS_QUOTA_PROG}" ]; then
+		# xfs quota tools not installed
+		return
+	fi
+
+	# Turn on all the quotas
+	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
+		# v5 filesystems can have group & project quotas
+		quota="usrquota,grpquota,prjquota"
+	else
+		# v4 filesystems cannot mix group & project quotas
+		quota="usrquota,grpquota"
+	fi
+
+	# Inject our quota mount options
+	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
+		return
+	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
+		_qmount_option "${quota}"
+	else
+		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
+		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
+	fi
+}
+
 # Populate an XFS on the scratch device with (we hope) all known
 # types of metadata block
 _scratch_xfs_populate() {
@@ -106,6 +148,7 @@ _scratch_xfs_populate() {
 		esac
 	done
 
+	_populate_xfs_qmount_option
 	_scratch_mount
 	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
 	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
@@ -720,7 +763,12 @@ _scratch_populate_cached() {
 		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
 		;;
 	"xfs")
-		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
+		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
+		_populate_xfs_qmount_option
+		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
+			extra_descr="${extra_descr} QUOTAS"
+		fi
+		;;
 	*)
 		extra_descr="";;
 	esac

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Eryu Guan]

On Fri, Jul 21, 2017 at 03:04:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're creating a populated xfs image, turn on quotas so that we can
> fuzz those fields too.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 498151f..b77c508 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -21,6 +21,7 @@
>  #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
>  #  Mountain View, CA 94043, USA, or: http://www.sgi.com
>  #-----------------------------------------------------------------------
> +. ./common/quota
>  
>  _require_populate_commands() {
>  	_require_xfs_io_command "falloc"
> @@ -94,6 +95,47 @@ __populate_fill_fs() {
>  	done
>  }
>  
> +# For XFS, force on all the quota options if quota is enabled
> +# and the user didn't feed us noquota.
> +_populate_xfs_qmount_option()
> +{
> +	# User explicitly told us not to quota
> +	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
> +		return
> +	fi
> +
> +	# Don't bother if we can't turn on quotas
> +	if [ ! -f /proc/fs/xfs/xqmstat ]; then
> +		# No quota support
> +		return
> +	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
> +		# Quotas not supported on rt filesystems
> +		return
> +	elif [ -z "${XFS_QUOTA_PROG}" ]; then
> +		# xfs quota tools not installed
> +		return
> +	fi
> +
> +	# Turn on all the quotas
> +	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
> +		# v5 filesystems can have group & project quotas
> +		quota="usrquota,grpquota,prjquota"
> +	else
> +		# v4 filesystems cannot mix group & project quotas
> +		quota="usrquota,grpquota"
> +	fi
> +
> +	# Inject our quota mount options
> +	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
> +		return
> +	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
> +		_qmount_option "${quota}"
> +	else
> +		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
> +		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> +	fi

It looks like an unconditional '_qmount_option "${quota}"' would be
sufficient here. Did I miss anything?

> +}
> +
>  # Populate an XFS on the scratch device with (we hope) all known
>  # types of metadata block
>  _scratch_xfs_populate() {
> @@ -106,6 +148,7 @@ _scratch_xfs_populate() {
>  		esac
>  	done
>  
> +	_populate_xfs_qmount_option
>  	_scratch_mount

Then use _qmount? But it's in populate function for xfs, _scratch_mount
and _qmount doesn't have much difference, so either way seems fine.

Thanks,
Eryu

>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> @@ -720,7 +763,12 @@ _scratch_populate_cached() {
>  		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
>  		;;
>  	"xfs")
> -		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
> +		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
> +		_populate_xfs_qmount_option
> +		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
> +			extra_descr="${extra_descr} QUOTAS"
> +		fi
> +		;;
>  	*)
>  		extra_descr="";;
>  	esac
> 

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Darrick J. Wong]

On Fri, Aug 04, 2017 at 09:54:35AM +0800, Eryu Guan wrote:
> On Fri, Jul 21, 2017 at 03:04:52PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > When we're creating a populated xfs image, turn on quotas so that we can
> > fuzz those fields too.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 49 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/common/populate b/common/populate
> > index 498151f..b77c508 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -21,6 +21,7 @@
> >  #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
> >  #  Mountain View, CA 94043, USA, or: http://www.sgi.com
> >  #-----------------------------------------------------------------------
> > +. ./common/quota
> >  
> >  _require_populate_commands() {
> >  	_require_xfs_io_command "falloc"
> > @@ -94,6 +95,47 @@ __populate_fill_fs() {
> >  	done
> >  }
> >  
> > +# For XFS, force on all the quota options if quota is enabled
> > +# and the user didn't feed us noquota.
> > +_populate_xfs_qmount_option()
> > +{
> > +	# User explicitly told us not to quota
> > +	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
> > +		return
> > +	fi
> > +
> > +	# Don't bother if we can't turn on quotas
> > +	if [ ! -f /proc/fs/xfs/xqmstat ]; then
> > +		# No quota support
> > +		return
> > +	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
> > +		# Quotas not supported on rt filesystems
> > +		return
> > +	elif [ -z "${XFS_QUOTA_PROG}" ]; then
> > +		# xfs quota tools not installed
> > +		return
> > +	fi
> > +
> > +	# Turn on all the quotas
> > +	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
> > +		# v5 filesystems can have group & project quotas
> > +		quota="usrquota,grpquota,prjquota"
> > +	else
> > +		# v4 filesystems cannot mix group & project quotas
> > +		quota="usrquota,grpquota"
> > +	fi
> > +
> > +	# Inject our quota mount options
> > +	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
> > +		return
> > +	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
> > +		_qmount_option "${quota}"
> > +	else
> > +		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
> > +		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> > +	fi
> 
> It looks like an unconditional '_qmount_option "${quota}"' would be
> sufficient here. Did I miss anything?

Turning on /all/ the quota types.

> > +}
> > +
> >  # Populate an XFS on the scratch device with (we hope) all known
> >  # types of metadata block
> >  _scratch_xfs_populate() {
> > @@ -106,6 +148,7 @@ _scratch_xfs_populate() {
> >  		esac
> >  	done
> >  
> > +	_populate_xfs_qmount_option
> >  	_scratch_mount
> 
> Then use _qmount? But it's in populate function for xfs, _scratch_mount
> and _qmount doesn't have much difference, so either way seems fine.

<shrug> fewer changes, then :)

--D

> 
> Thanks,
> Eryu
> 
> >  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> >  	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> > @@ -720,7 +763,12 @@ _scratch_populate_cached() {
> >  		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
> >  		;;
> >  	"xfs")
> > -		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
> > +		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
> > +		_populate_xfs_qmount_option
> > +		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
> > +			extra_descr="${extra_descr} QUOTAS"
> > +		fi
> > +		;;
> >  	*)
> >  		extra_descr="";;
> >  	esac
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Eryu Guan]

On Fri, Jul 21, 2017 at 03:04:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're creating a populated xfs image, turn on quotas so that we can
> fuzz those fields too.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 498151f..b77c508 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -21,6 +21,7 @@
>  #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
>  #  Mountain View, CA 94043, USA, or: http://www.sgi.com
>  #-----------------------------------------------------------------------
> +. ./common/quota
>  
>  _require_populate_commands() {
>  	_require_xfs_io_command "falloc"
> @@ -94,6 +95,47 @@ __populate_fill_fs() {
>  	done
>  }
>  
> +# For XFS, force on all the quota options if quota is enabled
> +# and the user didn't feed us noquota.
> +_populate_xfs_qmount_option()
> +{
> +	# User explicitly told us not to quota
> +	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
> +		return
> +	fi
> +
> +	# Don't bother if we can't turn on quotas
> +	if [ ! -f /proc/fs/xfs/xqmstat ]; then
> +		# No quota support
> +		return
> +	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
> +		# Quotas not supported on rt filesystems
> +		return
> +	elif [ -z "${XFS_QUOTA_PROG}" ]; then
> +		# xfs quota tools not installed
> +		return
> +	fi
> +
> +	# Turn on all the quotas
> +	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
> +		# v5 filesystems can have group & project quotas
> +		quota="usrquota,grpquota,prjquota"
> +	else
> +		# v4 filesystems cannot mix group & project quotas
> +		quota="usrquota,grpquota"
> +	fi
> +
> +	# Inject our quota mount options
> +	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
> +		return
> +	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
> +		_qmount_option "${quota}"
> +	else
> +		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
> +		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> +	fi
> +}
> +
>  # Populate an XFS on the scratch device with (we hope) all known
>  # types of metadata block
>  _scratch_xfs_populate() {
> @@ -106,6 +148,7 @@ _scratch_xfs_populate() {
>  		esac
>  	done
>  
> +	_populate_xfs_qmount_option

Sorry, another question just poped up..

This enables quota and modifies MOUNT_OPTIONS unconditionally and
implicitly, I suspect users of _scratch_populate_cached() or
_scratch_xfs_populate() may be bitten by this implicit change.

Would it be better if there's a argument to control this quota
enablement and default to disable quota? Just like the "nofill"
argument, introduce a new "quota" argument? So existing callers of
_scratch_populate_cached() are not affected by this change, and tests
want quota can enable it explicitly.

Thanks,
Eryu

>  	_scratch_mount
>  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
>  	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> @@ -720,7 +763,12 @@ _scratch_populate_cached() {
>  		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
>  		;;
>  	"xfs")
> -		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
> +		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
> +		_populate_xfs_qmount_option
> +		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
> +			extra_descr="${extra_descr} QUOTAS"
> +		fi
> +		;;
>  	*)
>  		extra_descr="";;
>  	esac
> 

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Darrick J. Wong]

On Fri, Aug 04, 2017 at 10:22:00AM +0800, Eryu Guan wrote:
> On Fri, Jul 21, 2017 at 03:04:52PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > When we're creating a populated xfs image, turn on quotas so that we can
> > fuzz those fields too.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 49 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/common/populate b/common/populate
> > index 498151f..b77c508 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -21,6 +21,7 @@
> >  #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
> >  #  Mountain View, CA 94043, USA, or: http://www.sgi.com
> >  #-----------------------------------------------------------------------
> > +. ./common/quota
> >  
> >  _require_populate_commands() {
> >  	_require_xfs_io_command "falloc"
> > @@ -94,6 +95,47 @@ __populate_fill_fs() {
> >  	done
> >  }
> >  
> > +# For XFS, force on all the quota options if quota is enabled
> > +# and the user didn't feed us noquota.
> > +_populate_xfs_qmount_option()
> > +{
> > +	# User explicitly told us not to quota
> > +	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
> > +		return
> > +	fi
> > +
> > +	# Don't bother if we can't turn on quotas
> > +	if [ ! -f /proc/fs/xfs/xqmstat ]; then
> > +		# No quota support
> > +		return
> > +	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
> > +		# Quotas not supported on rt filesystems
> > +		return
> > +	elif [ -z "${XFS_QUOTA_PROG}" ]; then
> > +		# xfs quota tools not installed
> > +		return
> > +	fi
> > +
> > +	# Turn on all the quotas
> > +	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
> > +		# v5 filesystems can have group & project quotas
> > +		quota="usrquota,grpquota,prjquota"
> > +	else
> > +		# v4 filesystems cannot mix group & project quotas
> > +		quota="usrquota,grpquota"
> > +	fi
> > +
> > +	# Inject our quota mount options
> > +	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
> > +		return
> > +	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
> > +		_qmount_option "${quota}"
> > +	else
> > +		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
> > +		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> > +	fi
> > +}
> > +
> >  # Populate an XFS on the scratch device with (we hope) all known
> >  # types of metadata block
> >  _scratch_xfs_populate() {
> > @@ -106,6 +148,7 @@ _scratch_xfs_populate() {
> >  		esac
> >  	done
> >  
> > +	_populate_xfs_qmount_option
> 
> Sorry, another question just poped up..
> 
> This enables quota and modifies MOUNT_OPTIONS unconditionally and
> implicitly, I suspect users of _scratch_populate_cached() or
> _scratch_xfs_populate() may be bitten by this implicit change.

I suppose we could be more explicit about the fact that we
unconditionally enable all quota types and change MOUNT_OPTIONS, though
I did go run all the _scratch_xfs_populate tests and they all seemed
fine.

(Well, as fine as we can get considering that most of them are fuzz
tests that cause all manners of other crazy behavior.)

> Would it be better if there's a argument to control this quota
> enablement and default to disable quota? Just like the "nofill"
> argument, introduce a new "quota" argument? So existing callers of
> _scratch_populate_cached() are not affected by this change, and tests
> want quota can enable it explicitly.

Hmm.  The way I look at it, though, _scratch_xfs_populate is supposed to
create all types of filesystem metadata, which includes quota files, so
we have to enable the quota options or we're not actually doing what the
documentation says it's supposed to be doing:

# Populate an XFS on the scratch device with (we hope) all known
# types of metadata block

The reason there's a 'nofill' option is that, having created an fs with
all known types of metadata, we can optionally use up any remaining free
space, but doing so isn't necessary to create all known types of
metadata block.

--D

> 
> Thanks,
> Eryu
> 
> >  	_scratch_mount
> >  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> >  	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> > @@ -720,7 +763,12 @@ _scratch_populate_cached() {
> >  		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
> >  		;;
> >  	"xfs")
> > -		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
> > +		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
> > +		_populate_xfs_qmount_option
> > +		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
> > +			extra_descr="${extra_descr} QUOTAS"
> > +		fi
> > +		;;
> >  	*)
> >  		extra_descr="";;
> >  	esac
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Darrick J. Wong]

On Tue, Aug 08, 2017 at 02:22:17PM -0700, Darrick J. Wong wrote:
> On Fri, Aug 04, 2017 at 10:22:00AM +0800, Eryu Guan wrote:
> > On Fri, Jul 21, 2017 at 03:04:52PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > When we're creating a populated xfs image, turn on quotas so that we can
> > > fuzz those fields too.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  common/populate |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 49 insertions(+), 1 deletion(-)
> > > 
> > > 
> > > diff --git a/common/populate b/common/populate
> > > index 498151f..b77c508 100644
> > > --- a/common/populate
> > > +++ b/common/populate
> > > @@ -21,6 +21,7 @@
> > >  #  Contact information: Silicon Graphics, Inc., 1500 Crittenden Lane,
> > >  #  Mountain View, CA 94043, USA, or: http://www.sgi.com
> > >  #-----------------------------------------------------------------------
> > > +. ./common/quota
> > >  
> > >  _require_populate_commands() {
> > >  	_require_xfs_io_command "falloc"
> > > @@ -94,6 +95,47 @@ __populate_fill_fs() {
> > >  	done
> > >  }
> > >  
> > > +# For XFS, force on all the quota options if quota is enabled
> > > +# and the user didn't feed us noquota.
> > > +_populate_xfs_qmount_option()
> > > +{
> > > +	# User explicitly told us not to quota
> > > +	if echo "${MOUNT_OPTIONS}" | grep -q 'noquota'; then
> > > +		return
> > > +	fi
> > > +
> > > +	# Don't bother if we can't turn on quotas
> > > +	if [ ! -f /proc/fs/xfs/xqmstat ]; then
> > > +		# No quota support
> > > +		return
> > > +	elif [ "${USE_EXTERNAL}" = "yes" ] && [ ! -z "${SCRATCH_RTDEV}" ]; then
> > > +		# Quotas not supported on rt filesystems
> > > +		return
> > > +	elif [ -z "${XFS_QUOTA_PROG}" ]; then
> > > +		# xfs quota tools not installed
> > > +		return
> > > +	fi
> > > +
> > > +	# Turn on all the quotas
> > > +	if xfs_info "${TEST_DIR}" | grep -q 'crc=1'; then
> > > +		# v5 filesystems can have group & project quotas
> > > +		quota="usrquota,grpquota,prjquota"
> > > +	else
> > > +		# v4 filesystems cannot mix group & project quotas
> > > +		quota="usrquota,grpquota"
> > > +	fi
> > > +
> > > +	# Inject our quota mount options
> > > +	if echo "${MOUNT_OPTIONS}" | grep -q "${quota}"; then
> > > +		return
> > > +	elif echo "${MOUNT_OPTIONS}" | egrep -q '(quota|noenforce)'; then
> > > +		_qmount_option "${quota}"
> > > +	else
> > > +		export MOUNT_OPTIONS="$MOUNT_OPTIONS -o ${quota}"
> > > +		echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
> > > +	fi
> > > +}
> > > +
> > >  # Populate an XFS on the scratch device with (we hope) all known
> > >  # types of metadata block
> > >  _scratch_xfs_populate() {
> > > @@ -106,6 +148,7 @@ _scratch_xfs_populate() {
> > >  		esac
> > >  	done
> > >  
> > > +	_populate_xfs_qmount_option
> > 
> > Sorry, another question just poped up..
> > 
> > This enables quota and modifies MOUNT_OPTIONS unconditionally and
> > implicitly, I suspect users of _scratch_populate_cached() or
> > _scratch_xfs_populate() may be bitten by this implicit change.
> 
> I suppose we could be more explicit about the fact that we
> unconditionally enable all quota types and change MOUNT_OPTIONS, though
> I did go run all the _scratch_xfs_populate tests and they all seemed
> fine.
> 
> (Well, as fine as we can get considering that most of them are fuzz
> tests that cause all manners of other crazy behavior.)
> 
> > Would it be better if there's a argument to control this quota
> > enablement and default to disable quota? Just like the "nofill"
> > argument, introduce a new "quota" argument? So existing callers of
> > _scratch_populate_cached() are not affected by this change, and tests
> > want quota can enable it explicitly.
> 
> Hmm.  The way I look at it, though, _scratch_xfs_populate is supposed to
> create all types of filesystem metadata, which includes quota files, so
> we have to enable the quota options or we're not actually doing what the
> documentation says it's supposed to be doing:
> 
> # Populate an XFS on the scratch device with (we hope) all known
> # types of metadata block

Ping?

--D

> 
> The reason there's a 'nofill' option is that, having created an fs with
> all known types of metadata, we can optionally use up any remaining free
> space, but doing so isn't necessary to create all known types of
> metadata block.
> 
> --D
> 
> > 
> > Thanks,
> > Eryu
> > 
> > >  	_scratch_mount
> > >  	blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> > >  	dblksz="$(xfs_info "${SCRATCH_MNT}" | grep naming.*bsize | sed -e 's/^.*bsize=//g' -e 's/\([0-9]*\).*$/\1/g')"
> > > @@ -720,7 +763,12 @@ _scratch_populate_cached() {
> > >  		test -n "${SCRATCH_LOGDEV}" && rm -f "${POPULATE_METADUMP}"
> > >  		;;
> > >  	"xfs")
> > > -		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}";;
> > > +		extra_descr="LOGDEV ${SCRATCH_LOGDEV} USE_EXTERNAL ${USE_EXTERNAL} RTDEV ${SCRATCH_RTDEV}"
> > > +		_populate_xfs_qmount_option
> > > +		if echo "${MOUNT_OPTIONS}" | grep -q 'usrquota'; then
> > > +			extra_descr="${extra_descr} QUOTAS"
> > > +		fi
> > > +		;;
> > >  	*)
> > >  		extra_descr="";;
> > >  	esac
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] common/populate: enable xfs quota accounting

[Eryu Guan]

On Wed, Aug 23, 2017 at 03:03:14PM -0700, Darrick J. Wong wrote:
> 
> Ping?

Sorry, somehow I thought there would be a resend or something. I'm
testing the quota fuzz tests now and will push them out if everything
goes fine. Thanks for the reminder!

Eryu

[PATCH 5/5] xfs: test fuzzing every field of a dquot

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

See what happens when we fuzz every field of a quota information structure.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/1380     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1380.out |    4 +++
 tests/xfs/1381     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1381.out |    4 +++
 tests/xfs/1382     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1382.out |    4 +++
 tests/xfs/1383     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1383.out |    4 +++
 tests/xfs/1384     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1384.out |    4 +++
 tests/xfs/1385     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1385.out |    4 +++
 tests/xfs/group    |    6 +++++
 13 files changed, 408 insertions(+)
 create mode 100755 tests/xfs/1380
 create mode 100644 tests/xfs/1380.out
 create mode 100755 tests/xfs/1381
 create mode 100644 tests/xfs/1381.out
 create mode 100755 tests/xfs/1382
 create mode 100644 tests/xfs/1382.out
 create mode 100755 tests/xfs/1383
 create mode 100644 tests/xfs/1383.out
 create mode 100755 tests/xfs/1384
 create mode 100644 tests/xfs/1384.out
 create mode 100755 tests/xfs/1385
 create mode 100644 tests/xfs/1385.out


diff --git a/tests/xfs/1380 b/tests/xfs/1380
new file mode 100755
index 0000000..6ce5cdf
--- /dev/null
+++ b/tests/xfs/1380
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1380
+#
+# Populate a XFS filesystem and fuzz every user dquot field.
+# Use xfs_repair to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'usrquota' || _notrun "user quota disabled"
+
+echo "Fuzz user 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'offline'  "dquot -u 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1380.out b/tests/xfs/1380.out
new file mode 100644
index 0000000..206445e
--- /dev/null
+++ b/tests/xfs/1380.out
@@ -0,0 +1,4 @@
+QA output created by 1380
+Format and populate
+Fuzz user 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/1381 b/tests/xfs/1381
new file mode 100755
index 0000000..fc5bca9
--- /dev/null
+++ b/tests/xfs/1381
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1381
+#
+# Populate a XFS filesystem and fuzz every user dquot field.
+# Use xfs_scrub to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'usrquota' || _notrun "user quota disabled"
+
+echo "Fuzz user 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'online'  "dquot -u 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1381.out b/tests/xfs/1381.out
new file mode 100644
index 0000000..f656609
--- /dev/null
+++ b/tests/xfs/1381.out
@@ -0,0 +1,4 @@
+QA output created by 1381
+Format and populate
+Fuzz user 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/1382 b/tests/xfs/1382
new file mode 100755
index 0000000..fd7e50d
--- /dev/null
+++ b/tests/xfs/1382
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1380
+#
+# Populate a XFS filesystem and fuzz every group dquot field.
+# Use xfs_repair to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'grpquota' || _notrun "group quota disabled"
+
+echo "Fuzz group 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'offline'  "dquot -g 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1382.out b/tests/xfs/1382.out
new file mode 100644
index 0000000..5350eb6
--- /dev/null
+++ b/tests/xfs/1382.out
@@ -0,0 +1,4 @@
+QA output created by 1382
+Format and populate
+Fuzz group 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/1383 b/tests/xfs/1383
new file mode 100755
index 0000000..0095b67
--- /dev/null
+++ b/tests/xfs/1383
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1383
+#
+# Populate a XFS filesystem and fuzz every group dquot field.
+# Use xfs_scrub to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'grpquota' || _notrun "group quota disabled"
+
+echo "Fuzz group 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'online'  "dquot -g 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1383.out b/tests/xfs/1383.out
new file mode 100644
index 0000000..3af271b
--- /dev/null
+++ b/tests/xfs/1383.out
@@ -0,0 +1,4 @@
+QA output created by 1383
+Format and populate
+Fuzz group 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/1384 b/tests/xfs/1384
new file mode 100755
index 0000000..53b0450
--- /dev/null
+++ b/tests/xfs/1384
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1384
+#
+# Populate a XFS filesystem and fuzz every project dquot field.
+# Use xfs_repair to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'prjquota' || _notrun "project quota disabled"
+
+echo "Fuzz project 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'offline'  "dquot -p 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1384.out b/tests/xfs/1384.out
new file mode 100644
index 0000000..0bba74e
--- /dev/null
+++ b/tests/xfs/1384.out
@@ -0,0 +1,4 @@
+QA output created by 1384
+Format and populate
+Fuzz project 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/1385 b/tests/xfs/1385
new file mode 100755
index 0000000..f6fc7b7
--- /dev/null
+++ b/tests/xfs/1385
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 1385
+#
+# Populate a XFS filesystem and fuzz every project dquot field.
+# Use xfs_scrub to fix the corruption.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1303  USA
+#-----------------------------------------------------------------------
+#
+
+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 7 15
+
+_cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/populate
+. ./common/fuzzy
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_xfs_fuzz_fields
+_require_quota
+
+echo "Format and populate"
+_scratch_populate_cached nofill > $seqres.full 2>&1
+echo "${MOUNT_OPTIONS}" | grep -q 'prjquota' || _notrun "project quota disabled"
+
+echo "Fuzz project 0 dquot"
+_scratch_xfs_fuzz_metadata '' 'online'  "dquot -p 0" >> $seqres.full
+echo "Done fuzzing dquot"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1385.out b/tests/xfs/1385.out
new file mode 100644
index 0000000..e980490
--- /dev/null
+++ b/tests/xfs/1385.out
@@ -0,0 +1,4 @@
+QA output created by 1385
+Format and populate
+Fuzz project 0 dquot
+Done fuzzing dquot
diff --git a/tests/xfs/group b/tests/xfs/group
index ffdb061..079dc48 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -421,3 +421,9 @@
 421 auto quick clone dedupe
 422 dangerous_scrub dangerous_online_repair
 423 dangerous_scrub
+1380 dangerous_fuzzers dangerous_scrub dangerous_repair
+1381 dangerous_fuzzers dangerous_scrub dangerous_online_repair
+1382 dangerous_fuzzers dangerous_scrub dangerous_repair
+1383 dangerous_fuzzers dangerous_scrub dangerous_online_repair
+1384 dangerous_fuzzers dangerous_scrub dangerous_repair
+1385 dangerous_fuzzers dangerous_scrub dangerous_online_repair

Re: [PATCH 5/5] xfs: test fuzzing every field of a dquot

[Eryu Guan]

On Fri, Jul 21, 2017 at 03:04:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> See what happens when we fuzz every field of a quota information structure.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

These tests all failed in the same way like:

    +scrub didn't fail with diskdq.magic = zeroes.
    +scrub didn't fail with diskdq.magic = ones.
    +scrub didn't fail with diskdq.magic = firstbit.
    +scrub didn't fail with diskdq.magic = middlebit.
    ...

with 4.13-rc5 kernel and xfsprogs for-next branch. I assume that's
something need to be fixed in the kernel?

...

> diff --git a/tests/xfs/1382 b/tests/xfs/1382
> new file mode 100755
> index 0000000..fd7e50d
> --- /dev/null
> +++ b/tests/xfs/1382
> @@ -0,0 +1,63 @@
> +#! /bin/bash
> +# FS QA Test No. 1380
                    ^^^^ I changed it to 1382 then renumbered.

Thanks,
Eryu

Re: [PATCH 5/5] xfs: test fuzzing every field of a dquot

[Darrick J. Wong]

On Thu, Aug 24, 2017 at 05:03:35PM +0800, Eryu Guan wrote:
> On Fri, Jul 21, 2017 at 03:04:58PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > See what happens when we fuzz every field of a quota information structure.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> These tests all failed in the same way like:
> 
>     +scrub didn't fail with diskdq.magic = zeroes.
>     +scrub didn't fail with diskdq.magic = ones.
>     +scrub didn't fail with diskdq.magic = firstbit.
>     +scrub didn't fail with diskdq.magic = middlebit.
>     ...
> 
> with 4.13-rc5 kernel and xfsprogs for-next branch. I assume that's
> something need to be fixed in the kernel?

Yes, that's testing whether or not online scrub actually notices when
we corrupt something.  I'll look into it, but output like that is a sign
that the test is working as designed. :)

--D

> 
> ...
> 
> > diff --git a/tests/xfs/1382 b/tests/xfs/1382
> > new file mode 100755
> > index 0000000..fd7e50d
> > --- /dev/null
> > +++ b/tests/xfs/1382
> > @@ -0,0 +1,63 @@
> > +#! /bin/bash
> > +# FS QA Test No. 1380
>                     ^^^^ I changed it to 1382 then renumbered.
> 
> Thanks,
> Eryu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] miscellaneous tests

[Eryu Guan]

Hi Darrick,

On Fri, Jul 21, 2017 at 03:04:19PM -0700, Darrick J. Wong wrote:
> I decided to roll everything up into a patch series, though these
> patches mostly address different things.
> 
> So... I've reposted the ext4 fsmap test, refactored the xfs error
> injection helpers to work with the new interface in 4.13, and added
> support for fuzzing xfs quota record fields.

I queued the first two patches (dryrun of xfs_scrub and ext4 fsmap
tests) in this week's update. Other patches may need more review cycles.

Thanks,
Eryu