[PATCH 0/5] miscellaneous fstests fixes

[PATCH 0/5] miscellaneous fstests fixes

[Darrick J. Wong]

Miscellaneous fstests fixes, starting with a patch to reduce periodic
failures seen in generic/23[23] on XFS; continuing with a bunch of
cleanups to xfs_scrub invocations in preparation for actually landing
xfs_scrub in 4.15; and ending with a few new tests to check for resource
leaks when mounting fails after a log recovery.

--D

[PATCH 1/5] quota: clear speculative delalloc when checking quota usage

[Darrick J. Wong]

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

Occasionally speculative preallocation kicks in when writing files to a
filesystem under test.  These preallocations consume quota and /usually/
aren't around after we drop_caches, but there's nothing to guarantee
that they actually have, so the quota reports will be different before
and after the fs remount, causing sporadic test failures in
generic/{23[123],270}.

We now have xfs_spaceman which can instruct XFS to forcibly remove the
speculative preallocations.  This fixes the sporadic failures, at
least for XFS.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/config |    1 +
 common/quota  |    6 ++++++
 2 files changed, 7 insertions(+)


diff --git a/common/config b/common/config
index 8844173..96503c6 100644
--- a/common/config
+++ b/common/config
@@ -149,6 +149,7 @@ export XFS_LOGPRINT_PROG="`set_prog_path xfs_logprint`"
 export XFS_REPAIR_PROG="`set_prog_path xfs_repair`"
 export XFS_DB_PROG="`set_prog_path xfs_db`"
 export XFS_GROWFS_PROG=`set_prog_path xfs_growfs`
+export XFS_SPACEMAN_PROG="`set_prog_path xfs_spaceman`"
 export XFS_SCRUB_PROG="`set_prog_path xfs_scrub`"
 export XFS_PARALLEL_REPAIR_PROG="`set_prog_path xfs_prepair`"
 export XFS_PARALLEL_REPAIR64_PROG="`set_prog_path xfs_prepair64`"
diff --git a/common/quota b/common/quota
index d027a8c..2611c48 100644
--- a/common/quota
+++ b/common/quota
@@ -267,6 +267,12 @@ _check_quota_usage()
 		VFS_QUOTA=1
 		quotaon -f -u -g $SCRATCH_MNT 2>/dev/null
 		;;
+	xfs)
+		# Clear out speculative preallocations to eliminate them
+		# as a source of intermittent orig/checked differences.
+		test -x "$XFS_SPACEMAN_PROG" && \
+			"$XFS_SPACEMAN_PROG" -c 'prealloc -s' $SCRATCH_MNT
+		;;
 	*)
 		;;
 	esac

[PATCH 2/5] common/xfs: refactor xfs_scrub presence testing

[Darrick J. Wong]

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

Move all the requirements checking for xfs_scrub into a helper function.
Make sure the helper properly detects the presence of the scrub ioctl
and situations where we can't run scrub (e.g. norecovery).

Refactor the existing three xfs_scrub call sites to use the helper to
check if it's appropriate to run scrub.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc         |    2 +-
 common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 4 files changed, 36 insertions(+), 28 deletions(-)


diff --git a/common/rc b/common/rc
index 1a4d81e..83aaced 100644
--- a/common/rc
+++ b/common/rc
@@ -2091,7 +2091,7 @@ _require_xfs_io_command()
 			_notrun "xfs_io $command support is missing"
 		;;
 	"scrub"|"repair")
-		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
+		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
 		echo $testio | grep -q "Inappropriate ioctl" && \
 			_notrun "xfs_io $command support is missing"
 		;;
diff --git a/common/xfs b/common/xfs
index dff8454..7d8f275 100644
--- a/common/xfs
+++ b/common/xfs
@@ -298,6 +298,29 @@ _require_xfs_db_command()
 		_notrun "xfs_db $command support is missing"
 }
 
+# Does the filesystem mounted from a particular device support scrub?
+_supports_xfs_scrub()
+{
+	mountpoint="$1"
+	device="$2"
+
+	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
+		echo "Usage: _supports_xfs_scrub mountpoint device"
+		exit 1
+	fi
+
+	test "$FSTYP" = "xfs" || return 1
+	test -x "$XFS_SCRUB_PROG" || return 1
+
+	# Probe for kernel support...
+	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
+
+	# Scrub can't run on norecovery mounts
+	_fs_options "$device" | grep -q "norecovery" && return 1
+
+	return 0
+}
+
 # run xfs_check and friends on a FS.
 _check_xfs_filesystem()
 {
@@ -330,14 +353,17 @@ _check_xfs_filesystem()
 	type=`_fs_type $device`
 	ok=1
 
-	if [ "$type" = "xfs" ]; then
-		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
-			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
-			if [ $? -ne 0 ]; then
-				_log_err "filesystem on $device failed scrub"
-				ok=0
-			fi
+	# Run online scrub if we can.
+	mntpt="$(_is_mounted $device)"
+	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
+		if [ $? -ne 0 ]; then
+			_log_err "filesystem on $device failed scrub"
+			ok=0
 		fi
+	fi
+
+	if [ "$type" = "xfs" ]; then
 		# mounted ...
 		mountpoint=`_umount_or_remount_ro $device`
 	fi
diff --git a/tests/generic/453 b/tests/generic/453
index ff29736..40fae91 100755
--- a/tests/generic/453
+++ b/tests/generic/453
@@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \
@@ -147,12 +144,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }
 
diff --git a/tests/generic/454 b/tests/generic/454
index 01279ee..462185a 100755
--- a/tests/generic/454
+++ b/tests/generic/454
@@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \
@@ -143,12 +140,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }
 

Re: [PATCH 2/5] common/xfs: refactor xfs_scrub presence testing

[Eryu Guan]

On Wed, Oct 18, 2017 at 04:37:42PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move all the requirements checking for xfs_scrub into a helper function.
> Make sure the helper properly detects the presence of the scrub ioctl
> and situations where we can't run scrub (e.g. norecovery).
> 
> Refactor the existing three xfs_scrub call sites to use the helper to
> check if it's appropriate to run scrub.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc         |    2 +-
>  common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
>  tests/generic/453 |   11 +----------
>  tests/generic/454 |   11 +----------
>  4 files changed, 36 insertions(+), 28 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index 1a4d81e..83aaced 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2091,7 +2091,7 @@ _require_xfs_io_command()
>  			_notrun "xfs_io $command support is missing"
>  		;;
>  	"scrub"|"repair")
> -		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
> +		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
>  		echo $testio | grep -q "Inappropriate ioctl" && \
>  			_notrun "xfs_io $command support is missing"
>  		;;
> diff --git a/common/xfs b/common/xfs
> index dff8454..7d8f275 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -298,6 +298,29 @@ _require_xfs_db_command()
>  		_notrun "xfs_db $command support is missing"
>  }
>  
> +# Does the filesystem mounted from a particular device support scrub?
> +_supports_xfs_scrub()
> +{
> +	mountpoint="$1"
> +	device="$2"
> +
> +	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
> +		echo "Usage: _supports_xfs_scrub mountpoint device"
> +		exit 1
> +	fi
> +
> +	test "$FSTYP" = "xfs" || return 1
> +	test -x "$XFS_SCRUB_PROG" || return 1
> +
> +	# Probe for kernel support...
> +	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
> +
> +	# Scrub can't run on norecovery mounts
> +	_fs_options "$device" | grep -q "norecovery" && return 1
> +
> +	return 0
> +}

Hmm, this enables scrub after each test by default, because
$TEST_XFS_SCRUB is not checked anymore. Either remove TEST_XFS_SCRUB
completely (it's documented in README) or check it in this
_supports_xfs_scrub() helper. I'm fine with either way :)

> +
>  # run xfs_check and friends on a FS.
>  _check_xfs_filesystem()
>  {
> @@ -330,14 +353,17 @@ _check_xfs_filesystem()
>  	type=`_fs_type $device`
>  	ok=1
>  
> -	if [ "$type" = "xfs" ]; then
> -		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
> -			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
> -			if [ $? -ne 0 ]; then
> -				_log_err "filesystem on $device failed scrub"
> -				ok=0
> -			fi
> +	# Run online scrub if we can.
> +	mntpt="$(_is_mounted $device)"
> +	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
> +		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full

Dump stderr to $seqres.full too? I noticed many warning messages printed
by xfs_scrub, though the tests didn't fail.

/mnt/testarea/test: Found more data blocks than reported (scrub.c line 382)
/mnt/testarea/test: 1 warnings found.
generic/450 0s ... 0s
/mnt/testarea/test: Found more data blocks than reported (scrub.c line 382)
/mnt/testarea/test: 1 warnings found.
generic/453 0s ... 0s
/mnt/testarea/scratch: Found more data blocks than reported (scrub.c line 382)
/mnt/testarea/scratch: 1 warnings found.
generic/454 1s ... 0s
/mnt/testarea/scratch: Found more data blocks than reported (scrub.c line 382)
/mnt/testarea/scratch: 1 warnings found.
Ran: generic/450 generic/453 generic/454
Passed all 3 tests

Thanks,
Eryu

> +		if [ $? -ne 0 ]; then
> +			_log_err "filesystem on $device failed scrub"
> +			ok=0
>  		fi
> +	fi
> +
> +	if [ "$type" = "xfs" ]; then
>  		# mounted ...
>  		mountpoint=`_umount_or_remount_ro $device`
>  	fi
> diff --git a/tests/generic/453 b/tests/generic/453
> index ff29736..40fae91 100755
> --- a/tests/generic/453
> +++ b/tests/generic/453
> @@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
>  
>  # Only run this on xfs if xfs_scrub is available and has the unicode checker
>  check_xfs_scrub() {
> -	# Ignore non-XFS fs or no scrub program...
> -	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
> -		return 1
> -	fi
> +	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
>  
>  	# We only care if xfs_scrub has unicode string support...
>  	if ! type ldd > /dev/null 2>&1 || \
> @@ -147,12 +144,6 @@ check_xfs_scrub() {
>  		return 1
>  	fi
>  
> -	# Does the ioctl work?
> -	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
> -	   grep -q "Inappropriate ioctl"; then
> -		return 1
> -	fi
> -
>  	return 0
>  }
>  
> diff --git a/tests/generic/454 b/tests/generic/454
> index 01279ee..462185a 100755
> --- a/tests/generic/454
> +++ b/tests/generic/454
> @@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
>  
>  # Only run this on xfs if xfs_scrub is available and has the unicode checker
>  check_xfs_scrub() {
> -	# Ignore non-XFS fs or no scrub program...
> -	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
> -		return 1
> -	fi
> +	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
>  
>  	# We only care if xfs_scrub has unicode string support...
>  	if ! type ldd > /dev/null 2>&1 || \
> @@ -143,12 +140,6 @@ check_xfs_scrub() {
>  		return 1
>  	fi
>  
> -	# Does the ioctl work?
> -	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
> -	   grep -q "Inappropriate ioctl"; then
> -		return 1
> -	fi
> -
>  	return 0
>  }
>  
> 

Re: [PATCH 2/5] common/xfs: refactor xfs_scrub presence testing

[Darrick J. Wong]

On Wed, Oct 25, 2017 at 07:04:47PM +0800, Eryu Guan wrote:
> On Wed, Oct 18, 2017 at 04:37:42PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move all the requirements checking for xfs_scrub into a helper function.
> > Make sure the helper properly detects the presence of the scrub ioctl
> > and situations where we can't run scrub (e.g. norecovery).
> > 
> > Refactor the existing three xfs_scrub call sites to use the helper to
> > check if it's appropriate to run scrub.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/rc         |    2 +-
> >  common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
> >  tests/generic/453 |   11 +----------
> >  tests/generic/454 |   11 +----------
> >  4 files changed, 36 insertions(+), 28 deletions(-)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index 1a4d81e..83aaced 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -2091,7 +2091,7 @@ _require_xfs_io_command()
> >  			_notrun "xfs_io $command support is missing"
> >  		;;
> >  	"scrub"|"repair")
> > -		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
> > +		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
> >  		echo $testio | grep -q "Inappropriate ioctl" && \
> >  			_notrun "xfs_io $command support is missing"
> >  		;;
> > diff --git a/common/xfs b/common/xfs
> > index dff8454..7d8f275 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -298,6 +298,29 @@ _require_xfs_db_command()
> >  		_notrun "xfs_db $command support is missing"
> >  }
> >  
> > +# Does the filesystem mounted from a particular device support scrub?
> > +_supports_xfs_scrub()
> > +{
> > +	mountpoint="$1"
> > +	device="$2"
> > +
> > +	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
> > +		echo "Usage: _supports_xfs_scrub mountpoint device"
> > +		exit 1
> > +	fi
> > +
> > +	test "$FSTYP" = "xfs" || return 1
> > +	test -x "$XFS_SCRUB_PROG" || return 1
> > +
> > +	# Probe for kernel support...
> > +	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
> > +
> > +	# Scrub can't run on norecovery mounts
> > +	_fs_options "$device" | grep -q "norecovery" && return 1
> > +
> > +	return 0
> > +}
> 
> Hmm, this enables scrub after each test by default, because
> $TEST_XFS_SCRUB is not checked anymore. Either remove TEST_XFS_SCRUB
> completely (it's documented in README) or check it in this
> _supports_xfs_scrub() helper. I'm fine with either way :)

Ok.  We're moving towards running scrub any time the fs is still mounted,
and eventually will drop xfs_check^Wxfs_db -c check.

> > +
> >  # run xfs_check and friends on a FS.
> >  _check_xfs_filesystem()
> >  {
> > @@ -330,14 +353,17 @@ _check_xfs_filesystem()
> >  	type=`_fs_type $device`
> >  	ok=1
> >  
> > -	if [ "$type" = "xfs" ]; then
> > -		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
> > -			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
> > -			if [ $? -ne 0 ]; then
> > -				_log_err "filesystem on $device failed scrub"
> > -				ok=0
> > -			fi
> > +	# Run online scrub if we can.
> > +	mntpt="$(_is_mounted $device)"
> > +	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
> > +		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
> 
> Dump stderr to $seqres.full too? I noticed many warning messages printed
> by xfs_scrub, though the tests didn't fail.
> 
> /mnt/testarea/test: Found more data blocks than reported (scrub.c line 382)
> /mnt/testarea/test: 1 warnings found.
> generic/450 0s ... 0s
> /mnt/testarea/test: Found more data blocks than reported (scrub.c line 382)
> /mnt/testarea/test: 1 warnings found.
> generic/453 0s ... 0s
> /mnt/testarea/scratch: Found more data blocks than reported (scrub.c line 382)
> /mnt/testarea/scratch: 1 warnings found.
> generic/454 1s ... 0s
> /mnt/testarea/scratch: Found more data blocks than reported (scrub.c line 382)
> /mnt/testarea/scratch: 1 warnings found.
> Ran: generic/450 generic/453 generic/454
> Passed all 3 tests

Will fix, even though as you point out the next patch fixes it anyway.

--D

> 
> Thanks,
> Eryu
> 
> > +		if [ $? -ne 0 ]; then
> > +			_log_err "filesystem on $device failed scrub"
> > +			ok=0
> >  		fi
> > +	fi
> > +
> > +	if [ "$type" = "xfs" ]; then
> >  		# mounted ...
> >  		mountpoint=`_umount_or_remount_ro $device`
> >  	fi
> > diff --git a/tests/generic/453 b/tests/generic/453
> > index ff29736..40fae91 100755
> > --- a/tests/generic/453
> > +++ b/tests/generic/453
> > @@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
> >  
> >  # Only run this on xfs if xfs_scrub is available and has the unicode checker
> >  check_xfs_scrub() {
> > -	# Ignore non-XFS fs or no scrub program...
> > -	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
> > -		return 1
> > -	fi
> > +	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
> >  
> >  	# We only care if xfs_scrub has unicode string support...
> >  	if ! type ldd > /dev/null 2>&1 || \
> > @@ -147,12 +144,6 @@ check_xfs_scrub() {
> >  		return 1
> >  	fi
> >  
> > -	# Does the ioctl work?
> > -	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
> > -	   grep -q "Inappropriate ioctl"; then
> > -		return 1
> > -	fi
> > -
> >  	return 0
> >  }
> >  
> > diff --git a/tests/generic/454 b/tests/generic/454
> > index 01279ee..462185a 100755
> > --- a/tests/generic/454
> > +++ b/tests/generic/454
> > @@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
> >  
> >  # Only run this on xfs if xfs_scrub is available and has the unicode checker
> >  check_xfs_scrub() {
> > -	# Ignore non-XFS fs or no scrub program...
> > -	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
> > -		return 1
> > -	fi
> > +	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
> >  
> >  	# We only care if xfs_scrub has unicode string support...
> >  	if ! type ldd > /dev/null 2>&1 || \
> > @@ -143,12 +140,6 @@ check_xfs_scrub() {
> >  		return 1
> >  	fi
> >  
> > -	# Does the ioctl work?
> > -	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
> > -	   grep -q "Inappropriate ioctl"; then
> > -		return 1
> > -	fi
> > -
> >  	return 0
> >  }
> >  
> > 

[PATCH 3/5] common/xfs: standardize the xfs_scrub output that gets recorded to $seqres.full

[Darrick J. Wong]

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

Make the xfs_scrub output that gets recorded to $seqres.full follow the
format of xfs_repair checks.

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


diff --git a/common/xfs b/common/xfs
index 7d8f275..25c2ce9 100644
--- a/common/xfs
+++ b/common/xfs
@@ -356,11 +356,15 @@ _check_xfs_filesystem()
 	# Run online scrub if we can.
 	mntpt="$(_is_mounted $device)"
 	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
-		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device > $tmp.scrub 2>&1
 		if [ $? -ne 0 ]; then
-			_log_err "filesystem on $device failed scrub"
+			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
+			echo "*** xfs_scrub $scrubflag -v -d -n output ***" >> $seqres.full
+			cat $tmp.scrub >> $seqres.full
+			echo "*** end xfs_scrub output" >> $serqres.full
 			ok=0
 		fi
+		rm -f $tmp.scrub
 	fi
 
 	if [ "$type" = "xfs" ]; then

Re: [PATCH 3/5] common/xfs: standardize the xfs_scrub output that gets recorded to $seqres.full

[Eryu Guan]

On Wed, Oct 18, 2017 at 04:37:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make the xfs_scrub output that gets recorded to $seqres.full follow the
> format of xfs_repair checks.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/xfs |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index 7d8f275..25c2ce9 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -356,11 +356,15 @@ _check_xfs_filesystem()
>  	# Run online scrub if we can.
>  	mntpt="$(_is_mounted $device)"
>  	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
> -		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
> +		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device > $tmp.scrub 2>&1

Ahh, you dumped stderr to $tmp.scrub here, that's fine too :)

Eryu

>  		if [ $? -ne 0 ]; then
> -			_log_err "filesystem on $device failed scrub"
> +			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
> +			echo "*** xfs_scrub $scrubflag -v -d -n output ***" >> $seqres.full
> +			cat $tmp.scrub >> $seqres.full
> +			echo "*** end xfs_scrub output" >> $serqres.full
>  			ok=0
>  		fi
> +		rm -f $tmp.scrub
>  	fi
>  
>  	if [ "$type" = "xfs" ]; then
> 

[PATCH 4/5] generic/45[34]: force UTF-8 codeset to enable utf-8 namer checks in xfs_scrub

[Darrick J. Wong]

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

The upcoming xfs_scrub tool will have the ability to warn about
suspicious UTF-8 normalization collisions.  We want generic/45[34] to be
able to test this functionality, but to do that we have to forcibly set
the codeset to UTF-8 via LC_ALL since the rest of xfstests only uses
LC_ALL=C.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/453 |    2 +-
 tests/generic/454 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/tests/generic/453 b/tests/generic/453
index 40fae91..16589d1 100755
--- a/tests/generic/453
+++ b/tests/generic/453
@@ -148,7 +148,7 @@ check_xfs_scrub() {
 }
 
 if check_xfs_scrub; then
-	output="$(${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
+	output="$(LC_ALL="C.UTF-8" ${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
 	echo "${output}" | grep -q "french_" || echo "No complaints about french e accent?"
 	echo "${output}" | grep -q "chinese_" || echo "No complaints about chinese width-different?"
 	echo "${output}" | grep -q "greek_" || echo "No complaints about greek letter mess?"
diff --git a/tests/generic/454 b/tests/generic/454
index 462185a..efac860 100755
--- a/tests/generic/454
+++ b/tests/generic/454
@@ -144,7 +144,7 @@ check_xfs_scrub() {
 }
 
 if check_xfs_scrub; then
-	output="$(${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
+	output="$(LC_ALL="C.UTF-8" ${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
 	echo "${output}" | grep -q "french_" || echo "No complaints about french e accent?"
 	echo "${output}" | grep -q "chinese_" || echo "No complaints about chinese width-different?"
 	echo "${output}" | grep -q "greek_" || echo "No complaints about greek letter mess?"

Re: [PATCH 4/5] generic/45[34]: force UTF-8 codeset to enable utf-8 namer checks in xfs_scrub

[Christoph Hellwig]

On Wed, Oct 18, 2017 at 04:37:55PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The upcoming xfs_scrub tool will have the ability to warn about
> suspicious UTF-8 normalization collisions.  We want generic/45[34] to be
> able to test this functionality, but to do that we have to forcibly set
> the codeset to UTF-8 via LC_ALL since the rest of xfstests only uses
> LC_ALL=C.

Wait.  Where do you want to validate UTF-8 normalization?  There is
absolutely no guarantee that someone uses UTF-8, so any reliance on
the character set in the file system is bogus.

Re: [PATCH 4/5] generic/45[34]: force UTF-8 codeset to enable utf-8 namer checks in xfs_scrub

[Darrick J. Wong]

On Thu, Oct 19, 2017 at 12:18:42AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 18, 2017 at 04:37:55PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > The upcoming xfs_scrub tool will have the ability to warn about
> > suspicious UTF-8 normalization collisions.  We want generic/45[34] to be
> > able to test this functionality, but to do that we have to forcibly set
> > the codeset to UTF-8 via LC_ALL since the rest of xfstests only uses
> > LC_ALL=C.
> 
> Wait.  Where do you want to validate UTF-8 normalization?  There is
> absolutely no guarantee that someone uses UTF-8, so any reliance on
> the character set in the file system is bogus.

I'll start by summarizing a problem statement[1].  In XFS (and nearly
all the other filesystems), neither the on-disk format nor the kernel
driver care about the contents of file names or attribute names; they
treat these as an arbitrary byte sequence.  Userspace can set whatever
localization and encoding parameters it wants, and the kernel doesn't
care except for '\0' and '/'.  That doesn't change.

In modern Linux userspace, however, we /do/ care about being able to
encode Unicode codepoints into byte streams, so we encode them in UTF8.
Because there's two different normalization methods in Unicode, this
leads to the funny situation where two unique filename byte sequences
can render the same but point to totally different files:

$ echo NFC > "$(echo -e "french_caf\xc3\xa9.txt")"
$ echo NFD > "$(echo -e "french_caf\xcc\x81.txt")"
$ ls -lai
133 -rw-r--r-- 1 root root   4 Oct 20 10:40 french_café.txt
132 -rw-r--r-- 1 root root   4 Oct 20 10:40 french_café.txt
$ echo $LANG
en_US.UTF-8

At least on my computer, the two filenames render identically yet point
to different inodes.  This could be used to mislead people into opening
a malicious file whose name appears identical to a legitimate file.

xfs_scrub is the (proposed) userspace component of XFS online fsck.  The
first four phases simply call the in-kernel fsck code and pass status
back, but the fifth phase walks the directory tree looking for problems.

If xfs_scrub (the userspace component of online fsck) was built with
libunistring and the LC_MESSAGES string contains "UTF-8", phase 5 will
warn if it finds multiple filenames in a directory that normalize to the
same string but point to different inodes.  Similarly, it will warn
about colliding attribute names.  Warnings in xfs_scrub are for
situations that warrant administrative review but are not filesystem
corruptions.

IOWs, if userspace is configured for UTF-8, the userspace part of online
fsck will flag suspicious-looking uses of Unicode for admin review.  The
kernel remains uninvolved.

--D

[1] https://eclecticlight.co/2017/04/06/apfs-is-currently-unusable-with-most-non-english-languages/

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

[PATCH 5/5] xfs: test that we don't leak inodes and dquots during failed cow recovery

[Darrick J. Wong]

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

Add a couple of tests to check that we don't leak inodes or dquots
if CoW recovery fails and therefore the mount fails.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc         |    1 
 tests/xfs/703     |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/703.out |    9 ++++
 tests/xfs/704     |   90 +++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/704.out |    5 ++
 tests/xfs/705     |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/705.out |    9 ++++
 tests/xfs/group   |    3 +
 8 files changed, 335 insertions(+)
 create mode 100755 tests/xfs/703
 create mode 100644 tests/xfs/703.out
 create mode 100755 tests/xfs/704
 create mode 100644 tests/xfs/704.out
 create mode 100755 tests/xfs/705
 create mode 100644 tests/xfs/705.out


diff --git a/common/rc b/common/rc
index 83aaced..fe68d67 100644
--- a/common/rc
+++ b/common/rc
@@ -3324,6 +3324,7 @@ _check_dmesg()
 	     -e "(INFO|ERR): suspicious RCU usage" \
 	     -e "INFO: possible circular locking dependency detected" \
 	     -e "general protection fault:" \
+	     -e "BUG .* remaining" \
 	     $seqres.dmesg
 	if [ $? -eq 0 ]; then
 		_dump_err "_check_dmesg: something found in dmesg (see $seqres.dmesg)"
diff --git a/tests/xfs/703 b/tests/xfs/703
new file mode 100755
index 0000000..f29b843
--- /dev/null
+++ b/tests/xfs/703
@@ -0,0 +1,112 @@
+#! /bin/bash
+# FS QA Test No. 703
+#
+# Ensure that we don't leak quota inodes when CoW recovery fails.
+#
+# Use xfs_fsr to inject bmap redo items in the log for a linked file and
+# an unlinked file; enable quota so that we always mount with the quota
+# inodes; and then corrupt the refcount btree to ensure that the CoW
+# garbage collection (and therefore the mount) fail.
+#
+# On a subsequent mount attempt, we should be able to replay the bmap
+# items for the linked and unlinked files without prematurely truncating
+# the unlinked inode and without leaking the linked inode, and we should
+# be able to release the quota inodes when we're aborting the mount.  We
+# also should not leak dquots.
+#
+#-----------------------------------------------------------------------
+# 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 -rf "$tmp".*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/reflink
+. ./common/inject
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_quota
+_require_scratch_reflink
+_require_cp_reflink
+_require_command "$XFS_FSR_PROG" "xfs_fsr"
+_require_xfs_io_error_injection "bmap_finish_one"
+_require_xfs_scratch_rmapbt
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount -o noquota >> "$seqres.full" 2>&1
+
+testdir="$SCRATCH_MNT/test-$seq"
+blksz=65536
+blks=3
+mkdir "$testdir"
+
+echo "Create a many-block file"
+_pwrite_byte 0x62 0 $((blksz * blks)) $testdir/file1 >> $seqres.full
+_pwrite_byte 0x63 0 $blksz $testdir/file2 >> $seqres.full
+_reflink_range $testdir/file2 0 $testdir/file1 $blksz $blksz >> $seqres.full
+_scratch_cycle_mount noquota
+
+echo "Inject error"
+_scratch_inject_error "bmap_finish_one"
+
+echo "Defrag the file"
+$XFS_FSR_PROG -v -d $testdir/file1 >> $seqres.full 2>&1
+
+echo "FS should be shut down, touch will fail"
+touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
+
+echo "Remount to replay log" | tee /dev/ttyprintk
+_scratch_unmount
+_scratch_dump_log >> $seqres.full
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c 'fuzz -d recs[1].startblock ones' >> $seqres.full
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c p >> $seqres.full
+
+# Suddenly enable quota to test if we can leak the quotacheck dquots!
+_scratch_mount -o quota >> $seqres.full 2>&1
+_scratch_unmount 2> /dev/null
+rm -f ${RESULT_DIR}/require_scratch
+
+echo "See if we leak"
+_test_unmount
+rmmod xfs
+_test_mount
+_check_dmesg
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/703.out b/tests/xfs/703.out
new file mode 100644
index 0000000..d811880
--- /dev/null
+++ b/tests/xfs/703.out
@@ -0,0 +1,9 @@
+QA output created by 703
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak
diff --git a/tests/xfs/704 b/tests/xfs/704
new file mode 100755
index 0000000..6d272e1
--- /dev/null
+++ b/tests/xfs/704
@@ -0,0 +1,90 @@
+#! /bin/bash
+# FS QA Test No. 704
+#
+# Ensure that we don't leak dquots when CoW recovery fails.
+#
+# Corrupt the refcount btree to ensure that the CoW garbage collection
+# (and therefore the mount) fail.
+#
+# On a subsequent mount attempt, we should be able to release the quota
+# inodes when we're aborting the mount.  We also should not leak dquots.
+#
+#-----------------------------------------------------------------------
+# 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 -rf "$tmp".*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/reflink
+. ./common/quota
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_quota
+_require_scratch_reflink
+_require_cp_reflink
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount -o quota >> "$seqres.full" 2>&1
+
+testdir="$SCRATCH_MNT/test-$seq"
+blksz=65536
+blks=3
+mkdir "$testdir"
+
+echo "Create a many-block file"
+_pwrite_byte 0x62 0 $((blksz * blks)) $testdir/file1 >> $seqres.full
+_pwrite_byte 0x63 0 $blksz $testdir/file2 >> $seqres.full
+_reflink_range $testdir/file2 0 $testdir/file1 $blksz $blksz >> $seqres.full
+
+echo "Remount to check recovery" | tee /dev/ttyprintk
+_scratch_unmount
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c 'fuzz -d recs[1].startblock ones' >> $seqres.full
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c p >> $seqres.full
+_scratch_mount -o quota >> $seqres.full 2>&1
+_scratch_unmount 2> /dev/null
+rm -f ${RESULT_DIR}/require_scratch
+
+echo "See if we leak"
+_test_unmount
+rmmod xfs
+_test_mount
+_check_dmesg
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/704.out b/tests/xfs/704.out
new file mode 100644
index 0000000..b237b2d
--- /dev/null
+++ b/tests/xfs/704.out
@@ -0,0 +1,5 @@
+QA output created by 704
+Format and mount
+Create a many-block file
+Remount to check recovery
+See if we leak
diff --git a/tests/xfs/705 b/tests/xfs/705
new file mode 100755
index 0000000..752a54b
--- /dev/null
+++ b/tests/xfs/705
@@ -0,0 +1,106 @@
+#! /bin/bash
+# FS QA Test No. 705
+#
+# Ensure that we don't leak inodes when CoW recovery fails.
+#
+# Use xfs_fsr to inject bmap redo items in the log for a linked file and
+# an unlinked file; and then corrupt the refcount btree to ensure that
+# the CoW garbage collection (and therefore the mount) fail.
+#
+# On a subsequent mount attempt, we should be able to replay the bmap
+# items for the linked and unlinked files without prematurely truncating
+# the unlinked inode and without leaking the linked inode, and we should
+# be able to release all the inodes when we're aborting the mount.
+#
+#-----------------------------------------------------------------------
+# 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 -rf "$tmp".*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/reflink
+. ./common/inject
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_reflink
+_require_cp_reflink
+_require_command "$XFS_FSR_PROG" "xfs_fsr"
+_require_xfs_io_error_injection "bmap_finish_one"
+_require_xfs_scratch_rmapbt
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount -o noquota >> "$seqres.full" 2>&1
+
+testdir="$SCRATCH_MNT/test-$seq"
+blksz=65536
+blks=3
+mkdir "$testdir"
+
+echo "Create a many-block file"
+_pwrite_byte 0x62 0 $((blksz * blks)) $testdir/file1 >> $seqres.full
+_pwrite_byte 0x63 0 $blksz $testdir/file2 >> $seqres.full
+_reflink_range $testdir/file2 0 $testdir/file1 $blksz $blksz >> $seqres.full
+_scratch_cycle_mount noquota
+
+echo "Inject error"
+_scratch_inject_error "bmap_finish_one"
+
+echo "Defrag the file"
+$XFS_FSR_PROG -v -d $testdir/file1 >> $seqres.full 2>&1
+
+echo "FS should be shut down, touch will fail"
+touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
+
+echo "Remount to replay log" | tee /dev/ttyprintk
+_scratch_unmount
+_scratch_dump_log >> $seqres.full
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c 'fuzz -d recs[1].startblock ones' >> $seqres.full
+_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c p >> $seqres.full
+_scratch_mount -o noquota >> $seqres.full 2>&1
+_scratch_unmount 2> /dev/null
+rm -f ${RESULT_DIR}/require_scratch
+
+echo "See if we leak"
+_test_unmount
+rmmod xfs
+_test_mount
+_check_dmesg
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/705.out b/tests/xfs/705.out
new file mode 100644
index 0000000..0bcd9ab
--- /dev/null
+++ b/tests/xfs/705.out
@@ -0,0 +1,9 @@
+QA output created by 705
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak
diff --git a/tests/xfs/group b/tests/xfs/group
index b439842..e56666b 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -431,3 +431,6 @@
 431 auto quick dangerous
 432 auto quick dir metadata
 433 auto quick attr
+703 auto quick clone fsr
+704 auto quick clone
+705 auto quick clone fsr

Re: [PATCH 5/5] xfs: test that we don't leak inodes and dquots during failed cow recovery

[Eryu Guan]

On Wed, Oct 18, 2017 at 04:38:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a couple of tests to check that we don't leak inodes or dquots
> if CoW recovery fails and therefore the mount fails.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc         |    1 
>  tests/xfs/703     |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/703.out |    9 ++++
>  tests/xfs/704     |   90 +++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/704.out |    5 ++
>  tests/xfs/705     |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/705.out |    9 ++++
>  tests/xfs/group   |    3 +
>  8 files changed, 335 insertions(+)
>  create mode 100755 tests/xfs/703
>  create mode 100644 tests/xfs/703.out
>  create mode 100755 tests/xfs/704
>  create mode 100644 tests/xfs/704.out
>  create mode 100755 tests/xfs/705
>  create mode 100644 tests/xfs/705.out
> 
> 
> diff --git a/common/rc b/common/rc
> index 83aaced..fe68d67 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3324,6 +3324,7 @@ _check_dmesg()
>  	     -e "(INFO|ERR): suspicious RCU usage" \
>  	     -e "INFO: possible circular locking dependency detected" \
>  	     -e "general protection fault:" \
> +	     -e "BUG .* remaining" \
>  	     $seqres.dmesg
>  	if [ $? -eq 0 ]; then
>  		_dump_err "_check_dmesg: something found in dmesg (see $seqres.dmesg)"
> diff --git a/tests/xfs/703 b/tests/xfs/703
> new file mode 100755
> index 0000000..f29b843
> --- /dev/null
> +++ b/tests/xfs/703
> @@ -0,0 +1,112 @@
> +#! /bin/bash
> +# FS QA Test No. 703
> +#
> +# Ensure that we don't leak quota inodes when CoW recovery fails.
> +#
> +# Use xfs_fsr to inject bmap redo items in the log for a linked file and
> +# an unlinked file; enable quota so that we always mount with the quota
> +# inodes; and then corrupt the refcount btree to ensure that the CoW
> +# garbage collection (and therefore the mount) fail.
> +#
> +# On a subsequent mount attempt, we should be able to replay the bmap
> +# items for the linked and unlinked files without prematurely truncating
> +# the unlinked inode and without leaking the linked inode, and we should
> +# be able to release the quota inodes when we're aborting the mount.  We
> +# also should not leak dquots.
> +#
> +#-----------------------------------------------------------------------
> +# 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 -rf "$tmp".*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +. ./common/reflink
> +. ./common/inject
> +. ./common/quota
> +
> +# real QA test starts here
> +_supported_os Linux
> +_supported_fs xfs
> +_require_quota
> +_require_scratch_reflink
> +_require_cp_reflink
> +_require_command "$XFS_FSR_PROG" "xfs_fsr"
> +_require_xfs_io_error_injection "bmap_finish_one"
> +_require_xfs_scratch_rmapbt
> +
> +rm -f "$seqres.full"
> +
> +echo "Format and mount"
> +_scratch_mkfs > "$seqres.full" 2>&1
> +_scratch_mount -o noquota >> "$seqres.full" 2>&1
> +
> +testdir="$SCRATCH_MNT/test-$seq"
> +blksz=65536
> +blks=3
> +mkdir "$testdir"
> +
> +echo "Create a many-block file"
> +_pwrite_byte 0x62 0 $((blksz * blks)) $testdir/file1 >> $seqres.full
> +_pwrite_byte 0x63 0 $blksz $testdir/file2 >> $seqres.full
> +_reflink_range $testdir/file2 0 $testdir/file1 $blksz $blksz >> $seqres.full
> +_scratch_cycle_mount noquota
> +
> +echo "Inject error"
> +_scratch_inject_error "bmap_finish_one"
> +
> +echo "Defrag the file"
> +$XFS_FSR_PROG -v -d $testdir/file1 >> $seqres.full 2>&1
> +
> +echo "FS should be shut down, touch will fail"
> +touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
> +
> +echo "Remount to replay log" | tee /dev/ttyprintk
> +_scratch_unmount
> +_scratch_dump_log >> $seqres.full
> +_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c 'fuzz -d recs[1].startblock ones' >> $seqres.full
> +_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c p >> $seqres.full
> +
> +# Suddenly enable quota to test if we can leak the quotacheck dquots!
> +_scratch_mount -o quota >> $seqres.full 2>&1
> +_scratch_unmount 2> /dev/null
> +rm -f ${RESULT_DIR}/require_scratch
> +
> +echo "See if we leak"
> +_test_unmount
> +rmmod xfs

Test fails if xfs is built in or there's another mounted xfs, e.g.
rootfs. There's a _require_btrfs_loadable btrfs-specific helper to check
if we're able to unload btrfs as a module, and _notrun if not. I think
we can make it generic easily, e.g. _require_module_loadable "$MOD_NAME"

> +_test_mount
> +_check_dmesg

check will do this for us :)

Thanks,
Eryu

Re: [PATCH 5/5] xfs: test that we don't leak inodes and dquots during failed cow recovery

[Darrick J. Wong]

On Wed, Oct 25, 2017 at 07:48:17PM +0800, Eryu Guan wrote:
> On Wed, Oct 18, 2017 at 04:38:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add a couple of tests to check that we don't leak inodes or dquots
> > if CoW recovery fails and therefore the mount fails.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/rc         |    1 
> >  tests/xfs/703     |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/703.out |    9 ++++
> >  tests/xfs/704     |   90 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/704.out |    5 ++
> >  tests/xfs/705     |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/705.out |    9 ++++
> >  tests/xfs/group   |    3 +
> >  8 files changed, 335 insertions(+)
> >  create mode 100755 tests/xfs/703
> >  create mode 100644 tests/xfs/703.out
> >  create mode 100755 tests/xfs/704
> >  create mode 100644 tests/xfs/704.out
> >  create mode 100755 tests/xfs/705
> >  create mode 100644 tests/xfs/705.out
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index 83aaced..fe68d67 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3324,6 +3324,7 @@ _check_dmesg()
> >  	     -e "(INFO|ERR): suspicious RCU usage" \
> >  	     -e "INFO: possible circular locking dependency detected" \
> >  	     -e "general protection fault:" \
> > +	     -e "BUG .* remaining" \
> >  	     $seqres.dmesg
> >  	if [ $? -eq 0 ]; then
> >  		_dump_err "_check_dmesg: something found in dmesg (see $seqres.dmesg)"
> > diff --git a/tests/xfs/703 b/tests/xfs/703
> > new file mode 100755
> > index 0000000..f29b843
> > --- /dev/null
> > +++ b/tests/xfs/703
> > @@ -0,0 +1,112 @@
> > +#! /bin/bash
> > +# FS QA Test No. 703
> > +#
> > +# Ensure that we don't leak quota inodes when CoW recovery fails.
> > +#
> > +# Use xfs_fsr to inject bmap redo items in the log for a linked file and
> > +# an unlinked file; enable quota so that we always mount with the quota
> > +# inodes; and then corrupt the refcount btree to ensure that the CoW
> > +# garbage collection (and therefore the mount) fail.
> > +#
> > +# On a subsequent mount attempt, we should be able to replay the bmap
> > +# items for the linked and unlinked files without prematurely truncating
> > +# the unlinked inode and without leaking the linked inode, and we should
> > +# be able to release the quota inodes when we're aborting the mount.  We
> > +# also should not leak dquots.
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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 -rf "$tmp".*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/attr
> > +. ./common/reflink
> > +. ./common/inject
> > +. ./common/quota
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs xfs
> > +_require_quota
> > +_require_scratch_reflink
> > +_require_cp_reflink
> > +_require_command "$XFS_FSR_PROG" "xfs_fsr"
> > +_require_xfs_io_error_injection "bmap_finish_one"
> > +_require_xfs_scratch_rmapbt
> > +
> > +rm -f "$seqres.full"
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs > "$seqres.full" 2>&1
> > +_scratch_mount -o noquota >> "$seqres.full" 2>&1
> > +
> > +testdir="$SCRATCH_MNT/test-$seq"
> > +blksz=65536
> > +blks=3
> > +mkdir "$testdir"
> > +
> > +echo "Create a many-block file"
> > +_pwrite_byte 0x62 0 $((blksz * blks)) $testdir/file1 >> $seqres.full
> > +_pwrite_byte 0x63 0 $blksz $testdir/file2 >> $seqres.full
> > +_reflink_range $testdir/file2 0 $testdir/file1 $blksz $blksz >> $seqres.full
> > +_scratch_cycle_mount noquota
> > +
> > +echo "Inject error"
> > +_scratch_inject_error "bmap_finish_one"
> > +
> > +echo "Defrag the file"
> > +$XFS_FSR_PROG -v -d $testdir/file1 >> $seqres.full 2>&1
> > +
> > +echo "FS should be shut down, touch will fail"
> > +touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
> > +
> > +echo "Remount to replay log" | tee /dev/ttyprintk
> > +_scratch_unmount
> > +_scratch_dump_log >> $seqres.full
> > +_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c 'fuzz -d recs[1].startblock ones' >> $seqres.full
> > +_scratch_xfs_db -x -c 'agf 0' -c 'addr refcntroot' -c p >> $seqres.full
> > +
> > +# Suddenly enable quota to test if we can leak the quotacheck dquots!
> > +_scratch_mount -o quota >> $seqres.full 2>&1
> > +_scratch_unmount 2> /dev/null
> > +rm -f ${RESULT_DIR}/require_scratch
> > +
> > +echo "See if we leak"
> > +_test_unmount
> > +rmmod xfs
> 
> Test fails if xfs is built in or there's another mounted xfs, e.g.
> rootfs. There's a _require_btrfs_loadable btrfs-specific helper to check
> if we're able to unload btrfs as a module, and _notrun if not. I think
> we can make it generic easily, e.g. _require_module_loadable "$MOD_NAME"

Ok, I'll insert a new patch to refactor that in a generic way and then
rework this patch to use the generics.

> > +_test_mount
> > +_check_dmesg
> 
> check will do this for us :)

Ok.

--D

> 
> Thanks,
> Eryu
> --
> 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