Re: [PATCH v2.1 1/2] check: detect and preserve all coredumps made by a test

[Zorro Lang <zlang@redhat.com>]

On Wed, Oct 12, 2022 at 05:19:50PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If someone sets kernel.core_uses_pid (or kernel.core_pattern), any
> coredumps generated by fstests might have names that are longer than
> just "core".  Since the pid isn't all that useful by itself, let's
> record the coredumps by hash when we save them, so that we don't waste
> space storing identical crash dumps.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v2.1: use REPORT_DIR per maintainer suggestion
> ---

This version looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  check     |   26 ++++++++++++++++++++++----
>  common/rc |   16 ++++++++++++++++
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/check b/check
> index d587a70546..29303db1c8 100755
> --- a/check
> +++ b/check
> @@ -923,11 +923,19 @@ function run_section()
>  			sts=$?
>  		fi
>  
> -		if [ -f core ]; then
> -			_dump_err_cont "[dumped core]"
> -			mv core $RESULT_BASE/$seqnum.core
> +		# If someone sets kernel.core_pattern or kernel.core_uses_pid,
> +		# coredumps generated by fstests might have a longer name than
> +		# just "core".  Use globbing to find the most common patterns,
> +		# assuming there are no other coredump capture packages set up.
> +		local cores=0
> +		for i in core core.*; do
> +			test -f "$i" || continue
> +			if ((cores++ == 0)); then
> +				_dump_err_cont "[dumped core]"
> +			fi
> +			_save_coredump "$i"
>  			tc_status="fail"
> -		fi
> +		done
>  
>  		if [ -f $seqres.notrun ]; then
>  			$timestamp && _timestamp
> @@ -960,6 +968,16 @@ function run_section()
>  			# of the check script itself.
>  			(_adjust_oom_score 250; _check_filesystems) || tc_status="fail"
>  			_check_dmesg || tc_status="fail"
> +
> +			# Save any coredumps from the post-test fs checks
> +			for i in core core.*; do
> +				test -f "$i" || continue
> +				if ((cores++ == 0)); then
> +					_dump_err_cont "[dumped core]"
> +				fi
> +				_save_coredump "$i"
> +				tc_status="fail"
> +			done
>  		fi
>  
>  		# Reload the module after each test to check for leaks or
> diff --git a/common/rc b/common/rc
> index d877ac77a0..2e1891180a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4949,6 +4949,22 @@ _create_file_sized()
>  	return $ret
>  }
>  
> +_save_coredump()
> +{
> +	local path="$1"
> +
> +	local core_hash="$(_md5_checksum "$path")"
> +	local out_file="$REPORT_DIR/$seqnum.core.$core_hash"
> +
> +	if [ -s "$out_file" ]; then
> +		rm -f "$path"
> +		return
> +	fi
> +	rm -f "$out_file"
> +
> +	mv "$path" "$out_file"
> +}
> +
>  init_rc
>  
>  ################################################################################
>