| Message ID | 20241116190800.1870975-3-zlang@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fstests: fix test issue of xfs/157 | expand |
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 > >
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 > > > > >
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
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(-)