Quoting issues in Bash script `grub-fs-tester` causes F2FS test failure

Quoting issues in Bash script `grub-fs-tester` causes F2FS test failure

[Paul Menzel]

Dear GRUB folks,


Running `sudo env make -e check VERBOSE=yes SUBDIRS=` the F2FS is one of 
the two tests failing. It returns 255.

Running only the specific test

     sudo env TESTS=f2fs_test make -e check VERBOSE=yes SUBDIRS=

which runs

     sudo ./grub-fs-tester f2fs

and using `set -ex` in `grub-fs-tester` the call below has quoting issues.

                 xf2fs)
                     "mkfs.f2fs" -l "$FSLABEL" -q "${LODEVICES[0]}"

fails due to the loop device being passed in single quotes, which does 
not exist with the leading space.

      + LODEVICE=/dev/loop11
      + LODEVICES=' /dev/loop11'

      + mkfs.f2fs -l 
'grub_;/testé䏌䐓䏕киритiurewfceniuewruewnuuireurevueurnievrewfnerfcnevirivinrewvnirewnivrewiuvcrewvnuewvrrrewniuerwreiuviurewiuviurewnuvewnvrenurnunuvrevuurerejiremvreijnvvcreivire 
nverivnreivrevnureiorfnfrvoeoiroireoireoifrefoieroifoirvcreivire 
nverivnreivrevnureiorfnfrvoeoiroireoireoifrefoieroifoircreivire 
nverivnreivrevnureiorfnfrvoeoiroireoireoifrefoieroifoireoifoiq' -q ' 
/dev/loop11'
      FAIL f2fs_test (exit status: 255)

Is there a Bash expert in here, able to fix it?


Kind regards,

Paul

[PATCH] tests: F2FS test should use MOUNTDEVICE like other tests.

[Glenn Washburn]

LODEVICES is not an array variable and should not be accessed as such. This
allows the f2fs test to pass as it was failing because a device name had a
space prepended to the path.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index bc14a05ca..bfc425e1f 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -796,7 +796,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    MOUNTFS=ext2
 		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
 		xf2fs)
-		    "mkfs.f2fs" -l "$FSLABEL" -q "${LODEVICES[0]}" ;;
+		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;
 		xnilfs2)
 		    "mkfs.nilfs2" -L "$FSLABEL" -b $BLKSIZE  -q "${MOUNTDEVICE}" ;;
 		xext2_old)
-- 
2.27.0

Re: [PATCH] tests: F2FS test should use MOUNTDEVICE like other tests.

[Daniel Kiper]

Adding Jaegeuk and Paul...

On Fri, Aug 28, 2020 at 06:30:25PM -0500, Glenn Washburn wrote:
> LODEVICES is not an array variable and should not be accessed as such. This
> allows the f2fs test to pass as it was failing because a device name had a
> space prepended to the path.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  tests/util/grub-fs-tester.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> index bc14a05ca..bfc425e1f 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -796,7 +796,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
>  		    MOUNTFS=ext2
>  		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
>  		xf2fs)
> -		    "mkfs.f2fs" -l "$FSLABEL" -q "${LODEVICES[0]}" ;;
> +		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;

LGTM...

Jaegeuk, Paul does this fix work for you?

Daniel

Re: [PATCH] tests: F2FS test should use MOUNTDEVICE like other tests.

[Jaegeuk Kim]

On 09/17, Daniel Kiper wrote:
> Adding Jaegeuk and Paul...
> 
> On Fri, Aug 28, 2020 at 06:30:25PM -0500, Glenn Washburn wrote:
> > LODEVICES is not an array variable and should not be accessed as such. This
> > allows the f2fs test to pass as it was failing because a device name had a
> > space prepended to the path.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  tests/util/grub-fs-tester.in | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> > index bc14a05ca..bfc425e1f 100644
> > --- a/tests/util/grub-fs-tester.in
> > +++ b/tests/util/grub-fs-tester.in
> > @@ -796,7 +796,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> >  		    MOUNTFS=ext2
> >  		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
> >  		xf2fs)
> > -		    "mkfs.f2fs" -l "$FSLABEL" -q "${LODEVICES[0]}" ;;
> > +		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;
> 
> LGTM...
> 
> Jaegeuk, Paul does this fix work for you?

Thank you. I'm okay and should follow the others.

Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>

> 
> Daniel

Re: [PATCH] tests: F2FS test should use MOUNTDEVICE like other tests.

[Paul Menzel]

Dear Glenn,


Am 29.08.20 um 01:30 schrieb Glenn Washburn:
> LODEVICES is not an array variable and should not be accessed as such. This
> allows the f2fs test to pass as it was failing because a device name had a
> space prepended to the path.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>   tests/util/grub-fs-tester.in | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> index bc14a05ca..bfc425e1f 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -796,7 +796,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
>   		    MOUNTFS=ext2
>   		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
>   		xf2fs)
> -		    "mkfs.f2fs" -l "$FSLABEL" -q "${LODEVICES[0]}" ;;
> +		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;
>   		xnilfs2)
>   		    "mkfs.nilfs2" -L "$FSLABEL" -b $BLKSIZE  -q "${MOUNTDEVICE}" ;;
>   		xext2_old)

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul