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

[PATCH 1/6] 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/6] 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>
---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 39 insertions(+), 31 deletions(-)


diff --git a/README b/README
index 4963d28..a9da4f0 100644
--- a/README
+++ b/README
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild
                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.
 
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..df7d029 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 2>&1
+		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/6] common/xfs: refactor xfs_scrub presence testing

[Eryu Guan]

On Wed, Oct 25, 2017 at 10:51:45PM -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>
> ---
>  README            |    6 +++---
>  common/rc         |    2 +-
>  common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
>  tests/generic/453 |   11 +----------
>  tests/generic/454 |   11 +----------
>  5 files changed, 39 insertions(+), 31 deletions(-)
> 
> 
> diff --git a/README b/README
> index 4963d28..a9da4f0 100644
> --- a/README
> +++ b/README
> @@ -88,9 +88,9 @@ Preparing system for tests:
>                 run xfs_repair -n to check the filesystem; xfs_repair to rebuild
>                 metadata indexes; and xfs_repair -n (a third time) to check the
>                 results of the rebuilding.
> -             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
> -               xfs_scrub -vd to scrub the filesystem metadata online before
> -               unmounting to run the offline check.
> +             - xfs_scrub, if present, will always check the test and scratch
> +               filesystems if they are still online at the end of the test.
> +               It is no longer necessary to set TEST_XFS_SCRUB.
>               - setenv LOGWRITES_DEV to a block device to use for power fail
>                 testing.

With xfs_scrub check enabled by default, I always see scrub failures
like this:

_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub
(see /root/xfstests/results//check.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
generic/445 1s ... 1s
_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub
(see /root/xfstests/results//xfs_4k_crc/generic/445.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
Ran: generic/445
Passed all 1 tests

And in check.full or 445.full:
*** xfs_scrub  -v -d -n output ***
Phase 1: Find filesystem geometry.
Info: /mnt/testarea/test: Kernel metadata scrub is required. (ioctl.c line 946)
Info: /mnt/testarea/test: Scrub aborted after phase 1. (scrub.c line 801)

And this happens to all tests and all test configs. Is this a known
issue of scrub?

The xfs_growfs message probably comes from _check_xfs_test_fs(), stderr
of xfs_growfs should go to the pipe too.

Thanks,
Eryu

>  
> 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..df7d029 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 2>&1
> +		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/6] common/xfs: refactor xfs_scrub presence testing

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 12:37:15PM +0800, Eryu Guan wrote:
> On Wed, Oct 25, 2017 at 10:51:45PM -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>
> > ---
> >  README            |    6 +++---
> >  common/rc         |    2 +-
> >  common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
> >  tests/generic/453 |   11 +----------
> >  tests/generic/454 |   11 +----------
> >  5 files changed, 39 insertions(+), 31 deletions(-)
> > 
> > 
> > diff --git a/README b/README
> > index 4963d28..a9da4f0 100644
> > --- a/README
> > +++ b/README
> > @@ -88,9 +88,9 @@ Preparing system for tests:
> >                 run xfs_repair -n to check the filesystem; xfs_repair to rebuild
> >                 metadata indexes; and xfs_repair -n (a third time) to check the
> >                 results of the rebuilding.
> > -             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
> > -               xfs_scrub -vd to scrub the filesystem metadata online before
> > -               unmounting to run the offline check.
> > +             - xfs_scrub, if present, will always check the test and scratch
> > +               filesystems if they are still online at the end of the test.
> > +               It is no longer necessary to set TEST_XFS_SCRUB.
> >               - setenv LOGWRITES_DEV to a block device to use for power fail
> >                 testing.
> 
> With xfs_scrub check enabled by default, I always see scrub failures
> like this:
> 
> _check_xfs_filesystem: filesystem on /dev/vda5 failed scrub
> (see /root/xfstests/results//check.full for details)
> xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
> generic/445 1s ... 1s
> _check_xfs_filesystem: filesystem on /dev/vda5 failed scrub
> (see /root/xfstests/results//xfs_4k_crc/generic/445.full for details)
> xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
> Ran: generic/445
> Passed all 1 tests
> 
> And in check.full or 445.full:
> *** xfs_scrub  -v -d -n output ***
> Phase 1: Find filesystem geometry.
> Info: /mnt/testarea/test: Kernel metadata scrub is required. (ioctl.c line 946)
> Info: /mnt/testarea/test: Scrub aborted after phase 1. (scrub.c line 801)
> 
> And this happens to all tests and all test configs. Is this a known
> issue of scrub?

It shouldn't be... but we /did/ change the name of the "does this kernel
support scrub" xfs_io command to 'probe' from 'test', so if you have an
old branch of djwong-dev sitting around that might be why it fails to
notice that the running kernel doesn't support scrub.

Ok, I should test that xfs_io has a scrub command and that it has a
probe subcommand.

> The xfs_growfs message probably comes from _check_xfs_test_fs(), stderr
> of xfs_growfs should go to the pipe too.

The xfs_growfs error happens because we fail scrub, which sets ok=0.
Then we unmount the filesystem to run repair, but we don't remount the
filesystem after that because ok=0.  _check_xfs_test_fs doesn't check
the return value of _check_xfs_filesystem, so it blindly assumes that
the testfs is still mounted and runs xfs_info, which fails as you
describe.  That whole thing is pointless anyway because we don't support
"inode-paths" (i.e. IRIX parent pointers) so we might as well rip it out.

--D

> Thanks,
> Eryu
> 
> >  
> > 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..df7d029 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 2>&1
> > +		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
> >  }
> >  
> > 
> --
> 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 v2 2/6] common/xfs: refactor xfs_scrub presence testing

[Darrick J. Wong]

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>
---
v2: make sure xfs_io has a scrub/probe command at all...
---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   41 ++++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/README b/README
index 4963d28..a9da4f0 100644
--- a/README
+++ b/README
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild
                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.
 
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..de3c560 100644
--- a/common/xfs
+++ b/common/xfs
@@ -298,6 +298,30 @@ _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 -c 'help scrub' 2>&1 | grep -q 'types are:.*probe' || return 1
+	$XFS_IO_PROG -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 +354,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 2>&1
+		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/6] 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 df7d029..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 2>&1
+		"$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

[PATCH 4/6] 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?"

[PATCH 5/6] misc: add module reloading helpers

[Darrick J. Wong]

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

Add some helper functions to require that we can reload a given module,
and add a helper to actually do that.  Refactor the existing users to
use the generics.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/btrfs    |   12 ------------
 common/rc       |   15 +++++++++++++++
 tests/btrfs/124 |    9 +++------
 tests/btrfs/125 |    9 +++------
 4 files changed, 21 insertions(+), 24 deletions(-)


diff --git a/common/btrfs b/common/btrfs
index fd762ef..c09206c 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib
diff --git a/common/rc b/common/rc
index 83aaced..5375f7b 100644
--- a/common/rc
+++ b/common/rc
@@ -3580,6 +3580,21 @@ _require_scratch_feature()
 	esac
 }
 
+# Check that we have a module that can be loaded.
+_require_loadable_module()
+{
+	module="$1"
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+}
+
+# Reload a particular module.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
 
 init_rc
 
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index 7206094..1b6cb24 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -64,10 +64,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 2
 
@@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
@@ -141,7 +138,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`
diff --git a/tests/btrfs/125 b/tests/btrfs/125
index 91aa8d8..dbb226c 100755
--- a/tests/btrfs/125
+++ b/tests/btrfs/125
@@ -63,10 +63,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 3
 
@@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1
@@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
 

Re: [PATCH 5/6] misc: add module reloading helpers

[Eryu Guan]

On Wed, Oct 25, 2017 at 10:52:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add some helper functions to require that we can reload a given module,
> and add a helper to actually do that.  Refactor the existing users to
> use the generics.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/btrfs    |   12 ------------
>  common/rc       |   15 +++++++++++++++
>  tests/btrfs/124 |    9 +++------
>  tests/btrfs/125 |    9 +++------
>  4 files changed, 21 insertions(+), 24 deletions(-)
> 
> 
> diff --git a/common/btrfs b/common/btrfs
> index fd762ef..c09206c 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -349,18 +349,6 @@ _btrfs_qgroup_units()
>  	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
>  }
>  
> -_require_btrfs_loadable()
> -{
> -	modprobe -r btrfs || _notrun "btrfs unloadable"
> -	modprobe btrfs || _notrun "Can't load btrfs"
> -}
> -
> -_reload_btrfs_ko()
> -{
> -	modprobe -r btrfs || _fail "btrfs unload failed"
> -	modprobe btrfs || _fail "btrfs load failed"
> -}
> -
>  _btrfs_compression_algos()
>  {
>  	echo zlib
> diff --git a/common/rc b/common/rc
> index 83aaced..5375f7b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3580,6 +3580,21 @@ _require_scratch_feature()
>  	esac
>  }
>  
> +# Check that we have a module that can be loaded.
> +_require_loadable_module()
> +{
> +	module="$1"
> +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."

I think we should try to unload the given module too to see if we really
could unload it, otherwise a later _reload_module() still fails if
rootfs is mounted as $module type, even if we already umounted TEST_DEV
and SCRATCH_DEV.

> +}
> +
> +# Reload a particular module.
> +_reload_module()
> +{
> +	module="$1"
> +
> +	modprobe -r "${module}" || _fail "${module} unload failed"
> +	modprobe "${module}" || _fail "${module} load failed"
> +}
>  
>  init_rc
>  
> diff --git a/tests/btrfs/124 b/tests/btrfs/124
> index 7206094..1b6cb24 100755
> --- a/tests/btrfs/124
> +++ b/tests/btrfs/124
> @@ -64,10 +64,7 @@ rm -f $seqres.full
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 2
> -
> -# the mounted test dir prevent btrfs unload, we need to unmount
> -_test_unmount
> -_require_btrfs_loadable
> +_require_loadable_module "btrfs"

Then keep the above comments and _test_unmount?

Thanks,
Eryu

>  
>  _scratch_dev_pool_get 2
>  
> @@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
>  _scratch_unmount
>  
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_module "btrfs"
>  
>  echo >> $seqres.full
>  echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
> @@ -141,7 +138,7 @@ echo
>  echo "Mount degraded with the other dev"
>  _scratch_unmount
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_module "btrfs"
>  _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
>  _run_btrfs_util_prog filesystem show
>  checkpoint3=`md5sum $SCRATCH_MNT/tf2`
> diff --git a/tests/btrfs/125 b/tests/btrfs/125
> index 91aa8d8..dbb226c 100755
> --- a/tests/btrfs/125
> +++ b/tests/btrfs/125
> @@ -63,10 +63,7 @@ rm -f $seqres.full
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 3
> -
> -# we need btrfs to unload, need test dir unmounted
> -_test_unmount
> -_require_btrfs_loadable
> +_require_loadable_module "btrfs"
>  
>  _scratch_dev_pool_get 3
>  
> @@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
>  _scratch_unmount
>  echo "clean btrfs ko" >> $seqres.full
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_module "btrfs"
>  _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
>  dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
>  					>>$seqres.full 2>&1
> @@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
>  
>  _scratch_unmount
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_module "btrfs"
>  
>  _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
>  
> 

Re: [PATCH 5/6] misc: add module reloading helpers

[Darrick J. Wong]

On Thu, Oct 26, 2017 at 02:43:13PM +0800, Eryu Guan wrote:
> On Wed, Oct 25, 2017 at 10:52:04PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add some helper functions to require that we can reload a given module,
> > and add a helper to actually do that.  Refactor the existing users to
> > use the generics.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/btrfs    |   12 ------------
> >  common/rc       |   15 +++++++++++++++
> >  tests/btrfs/124 |    9 +++------
> >  tests/btrfs/125 |    9 +++------
> >  4 files changed, 21 insertions(+), 24 deletions(-)
> > 
> > 
> > diff --git a/common/btrfs b/common/btrfs
> > index fd762ef..c09206c 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -349,18 +349,6 @@ _btrfs_qgroup_units()
> >  	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
> >  }
> >  
> > -_require_btrfs_loadable()
> > -{
> > -	modprobe -r btrfs || _notrun "btrfs unloadable"
> > -	modprobe btrfs || _notrun "Can't load btrfs"
> > -}
> > -
> > -_reload_btrfs_ko()
> > -{
> > -	modprobe -r btrfs || _fail "btrfs unload failed"
> > -	modprobe btrfs || _fail "btrfs load failed"
> > -}
> > -
> >  _btrfs_compression_algos()
> >  {
> >  	echo zlib
> > diff --git a/common/rc b/common/rc
> > index 83aaced..5375f7b 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3580,6 +3580,21 @@ _require_scratch_feature()
> >  	esac
> >  }
> >  
> > +# Check that we have a module that can be loaded.
> > +_require_loadable_module()
> > +{
> > +	module="$1"
> > +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> 
> I think we should try to unload the given module too to see if we really
> could unload it, otherwise a later _reload_module() still fails if
> rootfs is mounted as $module type, even if we already umounted TEST_DEV
> and SCRATCH_DEV.

Yeah, ok.  While I'm at it I'll drag all the module helpers out into a
separate file too.

--D

> > +}
> > +
> > +# Reload a particular module.
> > +_reload_module()
> > +{
> > +	module="$1"
> > +
> > +	modprobe -r "${module}" || _fail "${module} unload failed"
> > +	modprobe "${module}" || _fail "${module} load failed"
> > +}
> >  
> >  init_rc
> >  
> > diff --git a/tests/btrfs/124 b/tests/btrfs/124
> > index 7206094..1b6cb24 100755
> > --- a/tests/btrfs/124
> > +++ b/tests/btrfs/124
> > @@ -64,10 +64,7 @@ rm -f $seqres.full
> >  _supported_fs btrfs
> >  _supported_os Linux
> >  _require_scratch_dev_pool 2
> > -
> > -# the mounted test dir prevent btrfs unload, we need to unmount
> > -_test_unmount
> > -_require_btrfs_loadable
> > +_require_loadable_module "btrfs"
> 
> Then keep the above comments and _test_unmount?
> 
> Thanks,
> Eryu
> 
> >  
> >  _scratch_dev_pool_get 2
> >  
> > @@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
> >  _scratch_unmount
> >  
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_module "btrfs"
> >  
> >  echo >> $seqres.full
> >  echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
> > @@ -141,7 +138,7 @@ echo
> >  echo "Mount degraded with the other dev"
> >  _scratch_unmount
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_module "btrfs"
> >  _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
> >  _run_btrfs_util_prog filesystem show
> >  checkpoint3=`md5sum $SCRATCH_MNT/tf2`
> > diff --git a/tests/btrfs/125 b/tests/btrfs/125
> > index 91aa8d8..dbb226c 100755
> > --- a/tests/btrfs/125
> > +++ b/tests/btrfs/125
> > @@ -63,10 +63,7 @@ rm -f $seqres.full
> >  _supported_fs btrfs
> >  _supported_os Linux
> >  _require_scratch_dev_pool 3
> > -
> > -# we need btrfs to unload, need test dir unmounted
> > -_test_unmount
> > -_require_btrfs_loadable
> > +_require_loadable_module "btrfs"
> >  
> >  _scratch_dev_pool_get 3
> >  
> > @@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
> >  _scratch_unmount
> >  echo "clean btrfs ko" >> $seqres.full
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_module "btrfs"
> >  _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
> >  dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
> >  					>>$seqres.full 2>&1
> > @@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
> >  
> >  _scratch_unmount
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_module "btrfs"
> >  
> >  _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
> >  
> > 
> --
> 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 v2 5/6] misc: add module reloading helpers

[Darrick J. Wong]

Add some helper functions to require that we can reload a given module,
and add a helper to actually do that.  Refactor the existing users to
use the generics.

We need to hoist completely the behaviors of the old btrfs module helper
because we need to confirm before starting the test that we actually can
remove the module.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/btrfs    |   12 --------
 common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 -------
 tests/btrfs/124 |    9 +++---
 tests/btrfs/125 |    9 +++---
 6 files changed, 92 insertions(+), 33 deletions(-)
 create mode 100644 common/module

diff --git a/common/btrfs b/common/btrfs
index fd762ef..c09206c 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib
diff --git a/common/module b/common/module
new file mode 100644
index 0000000..59cbd8a
--- /dev/null
+++ b/common/module
@@ -0,0 +1,83 @@
+##/bin/bash
+
+# Routines for messing around with loadable kernel modules
+#
+#-----------------------------------------------------------------------
+#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will 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 to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+#  USA
+#-----------------------------------------------------------------------
+
+# Return the module name for this fs.
+_module_for_fs()
+{
+	echo "$FSTYP"
+}
+
+# Reload a particular module.  This module MUST NOT be the module that
+# underlies the filesystem.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
+
+# Reload the filesystem module.
+_reload_fs_module()
+{
+	module="$1"
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "$had_testfs" && _test_unmount
+	test -n "$had_scratchfs" && _scratch_unmount
+	_reload_module "$module"
+	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
+	test -n "$had_testfs" && _test_mount 2> /dev/null
+}
+
+# Check that we have a module that can be loaded.  This module MUST NOT
+# be the module that underlies the filesystem.
+_require_loadable_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_module "$module"
+}
+
+# Check that the module for FSTYP can be loaded.
+_require_loadable_fs_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_fs_module "$module"
+}
+
+# Print the value of a filesystem module parameter
+# at /sys/module/$FSTYP/parameters/$PARAM
+#
+# Usage example (FSTYP=overlay):
+#   _get_fs_module_param index
+_get_fs_module_param()
+{
+	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
+}
diff --git a/common/overlay b/common/overlay
index 79097ba..1da4ab1 100644
--- a/common/overlay
+++ b/common/overlay
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()
diff --git a/common/rc b/common/rc
index 83aaced..c708bfa 100644
--- a/common/rc
+++ b/common/rc
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index 7206094..a648627 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2
 
@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`
diff --git a/tests/btrfs/125 b/tests/btrfs/125
index 91aa8d8..19961a1 100755
--- a/tests/btrfs/125
+++ b/tests/btrfs/125
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3
 
@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1
@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
 

Re: [PATCH v2 5/6] misc: add module reloading helpers

[Eryu Guan]

On Thu, Oct 26, 2017 at 05:38:17PM -0700, Darrick J. Wong wrote:
> Add some helper functions to require that we can reload a given module,
> and add a helper to actually do that.  Refactor the existing users to
> use the generics.
> 
> We need to hoist completely the behaviors of the old btrfs module helper
> because we need to confirm before starting the test that we actually can
> remove the module.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/btrfs    |   12 --------
>  common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  common/overlay  |    1 +
>  common/rc       |   11 -------
>  tests/btrfs/124 |    9 +++---
>  tests/btrfs/125 |    9 +++---
>  6 files changed, 92 insertions(+), 33 deletions(-)
>  create mode 100644 common/module
> 
> diff --git a/common/btrfs b/common/btrfs
> index fd762ef..c09206c 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -349,18 +349,6 @@ _btrfs_qgroup_units()
>  	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
>  }
>  
> -_require_btrfs_loadable()
> -{
> -	modprobe -r btrfs || _notrun "btrfs unloadable"
> -	modprobe btrfs || _notrun "Can't load btrfs"
> -}
> -
> -_reload_btrfs_ko()
> -{
> -	modprobe -r btrfs || _fail "btrfs unload failed"
> -	modprobe btrfs || _fail "btrfs load failed"
> -}
> -
>  _btrfs_compression_algos()
>  {
>  	echo zlib
> diff --git a/common/module b/common/module
> new file mode 100644
> index 0000000..59cbd8a
> --- /dev/null
> +++ b/common/module
> @@ -0,0 +1,83 @@
> +##/bin/bash
> +
> +# Routines for messing around with loadable kernel modules
> +#
> +#-----------------------------------------------------------------------
> +#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
> +#  (at your option) any later version.
> +#
> +#  This program is distributed in the hope that it will 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 to the Free Software
> +#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> +#  USA
> +#-----------------------------------------------------------------------
> +
> +# Return the module name for this fs.
> +_module_for_fs()
> +{
> +	echo "$FSTYP"
> +}
> +
> +# Reload a particular module.  This module MUST NOT be the module that
> +# underlies the filesystem.
> +_reload_module()
> +{
> +	module="$1"
> +
> +	modprobe -r "${module}" || _fail "${module} unload failed"
> +	modprobe "${module}" || _fail "${module} load failed"
> +}
> +
> +# Reload the filesystem module.
> +_reload_fs_module()
> +{
> +	module="$1"
> +
> +	# Unload test fs, try to reload module, remount
> +	had_testfs=""
> +	had_scratchfs=""
> +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
> +	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
> +	test -n "$had_testfs" && _test_unmount
> +	test -n "$had_scratchfs" && _scratch_unmount
> +	_reload_module "$module"
> +	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
> +	test -n "$had_testfs" && _test_mount 2> /dev/null
> +}
> +
> +# Check that we have a module that can be loaded.  This module MUST NOT
> +# be the module that underlies the filesystem.
> +_require_loadable_module()
> +{
> +	module="$1"
> +
> +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> +	_reload_module "$module"
> +}
> +
> +# Check that the module for FSTYP can be loaded.
> +_require_loadable_fs_module()
> +{
> +	module="$1"
> +
> +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> +	_reload_fs_module "$module"

_reload_module() or _reload_fs_module() fails the test directly if the
module is still in use. IMHO we should _notrun in this case.

Currently xfs/70[345] still fail if rootfs is xfs.

Thanks,
Eryu

> +}
> +
> +# Print the value of a filesystem module parameter
> +# at /sys/module/$FSTYP/parameters/$PARAM
> +#
> +# Usage example (FSTYP=overlay):
> +#   _get_fs_module_param index
> +_get_fs_module_param()
> +{
> +	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
> +}
> diff --git a/common/overlay b/common/overlay
> index 79097ba..1da4ab1 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -1,6 +1,7 @@
>  #
>  # overlayfs specific common functions.
>  #
> +. ./common/module
>  
>  # helper function to do the actual overlayfs mount operation
>  _overlay_mount_dirs()
> diff --git a/common/rc b/common/rc
> index 83aaced..c708bfa 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
>  	cat /sys/fs/${FSTYP}/${dname}/${attr}
>  }
>  
> -# Print the value of a filesystem module parameter
> -# at /sys/module/$FSTYP/parameters/$PARAM
> -#
> -# Usage example (FSTYP=overlay):
> -#   _get_fs_module_param index
> -_get_fs_module_param()
> -{
> -	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
> -}
> -
>  # Generic test for specific filesystem feature.
>  # Currently only implemented to test overlayfs features.
>  _require_scratch_feature()
> diff --git a/tests/btrfs/124 b/tests/btrfs/124
> index 7206094..a648627 100755
> --- a/tests/btrfs/124
> +++ b/tests/btrfs/124
> @@ -55,6 +55,7 @@ _cleanup()
>  # get standard environment, filters and checks
>  . ./common/rc
>  . ./common/filter
> +. ./common/module
>  
>  # remove previous $seqres.full before test
>  rm -f $seqres.full
> @@ -64,10 +65,8 @@ rm -f $seqres.full
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 2
> -
> -# the mounted test dir prevent btrfs unload, we need to unmount
>  _test_unmount
> -_require_btrfs_loadable
> +_require_loadable_fs_module "btrfs"
>  
>  _scratch_dev_pool_get 2
>  
> @@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
>  _scratch_unmount
>  
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_fs_module "btrfs"
>  
>  echo >> $seqres.full
>  echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
> @@ -141,7 +140,7 @@ echo
>  echo "Mount degraded with the other dev"
>  _scratch_unmount
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_fs_module "btrfs"
>  _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
>  _run_btrfs_util_prog filesystem show
>  checkpoint3=`md5sum $SCRATCH_MNT/tf2`
> diff --git a/tests/btrfs/125 b/tests/btrfs/125
> index 91aa8d8..19961a1 100755
> --- a/tests/btrfs/125
> +++ b/tests/btrfs/125
> @@ -54,6 +54,7 @@ _cleanup()
>  # get standard environment, filters and checks
>  . ./common/rc
>  . ./common/filter
> +. ./common/module
>  
>  # remove previous $seqres.full before test
>  rm -f $seqres.full
> @@ -63,10 +64,8 @@ rm -f $seqres.full
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch_dev_pool 3
> -
> -# we need btrfs to unload, need test dir unmounted
>  _test_unmount
> -_require_btrfs_loadable
> +_require_loadable_fs_module "btrfs"
>  
>  _scratch_dev_pool_get 3
>  
> @@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
>  _scratch_unmount
>  echo "clean btrfs ko" >> $seqres.full
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_fs_module "btrfs"
>  _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
>  dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
>  					>>$seqres.full 2>&1
> @@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
>  
>  _scratch_unmount
>  # un-scan the btrfs devices
> -_reload_btrfs_ko
> +_reload_fs_module "btrfs"
>  
>  _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
>  

Re: [PATCH v2 5/6] misc: add module reloading helpers

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 12:41:12PM +0800, Eryu Guan wrote:
> On Thu, Oct 26, 2017 at 05:38:17PM -0700, Darrick J. Wong wrote:
> > Add some helper functions to require that we can reload a given module,
> > and add a helper to actually do that.  Refactor the existing users to
> > use the generics.
> > 
> > We need to hoist completely the behaviors of the old btrfs module helper
> > because we need to confirm before starting the test that we actually can
> > remove the module.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/btrfs    |   12 --------
> >  common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  common/overlay  |    1 +
> >  common/rc       |   11 -------
> >  tests/btrfs/124 |    9 +++---
> >  tests/btrfs/125 |    9 +++---
> >  6 files changed, 92 insertions(+), 33 deletions(-)
> >  create mode 100644 common/module
> > 
> > diff --git a/common/btrfs b/common/btrfs
> > index fd762ef..c09206c 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -349,18 +349,6 @@ _btrfs_qgroup_units()
> >  	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
> >  }
> >  
> > -_require_btrfs_loadable()
> > -{
> > -	modprobe -r btrfs || _notrun "btrfs unloadable"
> > -	modprobe btrfs || _notrun "Can't load btrfs"
> > -}
> > -
> > -_reload_btrfs_ko()
> > -{
> > -	modprobe -r btrfs || _fail "btrfs unload failed"
> > -	modprobe btrfs || _fail "btrfs load failed"
> > -}
> > -
> >  _btrfs_compression_algos()
> >  {
> >  	echo zlib
> > diff --git a/common/module b/common/module
> > new file mode 100644
> > index 0000000..59cbd8a
> > --- /dev/null
> > +++ b/common/module
> > @@ -0,0 +1,83 @@
> > +##/bin/bash
> > +
> > +# Routines for messing around with loadable kernel modules
> > +#
> > +#-----------------------------------------------------------------------
> > +#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
> > +#  (at your option) any later version.
> > +#
> > +#  This program is distributed in the hope that it will 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 to the Free Software
> > +#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> > +#  USA
> > +#-----------------------------------------------------------------------
> > +
> > +# Return the module name for this fs.
> > +_module_for_fs()
> > +{
> > +	echo "$FSTYP"
> > +}
> > +
> > +# Reload a particular module.  This module MUST NOT be the module that
> > +# underlies the filesystem.
> > +_reload_module()
> > +{
> > +	module="$1"
> > +
> > +	modprobe -r "${module}" || _fail "${module} unload failed"
> > +	modprobe "${module}" || _fail "${module} load failed"
> > +}
> > +
> > +# Reload the filesystem module.
> > +_reload_fs_module()
> > +{
> > +	module="$1"
> > +
> > +	# Unload test fs, try to reload module, remount
> > +	had_testfs=""
> > +	had_scratchfs=""
> > +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
> > +	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
> > +	test -n "$had_testfs" && _test_unmount
> > +	test -n "$had_scratchfs" && _scratch_unmount
> > +	_reload_module "$module"
> > +	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
> > +	test -n "$had_testfs" && _test_mount 2> /dev/null
> > +}
> > +
> > +# Check that we have a module that can be loaded.  This module MUST NOT
> > +# be the module that underlies the filesystem.
> > +_require_loadable_module()
> > +{
> > +	module="$1"
> > +
> > +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> > +	_reload_module "$module"
> > +}
> > +
> > +# Check that the module for FSTYP can be loaded.
> > +_require_loadable_fs_module()
> > +{
> > +	module="$1"
> > +
> > +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> > +	_reload_fs_module "$module"
> 
> _reload_module() or _reload_fs_module() fails the test directly if the
> module is still in use. IMHO we should _notrun in this case.

Ok, will fix.

> Currently xfs/70[345] still fail if rootfs is xfs.

I don't consider it a good idea to store xfsprogs/xfstests on a $FSTYP
filesystem because you're depending on the reliability of the same
filesystem whose reliability you're trying to prove.

That said, I suppose that pretty much requires nfsroot or a giant initramfs...

--D

> Thanks,
> Eryu
> 
> > +}
> > +
> > +# Print the value of a filesystem module parameter
> > +# at /sys/module/$FSTYP/parameters/$PARAM
> > +#
> > +# Usage example (FSTYP=overlay):
> > +#   _get_fs_module_param index
> > +_get_fs_module_param()
> > +{
> > +	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
> > +}
> > diff --git a/common/overlay b/common/overlay
> > index 79097ba..1da4ab1 100644
> > --- a/common/overlay
> > +++ b/common/overlay
> > @@ -1,6 +1,7 @@
> >  #
> >  # overlayfs specific common functions.
> >  #
> > +. ./common/module
> >  
> >  # helper function to do the actual overlayfs mount operation
> >  _overlay_mount_dirs()
> > diff --git a/common/rc b/common/rc
> > index 83aaced..c708bfa 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
> >  	cat /sys/fs/${FSTYP}/${dname}/${attr}
> >  }
> >  
> > -# Print the value of a filesystem module parameter
> > -# at /sys/module/$FSTYP/parameters/$PARAM
> > -#
> > -# Usage example (FSTYP=overlay):
> > -#   _get_fs_module_param index
> > -_get_fs_module_param()
> > -{
> > -	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
> > -}
> > -
> >  # Generic test for specific filesystem feature.
> >  # Currently only implemented to test overlayfs features.
> >  _require_scratch_feature()
> > diff --git a/tests/btrfs/124 b/tests/btrfs/124
> > index 7206094..a648627 100755
> > --- a/tests/btrfs/124
> > +++ b/tests/btrfs/124
> > @@ -55,6 +55,7 @@ _cleanup()
> >  # get standard environment, filters and checks
> >  . ./common/rc
> >  . ./common/filter
> > +. ./common/module
> >  
> >  # remove previous $seqres.full before test
> >  rm -f $seqres.full
> > @@ -64,10 +65,8 @@ rm -f $seqres.full
> >  _supported_fs btrfs
> >  _supported_os Linux
> >  _require_scratch_dev_pool 2
> > -
> > -# the mounted test dir prevent btrfs unload, we need to unmount
> >  _test_unmount
> > -_require_btrfs_loadable
> > +_require_loadable_fs_module "btrfs"
> >  
> >  _scratch_dev_pool_get 2
> >  
> > @@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
> >  _scratch_unmount
> >  
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_fs_module "btrfs"
> >  
> >  echo >> $seqres.full
> >  echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
> > @@ -141,7 +140,7 @@ echo
> >  echo "Mount degraded with the other dev"
> >  _scratch_unmount
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_fs_module "btrfs"
> >  _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
> >  _run_btrfs_util_prog filesystem show
> >  checkpoint3=`md5sum $SCRATCH_MNT/tf2`
> > diff --git a/tests/btrfs/125 b/tests/btrfs/125
> > index 91aa8d8..19961a1 100755
> > --- a/tests/btrfs/125
> > +++ b/tests/btrfs/125
> > @@ -54,6 +54,7 @@ _cleanup()
> >  # get standard environment, filters and checks
> >  . ./common/rc
> >  . ./common/filter
> > +. ./common/module
> >  
> >  # remove previous $seqres.full before test
> >  rm -f $seqres.full
> > @@ -63,10 +64,8 @@ rm -f $seqres.full
> >  _supported_fs btrfs
> >  _supported_os Linux
> >  _require_scratch_dev_pool 3
> > -
> > -# we need btrfs to unload, need test dir unmounted
> >  _test_unmount
> > -_require_btrfs_loadable
> > +_require_loadable_fs_module "btrfs"
> >  
> >  _scratch_dev_pool_get 3
> >  
> > @@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
> >  _scratch_unmount
> >  echo "clean btrfs ko" >> $seqres.full
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_fs_module "btrfs"
> >  _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
> >  dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
> >  					>>$seqres.full 2>&1
> > @@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
> >  
> >  _scratch_unmount
> >  # un-scan the btrfs devices
> > -_reload_btrfs_ko
> > +_reload_fs_module "btrfs"
> >  
> >  _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
> >  
> --
> 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 v2 5/6] misc: add module reloading helpers

[Eryu Guan]

On Fri, Oct 27, 2017 at 11:18:59AM -0700, Darrick J. Wong wrote:
> On Fri, Oct 27, 2017 at 12:41:12PM +0800, Eryu Guan wrote:
> > On Thu, Oct 26, 2017 at 05:38:17PM -0700, Darrick J. Wong wrote:
> > > Add some helper functions to require that we can reload a given module,
> > > and add a helper to actually do that.  Refactor the existing users to
> > > use the generics.
> > > 
> > > We need to hoist completely the behaviors of the old btrfs module helper
> > > because we need to confirm before starting the test that we actually can
> > > remove the module.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  common/btrfs    |   12 --------
> > >  common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  common/overlay  |    1 +
> > >  common/rc       |   11 -------
> > >  tests/btrfs/124 |    9 +++---
> > >  tests/btrfs/125 |    9 +++---
> > >  6 files changed, 92 insertions(+), 33 deletions(-)
> > >  create mode 100644 common/module
> > > 
> > > diff --git a/common/btrfs b/common/btrfs
> > > index fd762ef..c09206c 100644
> > > --- a/common/btrfs
> > > +++ b/common/btrfs
> > > @@ -349,18 +349,6 @@ _btrfs_qgroup_units()
> > >  	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
> > >  }
> > >  
> > > -_require_btrfs_loadable()
> > > -{
> > > -	modprobe -r btrfs || _notrun "btrfs unloadable"
> > > -	modprobe btrfs || _notrun "Can't load btrfs"
> > > -}
> > > -
> > > -_reload_btrfs_ko()
> > > -{
> > > -	modprobe -r btrfs || _fail "btrfs unload failed"
> > > -	modprobe btrfs || _fail "btrfs load failed"
> > > -}
> > > -
> > >  _btrfs_compression_algos()
> > >  {
> > >  	echo zlib
> > > diff --git a/common/module b/common/module
> > > new file mode 100644
> > > index 0000000..59cbd8a
> > > --- /dev/null
> > > +++ b/common/module
> > > @@ -0,0 +1,83 @@
> > > +##/bin/bash
> > > +
> > > +# Routines for messing around with loadable kernel modules
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
> > > +#  (at your option) any later version.
> > > +#
> > > +#  This program is distributed in the hope that it will 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 to the Free Software
> > > +#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> > > +#  USA
> > > +#-----------------------------------------------------------------------
> > > +
> > > +# Return the module name for this fs.
> > > +_module_for_fs()
> > > +{
> > > +	echo "$FSTYP"
> > > +}
> > > +
> > > +# Reload a particular module.  This module MUST NOT be the module that
> > > +# underlies the filesystem.
> > > +_reload_module()
> > > +{
> > > +	module="$1"
> > > +
> > > +	modprobe -r "${module}" || _fail "${module} unload failed"
> > > +	modprobe "${module}" || _fail "${module} load failed"
> > > +}
> > > +
> > > +# Reload the filesystem module.
> > > +_reload_fs_module()
> > > +{
> > > +	module="$1"
> > > +
> > > +	# Unload test fs, try to reload module, remount
> > > +	had_testfs=""
> > > +	had_scratchfs=""
> > > +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
> > > +	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
> > > +	test -n "$had_testfs" && _test_unmount
> > > +	test -n "$had_scratchfs" && _scratch_unmount
> > > +	_reload_module "$module"
> > > +	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
> > > +	test -n "$had_testfs" && _test_mount 2> /dev/null
> > > +}
> > > +
> > > +# Check that we have a module that can be loaded.  This module MUST NOT
> > > +# be the module that underlies the filesystem.
> > > +_require_loadable_module()
> > > +{
> > > +	module="$1"
> > > +
> > > +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> > > +	_reload_module "$module"
> > > +}
> > > +
> > > +# Check that the module for FSTYP can be loaded.
> > > +_require_loadable_fs_module()
> > > +{
> > > +	module="$1"
> > > +
> > > +	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
> > > +	_reload_fs_module "$module"
> > 
> > _reload_module() or _reload_fs_module() fails the test directly if the
> > module is still in use. IMHO we should _notrun in this case.
> 
> Ok, will fix.
> 
> > Currently xfs/70[345] still fail if rootfs is xfs.
> 
> I don't consider it a good idea to store xfsprogs/xfstests on a $FSTYP
> filesystem because you're depending on the reliability of the same
> filesystem whose reliability you're trying to prove.
> 
> That said, I suppose that pretty much requires nfsroot or a giant initramfs...

So I have two test vms, one with ext4 rootfs to test xfs, one with xfs
rootfs to test ext4, I didn't use btrfs root because RHEL dropped btrfs
support :)

BTW, I took patch 1, 4 and 7 for this week's update, the other patches
probably will go in next week. Thanks a lot for all the updates!

Eryu

[PATCH v3 5/6] misc: add module reloading helpers

[Darrick J. Wong]

Add some helper functions to require that we can reload a given module,
and add a helper to actually do that.  Refactor the existing users to
use the generics.

We need to hoist completely the behaviors of the old btrfs module helper
because we need to confirm before starting the test that we actually can
remove the module.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v3: be more judicious in our use of _notrun/_fail
---
 common/btrfs    |   12 -------
 common/module   |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 ------
 tests/btrfs/124 |    9 ++---
 tests/btrfs/125 |    9 ++---
 6 files changed, 108 insertions(+), 33 deletions(-)
 create mode 100644 common/module

diff --git a/common/btrfs b/common/btrfs
index fd762ef..c09206c 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib
diff --git a/common/module b/common/module
new file mode 100644
index 0000000..7473d20
--- /dev/null
+++ b/common/module
@@ -0,0 +1,99 @@
+##/bin/bash
+
+# Routines for messing around with loadable kernel modules
+#
+#-----------------------------------------------------------------------
+#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will 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 to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+#  USA
+#-----------------------------------------------------------------------
+
+# Return the module name for this fs.
+_module_for_fs()
+{
+	echo "${FSTYP}"
+}
+
+# Reload a particular module.  This module MUST NOT be the module that
+# underlies the filesystem.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
+
+# Reload the filesystem module.
+_reload_fs_module()
+{
+	module="$1"
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "${had_testfs}" && _test_unmount
+	test -n "${had_scratchfs}" && _scratch_unmount
+	_reload_module "${module}"
+	test -n "${had_scratchfs}" && _scratch_mount 2> /dev/null
+	test -n "${had_testfs}" && _test_mount 2> /dev/null
+}
+
+# Check that we have a module that can be loaded.  This module MUST NOT
+# be the module that underlies the filesystem.
+_require_loadable_module()
+{
+	module="$1"
+
+	modinfo "${module}" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	modprobe -r "${module}" || _notrun "${module} unload failed"
+	modprobe "${module}" || _notrun "${module} load failed"
+}
+
+# Check that the module for FSTYP can be loaded.
+_require_loadable_fs_module()
+{
+	module="$1"
+
+	modinfo "${module}" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "${had_testfs}" && _test_unmount
+	test -n "${had_scratchfs}" && _scratch_unmount
+	unload_ok=""
+	load_ok=""
+	modprobe -r "${module}" || unload_ok=0
+	modprobe "${module}" || load_ok=0
+	test -n "${had_scratchfs}" && _scratch_mount 2> /dev/null
+	test -n "${had_testfs}" && _test_mount 2> /dev/null
+	test -z "${unload_ok}" || _notrun "${module} unload failed"
+	test -z "${load_ok}" || _notrun "${module} load failed"
+}
+
+# Print the value of a filesystem module parameter
+# at /sys/module/$FSTYP/parameters/$PARAM
+#
+# Usage example (FSTYP=overlay):
+#   _get_fs_module_param index
+_get_fs_module_param()
+{
+	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
+}
diff --git a/common/overlay b/common/overlay
index 79097ba..1da4ab1 100644
--- a/common/overlay
+++ b/common/overlay
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()
diff --git a/common/rc b/common/rc
index 83aaced..c708bfa 100644
--- a/common/rc
+++ b/common/rc
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()
@@ -3580,7 +3570,6 @@ _require_scratch_feature()
 	esac
 }
 
-
 init_rc
 
 ################################################################################
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index 7206094..a648627 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2
 
@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`
diff --git a/tests/btrfs/125 b/tests/btrfs/125
index 91aa8d8..19961a1 100755
--- a/tests/btrfs/125
+++ b/tests/btrfs/125
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3
 
@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1
@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
 

[PATCH 6/6] 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 5375f7b..a0a348f 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..524c3d5
--- /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_loadable_module "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
+_reload_module "xfs"
+_test_mount
+
+# 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..f873efd
--- /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_loadable_module "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
+_reload_module "xfs"
+_test_mount
+
+# 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..bcb024b
--- /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_loadable_module "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
+_reload_module "xfs"
+_test_mount
+
+# 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

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

[Darrick J. Wong]

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     |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/703.out |    9 ++++
 tests/xfs/704     |   89 ++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/704.out |    5 ++
 tests/xfs/705     |  105 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/705.out |    9 ++++
 tests/xfs/group   |    3 +
 8 files changed, 332 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 c708bfa..7e453e0 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..b2e37e6
--- /dev/null
+++ b/tests/xfs/703
@@ -0,0 +1,111 @@
+#! /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
+. ./common/module
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_loadable_fs_module "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"
+_reload_fs_module "xfs"
+
+# 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..3f4d704
--- /dev/null
+++ b/tests/xfs/704
@@ -0,0 +1,89 @@
+#! /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
+. ./common/module
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_loadable_fs_module "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"
+_reload_fs_module "xfs"
+
+# 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..6019a4e
--- /dev/null
+++ b/tests/xfs/705
@@ -0,0 +1,105 @@
+#! /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
+. ./common/module
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_loadable_fs_module "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"
+_reload_fs_module "xfs"
+
+# 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

[PATCH 7/6] common/fuzzy: online re-scrub should not preen

[Darrick J. Wong]

When we're doing the second online scrub (to figure out if the repair
did any good) we shouldn't let that second scrub preen the filesystem in
any way.  If scrub finds things it can't/won't preen that turns into a
nonzero return code which gets reported (incorrectly) as a failure.

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

diff --git a/common/fuzzy b/common/fuzzy
index 7a4b03a..87f7acc 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -275,7 +275,7 @@ __scratch_xfs_fuzz_field_test() {
 		# which scrub doesn't know how to fix.
 		echo "++ Online scrub"
 		if [ "$1" != "sb 0" ]; then
-			_scratch_scrub -e continue 2>&1
+			_scratch_scrub -n -e continue 2>&1
 			res=$?
 			test $res -ne 0 && \
 				(>&2 echo "online re-scrub ($res) with ${field} = ${fuzzverb}.")

[PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

[Darrick J. Wong]

Fix test failures with new inode pointer verifiers... and also make sure
that the running xfs actually supports realtime rmap.  (This should stop
the current crop of weird test failures since nobody has rtrmap yet
anyway...)

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/333     |    4 +++-
 tests/xfs/333.out |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/333 b/tests/xfs/333
index f7f233d..af52373 100755
--- a/tests/xfs/333
+++ b/tests/xfs/333
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"
@@ -62,7 +64,7 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
+_scratch_mount 2>&1 | _filter_scratch
 
 echo "Check files"
 md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
diff --git a/tests/xfs/333.out b/tests/xfs/333.out
index bee9bbc..9f81ba4 100644
--- a/tests/xfs/333.out
+++ b/tests/xfs/333.out
@@ -2,8 +2,10 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
 Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
+md5sum: SCRATCH_MNT/f1: No such file or directory
 Try to create more files
 Repair fs
+umount: SCRATCH_DEV: not mounted
 Try to create more files (again)

Re: [PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

[Eryu Guan]

On Thu, Oct 26, 2017 at 05:44:01PM -0700, Darrick J. Wong wrote:
> Fix test failures with new inode pointer verifiers... and also make sure
> that the running xfs actually supports realtime rmap.  (This should stop
> the current crop of weird test failures since nobody has rtrmap yet
> anyway...)
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/333     |    4 +++-
>  tests/xfs/333.out |    4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/333 b/tests/xfs/333
> index f7f233d..af52373 100755
> --- a/tests/xfs/333
> +++ b/tests/xfs/333
> @@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
>  
>  echo "Format and mount"
>  _scratch_mkfs > "$seqres.full" 2>&1
> +rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
> +test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
>  _scratch_mount
>  
>  echo "Create some files"
> @@ -62,7 +64,7 @@ _scratch_unmount
>  
>  echo "Corrupt fs"
>  _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
> -_scratch_mount
> +_scratch_mount 2>&1 | _filter_scratch
>  
>  echo "Check files"
>  md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
> diff --git a/tests/xfs/333.out b/tests/xfs/333.out
> index bee9bbc..9f81ba4 100644
> --- a/tests/xfs/333.out
> +++ b/tests/xfs/333.out
> @@ -2,8 +2,10 @@ QA output created by 333
>  Format and mount
>  Create some files
>  Corrupt fs
> +mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning

So kernel refuses to mount the corrupted fs now, then all the subsequent
test steps before _repair_scratch_fs seem meaningless now, e.g. check
md5sum of the file, and since SCRATCH_DEV is not mounted, "Trying to
create more files" is actually writing to rootfs. All these don't seem
right and look confusing to me..

Thanks,
Eryu

>  Check files
> -8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
> +md5sum: SCRATCH_MNT/f1: No such file or directory
>  Try to create more files
>  Repair fs
> +umount: SCRATCH_DEV: not mounted
>  Try to create more files (again)

Re: [PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 02:04:18PM +0800, Eryu Guan wrote:
> On Thu, Oct 26, 2017 at 05:44:01PM -0700, Darrick J. Wong wrote:
> > Fix test failures with new inode pointer verifiers... and also make sure
> > that the running xfs actually supports realtime rmap.  (This should stop
> > the current crop of weird test failures since nobody has rtrmap yet
> > anyway...)
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/333     |    4 +++-
> >  tests/xfs/333.out |    4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/xfs/333 b/tests/xfs/333
> > index f7f233d..af52373 100755
> > --- a/tests/xfs/333
> > +++ b/tests/xfs/333
> > @@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
> >  
> >  echo "Format and mount"
> >  _scratch_mkfs > "$seqres.full" 2>&1
> > +rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
> > +test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
> >  _scratch_mount
> >  
> >  echo "Create some files"
> > @@ -62,7 +64,7 @@ _scratch_unmount
> >  
> >  echo "Corrupt fs"
> >  _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
> > -_scratch_mount
> > +_scratch_mount 2>&1 | _filter_scratch
> >  
> >  echo "Check files"
> >  md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
> > diff --git a/tests/xfs/333.out b/tests/xfs/333.out
> > index bee9bbc..9f81ba4 100644
> > --- a/tests/xfs/333.out
> > +++ b/tests/xfs/333.out
> > @@ -2,8 +2,10 @@ QA output created by 333
> >  Format and mount
> >  Create some files
> >  Corrupt fs
> > +mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
> 
> So kernel refuses to mount the corrupted fs now, then all the subsequent
> test steps before _repair_scratch_fs seem meaningless now, e.g. check
> md5sum of the file, and since SCRATCH_DEV is not mounted, "Trying to
> create more files" is actually writing to rootfs. All these don't seem
> right and look confusing to me..

Ok, yeah, the rest of the test can get deleted now.

--D

> Thanks,
> Eryu
> 
> >  Check files
> > -8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
> > +md5sum: SCRATCH_MNT/f1: No such file or directory
> >  Try to create more files
> >  Repair fs
> > +umount: SCRATCH_DEV: not mounted
> >  Try to create more files (again)
> --
> 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 v2 8/6] xfs/333: fix errors with new inode pointer verifiers

[Darrick J. Wong]

Fix test failures with new inode pointer verifiers... and also make sure
that the running xfs actually supports realtime rmap.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: remove the rest of the test after the mount failure
---
 tests/xfs/333     |   18 ++++--------------
 tests/xfs/333.out |    7 ++-----
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/tests/xfs/333 b/tests/xfs/333
index f7f233d..bf0c811 100755
--- a/tests/xfs/333
+++ b/tests/xfs/333
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"
@@ -62,21 +64,9 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
-
-echo "Check files"
-md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
-
-echo "Try to create more files"
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f3 >> $seqres.full 2>&1
+_scratch_mount 2>&1 | _filter_scratch
 
-echo "Repair fs"
-_scratch_unmount 2>&1 | _filter_scratch
-_repair_scratch_fs >> $seqres.full 2>&1
-
-echo "Try to create more files (again)"
-_scratch_mount
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f4 >> $seqres.full
+echo "Test done, mount should have failed"
 
 # success, all done
 status=0
diff --git a/tests/xfs/333.out b/tests/xfs/333.out
index bee9bbc..f7518f4 100644
--- a/tests/xfs/333.out
+++ b/tests/xfs/333.out
@@ -2,8 +2,5 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
-Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
-Try to create more files
-Repair fs
-Try to create more files (again)
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
+Test done, mount should have failed

Re: [PATCH v2 8/6] xfs/333: fix errors with new inode pointer verifiers

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 01:24:07PM -0700, Darrick J. Wong wrote:
> Fix test failures with new inode pointer verifiers... and also make sure
> that the running xfs actually supports realtime rmap.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v2: remove the rest of the test after the mount failure
> ---
>  tests/xfs/333     |   18 ++++--------------
>  tests/xfs/333.out |    7 ++-----
>  2 files changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/xfs/333 b/tests/xfs/333
> index f7f233d..bf0c811 100755
> --- a/tests/xfs/333
> +++ b/tests/xfs/333
> @@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
>  
>  echo "Format and mount"
>  _scratch_mkfs > "$seqres.full" 2>&1
> +rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
> +test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
>  _scratch_mount
>  
>  echo "Create some files"
> @@ -62,21 +64,9 @@ _scratch_unmount
>  
>  echo "Corrupt fs"
>  _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
> -_scratch_mount
> -
> -echo "Check files"
> -md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
> -
> -echo "Try to create more files"
> -$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f3 >> $seqres.full 2>&1
> +_scratch_mount 2>&1 | _filter_scratch
>  
> -echo "Repair fs"
> -_scratch_unmount 2>&1 | _filter_scratch
> -_repair_scratch_fs >> $seqres.full 2>&1
> -
> -echo "Try to create more files (again)"
> -_scratch_mount
> -$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f4 >> $seqres.full
> +echo "Test done, mount should have failed"

Hmm, we ought to repair the filesystem to make sure that repair actually
knows how to deal with rrmapino problems.

(I already fixed this; will be sending a rollup of this week's patches
shortly.)

--D

>  
>  # success, all done
>  status=0
> diff --git a/tests/xfs/333.out b/tests/xfs/333.out
> index bee9bbc..f7518f4 100644
> --- a/tests/xfs/333.out
> +++ b/tests/xfs/333.out
> @@ -2,8 +2,5 @@ QA output created by 333
>  Format and mount
>  Create some files
>  Corrupt fs
> -Check files
> -8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
> -Try to create more files
> -Repair fs
> -Try to create more files (again)
> +mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
> +Test done, mount should have failed
> --
> 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 9/6] generic/459: fix test running errors

[Darrick J. Wong]

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Then we can end up with arbitrary gunk in the thin device.  This causes
mkfs to fail because it's afraid to format the device.  Don't be afraid,
just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
external log because sometimes the thinp device just happen to be backed
by the log of the previous test's scratch fs.

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

diff --git a/tests/generic/459 b/tests/generic/459
index 7c10c2a..3dd3681 100755
--- a/tests/generic/459
+++ b/tests/generic/459
@@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
 		    -T $vgname/$poolname \
 		    -n $lvname >>$seqres.full 2>&1
 
-_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
+_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
 
 
 # Running the test over the original volume doesn't reproduce the problem

Re: [PATCH 9/6] generic/459: fix test running errors

[Eryu Guan]

On Thu, Oct 26, 2017 at 05:44:46PM -0700, Darrick J. Wong wrote:
> If the DISCARD of the thin device somehow fails with this message:
> 
> device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
> 
> Then we can end up with arbitrary gunk in the thin device.  This causes
> mkfs to fail because it's afraid to format the device.  Don't be afraid,
> just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
> external log because sometimes the thinp device just happen to be backed
> by the log of the previous test's scratch fs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/generic/459 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/459 b/tests/generic/459
> index 7c10c2a..3dd3681 100755
> --- a/tests/generic/459
> +++ b/tests/generic/459
> @@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
>  		    -T $vgname/$poolname \
>  		    -n $lvname >>$seqres.full 2>&1
>  
> -_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
> +_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1

I think we should fix it in _mkfs_dev, add a new switch entry for xfs.

Thanks,
Eryu

>  
>  
>  # Running the test over the original volume doesn't reproduce the problem

Re: [PATCH 9/6] generic/459: fix test running errors

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 12:42:01PM +0800, Eryu Guan wrote:
> On Thu, Oct 26, 2017 at 05:44:46PM -0700, Darrick J. Wong wrote:
> > If the DISCARD of the thin device somehow fails with this message:
> > 
> > device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
> > 
> > Then we can end up with arbitrary gunk in the thin device.  This causes
> > mkfs to fail because it's afraid to format the device.  Don't be afraid,
> > just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
> > external log because sometimes the thinp device just happen to be backed
> > by the log of the previous test's scratch fs.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/generic/459 |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/generic/459 b/tests/generic/459
> > index 7c10c2a..3dd3681 100755
> > --- a/tests/generic/459
> > +++ b/tests/generic/459
> > @@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
> >  		    -T $vgname/$poolname \
> >  		    -n $lvname >>$seqres.full 2>&1
> >  
> > -_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
> > +_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
> 
> I think we should fix it in _mkfs_dev, add a new switch entry for xfs.

Will do.

--D

> Thanks,
> Eryu
> 
> >  
> >  
> >  # Running the test over the original volume doesn't reproduce the problem
> --
> 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 v2 9/6] generic/459: fix test running errors

[Darrick J. Wong]

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Then we can end up with arbitrary gunk in the thin device.  This causes
mkfs to fail because it's afraid to format the device.  Don't be afraid,
just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
external log because sometimes the thinp device just happen to be backed
by the log of the previous test's scratch fs.

Fix this by making the _mkfs_dev helper always format the device, per
Eryu Guan's suggestion.

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

diff --git a/common/rc b/common/rc
index 7e453e0..a985f4d 100644
--- a/common/rc
+++ b/common/rc
@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -f -t $FSTYP -- $MKFS_OPTIONS $* \
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd

Re: [PATCH v2 9/6] generic/459: fix test running errors

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 01:25:21PM -0700, Darrick J. Wong wrote:
> If the DISCARD of the thin device somehow fails with this message:
> 
> device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
> 
> Then we can end up with arbitrary gunk in the thin device.  This causes
> mkfs to fail because it's afraid to format the device.  Don't be afraid,
> just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
> external log because sometimes the thinp device just happen to be backed
> by the log of the previous test's scratch fs.
> 
> Fix this by making the _mkfs_dev helper always format the device, per
> Eryu Guan's suggestion.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 7e453e0..a985f4d 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -642,7 +642,10 @@ _mkfs_dev()
>  	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	;;
> -
> +    xfs)
> +	yes | $MKFS_PROG -f -t $FSTYP -- $MKFS_OPTIONS $* \

The '-f' should be after the '--', will resend patch.

--D

> +		2>$tmp.mkfserr 1>$tmp.mkfsstd
> +	;;
>      *)
>  	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
> --
> 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 9/6] generic/459: fix test running errors

[Darrick J. Wong]

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Then we can end up with arbitrary gunk in the thin device.  This causes
mkfs to fail because it's afraid to format the device.  Don't be afraid,
just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
external log because sometimes the thinp device just happen to be backed
by the log of the previous test's scratch fs.

Fix this by making the _mkfs_dev helper always format the device, per
Eryu Guan's suggestion.

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

diff --git a/common/rc b/common/rc
index 7e453e0..c441199 100644
--- a/common/rc
+++ b/common/rc
@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd

Re: [PATCH v3 9/6] generic/459: fix test running errors

[Eryu Guan]

On Sat, Oct 28, 2017 at 10:08:56AM -0700, Darrick J. Wong wrote:
> If the DISCARD of the thin device somehow fails with this message:
> 
> device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
> 
> Then we can end up with arbitrary gunk in the thin device.  This causes
> mkfs to fail because it's afraid to format the device.  Don't be afraid,
> just zap it.  FWIW mkfs.xfs thinks that the thinp device has an xfs
> external log because sometimes the thinp device just happen to be backed
> by the log of the previous test's scratch fs.
> 
> Fix this by making the _mkfs_dev helper always format the device, per
> Eryu Guan's suggestion.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 7e453e0..c441199 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -642,7 +642,10 @@ _mkfs_dev()
>  	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	;;
> -
> +    xfs)
> +	yes | $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \
> +		2>$tmp.mkfserr 1>$tmp.mkfsstd
> +	;;

I think the 'yes' pipe can be omitted, I'll fix that on commit.

Thanks,
Eryu

>      *)
>  	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd

[PATCH 10/6] common/xfs: remove inode-paths cruft

[Darrick J. Wong]

Remove the inode-paths check from _check_xfs_test_fs because we don't
support inode paths, xfsprogs doesn't have a xfs_{check,repair}_ipaths
tool, and it's broken anyway because we ignore _check_xfs_filesystem
(which tells whether or not the filesystem is even still mounted).

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

diff --git a/common/xfs b/common/xfs
index 5643adf..79efb78 100644
--- a/common/xfs
+++ b/common/xfs
@@ -471,13 +471,6 @@ _check_xfs_test_fs()
 		TEST_RT="$TEST_RTDEV"
 
 	_check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
-
-	# check for ipath consistency
-	if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
-		# errors go to stderr
-		xfs_check_ipaths $TEST_DIR >/dev/null
-		xfs_repair_ipaths -n $TEST_DIR >/dev/null
-	fi
 }
 
 _require_xfs_test_rmapbt()

Re: [PATCH 10/6] common/xfs: remove inode-paths cruft

[Eryu Guan]

On Fri, Oct 27, 2017 at 01:25:50PM -0700, Darrick J. Wong wrote:
> Remove the inode-paths check from _check_xfs_test_fs because we don't
> support inode paths, xfsprogs doesn't have a xfs_{check,repair}_ipaths
> tool, and it's broken anyway because we ignore _check_xfs_filesystem
> (which tells whether or not the filesystem is even still mounted).
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/xfs |    7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 5643adf..79efb78 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -471,13 +471,6 @@ _check_xfs_test_fs()
>  		TEST_RT="$TEST_RTDEV"
>  
>  	_check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
> -
> -	# check for ipath consistency
> -	if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
> -		# errors go to stderr
> -		xfs_check_ipaths $TEST_DIR >/dev/null
> -		xfs_repair_ipaths -n $TEST_DIR >/dev/null
> -	fi

I added a "return $?" here to report _check_xfs_filesystem result to the
caller, otherwise test was reported as a PASS even _check_xfs_filesystem
found something and printed out the error messages.

Thanks,
Eryu

>  }
>  
>  _require_xfs_test_rmapbt()