On Thu, Jun 23, 2022 at 11:17:44AM -0700, Darrick J. Wong wrote: > 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@xxxxxxxx> > > --- > > 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. Hi Darrick, That's another story from this patch review: https://lore.kernel.org/fstests/20220616043845.14320-1-lan@xxxxxxxx/ Any more review points about that? Thanks, Zorro > > --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 > > >
