[PATCH 0/3] Fix/harden "quick" tests for realtime subvolumes

[PATCH 0/3] Fix/harden "quick" tests for realtime subvolumes

[Richard Wareing]

Some tests have issues when being run with realtime subvolumes.  This patch
set aims to harden fs tests so we can get reliable/reproducible test runs
with fstests for realtime subvolumes.

Two new _require functions are introduced along with a new flag to
automatically set the rtinherit=1 option upon mkfs'ing.

The problems I found were usually one of:
1. Snapshot + subsequent test being performed without regard to the realtime
subvolume.
2. Use of functions which are not supported by realtime devices.  Not setting
rtinherit on scratch devices for these cases fixes these tests.  Non-scratch
cases are more tricky since we don't control what goes into the MKFS_OPTIONS
variable.
3. xfs_repair being used directly without checking for RT devices.

After fixing these cases, you get 86 good "quick: xfs tests & 180 good "quick"
generic tests with the bulk are not run due to the lack of reflink and/or quota
support on realtime scratch targets.

P.S. I haven't forgotten about the non-"quick" tests, but I had to start
somewhere, and break this up a bit into a more manageable problem.


Richard Wareing (3):
  xfs/realtime: Add require_no_realtime function
  xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
  xfs/realtime: Fix direct invocations of xfs_repair

 common/rc                      | 32 +++++++++++++++++++++++++++++++-
 tests/generic/250              |  1 +
 tests/generic/252              |  1 +
 tests/generic/409              |  1 +
 tests/generic/410              |  1 +
 tests/generic/411              |  1 +
 tests/generic/427              |  1 +
 tests/generic/441              |  1 +
 tests/xfs/019                  |  1 +
 tests/xfs/031                  |  1 +
 tests/xfs/070                  |  4 ++--
 tests/xfs/077                  |  1 +
 tests/xfs/170                  |  1 +
 tests/xfs/187                  |  1 +
 tests/xfs/189                  |  1 +
 tests/xfs/191-input-validation |  1 +
 tests/xfs/202                  |  1 +
 tests/xfs/284                  |  1 +
 tests/xfs/291                  |  3 ++-
 19 files changed, 51 insertions(+), 4 deletions(-)

-- 
2.9.5

[PATCH 1/3] xfs/realtime: Add require_no_realtime function

[Richard Wareing]

Some tests do not play well with realtime devices, in an effort to
produce a stable set of test which exercise the realtime code paths
we introduce a _require_no_realtime function to allow tests to opt
out of realtime subvolume test runs.

Signed-off-by: Richard Wareing <rwareing@fb.com>
---
 common/rc                      | 8 ++++++++
 tests/generic/409              | 1 +
 tests/generic/410              | 1 +
 tests/generic/411              | 1 +
 tests/xfs/077                  | 1 +
 tests/xfs/189                  | 1 +
 tests/xfs/191-input-validation | 1 +
 tests/xfs/202                  | 1 +
 tests/xfs/284                  | 1 +
 9 files changed, 16 insertions(+)

diff --git a/common/rc b/common/rc
index cd53a37..c65b4b3 100644
--- a/common/rc
+++ b/common/rc
@@ -1829,6 +1829,14 @@ _require_realtime()
 	_notrun "Realtime device required, skipped this test"
 }
 
+# This test requires that a realtime subvolume is not in use
+#
+_require_no_realtime()
+{
+    [ -n "$SCRATCH_RTDEV" ] && \
+	_notrun "Test not compatible with realtime subvolumes, skipped this test"
+}
+
 # this test requires that a specified command (executable) exists
 # $1 - command, $2 - name for error message
 #
diff --git a/tests/generic/409 b/tests/generic/409
index 3ad65c9..8ed3e4e 100755
--- a/tests/generic/409
+++ b/tests/generic/409
@@ -65,6 +65,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {
diff --git a/tests/generic/410 b/tests/generic/410
index 63ab716..1bbaff8 100755
--- a/tests/generic/410
+++ b/tests/generic/410
@@ -73,6 +73,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {
diff --git a/tests/generic/411 b/tests/generic/411
index 83f6d26..ea718fc 100755
--- a/tests/generic/411
+++ b/tests/generic/411
@@ -54,6 +54,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {
diff --git a/tests/xfs/077 b/tests/xfs/077
index eba4f08..d202fa4 100755
--- a/tests/xfs/077
+++ b/tests/xfs/077
@@ -50,6 +50,7 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_crc
 _require_meta_uuid
diff --git a/tests/xfs/189 b/tests/xfs/189
index 636f6f0..699eb3c 100755
--- a/tests/xfs/189
+++ b/tests/xfs/189
@@ -236,6 +236,7 @@ _putback_scratch_fstab()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 _require_scratch
 _require_noattr2
 
diff --git a/tests/xfs/191-input-validation b/tests/xfs/191-input-validation
index cff3efa..764ac9b 100755
--- a/tests/xfs/191-input-validation
+++ b/tests/xfs/191-input-validation
@@ -47,6 +47,7 @@ _cleanup()
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_mkfs_validation
 
diff --git a/tests/xfs/202 b/tests/xfs/202
index b9827a7..f887873 100755
--- a/tests/xfs/202
+++ b/tests/xfs/202
@@ -38,6 +38,7 @@ status=1	# failure is the default!
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 # single AG will cause default xfs_repair to fail. This test is actually
 # testing the special corner case option needed to repair a single AG fs.
 _require_scratch_nocheck
diff --git a/tests/xfs/284 b/tests/xfs/284
index e3625fe..fa5dac8 100755
--- a/tests/xfs/284
+++ b/tests/xfs/284
@@ -49,6 +49,7 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_test
 _require_scratch
 
-- 
2.9.5

[PATCH 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function

[Richard Wareing]

To better exercise the data path code of realtime subvolumes, we will
set rtinherit=1 during mkfs calls.  For tests which this is not desired
we introduce a _require_no_rtinherit function to opt out of this
behavior.

Signed-off-by: Richard Wareing <rwareing@fb.com>
---
 common/rc         | 24 +++++++++++++++++++++++-
 tests/generic/250 |  1 +
 tests/generic/252 |  1 +
 tests/generic/427 |  1 +
 tests/generic/441 |  1 +
 tests/xfs/019     |  1 +
 tests/xfs/031     |  1 +
 tests/xfs/170     |  1 +
 tests/xfs/187     |  1 +
 9 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index c65b4b3..4408c47 100644
--- a/common/rc
+++ b/common/rc
@@ -33,6 +33,16 @@ BC=$(which bc 2> /dev/null) || BC=
 VALID_TEST_ID="[0-9]\{3\}"
 VALID_TEST_NAME="$VALID_TEST_ID-\?[[:alnum:]-]*"
 
+# When running tests with a realtime device configured, the realtime inherit
+# flag will be set during mkfs via -d rtinherit=1 option.  For some tests
+# this may render the test invalid (i.e. it uses a function which is not
+# supported by the realtime subvolume); to prevent failure these tests may
+# disable this behavior by calling _require_no_rtinherit .
+_require_no_rtinherit()
+{
+	RT_INHERIT=false
+}
+
 _require_math()
 {
 	if [ -z "$BC" ]; then
@@ -562,6 +572,13 @@ _scratch_do_mkfs()
 	local mkfs_status
 	local tmp=`mktemp -u`
 
+	# Add rtinherit=1 to mkfs so we exercise realtime subvolume during
+	# our tests.  Tests can opts out of this behavior by calling
+	# _require_no_rtinherit.
+	if $RT_INHERIT && echo "$mkfs_cmd" | grep rtdev &> /dev/null; then
+		extra_mkfs_options="$extra_mkfs_options -d rtinherit=1"
+	fi
+
 	# save mkfs output in case conflict means we need to run again.
 	# only the output for the mkfs that applies should be shown
 	eval "$mkfs_cmd $MKFS_OPTIONS $extra_mkfs_options $SCRATCH_DEV" \
@@ -760,7 +777,12 @@ _mkfs_dev()
 	;;
 
     *)
-	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
+	local extra_mkfs_options="$*"
+	# Similar behavior to the scratch variant of this
+	if $RT_INHERIT && echo $extra_mkfs_options | grep rtdev &> /dev/null; then
+		extra_mkfs_options="$extra_mkfs_options -d rtinherit=1"
+	fi
+	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $extra_mkfs_options \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
     esac
diff --git a/tests/generic/250 b/tests/generic/250
index 3c4fe6d..9f4e364 100755
--- a/tests/generic/250
+++ b/tests/generic/250
@@ -48,6 +48,7 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+_require_no_rtinherit
 
 rm -f $seqres.full
 
diff --git a/tests/generic/252 b/tests/generic/252
index ffedd56..1156902 100755
--- a/tests/generic/252
+++ b/tests/generic/252
@@ -47,6 +47,7 @@ _supported_os Linux
 _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
+_require_no_rtinherit
 _require_aiodio "aiocp"
 AIO_TEST="src/aio-dio-regress/aiocp"
 
diff --git a/tests/generic/427 b/tests/generic/427
index 9cde5f5..18f8476 100755
--- a/tests/generic/427
+++ b/tests/generic/427
@@ -53,6 +53,7 @@ _supported_os Linux
 _require_scratch
 _require_test_program "feature"
 _require_aiodio aio-dio-eof-race
+_require_no_rtinherit
 
 # limit the filesystem size, to save the time of filling filesystem
 _scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1
diff --git a/tests/generic/441 b/tests/generic/441
index 075d877..589069a 100755
--- a/tests/generic/441
+++ b/tests/generic/441
@@ -47,6 +47,7 @@ _cleanup()
 # real QA test starts here
 _supported_os Linux
 _require_scratch
+_require_no_rtinherit
 
 # Generally, we want to avoid journal errors on the extended testcase. Only
 # unset the -s flag if we have a logdev
diff --git a/tests/xfs/019 b/tests/xfs/019
index 3e4f169..1ab8991 100755
--- a/tests/xfs/019
+++ b/tests/xfs/019
@@ -66,6 +66,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 protofile=$tmp.proto
 tempfile=$tmp.file
diff --git a/tests/xfs/031 b/tests/xfs/031
index b05f28b..321f67a 100755
--- a/tests/xfs/031
+++ b/tests/xfs/031
@@ -96,6 +96,7 @@ _supported_os Linux
 
 _require_scratch
 _require_no_large_scratch_dev
+_require_no_rtinherit
 
 # sanity test - default + one root directory entry
 # Note: must do this proto/mkfs now for later inode size calcs
diff --git a/tests/xfs/170 b/tests/xfs/170
index c5ae8e4..6deef1b 100755
--- a/tests/xfs/170
+++ b/tests/xfs/170
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"
 
diff --git a/tests/xfs/187 b/tests/xfs/187
index 07ef3ae..89e7b11 100755
--- a/tests/xfs/187
+++ b/tests/xfs/187
@@ -56,6 +56,7 @@ _filter_version()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_rtinherit
 _require_scratch
 _require_attrs
 _require_attr_v1
-- 
2.9.5

[PATCH 3/3] xfs/realtime: Fix direct invocations of xfs_repair

[Richard Wareing]

Fixes direct invocations of xfs_repair to add in -r option if required.

Signed-off-by: Richard Wareing <rwareing@fb.com>
---
 tests/xfs/070 | 4 ++--
 tests/xfs/291 | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/xfs/070 b/tests/xfs/070
index 0ae6eff..453f6cc 100755
--- a/tests/xfs/070
+++ b/tests/xfs/070
@@ -56,8 +56,8 @@ _cleanup()
 _xfs_repair_noscan()
 {
 	# invoke repair directly so we can kill the process if need be
-	$XFS_REPAIR_PROG $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
-	repair_pid=$!
+	[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
+	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
 
 	# monitor progress for as long as it is running
 	while [ `pgrep xfs_repair` ]; do
diff --git a/tests/xfs/291 b/tests/xfs/291
index 3f5295c..140fa33 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -122,7 +122,8 @@ _xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
 # Can xfs_metadump cope with this monster?
 _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
 xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
-xfs_repair -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of metadump failed"
+[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
+xfs_repair $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of metadump failed"
 
 # Yes it can; success, all done
 status=0
-- 
2.9.5

Re: [PATCH 0/3] Fix/harden "quick" tests for realtime subvolumes

[Richard Wareing]

Oh, and here's my exclude list:

generic/050 <--- Test is fine, message mis-match (xfsprogs?)
generic/449 <--- Test fails on XFS without realtime subvol
generic/451 <--- Test fails on XFS without realtime subvol
xfs/005     <--- Test is fine, message mis-match (xfsprogs?)
xfs/187     <---- Fails with "Need to update test 187 so that
                   initial subtests do not use features2"

So these are unrelated to realtime sub-volumes.

Richard


Richard Wareing <rwareing@fb.com> wrote:

> Some tests have issues when being run with realtime subvolumes.  This patch
> set aims to harden fs tests so we can get reliable/reproducible test runs
> with fstests for realtime subvolumes.
>
> Two new _require functions are introduced along with a new flag to
> automatically set the rtinherit=1 option upon mkfs'ing.
>
> The problems I found were usually one of:
> 1. Snapshot + subsequent test being performed without regard to the  
> realtime
> subvolume.
> 2. Use of functions which are not supported by realtime devices.  Not  
> setting
> rtinherit on scratch devices for these cases fixes these tests.   
> Non-scratch
> cases are more tricky since we don't control what goes into the  
> MKFS_OPTIONS
> variable.
> 3. xfs_repair being used directly without checking for RT devices.
>
> After fixing these cases, you get 86 good "quick: xfs tests & 180 good  
> "quick"
> generic tests with the bulk are not run due to the lack of reflink and/or  
> quota
> support on realtime scratch targets.
>
> P.S. I haven't forgotten about the non-"quick" tests, but I had to start
> somewhere, and break this up a bit into a more manageable problem.
>
>
> Richard Wareing (3):
>   xfs/realtime: Add require_no_realtime function
>   xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
>   xfs/realtime: Fix direct invocations of xfs_repair
>
>  common/rc                      | 32 +++++++++++++++++++++++++++++++-
>  tests/generic/250              |  1 +
>  tests/generic/252              |  1 +
>  tests/generic/409              |  1 +
>  tests/generic/410              |  1 +
>  tests/generic/411              |  1 +
>  tests/generic/427              |  1 +
>  tests/generic/441              |  1 +
>  tests/xfs/019                  |  1 +
>  tests/xfs/031                  |  1 +
>  tests/xfs/070                  |  4 ++--
>  tests/xfs/077                  |  1 +
>  tests/xfs/170                  |  1 +
>  tests/xfs/187                  |  1 +
>  tests/xfs/189                  |  1 +
>  tests/xfs/191-input-validation |  1 +
>  tests/xfs/202                  |  1 +
>  tests/xfs/284                  |  1 +
>  tests/xfs/291                  |  3 ++-
>  19 files changed, 51 insertions(+), 4 deletions(-)
>
> -- 
> 2.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] xfs/realtime: Fix direct invocations of xfs_repair

[Darrick J. Wong]

On Thu, Sep 21, 2017 at 07:02:40PM -0700, Richard Wareing wrote:
> Fixes direct invocations of xfs_repair to add in -r option if required.
> 
> Signed-off-by: Richard Wareing <rwareing@fb.com>
> ---
>  tests/xfs/070 | 4 ++--
>  tests/xfs/291 | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/xfs/070 b/tests/xfs/070
> index 0ae6eff..453f6cc 100755
> --- a/tests/xfs/070
> +++ b/tests/xfs/070
> @@ -56,8 +56,8 @@ _cleanup()
>  _xfs_repair_noscan()
>  {
>  	# invoke repair directly so we can kill the process if need be
> -	$XFS_REPAIR_PROG $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
> -	repair_pid=$!

This breaks the 'kill -9 $repair_pid' further down.

--D

> +	[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
> +	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
>  
>  	# monitor progress for as long as it is running
>  	while [ `pgrep xfs_repair` ]; do
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index 3f5295c..140fa33 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -122,7 +122,8 @@ _xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
>  # Can xfs_metadump cope with this monster?
>  _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
>  xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
> -xfs_repair -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of metadump failed"
> +[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
> +xfs_repair $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of metadump failed"
>  
>  # Yes it can; success, all done
>  status=0
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] xfs/realtime: Fix direct invocations of xfs_repair

[Richard Wareing]

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

> On Thu, Sep 21, 2017 at 07:02:40PM -0700, Richard Wareing wrote:
>> Fixes direct invocations of xfs_repair to add in -r option if required.
>>
>> Signed-off-by: Richard Wareing <rwareing@fb.com>
>> ---
>>  tests/xfs/070 | 4 ++--
>>  tests/xfs/291 | 3 ++-
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/xfs/070 b/tests/xfs/070
>> index 0ae6eff..453f6cc 100755
>> --- a/tests/xfs/070
>> +++ b/tests/xfs/070
>> @@ -56,8 +56,8 @@ _cleanup()
>>  _xfs_repair_noscan()
>>  {
>>  	# invoke repair directly so we can kill the process if need be
>> -	$XFS_REPAIR_PROG $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
>> -	repair_pid=$!
>
> This breaks the 'kill -9 $repair_pid' further down.
>

Nice catch, my bad.  Fixing...

> --D
>
>> +	[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
>> +	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 | tee -a  
>> $seqres.full > $tmp.repair &
>>
>>  	# monitor progress for as long as it is running
>>  	while [ `pgrep xfs_repair` ]; do
>> diff --git a/tests/xfs/291 b/tests/xfs/291
>> index 3f5295c..140fa33 100755
>> --- a/tests/xfs/291
>> +++ b/tests/xfs/291
>> @@ -122,7 +122,8 @@ _xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 ||  
>> _fail "xfs_check failed"
>>  # Can xfs_metadump cope with this monster?
>>  _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
>>  xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
>> -xfs_repair -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of  
>> metadump failed"
>> +[ -n "$SCRATCH_RTDEV" ] && rt_repair_opts="-r $SCRATCH_RTDEV"
>> +xfs_repair $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || _fail  
>> "xfs_repair of metadump failed"
>>
>>  # Yes it can; success, all done
>>  status=0
>> -- 
>> 2.9.5
>>
>> --
>> 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