Re: [PATCH] common/config: Fix use of MKFS_OPTIONS

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

On Fri, Jun 24, 2022 at 12:08:25AM +0800, An Long wrote:
> _scratch_mkfs_sized() only receive integer number of bytes as a valid
> input. But if the MKFS_OPTIONS variable exists, it will use the value of
> block size in MKFS_OPTIONS to override input. In case of
> MKFS_OPTIONS="-b 4k", would result in blocksize=4 but not 4096. This
> will give errors to ext2/3/4 etc, and brings potential bugs to xfs or
> btrfs.
> 
> Therefore, add validation check to force MKFS_OPTIONS to use pure
> integer in bytes for any size value.
> 
> Signed-off-by: An Long <lan@suse.com>
> ---
>  README        | 3 ++-
>  common/config | 5 +++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/README b/README
> index 80d148be..7c2d3334 100644
> --- a/README
> +++ b/README
> @@ -241,7 +241,8 @@ Misc:
>     this option is supported for all filesystems currently only -overlay is
>     expected to run without issues. For other filesystems additional patches
>     and fixes to the test suite might be needed.
> -
> + - If MKFS_OPTIONS is used, the size value must be an integer number of bytes
> +   without units. For example, set MKFS_OPTIONS="-b 4096" but not "-b 4k".

IDGI.  mkfs.$FSTYP does its own input validation, which means that fstest
runners are required to set MKFS_OPTIONS to something that mkfs will
accept.  It's not the job of fstests to add /another/ layer of
validation...

>  ______________________
>  USING THE FSQA SUITE
>  ______________________
> diff --git a/common/config b/common/config
> index de3aba15..ec723ac4 100644
> --- a/common/config
> +++ b/common/config
> @@ -896,5 +896,10 @@ else
>  	fi
>  fi
>  
> +# check the size value in $MKFS_OPTIONS is an integer
> +if [[ $MKFS_OPTIONS =~ [0-9]+[^0-9\ ] ]] ; then

...because this regex will break /any/ MKFS_OPTIONS with a number
followed by a letter.  mkfs.xfs handles units just fine, so I don't
understand why you're adding this blunt instrument.

MKFS_OPTIONS='-b size=1k' works just fine for XFS.

--D

> +        _fatal "error: \$MKFS_OPTIONS: Only number in bytes is allowed, no units."
> +fi
> +
>  # make sure this script returns success
>  /bin/true
> -- 
> 2.35.3
>