[PATCH 0/4] fstests: more random fixes from Ted

[PATCH 0/4] fstests: more random fixes from Ted

[Theodore Ts'o]

Here are few more hopefully non-controvial patches from my tytso-local
branch on https://github.com/tytso/xfstests.  The xuniit-quet patch
has previously been posted; this has a minor clarification in the
commit description, but is otherwise unchanged from the previous v2
version on the list.

The other fixes fixes some problems found when running tests with an
external log, and to address generic/556 failing on older LTS kernels.

Theodore Ts'o (4):
  report: add support for the xunit-quiet format
  generic/556: add a check to make sure ext4 supports encrypted
    casefolding
  dmflakey: don't run dmflakey tests with an external log device
  generic/475: skip test when using an external log device

 check             |  2 +-
 common/casefold   | 17 +++++++++++++++++
 common/dmflakey   |  4 ++++
 common/report     | 21 ++++++++++++++-------
 tests/generic/475 |  4 ++++
 tests/generic/556 |  1 +
 6 files changed, 41 insertions(+), 8 deletions(-)

-- 
2.31.0

[PATCH 1/4] report: add support for the xunit-quiet format

[Theodore Ts'o]

The xunit-quiet format excludes the NNN.{full,dmesg,bad} files in
<system-out> and <system-err> nodes which are included in the xunit
report format.

For test runners that save the entire results directory to preserve
all of the test artifacts, capturing the NNN.{full,dmesg,bad} in the
results.xml file is redundant.  In addition, if the NNN.bad is too
large, it can cause the junitparser python library to refuse to parse
the XML file to prevent potential denial of service attacks[1].  A
simple way to avoid this problem is to simply to omit the <system-out>
and <system-err> nodes in the results.xml file.

[1] https://gitlab.com/gitlab-org/gitlab/-/issues/268035

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: David Disseldorp <ddiss@suse.de>
---
 check         |  2 +-
 common/report | 21 ++++++++++++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/check b/check
index 5f6d86b4..0b2f10ed 100755
--- a/check
+++ b/check
@@ -75,7 +75,7 @@ check options
     -I <n>		iterate the test list <n> times, but stops iterating further in case of any test failure
     -d			dump test output to stdout
     -b			brief test summary
-    -R fmt[,fmt]	generate report in formats specified. Supported format: [xunit]
+    -R fmt[,fmt]	generate report in formats specified. Supported formats: xunit, xunit-quiet
     --large-fs		optimise scratch device for large filesystems
     -s section		run only specified section from config file
     -S section		exclude the specified section from the config file
diff --git a/common/report b/common/report
index 5ca41bc4..64f9c866 100644
--- a/common/report
+++ b/common/report
@@ -71,11 +71,16 @@ _xunit_make_testcase_report()
 	local test_name="$2"
 	local test_status="$3"
 	local test_time="$4"
+	local report_format="$5"
+	local quiet
+
+	if [ "$report_format" = xunit-quiet ]; then
+		quiet=yes
+	fi
 
 	# TODO: other places may also win if no-section mode will be named like 'default/global'
 	if [ $sect_name == '-no-sections-' ]; then
 		sect_name='global'
-
 	fi
 	local report=$tmp.report.xunit.$sect_name.xml
 
@@ -104,14 +109,16 @@ _xunit_make_testcase_report()
 			_err_msg="Test $test_name failed, reason unknown"
 		fi
 		echo -e "\t\t<failure message=\"$_err_msg\" type=\"TestFail\" />" >> $report
-		if [ -s "$full_file" ]; then
+		if [ -z "$quiet" -a -s "$full_file" ]; then
 			echo -e "\t\t<system-out>" >> $report
 			printf	'<![CDATA[\n' >>$report
 			cat "$full_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
 			printf ']]>\n'	>>$report
 			echo -e "\t\t</system-out>" >> $report
 		fi
-		if [ -f "$dmesg_file" ]; then
+		if [ -n "$quiet" ]; then
+		   :
+		elif [ -f "$dmesg_file" ]; then
 			echo -e "\t\t<system-err>" >> $report
 			printf	'<![CDATA[\n' >>$report
 			cat "$dmesg_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
@@ -144,7 +151,7 @@ _make_section_report()
 	local sect_time="$5"
 	for report in $REPORT_LIST; do
 		case "$report" in
-		"xunit")
+		"xunit"|"xunit-quiet")
 			_xunit_make_section_report "$sect_name" "$tests_count" \
 						   "$bad_count" "$notrun_count" \
 						   "$sect_time"
@@ -164,9 +171,9 @@ _make_testcase_report()
 	local test_time="$4"
 	for report in $REPORT_LIST; do
 		case "$report" in
-		"xunit")
+		"xunit"|"xunit-quiet")
 			_xunit_make_testcase_report "$sect_name" "$test_seq" \
-						    "$test_status" "$test_time"
+						    "$test_status" "$test_time" "$report"
 			;;
 		*)
 			_dump_err "report format '$report' is not supported"
@@ -178,7 +185,7 @@ _make_testcase_report()
 _assert_report_list() {
 	for report in $REPORT_LIST; do
 		case "$report" in
-		"xunit")
+		"xunit"|"xunit-quiet")
 			;;
 		*)
 			_fatal "report format '$report' is not supported"
-- 
2.31.0

Re: [PATCH 1/4] report: add support for the xunit-quiet format

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 12:43:53PM -0400, Theodore Ts'o wrote:
> The xunit-quiet format excludes the NNN.{full,dmesg,bad} files in
> <system-out> and <system-err> nodes which are included in the xunit
> report format.
> 
> For test runners that save the entire results directory to preserve
> all of the test artifacts, capturing the NNN.{full,dmesg,bad} in the
> results.xml file is redundant.  In addition, if the NNN.bad is too
> large, it can cause the junitparser python library to refuse to parse
> the XML file to prevent potential denial of service attacks[1].  A
> simple way to avoid this problem is to simply to omit the <system-out>
> and <system-err> nodes in the results.xml file.
> 
> [1] https://gitlab.com/gitlab-org/gitlab/-/issues/268035
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reviewed-by: David Disseldorp <ddiss@suse.de>

Looks fine to me, though I wonder if we should document what these
report formats actually do?

I really dislike the "xunit" name since AFAICT it's really the junit xml
format, not the xunit xml format, and this trips me up **every** single
time I have to go look at the fstests reporting code.

For this bit though,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  check         |  2 +-
>  common/report | 21 ++++++++++++++-------
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/check b/check
> index 5f6d86b4..0b2f10ed 100755
> --- a/check
> +++ b/check
> @@ -75,7 +75,7 @@ check options
>      -I <n>		iterate the test list <n> times, but stops iterating further in case of any test failure
>      -d			dump test output to stdout
>      -b			brief test summary
> -    -R fmt[,fmt]	generate report in formats specified. Supported format: [xunit]
> +    -R fmt[,fmt]	generate report in formats specified. Supported formats: xunit, xunit-quiet
>      --large-fs		optimise scratch device for large filesystems
>      -s section		run only specified section from config file
>      -S section		exclude the specified section from the config file
> diff --git a/common/report b/common/report
> index 5ca41bc4..64f9c866 100644
> --- a/common/report
> +++ b/common/report
> @@ -71,11 +71,16 @@ _xunit_make_testcase_report()
>  	local test_name="$2"
>  	local test_status="$3"
>  	local test_time="$4"
> +	local report_format="$5"
> +	local quiet
> +
> +	if [ "$report_format" = xunit-quiet ]; then
> +		quiet=yes
> +	fi
>  
>  	# TODO: other places may also win if no-section mode will be named like 'default/global'
>  	if [ $sect_name == '-no-sections-' ]; then
>  		sect_name='global'
> -
>  	fi
>  	local report=$tmp.report.xunit.$sect_name.xml
>  
> @@ -104,14 +109,16 @@ _xunit_make_testcase_report()
>  			_err_msg="Test $test_name failed, reason unknown"
>  		fi
>  		echo -e "\t\t<failure message=\"$_err_msg\" type=\"TestFail\" />" >> $report
> -		if [ -s "$full_file" ]; then
> +		if [ -z "$quiet" -a -s "$full_file" ]; then
>  			echo -e "\t\t<system-out>" >> $report
>  			printf	'<![CDATA[\n' >>$report
>  			cat "$full_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
>  			printf ']]>\n'	>>$report
>  			echo -e "\t\t</system-out>" >> $report
>  		fi
> -		if [ -f "$dmesg_file" ]; then
> +		if [ -n "$quiet" ]; then
> +		   :
> +		elif [ -f "$dmesg_file" ]; then
>  			echo -e "\t\t<system-err>" >> $report
>  			printf	'<![CDATA[\n' >>$report
>  			cat "$dmesg_file" | tr -dc '[:print:][:space:]' | encode_xml >>$report
> @@ -144,7 +151,7 @@ _make_section_report()
>  	local sect_time="$5"
>  	for report in $REPORT_LIST; do
>  		case "$report" in
> -		"xunit")
> +		"xunit"|"xunit-quiet")
>  			_xunit_make_section_report "$sect_name" "$tests_count" \
>  						   "$bad_count" "$notrun_count" \
>  						   "$sect_time"
> @@ -164,9 +171,9 @@ _make_testcase_report()
>  	local test_time="$4"
>  	for report in $REPORT_LIST; do
>  		case "$report" in
> -		"xunit")
> +		"xunit"|"xunit-quiet")
>  			_xunit_make_testcase_report "$sect_name" "$test_seq" \
> -						    "$test_status" "$test_time"
> +						    "$test_status" "$test_time" "$report"
>  			;;
>  		*)
>  			_dump_err "report format '$report' is not supported"
> @@ -178,7 +185,7 @@ _make_testcase_report()
>  _assert_report_list() {
>  	for report in $REPORT_LIST; do
>  		case "$report" in
> -		"xunit")
> +		"xunit"|"xunit-quiet")
>  			;;
>  		*)
>  			_fatal "report format '$report' is not supported"
> -- 
> 2.31.0
> 

[PATCH 2/4] generic/556: add a check to make sure ext4 supports encrypted casefolding

[Theodore Ts'o]

Some older kernels support ext4 file systems with encryption enabled,
and with casefold enabled, but not file systems have both encryption
*and* casefolding enabled.  On those kernels, generic/556 will fail.
Fortunately, we can test if ext4 supports encrypted casefold via the
presence of /sys/fs/ext4/features/encrypted_casefold.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 common/casefold   | 17 +++++++++++++++++
 tests/generic/556 |  1 +
 2 files changed, 18 insertions(+)

diff --git a/common/casefold b/common/casefold
index d9126f4c..e3e89508 100644
--- a/common/casefold
+++ b/common/casefold
@@ -43,6 +43,23 @@ _require_scratch_casefold()
 	_require_command "$LSATTR_PROG" lsattr
 }
 
+_require_encrypted_casefold ()
+{
+    case $FSTYP in
+	ext4)
+	    if test ! -f /sys/fs/ext4/features/casefold ; then
+		_notrun "casefolding not supported"
+	    fi
+	    if test ! -f /sys/fs/ext4/features/encryption ; then
+		_notrun "file system encryption not supported"
+	    fi
+	    if test ! -f /sys/fs/ext4/features/encrypted_casefold ; then
+		_notrun "encrypted casefolding not supported"
+	    fi
+	    ;;
+	esac
+}
+
 _scratch_mkfs_casefold()
 {
 	case $FSTYP in
diff --git a/tests/generic/556 b/tests/generic/556
index 404a3243..80563502 100755
--- a/tests/generic/556
+++ b/tests/generic/556
@@ -13,6 +13,7 @@ _begin_fstest auto quick casefold
 . ./common/attr
 
 _supported_fs generic
+_require_encrypted_casefold
 _require_scratch_nocheck
 _require_scratch_casefold
 _require_symlinks
-- 
2.31.0

Re: [PATCH 2/4] generic/556: add a check to make sure ext4 supports encrypted casefolding

[Eric Biggers]

Hi Ted,

On Wed, Jul 20, 2022 at 12:43:54PM -0400, Theodore Ts'o wrote:
> Some older kernels support ext4 file systems with encryption enabled,
> and with casefold enabled, but not file systems have both encryption
> *and* casefolding enabled.  On those kernels, generic/556 will fail.
> Fortunately, we can test if ext4 supports encrypted casefold via the
> presence of /sys/fs/ext4/features/encrypted_casefold.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

Please see my previous comments on this same patch:

    * https://lore.kernel.org/fstests/YNDOdrW0cEOhC9rZ@gmail.com
    * https://lore.kernel.org/fstests/YmGcDdLRlfSwUxmZ@sol.localdomain

- Eric

[PATCH 3/4] dmflakey: don't run dmflakey tests with an external log device

[Theodore Ts'o]

dmflakey works by dropping all writes before unmounting to simulate a
crash/power loss.  This doesn't work if there is an external log
device, since we only drop writes to the primary block device, and not
the external log device.  Fixing this for real would require somehow
arranging to atomically loading a new dmflakey table for two block
devices at the same time, so for now, just skip tests using dmflakey
if the external log device is enabled.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 common/dmflakey | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/dmflakey b/common/dmflakey
index 52da3b10..c2be78e9 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -12,6 +12,10 @@ _init_flakey()
 {
 	# Scratch device
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
+
+	if test "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ; then
+		_notrun "dmflakey tests don't work with an external log device"
+	fi
 	FLAKEY_DEV=/dev/mapper/flakey-test
 	FLAKEY_TABLE="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 180 0"
 	FLAKEY_TABLE_DROP="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 drop_writes"
-- 
2.31.0

Re: [PATCH 3/4] dmflakey: don't run dmflakey tests with an external log device

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 12:43:55PM -0400, Theodore Ts'o wrote:
> dmflakey works by dropping all writes before unmounting to simulate a
> crash/power loss.  This doesn't work if there is an external log
> device, since we only drop writes to the primary block device, and not
> the external log device.  Fixing this for real would require somehow
> arranging to atomically loading a new dmflakey table for two block
> devices at the same time, so for now, just skip tests using dmflakey
> if the external log device is enabled.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  common/dmflakey | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/common/dmflakey b/common/dmflakey
> index 52da3b10..c2be78e9 100644
> --- a/common/dmflakey
> +++ b/common/dmflakey
> @@ -12,6 +12,10 @@ _init_flakey()
>  {
>  	# Scratch device
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> +
> +	if test "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ; then
> +		_notrun "dmflakey tests don't work with an external log device"
> +	fi

I'm kinda surprised this still doesn't work -- last year Eryu merged a
patch[1] from me that's supposed to make common/dmflakey work with
external log devices.  What kind of failures do you see?

--D

[1] 9c1f3149 ("dmflakey: support external log and realtime devices")

>  	FLAKEY_DEV=/dev/mapper/flakey-test
>  	FLAKEY_TABLE="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 180 0"
>  	FLAKEY_TABLE_DROP="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 drop_writes"
> -- 
> 2.31.0
> 

[PATCH 4/4] generic/475: skip test when using an external log device

[Theodore Ts'o]

This test tests log recovery by kicking off fstress on the scratch fs,
then switching out the underlying the device with dm-error to see what
happens when the disk goes down.  But when there is an external log
device writes to the log device still succeed, while writes to the
rest of the file system fail, and this leads to file system
corruption.  So skip this test when there is an external log.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/generic/475 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/generic/475 b/tests/generic/475
index c426402e..43c26303 100755
--- a/tests/generic/475
+++ b/tests/generic/475
@@ -34,6 +34,10 @@ _require_scratch
 _require_dm_target error
 _require_command "$KILLALL_PROG" "killall"
 
+if test "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ; then
+    _notrun "Cannot have external log device"
+fi
+
 echo "Silence is golden."
 
 _scratch_mkfs >> $seqres.full 2>&1
-- 
2.31.0

Re: [PATCH 4/4] generic/475: skip test when using an external log device

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 12:43:56PM -0400, Theodore Ts'o wrote:
> This test tests log recovery by kicking off fstress on the scratch fs,
> then switching out the underlying the device with dm-error to see what
> happens when the disk goes down.  But when there is an external log
> device writes to the log device still succeed, while writes to the
> rest of the file system fail, and this leads to file system
> corruption.  So skip this test when there is an external log.

Hm, could you review the congruent operation length series[1], please?

Once that's done, I can move on to the next push, which is fixing up
dmerror[2] and fail-make-request[3] to handle external devices:

[1] https://lore.kernel.org/fstests/165826663647.3249494.13640199673218669145.stgit@magnolia/T/#t
[2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=dmerror-on-rt-devices
[3] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fix-fail-make-reqest

--D

> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  tests/generic/475 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/475 b/tests/generic/475
> index c426402e..43c26303 100755
> --- a/tests/generic/475
> +++ b/tests/generic/475
> @@ -34,6 +34,10 @@ _require_scratch
>  _require_dm_target error
>  _require_command "$KILLALL_PROG" "killall"
>  
> +if test "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ; then
> +    _notrun "Cannot have external log device"
> +fi
> +
>  echo "Silence is golden."
>  
>  _scratch_mkfs >> $seqres.full 2>&1
> -- 
> 2.31.0
>