From: "Darrick J. Wong" <djwong@kernel.org>
To: An Long <lan@suse.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH] common/config: Fix use of MKFS_OPTIONS
Date: Thu, 23 Jun 2022 11:17:44 -0700 [thread overview]
Message-ID: <YrSuSKLKA6LMjyf/@magnolia> (raw)
In-Reply-To: <20220623160825.31788-1-lan@suse.com>
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
>
next prev parent reply other threads:[~2022-06-23 19:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-23 16:08 [PATCH] common/config: Fix use of MKFS_OPTIONS An Long
2022-06-23 18:17 ` Darrick J. Wong [this message]
2022-06-24 2:14 ` Zorro Lang
2022-06-24 2:41 ` Dave Chinner
2022-06-24 3:29 ` Long An
2022-06-25 3:15 ` Darrick J. Wong
2022-06-25 16:38 ` Zorro Lang
2022-06-25 16:39 ` Theodore Ts'o
2022-06-25 22:47 ` Dave Chinner
2022-06-26 2:09 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YrSuSKLKA6LMjyf/@magnolia \
--to=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=lan@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.