| Message ID | 20241116190800.1870975-3-zlang@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| 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 > > > > >
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 > > > > > > > > > >
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(-)