* [PATCH 0/2] fstests: fix test issue of xfs/157
@ 2024-11-16 19:07 Zorro Lang
2024-11-16 19:07 ` [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments Zorro Lang
2024-11-16 19:08 ` [PATCH 2/2] xfs/157: do not drop necessary mkfs options Zorro Lang
0 siblings, 2 replies; 10+ messages in thread
From: Zorro Lang @ 2024-11-16 19:07 UTC (permalink / raw)
To: fstests; +Cc: linux-xfs
xfs/157 started to fail since 2f7e1b8a6f09 ("xfs/157,xfs/547,xfs/548: switch to
using _scratch_mkfs_sized") was merged.
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64
MKFS_OPTIONS -- -f -m rmapbt=1 /dev/sda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/scratch
xfs/157 7s ... - output mismatch (see /root/git/xfstests/results//xfs/157.out.bad)
--- tests/xfs/157.out 2024-11-01 01:05:03.664543576 +0800
+++ /root/git/xfstests/results//xfs/157.out.bad 2024-11-01 02:56:47.994007900 +0800
@@ -6,10 +6,10 @@
label = "oldlabel"
label = "newlabel"
S3: Check that setting with rtdev works
-label = "oldlabel"
+label = ""
label = "newlabel"
S4: Check that setting with rtdev + logdev works
...
(Run 'diff -u /root/git/xfstests/tests/xfs/157.out /root/git/xfstests/results//xfs/157.out.bad' to see the entire diff)
Ran: xfs/157
Failures: xfs/157
Failed 1 of 1 tests
Before that change, the _scratch_mkfs can drop "rmapbt=1" option from $MKFS_OPTIONS,
only keep the "-L label" option. That's why this test never failed before. To fix
this failure, this patchset does:
The [PATCH 1/2] helps _scratch_mkfs_sized to support extra argument, then the
extra argument can be sent to _scratch_mkfs, which will try to "protect"
the extra argument from being dropped.
The [PATCH 2/2] invokes the new _scratch_mkfs_sized in xfs/157 to fix the
test failure of it.
Thanks,
Zorro
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments 2024-11-16 19:07 [PATCH 0/2] fstests: fix test issue of xfs/157 Zorro Lang @ 2024-11-16 19:07 ` Zorro Lang 2024-11-18 22:21 ` Darrick J. Wong 2024-11-16 19:08 ` [PATCH 2/2] xfs/157: do not drop necessary mkfs options Zorro Lang 1 sibling, 1 reply; 10+ messages in thread From: Zorro Lang @ 2024-11-16 19:07 UTC (permalink / raw) To: fstests; +Cc: linux-xfs To give more arguments to _scratch_mkfs_sized, we generally do as: MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it will get rid of the whole MKFS_OPTIONS and try to mkfs again. Likes: ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** But that's not the fault of "-L oldlabel". So for keeping the mkfs options ("-L oldlabel") we need, we'd better to let the scratch_mkfs_sized to support extra arguments, rather than using global MKFS_OPTIONS. Signed-off-by: Zorro Lang <zlang@kernel.org> --- common/rc | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/common/rc b/common/rc index 2af26f23f..ce8602383 100644 --- a/common/rc +++ b/common/rc @@ -1023,11 +1023,13 @@ _small_fs_size_mb() } # Create fs of certain size on scratch device -# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] +# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options] _try_scratch_mkfs_sized() { local fssize=$1 - local blocksize=$2 + shift + local blocksize=$1 + shift local def_blksz local blocksize_opt local rt_ops @@ -1091,10 +1093,10 @@ _try_scratch_mkfs_sized() # don't override MKFS_OPTIONS that set a block size. echo $MKFS_OPTIONS |grep -E -q "b\s*size=" if [ $? -eq 0 ]; then - _try_scratch_mkfs_xfs -d size=$fssize $rt_ops + _try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@" else _try_scratch_mkfs_xfs -d size=$fssize $rt_ops \ - -b size=$blocksize + -b size=$blocksize "$@" fi ;; ext2|ext3|ext4) @@ -1105,7 +1107,7 @@ _try_scratch_mkfs_sized() _notrun "Could not make scratch logdev" MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV" fi - ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks + ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks ;; gfs2) # mkfs.gfs2 doesn't automatically shrink journal files on small @@ -1120,13 +1122,13 @@ _try_scratch_mkfs_sized() (( journal_size >= min_journal_size )) || journal_size=$min_journal_size MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS" fi - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks ;; ocfs2) - yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks + yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks ;; udf) - $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks + $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks ;; btrfs) local mixed_opt= @@ -1134,33 +1136,33 @@ _try_scratch_mkfs_sized() # the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size() (( fssize < $((256 * 1024 * 1024)) )) && ! _scratch_btrfs_is_zoned && mixed_opt='--mixed' - $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV + $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV ;; jfs) - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks ;; reiserfs) - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks ;; reiser4) # mkfs.resier4 requires size in KB as input for creating filesystem - $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \ + $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \ `expr $fssize / 1024` ;; f2fs) # mkfs.f2fs requires # of sectors as an input for the size local sector_size=`blockdev --getss $SCRATCH_DEV` - $MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size` + $MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size` ;; tmpfs) local free_mem=`_free_memory_bytes` if [ "$free_mem" -lt "$fssize" ] ; then _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes" fi - export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" + export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS" ;; bcachefs) - $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV ;; *) _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" @@ -1170,7 +1172,7 @@ _try_scratch_mkfs_sized() _scratch_mkfs_sized() { - _try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)" + _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($@)" } # Emulate an N-data-disk stripe w/ various stripe units -- 2.45.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments 2024-11-16 19:07 ` [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments Zorro Lang @ 2024-11-18 22:21 ` Darrick J. Wong 2024-11-18 22:43 ` Darrick J. Wong 2024-11-21 9:17 ` Zorro Lang 0 siblings, 2 replies; 10+ messages in thread From: Darrick J. Wong @ 2024-11-18 22:21 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-xfs On Sun, Nov 17, 2024 at 03:07:59AM +0800, Zorro Lang wrote: > To give more arguments to _scratch_mkfs_sized, we generally do as: > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it > will get rid of the whole MKFS_OPTIONS and try to mkfs again. > Likes: > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > But that's not the fault of "-L oldlabel". So for keeping the mkfs > options ("-L oldlabel") we need, we'd better to let the > scratch_mkfs_sized to support extra arguments, rather than using > global MKFS_OPTIONS. > > Signed-off-by: Zorro Lang <zlang@kernel.org> > --- > common/rc | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/common/rc b/common/rc > index 2af26f23f..ce8602383 100644 > --- a/common/rc > +++ b/common/rc > @@ -1023,11 +1023,13 @@ _small_fs_size_mb() > } > > # Create fs of certain size on scratch device > -# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] > +# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options] > _try_scratch_mkfs_sized() > { > local fssize=$1 > - local blocksize=$2 > + shift > + local blocksize=$1 > + shift > local def_blksz > local blocksize_opt > local rt_ops > @@ -1091,10 +1093,10 @@ _try_scratch_mkfs_sized() > # don't override MKFS_OPTIONS that set a block size. > echo $MKFS_OPTIONS |grep -E -q "b\s*size=" > if [ $? -eq 0 ]; then > - _try_scratch_mkfs_xfs -d size=$fssize $rt_ops > + _try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@" > else > _try_scratch_mkfs_xfs -d size=$fssize $rt_ops \ > - -b size=$blocksize > + -b size=$blocksize "$@" > fi > ;; > ext2|ext3|ext4) > @@ -1105,7 +1107,7 @@ _try_scratch_mkfs_sized() > _notrun "Could not make scratch logdev" > MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV" > fi > - ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > + ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > ;; > gfs2) > # mkfs.gfs2 doesn't automatically shrink journal files on small > @@ -1120,13 +1122,13 @@ _try_scratch_mkfs_sized() > (( journal_size >= min_journal_size )) || journal_size=$min_journal_size > MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS" > fi > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks > ;; > ocfs2) > - yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > + yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > ;; > udf) > - $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > + $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > ;; > btrfs) > local mixed_opt= > @@ -1134,33 +1136,33 @@ _try_scratch_mkfs_sized() > # the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size() > (( fssize < $((256 * 1024 * 1024)) )) && > ! _scratch_btrfs_is_zoned && mixed_opt='--mixed' > - $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV > + $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV > ;; > jfs) > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks > ;; > reiserfs) > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > ;; > reiser4) > # mkfs.resier4 requires size in KB as input for creating filesystem > - $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \ > + $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \ > `expr $fssize / 1024` > ;; > f2fs) > # mkfs.f2fs requires # of sectors as an input for the size > local sector_size=`blockdev --getss $SCRATCH_DEV` > - $MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size` > + $MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size` > ;; > tmpfs) > local free_mem=`_free_memory_bytes` > if [ "$free_mem" -lt "$fssize" ] ; then > _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes" > fi > - export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > + export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS" > ;; > bcachefs) > - $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV > + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV > ;; > *) > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > @@ -1170,7 +1172,7 @@ _try_scratch_mkfs_sized() > > _scratch_mkfs_sized() > { > - _try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)" > + _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($@)" Nit: Don't use '$@' within a longer string -- either it's "$@" so that each element in the arg array is rendered individually as a separate string parameter to the program being called, or "foo $*" so that you end up with a single string. shellcheck will complain about that, though bash itself doesn't seem to care. --D > } > > # Emulate an N-data-disk stripe w/ various stripe units > -- > 2.45.2 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments 2024-11-18 22:21 ` Darrick J. Wong @ 2024-11-18 22:43 ` Darrick J. Wong 2024-11-21 9:17 ` Zorro Lang 1 sibling, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2024-11-18 22:43 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-xfs On Mon, Nov 18, 2024 at 02:21:36PM -0800, Darrick J. Wong wrote: > On Sun, Nov 17, 2024 at 03:07:59AM +0800, Zorro Lang wrote: > > To give more arguments to _scratch_mkfs_sized, we generally do as: > > > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > > > to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it > > will get rid of the whole MKFS_OPTIONS and try to mkfs again. > > Likes: > > > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > > > But that's not the fault of "-L oldlabel". So for keeping the mkfs > > options ("-L oldlabel") we need, we'd better to let the > > scratch_mkfs_sized to support extra arguments, rather than using > > global MKFS_OPTIONS. > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > --- > > common/rc | 34 ++++++++++++++++++---------------- > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > diff --git a/common/rc b/common/rc > > index 2af26f23f..ce8602383 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -1023,11 +1023,13 @@ _small_fs_size_mb() > > } > > > > # Create fs of certain size on scratch device > > -# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] > > +# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options] > > _try_scratch_mkfs_sized() > > { > > local fssize=$1 > > - local blocksize=$2 > > + shift > > + local blocksize=$1 > > + shift > > local def_blksz > > local blocksize_opt > > local rt_ops > > @@ -1091,10 +1093,10 @@ _try_scratch_mkfs_sized() > > # don't override MKFS_OPTIONS that set a block size. > > echo $MKFS_OPTIONS |grep -E -q "b\s*size=" > > if [ $? -eq 0 ]; then > > - _try_scratch_mkfs_xfs -d size=$fssize $rt_ops > > + _try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@" > > else > > _try_scratch_mkfs_xfs -d size=$fssize $rt_ops \ > > - -b size=$blocksize > > + -b size=$blocksize "$@" > > fi > > ;; > > ext2|ext3|ext4) > > @@ -1105,7 +1107,7 @@ _try_scratch_mkfs_sized() > > _notrun "Could not make scratch logdev" > > MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV" > > fi > > - ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > gfs2) > > # mkfs.gfs2 doesn't automatically shrink journal files on small > > @@ -1120,13 +1122,13 @@ _try_scratch_mkfs_sized() > > (( journal_size >= min_journal_size )) || journal_size=$min_journal_size > > MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS" > > fi > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > ocfs2) > > - yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > udf) > > - $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > btrfs) > > local mixed_opt= > > @@ -1134,33 +1136,33 @@ _try_scratch_mkfs_sized() > > # the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size() > > (( fssize < $((256 * 1024 * 1024)) )) && > > ! _scratch_btrfs_is_zoned && mixed_opt='--mixed' > > - $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV > > + $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV > > ;; > > jfs) > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks > > ;; > > reiserfs) > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > reiser4) > > # mkfs.resier4 requires size in KB as input for creating filesystem > > - $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \ > > + $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \ > > `expr $fssize / 1024` > > ;; > > f2fs) > > # mkfs.f2fs requires # of sectors as an input for the size > > local sector_size=`blockdev --getss $SCRATCH_DEV` > > - $MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size` > > + $MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size` > > ;; > > tmpfs) > > local free_mem=`_free_memory_bytes` > > if [ "$free_mem" -lt "$fssize" ] ; then > > _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes" > > fi > > - export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > > + export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS" > > ;; > > bcachefs) > > - $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV > > + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV > > ;; > > *) > > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > > @@ -1170,7 +1172,7 @@ _try_scratch_mkfs_sized() > > > > _scratch_mkfs_sized() > > { > > - _try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)" > > + _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($@)" > > Nit: Don't use '$@' within a longer string -- either it's "$@" so that > each element in the arg array is rendered individually as a separate > string parameter to the program being called, or "foo $*" so that you > end up with a single string. > > shellcheck will complain about that, though bash itself doesn't seem to > care. Oh, also, as I was rebasing my branch atop yours, I wondered, should this patch convert these MKFS_OPTIONS overrides? ext4/059 ext4/060 xfs/107 xfs/547 --D > --D > > > } > > > > # Emulate an N-data-disk stripe w/ various stripe units > > -- > > 2.45.2 > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments 2024-11-18 22:21 ` Darrick J. Wong 2024-11-18 22:43 ` Darrick J. Wong @ 2024-11-21 9:17 ` Zorro Lang 2024-11-21 15:50 ` Darrick J. Wong 1 sibling, 1 reply; 10+ messages in thread From: Zorro Lang @ 2024-11-21 9:17 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Zorro Lang, fstests, linux-xfs On Mon, Nov 18, 2024 at 02:21:36PM -0800, Darrick J. Wong wrote: > On Sun, Nov 17, 2024 at 03:07:59AM +0800, Zorro Lang wrote: > > To give more arguments to _scratch_mkfs_sized, we generally do as: > > > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > > > to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it > > will get rid of the whole MKFS_OPTIONS and try to mkfs again. > > Likes: > > > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > > > But that's not the fault of "-L oldlabel". So for keeping the mkfs > > options ("-L oldlabel") we need, we'd better to let the > > scratch_mkfs_sized to support extra arguments, rather than using > > global MKFS_OPTIONS. > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > --- > > common/rc | 34 ++++++++++++++++++---------------- > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > diff --git a/common/rc b/common/rc > > index 2af26f23f..ce8602383 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -1023,11 +1023,13 @@ _small_fs_size_mb() > > } > > > > # Create fs of certain size on scratch device > > -# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] > > +# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options] > > _try_scratch_mkfs_sized() > > { > > local fssize=$1 > > - local blocksize=$2 > > + shift > > + local blocksize=$1 > > + shift > > local def_blksz > > local blocksize_opt > > local rt_ops > > @@ -1091,10 +1093,10 @@ _try_scratch_mkfs_sized() > > # don't override MKFS_OPTIONS that set a block size. > > echo $MKFS_OPTIONS |grep -E -q "b\s*size=" > > if [ $? -eq 0 ]; then > > - _try_scratch_mkfs_xfs -d size=$fssize $rt_ops > > + _try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@" > > else > > _try_scratch_mkfs_xfs -d size=$fssize $rt_ops \ > > - -b size=$blocksize > > + -b size=$blocksize "$@" > > fi > > ;; > > ext2|ext3|ext4) > > @@ -1105,7 +1107,7 @@ _try_scratch_mkfs_sized() > > _notrun "Could not make scratch logdev" > > MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV" > > fi > > - ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > gfs2) > > # mkfs.gfs2 doesn't automatically shrink journal files on small > > @@ -1120,13 +1122,13 @@ _try_scratch_mkfs_sized() > > (( journal_size >= min_journal_size )) || journal_size=$min_journal_size > > MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS" > > fi > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > ocfs2) > > - yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > udf) > > - $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > btrfs) > > local mixed_opt= > > @@ -1134,33 +1136,33 @@ _try_scratch_mkfs_sized() > > # the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size() > > (( fssize < $((256 * 1024 * 1024)) )) && > > ! _scratch_btrfs_is_zoned && mixed_opt='--mixed' > > - $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV > > + $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV > > ;; > > jfs) > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks > > ;; > > reiserfs) > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > ;; > > reiser4) > > # mkfs.resier4 requires size in KB as input for creating filesystem > > - $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \ > > + $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \ > > `expr $fssize / 1024` > > ;; > > f2fs) > > # mkfs.f2fs requires # of sectors as an input for the size > > local sector_size=`blockdev --getss $SCRATCH_DEV` > > - $MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size` > > + $MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size` > > ;; > > tmpfs) > > local free_mem=`_free_memory_bytes` > > if [ "$free_mem" -lt "$fssize" ] ; then > > _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes" > > fi > > - export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > > + export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS" > > ;; > > bcachefs) > > - $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV > > + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV > > ;; > > *) > > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > > @@ -1170,7 +1172,7 @@ _try_scratch_mkfs_sized() > > > > _scratch_mkfs_sized() > > { > > - _try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)" > > + _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($@)" > > Nit: Don't use '$@' within a longer string -- either it's "$@" so that > each element in the arg array is rendered individually as a separate > string parameter to the program being called, or "foo $*" so that you > end up with a single string. Hi Darrick, do you mean I should keep the later $*? Likes: _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($*)" > > shellcheck will complain about that, though bash itself doesn't seem to > care. > > --D > > > } > > > > # Emulate an N-data-disk stripe w/ various stripe units > > -- > > 2.45.2 > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments 2024-11-21 9:17 ` Zorro Lang @ 2024-11-21 15:50 ` Darrick J. Wong 0 siblings, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2024-11-21 15:50 UTC (permalink / raw) To: Zorro Lang; +Cc: Zorro Lang, fstests, linux-xfs On Thu, Nov 21, 2024 at 05:17:33PM +0800, Zorro Lang wrote: > On Mon, Nov 18, 2024 at 02:21:36PM -0800, Darrick J. Wong wrote: > > On Sun, Nov 17, 2024 at 03:07:59AM +0800, Zorro Lang wrote: > > > To give more arguments to _scratch_mkfs_sized, we generally do as: > > > > > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > > > > > to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it > > > will get rid of the whole MKFS_OPTIONS and try to mkfs again. > > > Likes: > > > > > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > > > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > > > > > But that's not the fault of "-L oldlabel". So for keeping the mkfs > > > options ("-L oldlabel") we need, we'd better to let the > > > scratch_mkfs_sized to support extra arguments, rather than using > > > global MKFS_OPTIONS. > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > --- > > > common/rc | 34 ++++++++++++++++++---------------- > > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > > > diff --git a/common/rc b/common/rc > > > index 2af26f23f..ce8602383 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -1023,11 +1023,13 @@ _small_fs_size_mb() > > > } > > > > > > # Create fs of certain size on scratch device > > > -# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] > > > +# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options] > > > _try_scratch_mkfs_sized() > > > { > > > local fssize=$1 > > > - local blocksize=$2 > > > + shift > > > + local blocksize=$1 > > > + shift > > > local def_blksz > > > local blocksize_opt > > > local rt_ops > > > @@ -1091,10 +1093,10 @@ _try_scratch_mkfs_sized() > > > # don't override MKFS_OPTIONS that set a block size. > > > echo $MKFS_OPTIONS |grep -E -q "b\s*size=" > > > if [ $? -eq 0 ]; then > > > - _try_scratch_mkfs_xfs -d size=$fssize $rt_ops > > > + _try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@" > > > else > > > _try_scratch_mkfs_xfs -d size=$fssize $rt_ops \ > > > - -b size=$blocksize > > > + -b size=$blocksize "$@" > > > fi > > > ;; > > > ext2|ext3|ext4) > > > @@ -1105,7 +1107,7 @@ _try_scratch_mkfs_sized() > > > _notrun "Could not make scratch logdev" > > > MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV" > > > fi > > > - ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > > + ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > > ;; > > > gfs2) > > > # mkfs.gfs2 doesn't automatically shrink journal files on small > > > @@ -1120,13 +1122,13 @@ _try_scratch_mkfs_sized() > > > (( journal_size >= min_journal_size )) || journal_size=$min_journal_size > > > MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS" > > > fi > > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks > > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks > > > ;; > > > ocfs2) > > > - yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > > + yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > > ;; > > > udf) > > > - $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > > + $MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > > ;; > > > btrfs) > > > local mixed_opt= > > > @@ -1134,33 +1136,33 @@ _try_scratch_mkfs_sized() > > > # the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size() > > > (( fssize < $((256 * 1024 * 1024)) )) && > > > ! _scratch_btrfs_is_zoned && mixed_opt='--mixed' > > > - $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV > > > + $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV > > > ;; > > > jfs) > > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks > > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks > > > ;; > > > reiserfs) > > > - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks > > > + ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks > > > ;; > > > reiser4) > > > # mkfs.resier4 requires size in KB as input for creating filesystem > > > - $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \ > > > + $MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \ > > > `expr $fssize / 1024` > > > ;; > > > f2fs) > > > # mkfs.f2fs requires # of sectors as an input for the size > > > local sector_size=`blockdev --getss $SCRATCH_DEV` > > > - $MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size` > > > + $MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size` > > > ;; > > > tmpfs) > > > local free_mem=`_free_memory_bytes` > > > if [ "$free_mem" -lt "$fssize" ] ; then > > > _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes" > > > fi > > > - export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > > > + export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS" > > > ;; > > > bcachefs) > > > - $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV > > > + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV > > > ;; > > > *) > > > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > > > @@ -1170,7 +1172,7 @@ _try_scratch_mkfs_sized() > > > > > > _scratch_mkfs_sized() > > > { > > > - _try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)" > > > + _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($@)" > > > > Nit: Don't use '$@' within a longer string -- either it's "$@" so that > > each element in the arg array is rendered individually as a separate > > string parameter to the program being called, or "foo $*" so that you > > end up with a single string. > > Hi Darrick, do you mean I should keep the later $*? Likes: > > _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($*)" Yes, as you saw in my more recent reposting of this. :) --D > > > > shellcheck will complain about that, though bash itself doesn't seem to > > care. > > > > --D > > > > > } > > > > > > # Emulate an N-data-disk stripe w/ various stripe units > > > -- > > > 2.45.2 > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] xfs/157: do not drop necessary mkfs options 2024-11-16 19:07 [PATCH 0/2] fstests: fix test issue of xfs/157 Zorro Lang 2024-11-16 19:07 ` [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments Zorro Lang @ 2024-11-16 19:08 ` Zorro Lang 2024-11-18 22:26 ` Darrick J. Wong 1 sibling, 1 reply; 10+ messages in thread From: Zorro Lang @ 2024-11-16 19:08 UTC (permalink / raw) To: fstests; +Cc: linux-xfs To give the test option "-L oldlabel" to _scratch_mkfs_sized, xfs/157 does: MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size but the _scratch_mkfs_sized trys to keep the $fs_size, when mkfs fails with incompatible $MKFS_OPTIONS options, likes this: ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** but the "-L oldlabel" is necessary, we shouldn't drop it. To avoid that, we give the "-L oldlabel" to _scratch_mkfs_sized through function parameters, not through global MKFS_OPTIONS. Signed-off-by: Zorro Lang <zlang@kernel.org> --- tests/xfs/157 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/xfs/157 b/tests/xfs/157 index 9b5badbae..f8f102d78 100755 --- a/tests/xfs/157 +++ b/tests/xfs/157 @@ -66,8 +66,7 @@ scenario() { } check_label() { - MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \ - >> $seqres.full + _scratch_mkfs_sized "$fs_size" "" "-L oldlabel" >> $seqres.full 2>&1 _scratch_xfs_db -c label _scratch_xfs_admin -L newlabel "$@" >> $seqres.full _scratch_xfs_db -c label -- 2.45.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfs/157: do not drop necessary mkfs options 2024-11-16 19:08 ` [PATCH 2/2] xfs/157: do not drop necessary mkfs options Zorro Lang @ 2024-11-18 22:26 ` Darrick J. Wong 2024-11-21 9:35 ` Zorro Lang 0 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2024-11-18 22:26 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-xfs On Sun, Nov 17, 2024 at 03:08:00AM +0800, Zorro Lang wrote: > To give the test option "-L oldlabel" to _scratch_mkfs_sized, xfs/157 > does: > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > but the _scratch_mkfs_sized trys to keep the $fs_size, when mkfs > fails with incompatible $MKFS_OPTIONS options, likes this: > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > but the "-L oldlabel" is necessary, we shouldn't drop it. To avoid > that, we give the "-L oldlabel" to _scratch_mkfs_sized through > function parameters, not through global MKFS_OPTIONS. > > Signed-off-by: Zorro Lang <zlang@kernel.org> > --- > tests/xfs/157 | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/tests/xfs/157 b/tests/xfs/157 > index 9b5badbae..f8f102d78 100755 > --- a/tests/xfs/157 > +++ b/tests/xfs/157 > @@ -66,8 +66,7 @@ scenario() { > } > > check_label() { > - MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \ > - >> $seqres.full > + _scratch_mkfs_sized "$fs_size" "" "-L oldlabel" >> $seqres.full 2>&1 Don't quote the "-L" and "oldlabel" within the same string unless you want them passed as a single string to _scratch_mkfs. Right now that works because although you have _scratch_mkfs_sized using "$@" (doublequote-dollarsign-atsign-doublequote) to pass its arguments intact to _scratch_mkfs, it turns out that _scratch_mkfs just brazely passes $* (with no quoting) to the actual MKFS_PROG which results in any space in any single argument being treated as an argument separator and the string is broken into multiple arguments. This is why you *can't* do _scratch_mkfs -L "moo cow". This is also part of why everyone hates bash. --D > _scratch_xfs_db -c label > _scratch_xfs_admin -L newlabel "$@" >> $seqres.full > _scratch_xfs_db -c label > -- > 2.45.2 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfs/157: do not drop necessary mkfs options 2024-11-18 22:26 ` Darrick J. Wong @ 2024-11-21 9:35 ` Zorro Lang 2024-11-21 15:52 ` Darrick J. Wong 0 siblings, 1 reply; 10+ messages in thread From: Zorro Lang @ 2024-11-21 9:35 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Zorro Lang, fstests, linux-xfs On Mon, Nov 18, 2024 at 02:26:14PM -0800, Darrick J. Wong wrote: > On Sun, Nov 17, 2024 at 03:08:00AM +0800, Zorro Lang wrote: > > To give the test option "-L oldlabel" to _scratch_mkfs_sized, xfs/157 > > does: > > > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > > > but the _scratch_mkfs_sized trys to keep the $fs_size, when mkfs > > fails with incompatible $MKFS_OPTIONS options, likes this: > > > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > > > but the "-L oldlabel" is necessary, we shouldn't drop it. To avoid > > that, we give the "-L oldlabel" to _scratch_mkfs_sized through > > function parameters, not through global MKFS_OPTIONS. > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > --- > > tests/xfs/157 | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/tests/xfs/157 b/tests/xfs/157 > > index 9b5badbae..f8f102d78 100755 > > --- a/tests/xfs/157 > > +++ b/tests/xfs/157 > > @@ -66,8 +66,7 @@ scenario() { > > } > > > > check_label() { > > - MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \ > > - >> $seqres.full > > + _scratch_mkfs_sized "$fs_size" "" "-L oldlabel" >> $seqres.full 2>&1 > > Don't quote the "-L" and "oldlabel" within the same string unless you > want them passed as a single string to _scratch_mkfs. Right now that > works because although you have _scratch_mkfs_sized using "$@" I use "$@" just for _scratch_mkfs_sized can give an empty argument to _try_scratch_mkfs_sized to be its second argument. how about: _scratch_mkfs_sized "$fs_size" "" -L oldlabel > (doublequote-dollarsign-atsign-doublequote) to pass its arguments intact > to _scratch_mkfs, it turns out that _scratch_mkfs just brazely passes $* > (with no quoting) to the actual MKFS_PROG which results in any space in > any single argument being treated as an argument separator and the > string is broken into multiple arguments. > > This is why you *can't* do _scratch_mkfs -L "moo cow". > > This is also part of why everyone hates bash. Hmm... do you need to change the $* of _scratch_mkfs to $@ too? > > --D > > > _scratch_xfs_db -c label > > _scratch_xfs_admin -L newlabel "$@" >> $seqres.full > > _scratch_xfs_db -c label > > -- > > 2.45.2 > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfs/157: do not drop necessary mkfs options 2024-11-21 9:35 ` Zorro Lang @ 2024-11-21 15:52 ` Darrick J. Wong 0 siblings, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2024-11-21 15:52 UTC (permalink / raw) To: Zorro Lang; +Cc: Zorro Lang, fstests, linux-xfs On Thu, Nov 21, 2024 at 05:35:37PM +0800, Zorro Lang wrote: > On Mon, Nov 18, 2024 at 02:26:14PM -0800, Darrick J. Wong wrote: > > On Sun, Nov 17, 2024 at 03:08:00AM +0800, Zorro Lang wrote: > > > To give the test option "-L oldlabel" to _scratch_mkfs_sized, xfs/157 > > > does: > > > > > > MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size > > > > > > but the _scratch_mkfs_sized trys to keep the $fs_size, when mkfs > > > fails with incompatible $MKFS_OPTIONS options, likes this: > > > > > > ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 ** > > > ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 ** > > > > > > but the "-L oldlabel" is necessary, we shouldn't drop it. To avoid > > > that, we give the "-L oldlabel" to _scratch_mkfs_sized through > > > function parameters, not through global MKFS_OPTIONS. > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > --- > > > tests/xfs/157 | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/tests/xfs/157 b/tests/xfs/157 > > > index 9b5badbae..f8f102d78 100755 > > > --- a/tests/xfs/157 > > > +++ b/tests/xfs/157 > > > @@ -66,8 +66,7 @@ scenario() { > > > } > > > > > > check_label() { > > > - MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \ > > > - >> $seqres.full > > > + _scratch_mkfs_sized "$fs_size" "" "-L oldlabel" >> $seqres.full 2>&1 > > > > Don't quote the "-L" and "oldlabel" within the same string unless you > > want them passed as a single string to _scratch_mkfs. Right now that > > works because although you have _scratch_mkfs_sized using "$@" > > I use "$@" just for _scratch_mkfs_sized can give an empty argument to > _try_scratch_mkfs_sized to be its second argument. > > how about: > _scratch_mkfs_sized "$fs_size" "" -L oldlabel > > > (doublequote-dollarsign-atsign-doublequote) to pass its arguments intact > > to _scratch_mkfs, it turns out that _scratch_mkfs just brazely passes $* > > (with no quoting) to the actual MKFS_PROG which results in any space in > > any single argument being treated as an argument separator and the > > string is broken into multiple arguments. > > > > This is why you *can't* do _scratch_mkfs -L "moo cow". > > > > This is also part of why everyone hates bash. > > Hmm... do you need to change the $* of _scratch_mkfs to $@ too? Yeah, that's something that needs to be done treewide. The trouble is, I bet there's other tests out there that have come to rely on the bad splitting behavior and do things like _scratch_fubar "-x 555" becoming execve("foobar", "-x" "555"); and will break if we try to fix it now. --D > > > > --D > > > > > _scratch_xfs_db -c label > > > _scratch_xfs_admin -L newlabel "$@" >> $seqres.full > > > _scratch_xfs_db -c label > > > -- > > > 2.45.2 > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-11-21 15:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-16 19:07 [PATCH 0/2] fstests: fix test issue of xfs/157 Zorro Lang 2024-11-16 19:07 ` [PATCH 1/2] common/rc: _scratch_mkfs_sized supports extra arguments Zorro Lang 2024-11-18 22:21 ` Darrick J. Wong 2024-11-18 22:43 ` Darrick J. Wong 2024-11-21 9:17 ` Zorro Lang 2024-11-21 15:50 ` Darrick J. Wong 2024-11-16 19:08 ` [PATCH 2/2] xfs/157: do not drop necessary mkfs options Zorro Lang 2024-11-18 22:26 ` Darrick J. Wong 2024-11-21 9:35 ` Zorro Lang 2024-11-21 15:52 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox