Re: [PATCH 2/2] check: capture dmesg of mount failures if test fails

On Fri, Apr 17, 2026 at 01:56:23AM +0800, Zorro Lang wrote:
> On Mon, Apr 13, 2026 at 10:51:25AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <[email protected]>
> > 
> > Capture the kernel output after a mount failure occurs.  If the test
> > itself fails, then keep the logging output for further diagnosis.
> > 
> > Signed-off-by: "Darrick J. Wong" <[email protected]>
> > ---
> >  check                  |   22 +++++++++++++++++++++-
> >  common/rc              |   26 +++++++++++++++++++++++++-
> >  common/report          |    8 ++++++++
> >  doc/xunit.xsd          |   12 +++++++++++-
> >  tests/selftest/008     |   20 ++++++++++++++++++++
> >  tests/selftest/008.out |    1 +
> >  6 files changed, 86 insertions(+), 3 deletions(-)
> >  create mode 100755 tests/selftest/008
> >  create mode 100644 tests/selftest/008.out
> > 
> > 
> > diff --git a/check b/check
> > index cd7a79347eac28..c37921ea58e558 100755
> > --- a/check
> > +++ b/check
> > @@ -608,7 +608,7 @@ _stash_fail_loop_files() {
> >  	local seq_prefix="${REPORT_DIR}/${1}"
> >  	local cp_suffix="$2"
> >  
> > -	for i in ".full" ".dmesg" ".out.bad" ".notrun" ".core" ".hints"; do
> > +	for i in ".full" ".dmesg" ".out.bad" ".notrun" ".core" ".hints" ".mountfail"; do
> >  		rm -f "${seq_prefix}${i}${cp_suffix}"
> >  		if [ -f "${seq_prefix}${i}" ]; then
> >  			cp "${seq_prefix}${i}" "${seq_prefix}${i}${cp_suffix}"
> > @@ -982,6 +982,7 @@ function run_section()
> >  				      echo -n "	$seqnum -- "
> >  			cat $seqres.notrun
> >  			tc_status="notrun"
> > +			rm -f "$seqres.mountfail?"
> >  			_stash_test_status "$seqnum" "$tc_status"
> >  
> >  			# Unmount the scratch fs so that we can wipe the scratch
> > @@ -1045,6 +1046,7 @@ function run_section()
> >  		if [ ! -f $seq.out ]; then
> >  			_dump_err "no qualified output"
> >  			tc_status="fail"
> > +			rm -f "$seqres.mountfail?"
> >  			_stash_test_status "$seqnum" "$tc_status"
> >  			continue;
> >  		fi
> > @@ -1081,6 +1083,24 @@ function run_section()
> >  				rm -f $seqres.hints
> >  			fi
> >  		fi
> > +
> > +		if [ -f "$seqres.mountfail?" ]; then
> > +			if [ "$tc_status" = "fail" ]; then
> > +				# Let the user know if there were mount
> > +				# failures on a test that failed because that
> > +				# could be interesting.
> > +				mv "$seqres.mountfail?" "$seqres.mountfail"
> > +				_dump_err "check: possible mount failures (see $seqres.mountfail)"
> > +				test -f $seqres.mountfail && \
> > +					maybe_compress_logfile $seqres.mountfail $MAX_MOUNTFAIL_SIZE
>                                         ^^^^^^^^^^^^^^^^^^^^^^
> What's the "maybe_compress_logfile" ?

Oops, that's part of another debugging patch that isn't in this batch.

I'll clear it out.

> > +			else
> > +				# Don't retain mount failure logs for tests
> > +				# that pass or were skipped because some tests
> > +				# intentionally drive mount failures.
> > +				rm -f "$seqres.mountfail?"
> > +			fi
> > +		fi
> > +
> >  		_stash_test_status "$seqnum" "$tc_status"
> >  	done
> >  
> > diff --git a/common/rc b/common/rc
> > index 5fe44e2158ffb3..18ceda1b32cd63 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -288,9 +288,33 @@ _get_hugepagesize()
> >  	awk '/Hugepagesize/ {print $2 * 1024}' /proc/meminfo
> >  }
> >  
> > +# Does dmesg have a --since flag?
> > +_dmesg_detect_since()
> > +{
> > +	if [ -z "$DMESG_HAS_SINCE" ]; then
> > +		test "$DMESG_HAS_SINCE" = "yes"
>                        ^^^^^^^^^^^^^^^
> I can't find you use this parameter anywhere. But you do:

That's exactly where DMESG_HAS_SINCE gets used.

> 
>   if _dmesg_detect_since; then
>           dmesg --since '30s ago' >> "$seqres.mountfail?"
>   else
>           dmesg | tail -n 100 >> "$seqres.mountfail?"
>   fi
> 
> do you want to treat this function as below?
> 
>   _has_dmesg_since_option()
>   {
>           if grep -q -- --since <(dmesg --help);then
>                   return 0
>           fi
>           return 1
>   }

_has_dmesg_since_option()
{
	grep -q -- --since <(dmesg --help)
}

would be shorter?

> 
> > +		return
> > +	elif dmesg --help | grep -q -- --since; then
> > +		DMESG_HAS_SINCE=yes
> > +	else
> > +		DMESG_HAS_SINCE=no

...as this case is broken anyway. :(

Will resend with less cruft

--D

> > +	fi
> > +}
> > +
> >  _mount()
> >  {
> > -    $MOUNT_PROG $*
> > +	$MOUNT_PROG $*
> > +	ret=$?
> > +	if [ "$ret" -ne 0 ]; then
> > +		echo "\"$MOUNT_PROG $*\" failed at $(date)" >> "$seqres.mountfail?"
> > +		if _dmesg_detect_since; then
> > +			dmesg --since '30s ago' >> "$seqres.mountfail?"
> > +		else
> > +			dmesg | tail -n 100 >> "$seqres.mountfail?"
> > +		fi
> > +	fi
> > +
> > +	return $ret
> >  }
> >  
> >  # Call _mount to do mount operation but also save mountpoint to
> > diff --git a/common/report b/common/report
> > index 7128bbebac8b75..a41a58f790b784 100644
> > --- a/common/report
> > +++ b/common/report
> > @@ -199,6 +199,7 @@ _xunit_make_testcase_report()
> >  		local out_src="${SRC_DIR}/${test_name}.out"
> >  		local full_file="${REPORT_DIR}/${test_name}.full"
> >  		local dmesg_file="${REPORT_DIR}/${test_name}.dmesg"
> > +		local mountfail_file="${REPORT_DIR}/${test_name}.mountfail"
> >  		local outbad_file="${REPORT_DIR}/${test_name}.out.bad"
> >  		if [ -z "$_err_msg" ]; then
> >  			_err_msg="Test $test_name failed, reason unknown"
> > @@ -225,6 +226,13 @@ _xunit_make_testcase_report()
> >  			printf ']]>\n'	>>$report
> >  			echo -e "\t\t</system-err>" >> $report
> >  		fi
> > +		if [ -z "$quiet" -a -f "$mountfail_file" ]; then
> > +			echo -e "\t\t<mount-failure>" >> $report
> > +			printf	'<![CDATA[\n' >>$report
> > +			cat "$mountfail_file" | tr -dc '[:print:][:space:]' | encode_cdata >>$report
> > +			printf ']]>\n'	>>$report
> > +			echo -e "\t\t</mount-failure>" >> $report
> > +		fi
> >  		;;
> >  	*)
> >  		echo -e "\t\t<failure message=\"Unknown test_status=$test_status\" type=\"TestFail\"/>" >> $report
> > diff --git a/doc/xunit.xsd b/doc/xunit.xsd
> > index d287eaf5a25fb6..efe0badbb338b5 100644
> > --- a/doc/xunit.xsd
> > +++ b/doc/xunit.xsd
> > @@ -131,7 +131,7 @@
> >                                  </xs:complexType>
> >                              </xs:element>
> >                          </xs:choice>
> > -                        <xs:choice minOccurs="0" maxOccurs="3">
> > +                        <xs:choice minOccurs="0" maxOccurs="4">
> >                              <xs:element name="system-out" minOccurs="0" maxOccurs="1">
> >                                  <xs:annotation>
> >                                      <xs:documentation xml:lang="en">Data that was written to the .full log file while the test was executed.</xs:documentation>
> > @@ -162,6 +162,16 @@
> >                                      </xs:restriction>
> >                                  </xs:simpleType>
> >                              </xs:element>
> > +                            <xs:element name="mount-failure" minOccurs="0" maxOccurs="1">
> > +                                <xs:annotation>
> > +                                    <xs:documentation xml:lang="en">Kernel log recorded when mount failed.</xs:documentation>
> > +                                </xs:annotation>
> > +                                <xs:simpleType>
> > +                                    <xs:restriction base="pre-string">
> > +                                        <xs:whiteSpace value="preserve"/>
> > +                                    </xs:restriction>
> > +                                </xs:simpleType>
> > +                            </xs:element>
> >                          </xs:choice>
> >                      </xs:sequence>
> >                      <xs:attribute name="name" type="xs:token" use="required">
> > diff --git a/tests/selftest/008 b/tests/selftest/008
> > new file mode 100755
> > index 00000000000000..bcd1f13ed27778
> > --- /dev/null
> > +++ b/tests/selftest/008
> > @@ -0,0 +1,20 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2024-2026 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test 008
> > +#
> > +# Test mount failure capture.
> > +#
> > +. ./common/preamble
> > +_begin_fstest selftest
> > +
> > +_require_command "$WIPEFS_PROG" wipefs
> > +_require_scratch
> > +
> > +$WIPEFS_PROG -a $SCRATCH_DEV
> > +_scratch_mount &>> $seqres.full
> > +
> 
> echo "Silence is golden"
> 
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/selftest/008.out b/tests/selftest/008.out
> > new file mode 100644
> > index 00000000000000..aaff95f3f48372
> > --- /dev/null
> > +++ b/tests/selftest/008.out
> > @@ -0,0 +1 @@
> > +QA output created by 008
> 
> Silence is golden
> 
> > 
>