xfs/realtime: Add require_no_realtime function

PATCH Superseded Richard Wareing 3 participants 11 messages Sep 25, 2017 – Sep 26, 2017

Share & tools

Richard Wareing

author

2017-09-25 19:56 UTC #0 lore raw

PATCH v2 1/3 xfs/realtime: Add require_no_realtime function

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 <[email protected]>

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

common/rc +8 -0

index 53bbb11..a0081f1 100644
@@ -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
 #

tests/generic/409 +1 -0

index 3ad65c9..8ed3e4e 100755
@@ -65,6 +65,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/410 +1 -0

index 63ab716..1bbaff8 100755
@@ -73,6 +73,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/411 +1 -0

index 83f6d26..ea718fc 100755
@@ -54,6 +54,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/xfs/077 +1 -0

index eba4f08..d202fa4 100755
@@ -50,6 +50,7 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_crc
 _require_meta_uuid

tests/xfs/189 +1 -0

index 636f6f0..699eb3c 100755
@@ -236,6 +236,7 @@ _putback_scratch_fstab()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 _require_scratch
 _require_noattr2

tests/xfs/191-input-validation +1 -0

index cff3efa..764ac9b 100755
@@ -47,6 +47,7 @@ _cleanup()
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_mkfs_validation

tests/xfs/202 +1 -0

index b9827a7..f887873 100755
@@ -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

tests/xfs/284 +1 -0

index e3625fe..fa5dac8 100755
@@ -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

Signature · 1 line

2.9.5

Richard Wareing

author

2017-09-25 19:56 UTC #1 lore raw

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

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 <[email protected]>

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

common/rc +23 -1

index a0081f1..feed17f 100644
@@ -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

tests/generic/250 +1 -0

index 3c4fe6d..9f4e364 100755
@@ -48,6 +48,7 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+_require_no_rtinherit
 
 rm -f $seqres.full

tests/generic/252 +1 -0

index ffedd56..1156902 100755
@@ -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"

tests/generic/427 +1 -0

index 9cde5f5..18f8476 100755
@@ -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

tests/generic/441 +1 -0

index 075d877..589069a 100755
@@ -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

tests/xfs/019 +1 -0

index 3e4f169..1ab8991 100755
@@ -66,6 +66,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 protofile=$tmp.proto
 tempfile=$tmp.file

tests/xfs/031 +1 -0

index b05f28b..321f67a 100755
@@ -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

tests/xfs/170 +1 -0

index c5ae8e4..6deef1b 100755
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"

tests/xfs/187 +1 -0

index 07ef3ae..89e7b11 100755
@@ -56,6 +56,7 @@ _filter_version()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_rtinherit
 _require_scratch
 _require_attrs
 _require_attr_v1

Signature · 1 line

2.9.5

Darrick J. Wong

2017-09-25 21:34 UTC #2 lore raw

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

Quoted 43 lines · Richard Wareing

Signed-off-by Richard Wareing <[email protected]>

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

common/rc +10 -0

index a0081f1..feed17f 100644
@@ -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

common/rc _scratch_do_mkfs() · line 578

Quoted 7 lines

common/rc +4 -0

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

Quoted 123 lines

common/rc +9 -1

+		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

tests/generic/250 +1 -0

index 3c4fe6d..9f4e364 100755
@@ -48,6 +48,7 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+_require_no_rtinherit
 
 rm -f $seqres.full

tests/generic/252 +1 -0

index ffedd56..1156902 100755
@@ -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"

tests/generic/427 +1 -0

index 9cde5f5..18f8476 100755
@@ -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

tests/generic/441 +1 -0

index 075d877..589069a 100755
@@ -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

tests/xfs/019 +1 -0

index 3e4f169..1ab8991 100755
@@ -66,6 +66,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 protofile=$tmp.proto
 tempfile=$tmp.file

tests/xfs/031 +1 -0

index b05f28b..321f67a 100755
@@ -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

tests/xfs/170 +1 -0

index c5ae8e4..6deef1b 100755
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"

tests/xfs/187 +1 -0

index 07ef3ae..89e7b11 100755
@@ -56,6 +56,7 @@ _filter_version()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_rtinherit
 _require_scratch
 _require_attrs
 _require_attr_v1

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Richard Wareing

author

2017-09-26 01:54 UTC #3 lore raw

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

Quoted 49 lines · Darrick J. Wong

Quoted 48 lines · Richard Wareing

Signed-off-by Richard Wareing <[email protected]>

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

common/rc +1 -0

index a0081f1..feed17f 100644
@@ -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

common/rc +3 -0

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

common/rc +6 -0

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

Quoted 8 lines

common/rc _scratch_do_mkfs() · line 578
Quoted 4 lines
common/rc +4 -0
+	# 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.

Quoted 135 lines

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

Quoted 124 lines

common/rc +6 -1

+		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

common/rc +3 -0

+		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

tests/generic/250 +1 -0

index 3c4fe6d..9f4e364 100755
@@ -48,6 +48,7 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+_require_no_rtinherit
 
 rm -f $seqres.full

tests/generic/252 +1 -0

index ffedd56..1156902 100755
@@ -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"

tests/generic/427 +1 -0

index 9cde5f5..18f8476 100755
@@ -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

tests/generic/441 +1 -0

index 075d877..589069a 100755
@@ -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

tests/xfs/019 +1 -0

index 3e4f169..1ab8991 100755
@@ -66,6 +66,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 protofile=$tmp.proto
 tempfile=$tmp.file

tests/xfs/031 +1 -0

index b05f28b..321f67a 100755
@@ -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

tests/xfs/170 +1 -0

index c5ae8e4..6deef1b 100755
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"

tests/xfs/187 +1 -0

index 07ef3ae..89e7b11 100755
@@ -56,6 +56,7 @@ _filter_version()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_rtinherit
 _require_scratch
 _require_attrs
 _require_attr_v1

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eryu Guan

2017-09-26 10:50 UTC #4 lore raw

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

In reply to Richard Wareing

Quoted 10 lines · 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 <[email protected]>
---
Changes since v1:
* None

Quoted 10 lines

 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

Richard Wareing

author

2017-09-25 19:56 UTC #5 lore raw

PATCH v2 3/3 xfs/realtime: Fix direct invocations of xfs_repair

In reply to Richard Wareing

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

Signed-off-by Richard Wareing <[email protected]>

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

tests/xfs/070 +2 -1

index 0ae6eff..a06e998 100755
@@ -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

tests/xfs/291 +2 -1

index 3f5295c..140fa33 100755
@@ -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

Signature · 1 line

2.9.5

Darrick J. Wong

Reviewed-by on v2 3/3

2017-09-25 21:34 UTC #6 lore raw

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

Quoted 3 lines · Richard Wareing

Fixes direct invocations of xfs_repair to add in -r option if required.
Signed-off-by Richard Wareing <[email protected]>

Looks ok,

Reviewed-by Darrick J. Wong <[email protected]>

Quoted 43 lines

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

tests/xfs/070 +2 -1

index 0ae6eff..a06e998 100755
@@ -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

tests/xfs/291 +2 -1

index 3f5295c..140fa33 100755
@@ -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

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eryu Guan

2017-09-26 11:02 UTC #7 lore raw

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

In reply to Richard Wareing

Quoted 16 lines · Richard Wareing

Fixes direct invocations of xfs_repair to add in -r option if required.
Signed-off-by Richard Wareing <[email protected]>
---
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(-)
tests/xfs/070 +0 -0
index 0ae6eff..a06e998 100755

tests/xfs/070 _cleanup() · line 60

Quoted 6 lines

tests/xfs/070 +2 -1

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

Quoted 8 lines

tests/xfs/070 +0 -0

 	repair_pid=$!
 
 	# monitor progress for as long as it is running

tests/xfs/291 +0 -0

index 3f5295c..140fa33 100755

tests/xfs/291 _xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed" · line 126

Quoted 6 lines

tests/xfs/291 +2 -1

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

Quoted 10 lines

tests/xfs/291 +0 -0

 # Yes it can; success, all done
 status=0

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

2017-09-25 21:38 UTC #8 lore raw

In reply to Richard Wareing

Quoted 5 lines · 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.

Quoted 8 lines

Signed-off-by Richard Wareing <[email protected]>
---
Changes since v1:
* None
 common/rc                      | 8 ++++++++
 tests/generic/409              | 1 +
 tests/generic/410              | 1 +

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

Quoted 1 line

 tests/generic/411              | 1 +

Some sort of mount regression?

Quoted 5 lines

 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

Quoted 124 lines

 9 files changed, 16 insertions(+)

common/rc +8 -0

index 53bbb11..a0081f1 100644
@@ -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
 #

tests/generic/409 +1 -0

index 3ad65c9..8ed3e4e 100755
@@ -65,6 +65,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/410 +1 -0

index 63ab716..1bbaff8 100755
@@ -73,6 +73,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/411 +1 -0

index 83f6d26..ea718fc 100755
@@ -54,6 +54,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/xfs/077 +1 -0

index eba4f08..d202fa4 100755
@@ -50,6 +50,7 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_crc
 _require_meta_uuid

tests/xfs/189 +1 -0

index 636f6f0..699eb3c 100755
@@ -236,6 +236,7 @@ _putback_scratch_fstab()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 _require_scratch
 _require_noattr2

tests/xfs/191-input-validation +1 -0

index cff3efa..764ac9b 100755
@@ -47,6 +47,7 @@ _cleanup()
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_mkfs_validation

tests/xfs/202 +1 -0

index b9827a7..f887873 100755
@@ -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

tests/xfs/284 +1 -0

index e3625fe..fa5dac8 100755
@@ -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

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Richard Wareing

author

2017-09-26 02:25 UTC #9 lore raw

Quoted 11 lines · Darrick J. Wong

Quoted 10 lines · 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 <[email protected]>
---
Changes since v1:
* None

Quoted 6 lines

Quoted 3 lines
 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.

Quoted 4 lines

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

Same as 409/410.

Quoted 1 line

Quoted 1 line
tests/xfs/077 | 1 +

Uses xfs_copy which does not have realtime support.

Quoted 1 line

 tests/xfs/189                  | 1 +

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

Quoted 1 line

 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.

Quoted 1 line

 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.

Quoted 1 line

 tests/xfs/284                  | 1 +

Uses xfs_copy which does not support realtime.

Quoted 7 lines

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

Quoted 127 lines

--D

Quoted 125 lines

9 files changed, 16 insertions(+)

common/rc +6 -0

index 53bbb11..a0081f1 100644
@@ -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"

common/rc +2 -0

+}
+
 # this test requires that a specified command (executable) exists
 # $1 - command, $2 - name for error message
 #

tests/generic/409 +1 -0

index 3ad65c9..8ed3e4e 100755
@@ -65,6 +65,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/410 +1 -0

index 63ab716..1bbaff8 100755
@@ -73,6 +73,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/generic/411 +1 -0

index 83f6d26..ea718fc 100755
@@ -54,6 +54,7 @@ _supported_os Linux
 _require_test
 _require_scratch
 _require_local_device $SCRATCH_DEV
+_require_no_realtime
 
 fs_stress()
 {

tests/xfs/077 +1 -0

index eba4f08..d202fa4 100755
@@ -50,6 +50,7 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_crc
 _require_meta_uuid

tests/xfs/189 +1 -0

index 636f6f0..699eb3c 100755
@@ -236,6 +236,7 @@ _putback_scratch_fstab()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 _require_scratch
 _require_noattr2

tests/xfs/191-input-validation +1 -0

index cff3efa..764ac9b 100755
@@ -47,6 +47,7 @@ _cleanup()
 # Modify as appropriate.
 _supported_fs xfs
 _supported_os Linux
+_require_no_realtime
 _require_scratch
 _require_xfs_mkfs_validation

tests/xfs/202 +1 -0

index b9827a7..f887873 100755
@@ -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

tests/xfs/284 +1 -0

index e3625fe..fa5dac8 100755
@@ -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

Signature · 6 lines

2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eryu Guan

2017-09-26 10:01 UTC #10 lore raw

Quoted 25 lines · Richard Wareing

Quoted 11 lines · Darrick J. Wong
Quoted 10 lines · 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 <[email protected]>
---
Changes since v1:
* None
Quoted 6 lines
Quoted 3 lines
 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.

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
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.

Quoted 9 lines

Quoted 2 lines
Quoted 2 lines
tests/xfs/077 | 1 +
Uses xfs_copy which does not have realtime support.
Quoted 1 line
Quoted 1 line
 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.

Quoted 12 lines

Quoted 2 lines
Quoted 2 lines
 tests/xfs/191-input-validation | 1 +
Fails with:
QA output created by 191-input-validation
silence is golden
(quoted diff) +5 -0
+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

Quoted 2 lines

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

Quoted 7 lines

Quoted 3 lines
Quoted 3 lines
 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.

Quoted 6 lines

Quoted 2 lines
Quoted 2 lines
 tests/xfs/284                  | 1 +
Uses xfs_copy which does not support realtime.
Quoted 1 line

Quoted 8 lines

Quoted 6 lines
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.

Quoted 19 lines

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
Quoted 12 lines
--D
Quoted 10 lines
9 files changed, 16 insertions(+)
common/rc +0 -0
index 53bbb11..a0081f1 100644
@@ -1835,6 +1835,14 @@ _require_realtime()
 	_notrun "Realtime device required, skipped this test"
 }
 

Quoted 7 lines

common/rc +6 -0

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