Re: [PATCH v6 3/7] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly

[Ira Weiny <ira.weiny@intel.com>]

On Tue, Jul 14, 2020 at 05:40:05PM +0800, Xiao Yang wrote:
> ext4 can accept the last one if the same mkfs options are passed but xfs cannot

I'm having trouble parsing this commit message.  What does 'last one' refer to?

> accept the same mkfs options and reports "xxx option is respecified" error.

Ok I think I understand now.  Some FS's (XFS) do not accept an option more than
once.  So we can't just blindly add options to the end of the MKFS_OPTIONS.  Is
that correct?

> I
> prefer to override the same mkfs option which is defined in MKFS_OPTION so that
> we can have a chance to pass other mkfs options to _scratch_mkfs_geom().

Instead the patch parses the current option string and replaces the value if
the option is already there.  This allows us to specify MKFS_OPTIONS to
generic/223.

I think the code is reasonable although my sed skills are not good enough to
tell for sure...  ;-)

Ira

> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  common/rc         | 14 +++++++++++++-
>  tests/generic/223 |  1 -
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 6c908f2e..567cf83b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1051,7 +1051,19 @@ _scratch_mkfs_geom()
>  
>      case $FSTYP in
>      xfs)
> -	MKFS_OPTIONS+=" -b size=$blocksize, -d su=$sunit_bytes,sw=$swidth_mult"
> +	if echo "$MKFS_OPTIONS" | egrep -q "b?size="; then
> +		MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r "s/(b?size=)[0-9]+/\1$blocksize/")
> +	else
> +		MKFS_OPTIONS+=" -b size=$blocksize"
> +	fi
> +
> +	if echo "$MKFS_OPTIONS" | egrep -q "(su|sunit|sw|swidth)="; then
> +		MKFS_OPTIONS=$(echo "$MKFS_OPTIONS" | sed -r \
> +			-e "s/(su|sunit)=[0-9kmg]+/su=$sunit_bytes/" \
> +			-e "s/(sw|swidth)=[0-9kmg]+/sw=$swidth_mult/")
> +	else
> +		MKFS_OPTIONS+=" -d su=$sunit_bytes,sw=$swidth_mult"
> +	fi
>  	;;
>      ext4|ext4dev)
>  	MKFS_OPTIONS+=" -b $blocksize -E stride=$sunit_blocks,stripe_width=$swidth_blocks"
> diff --git a/tests/generic/223 b/tests/generic/223
> index 6cfd00dd..ba7c9a44 100755
> --- a/tests/generic/223
> +++ b/tests/generic/223
> @@ -41,7 +41,6 @@ for SUNIT_K in 8 16 32 64 128; do
>  	let SUNIT_BLOCKS=$SUNIT_BYTES/$BLOCKSIZE
>  
>  	echo "=== mkfs with su $SUNIT_BLOCKS blocks x 4 ==="
> -	export MKFS_OPTIONS=""
>  	_scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1
>  	_scratch_mount
>  
> -- 
> 2.21.0
> 
> 
>