Re: [PATCH 2/2] check: optionally compress core dumps

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Oct 13, 2022 at 07:51:02PM +0800, Zorro Lang wrote:
> On Tue, Oct 11, 2022 at 06:45:18PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Add a new option, COREDUMP_COMPRESSOR, that will be used to compress
> > core dumps collected during a fstests run.  The program specified must
> > accept the -f -9 arguments that gzip has.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  README    |    4 ++++
> >  common/rc |   14 +++++++++-----
> >  2 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > 
> > diff --git a/README b/README
> > index 80d148be82..4c4f22f853 100644
> > --- a/README
> > +++ b/README
> > @@ -212,6 +212,10 @@ Tools specification:
> >      - Set FSSTRESS_AVOID and/or FSX_AVOID, which contain options added to
> >        the end of fsstresss and fsx invocations, respectively, in case you wish
> >        to exclude certain operational modes from these tests.
> > + - core dumps:
> > +    - Set COREDUMP_COMPRESSOR to a compression program to compress crash dumps.
> > +      This program must accept '-f' and the name of a file to compress.  In
> > +      other words, it must emulate gzip.
> >  
> >  Kernel/Modules related configuration:
> >   - Set TEST_FS_MODULE_RELOAD=1 to unload the module and reload it between
> > diff --git a/common/rc b/common/rc
> > index 152b8bb414..c68869b7dc 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -4956,13 +4956,17 @@ _save_coredump()
> >  	local core_hash="$(_md5_checksum "$path")"
> >  	local out_file="$RESULT_BASE/$seqnum.core.$core_hash"
> >  
> > -	if [ -s "$out_file" ]; then
> > -		rm -f "$path"
> > -		return
> > -	fi
> > -	rm -f "$out_file"
> > +	for dump in "$out_file"*; do
> > +		if [ -s "$dump" ]; then
> > +			rm -f "$path"
> > +			return 0
> > +		fi
> > +	done
> >  
> >  	mv "$path" "$out_file"
> > +	test -z "$COREDUMP_COMPRESSOR" && return 0
> > +
> > +	$COREDUMP_COMPRESSOR -f "$out_file"
> 
> This patch looks good to me,
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> I'm just not sure if all/most compressor supports "-f" option, I use bzip2
> and gzip mostly, they both support that.

As do xz, lz4, and zstd, so I think that's sufficient coverage.

The only one I know of that won't be compatible is zip, since it uses -f
for "freshen archive".

--D

> Thanks,
> Zorro
> 
> >  }
> >  
> >  init_rc
> > 
>