All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function
@ 2017-09-25 19:56 Richard Wareing
  2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Richard Wareing @ 2017-09-25 19:56 UTC (permalink / raw)
  To: fstests; +Cc: linux-xfs, darrick.wong

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>
---
Changes since v1:
* None

 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 53bbb11..a0081f1 100644
--- a/common/rc
+++ b/common/rc
@@ -1835,6 +1835,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


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

* [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
  2017-09-25 19:56 [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Richard Wareing
@ 2017-09-25 19:56 ` Richard Wareing
  2017-09-25 21:33   ` Darrick J. Wong
  2017-09-26 10:50   ` Eryu Guan
  2017-09-25 19:56 ` [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair Richard Wareing
  2017-09-25 21:38 ` [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Darrick J. Wong
  2 siblings, 2 replies; 11+ messages in thread
From: Richard Wareing @ 2017-09-25 19:56 UTC (permalink / raw)
  To: fstests; +Cc: linux-xfs, darrick.wong

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>
---
Changes since v1:
* None

 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 a0081f1..feed17f 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


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

* [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair
  2017-09-25 19:56 [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Richard Wareing
  2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
@ 2017-09-25 19:56 ` Richard Wareing
  2017-09-25 21:34   ` Darrick J. Wong
  2017-09-26 11:02   ` Eryu Guan
  2017-09-25 21:38 ` [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Darrick J. Wong
  2 siblings, 2 replies; 11+ messages in thread
From: Richard Wareing @ 2017-09-25 19:56 UTC (permalink / raw)
  To: fstests; +Cc: linux-xfs, darrick.wong

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

Signed-off-by: Richard Wareing <rwareing@fb.com>
---
Changes since v1:
* Fixed kill -9 in test xfs/070

 tests/xfs/070 | 3 ++-
 tests/xfs/291 | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/070 b/tests/xfs/070
index 0ae6eff..a06e998 100755
--- a/tests/xfs/070
+++ b/tests/xfs/070
@@ -56,7 +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 &
+	[ -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 &
 	repair_pid=$!
 
 	# monitor progress for as long as it is running
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


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

* Re: [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
  2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
@ 2017-09-25 21:33   ` Darrick J. Wong
  2017-09-26  1:54     ` Richard Wareing
  2017-09-26 10:50   ` Eryu Guan
  1 sibling, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2017-09-25 21:33 UTC (permalink / raw)
  To: Richard Wareing; +Cc: fstests, linux-xfs

On Mon, Sep 25, 2017 at 12:56:24PM -0700, Richard Wareing wrote:
> 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>
> ---
> Changes since v1:
> * None
> 
>  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 a0081f1..feed17f 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

Doesn't this if test fail if RT_INHERIT isn't defined, and isn't
RT_INHERIT undefined unless _reuqire_no_rtinherit?

Also, what happens if I forget the syntax and run 'RT_INHERIT=1 ./check'?
Deosn't that just spit out errors everywhere because '1' isn't a
command?

IOWs I was expecting some kind of string test, like

if [ "${RT_INHERIT}" != "false" ] && echo "${mkfs_cmd}" | grep -q rtdev;
then

--D

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

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

* Re: [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair
  2017-09-25 19:56 ` [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair Richard Wareing
@ 2017-09-25 21:34   ` Darrick J. Wong
  2017-09-26 11:02   ` Eryu Guan
  1 sibling, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2017-09-25 21:34 UTC (permalink / raw)
  To: Richard Wareing; +Cc: fstests, linux-xfs

On Mon, Sep 25, 2017 at 12:56:25PM -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>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
> Changes since v1:
> * Fixed kill -9 in test xfs/070
> 
>  tests/xfs/070 | 3 ++-
>  tests/xfs/291 | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/070 b/tests/xfs/070
> index 0ae6eff..a06e998 100755
> --- a/tests/xfs/070
> +++ b/tests/xfs/070
> @@ -56,7 +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 &
> +	[ -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 &
>  	repair_pid=$!
>  
>  	# monitor progress for as long as it is running
> 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

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

* Re: [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function
  2017-09-25 19:56 [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Richard Wareing
  2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
  2017-09-25 19:56 ` [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair Richard Wareing
@ 2017-09-25 21:38 ` Darrick J. Wong
  2017-09-26  2:25   ` Richard Wareing
  2 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2017-09-25 21:38 UTC (permalink / raw)
  To: Richard Wareing; +Cc: fstests, linux-xfs

On Mon, Sep 25, 2017 at 12:56:23PM -0700, Richard Wareing wrote:
> 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>
> ---
> Changes since v1:
> * None
> 
>  common/rc                      | 8 ++++++++
>  tests/generic/409              | 1 +
>  tests/generic/410              | 1 +

Why don't shared subtree tests work w/ rt?

>  tests/generic/411              | 1 +

Some sort of mount regression?

>  tests/xfs/077                  | 1 +
>  tests/xfs/189                  | 1 +
>  tests/xfs/191-input-validation | 1 +
>  tests/xfs/202                  | 1 +
>  tests/xfs/284                  | 1 +

This test checks that xfsprogs don't run on a mounted fs.  Why would
that be excluded from rt testing?

Since there's only 8 tests on your list, I'm curious why each of them
gets disabled for rt filesystems?

--D

>  9 files changed, 16 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 53bbb11..a0081f1 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1835,6 +1835,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
> 
> --
> 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

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

* Re: [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
  2017-09-25 21:33   ` Darrick J. Wong
@ 2017-09-26  1:54     ` Richard Wareing
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Wareing @ 2017-09-26  1:54 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

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

> On Mon, Sep 25, 2017 at 12:56:24PM -0700, Richard Wareing wrote:
>> 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>
>> ---
>> Changes since v1:
>> * None
>>
>>  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 a0081f1..feed17f 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
>
> Doesn't this if test fail if RT_INHERIT isn't defined, and isn't
> RT_INHERIT undefined unless _reuqire_no_rtinherit?
>

My bad on this, I have RT_INHERIT defined in my config.local, which explains
why it works so nicely for me :).   So yes this will have to be changed to
default to something sane or a test.


> Also, what happens if I forget the syntax and run 'RT_INHERIT=1 ./check'?
> Deosn't that just spit out errors everywhere because '1' isn't a
> command?
>
> IOWs I was expecting some kind of string test, like
>
> if [ "${RT_INHERIT}" != "false" ] && echo "${mkfs_cmd}" | grep -q rtdev;
> then
>
> --D
>
>> +		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
>>
>> --
>> 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



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

* Re: [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function
  2017-09-25 21:38 ` [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Darrick J. Wong
@ 2017-09-26  2:25   ` Richard Wareing
  2017-09-26 10:01     ` Eryu Guan
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Wareing @ 2017-09-26  2:25 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

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

> On Mon, Sep 25, 2017 at 12:56:23PM -0700, Richard Wareing wrote:
>> 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>
>> ---
>> Changes since v1:
>> * None
>>
>>  common/rc                      | 8 ++++++++
>>  tests/generic/409              | 1 +
>>  tests/generic/410              | 1 +
>
> Why don't shared subtree tests work w/ rt?
>

409, 410 decide to mount without the -o rtdev, option.  The fix
here could be trivial, or more involved I'm not really sure.



>> tests/generic/411              | 1 +
>
> Some sort of mount regression?
>

Same as 409/410.

>> tests/xfs/077                  | 1 +

Uses xfs_copy which does not have realtime support.

>>  tests/xfs/189                  | 1 +

Bails with "noattr2 mount option not supported on /dev/loop32".
Possibly a trivial fix?

>>  tests/xfs/191-input-validation | 1 +

Fails with:
  QA output created by 191-input-validation
  silence is golden
+pass -b size=512 -l sectsize=1024 /dev/loop32
+fail -d size=4294967296 /dev/loop32
+fail -d agsize=1g /dev/loop32
+fail -l size=419430400 -d size=4294967296 /dev/loop32
+pass -n ftype=0 /dev/loop32

This test makes my eyes bleed, hoping somebody with more context on
this test can point me in the right direction here.


>>  tests/xfs/202                  | 1 +

Upon further inspection, this fails because it assumes your
scratch device is >1GB.  Though it should probably throw a nice
error indicating your scratch device isn't large enough.

>>  tests/xfs/284                  | 1 +

Uses xfs_copy which does not support realtime.

>
> This test checks that xfsprogs don't run on a mounted fs.  Why would
> that be excluded from rt testing?
>
> Since there's only 8 tests on your list, I'm curious why each of them
> gets disabled for rt filesystems?
>

Aside from the tests using xfs_copy, I can probably add RT support
to most of these tests, but I figured the first step was to first
get to a  stable set of working tests for realtime.

Richard



> --D
>
>> 9 files changed, 16 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 53bbb11..a0081f1 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -1835,6 +1835,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
>>
>> --
>> 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



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

* Re: [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function
  2017-09-26  2:25   ` Richard Wareing
@ 2017-09-26 10:01     ` Eryu Guan
  0 siblings, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2017-09-26 10:01 UTC (permalink / raw)
  To: Richard Wareing; +Cc: Darrick J. Wong, fstests, linux-xfs

On Mon, Sep 25, 2017 at 07:25:27PM -0700, Richard Wareing wrote:
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> > On Mon, Sep 25, 2017 at 12:56:23PM -0700, Richard Wareing wrote:
> > > 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>
> > > ---
> > > Changes since v1:
> > > * None
> > > 
> > >  common/rc                      | 8 ++++++++
> > >  tests/generic/409              | 1 +
> > >  tests/generic/410              | 1 +
> > 
> > Why don't shared subtree tests work w/ rt?
> > 
> 
> 409, 410 decide to mount without the -o rtdev, option.  The fix
> here could be trivial, or more involved I'm not really sure.
> 
> 
> 
> > > tests/generic/411              | 1 +
> > 
> > Some sort of mount regression?
> > 
> 
> Same as 409/410.

It seems generic/409-411 are not hard to fix, just make _get_mount honor
$SCRATCH_OPTIONS (need to make 'type' in _scratch_options local,
otherwise 'type' in the test would be overwritten). But these three
tests are testing vfs level mount semantics, realtime or not should not
make any difference, seems not worth the effort to me.

> 
> > > tests/xfs/077                  | 1 +
> 
> Uses xfs_copy which does not have realtime support.
> 
> > >  tests/xfs/189                  | 1 +
> 
> Bails with "noattr2 mount option not supported on /dev/loop32".
> Possibly a trivial fix?

This test already _notrun with above message gracefully when testing
with rtdev set, I don't think it needs an extra no_realtime rule.

> 
> > >  tests/xfs/191-input-validation | 1 +
> 
> Fails with:
>  QA output created by 191-input-validation
>  silence is golden
> +pass -b size=512 -l sectsize=1024 /dev/loop32
> +fail -d size=4294967296 /dev/loop32
> +fail -d agsize=1g /dev/loop32
> +fail -l size=419430400 -d size=4294967296 /dev/loop32
> +pass -n ftype=0 /dev/loop32
> 
> This test makes my eyes bleed, hoping somebody with more context on
> this test can point me in the right direction here.

This test passes for me with rtdev set, I'm using latest for-next branch
of upstream xfsprogs repo. (Maybe because you're using a too small
SCRATCH_DEV?)

> 
> 
> > >  tests/xfs/202                  | 1 +
> 
> Upon further inspection, this fails because it assumes your
> scratch device is >1GB.  Though it should probably throw a nice
> error indicating your scratch device isn't large enough.

Tested with 10G SCRATCH_DEV with rtdev set, and test passed, so the
failure has nothing to do with realtime. I agreed it should _notrun if
SCRATCH_DEV doesn't have enough space, perhaps do a

_scratch_mkfs_sized $((1024 * 1024 * 1024))

first, which would _notrun gracefully if SCRATCH_DEV is smaller than 1G.

But I'd recommend testing with larger devices, 15G should be good
enough.

> 
> > >  tests/xfs/284                  | 1 +
> 
> Uses xfs_copy which does not support realtime.
> 
> > 
> > This test checks that xfsprogs don't run on a mounted fs.  Why would
> > that be excluded from rt testing?
> > 
> > Since there's only 8 tests on your list, I'm curious why each of them
> > gets disabled for rt filesystems?
> > 
> 
> Aside from the tests using xfs_copy, I can probably add RT support

For the xfs_copy tests, a comment saying why we need
_require_no_realtime would be good.

> to most of these tests, but I figured the first step was to first
> get to a  stable set of working tests for realtime.
> 
> Richard
> 
> 
> 
> > --D
> > 
> > > 9 files changed, 16 insertions(+)
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 53bbb11..a0081f1 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1835,6 +1835,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"

We should check if $USE_EXTERNAL is 'yes' too, SCRATCH_RTDEV alone won't
enable realtime testings.

	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \ ...

Thanks,
Eryu

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

* Re: [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function
  2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
  2017-09-25 21:33   ` Darrick J. Wong
@ 2017-09-26 10:50   ` Eryu Guan
  1 sibling, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2017-09-26 10:50 UTC (permalink / raw)
  To: Richard Wareing; +Cc: fstests, linux-xfs, darrick.wong

On Mon, Sep 25, 2017 at 12:56:24PM -0700, Richard Wareing wrote:
> 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>
> ---
> Changes since v1:
> * None
> 
>  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(-)

I don't think we need an extra RT_INHERT flag, this is just one of the
normal test configurations and can be easily tested by setting correct
MKFS_OPTIONS, e.g.

MKFS_OPTIONS="-d rtinherit=1" ./check ...

Then we can check if MKFS_OPTIONS has rtinherit defined in
_require_no_rtinherit(), e.g.

_require_no_rtinherit()
{
	echo "$MKFS_OPTIONS" | egrep -q "rtinherit([^=]|=1)" && \
		_notrun "<notrun message here>"
}

Unfortunately, mkfs.xfs doesn't print rtinherit status at mkfs time,
otherwise we could check mkfs.xfs output directly.

And xfs/019 xfs/031 passed even with "-d rtinherit=1", and xfs/187
already _notrun with proper message, so seems they don't need
_require_no_rtinherit rule.

generic/427 fails because xfs_io prints out its write log, after
redirecting stdout of xfs_io to /dev/null, generic/427 passed for me
too, the xfs_io write is meant to write a special pattern to the disk
anyway.

For other tests that do need _require_no_rtinherit, a comment to explain
why this rule is needed would be good.

Thanks,
Eryu

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

* Re: [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair
  2017-09-25 19:56 ` [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair Richard Wareing
  2017-09-25 21:34   ` Darrick J. Wong
@ 2017-09-26 11:02   ` Eryu Guan
  1 sibling, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2017-09-26 11:02 UTC (permalink / raw)
  To: Richard Wareing; +Cc: fstests, linux-xfs, darrick.wong

On Mon, Sep 25, 2017 at 12:56:25PM -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>
> ---
> Changes since v1:
> * Fixed kill -9 in test xfs/070
> 
>  tests/xfs/070 | 3 ++-
>  tests/xfs/291 | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/070 b/tests/xfs/070
> index 0ae6eff..a06e998 100755
> --- a/tests/xfs/070
> +++ b/tests/xfs/070
> @@ -56,7 +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 &
> +	[ -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 &

Need to check $USE_EXTERNAL too.

>  	repair_pid=$!
>  
>  	# monitor progress for as long as it is running
> 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"

Same here. And replace xfs_repair with $XFS_REPAIR_PROG too?

Thanks,
Eryu

>  
>  # Yes it can; success, all done
>  status=0
> -- 
> 2.9.5
> 
> --
> 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

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

end of thread, other threads:[~2017-09-26 11:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-25 19:56 [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Richard Wareing
2017-09-25 19:56 ` [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function Richard Wareing
2017-09-25 21:33   ` Darrick J. Wong
2017-09-26  1:54     ` Richard Wareing
2017-09-26 10:50   ` Eryu Guan
2017-09-25 19:56 ` [PATCH v2 3/3] xfs/realtime: Fix direct invocations of xfs_repair Richard Wareing
2017-09-25 21:34   ` Darrick J. Wong
2017-09-26 11:02   ` Eryu Guan
2017-09-25 21:38 ` [PATCH v2 1/3] xfs/realtime: Add require_no_realtime function Darrick J. Wong
2017-09-26  2:25   ` Richard Wareing
2017-09-26 10:01     ` Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.