On Wed, Jul 15, 2020 at 11:12:36AM +0800, Xiao Yang wrote: > On 2020/7/15 10:31, Ira Weiny wrote: > > 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? > Hi Ira, > > 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. > Right. :-) > > XFS_MKFS_OPTIONS/MKFS_OPTIONS can be used to specify some custom options by user, > so I don't want to clear it blindly. > > Thanks, > Xiao Yang > > 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@xxxxxxxxxxxxxx> > > > --- > > > 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 ...ok, I see, this makes the function smart enough to substitute geometry parameters instead of dumping them on the end and letting that blow up. Heh, ok, that's definitely a weird quirk I've noticed. > > > ;; > > > 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="" So I guess you're deleting this so that the test runs with whatever MKFS_OPTIONS the test runner specified, while letting the test edit blocksize and stripe parameters? Proving I'm still bad at remembering to read commit messages, Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > > > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE>> $seqres.full 2>&1 > > > _scratch_mount > > > > > > -- > > > 2.21.0 > > > > > > > > > > > > > . > > > > >
