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

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



于 2020/7/16 0:07, Darrick J. Wong 写道:
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?
Hi Darrick,

Right :-) . My change wants to achieve it.

Thanks,
Xiao Yang
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



.




.







[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux