[PATCH 0/5] Various test fixes and improvements

[PATCH 0/5] Various test fixes and improvements

[Glenn Washburn]

Updated with Daniel's suggestions.

Glenn

Glenn Washburn (5):
  tests: Do not remove image file on error in pata_test
  tests: Skip pata_test on i386-efi
  tests: Remove $((BASE#NUM)) bashism in grub-fs-tester
  tests: Ensure that mountpoints are unmounted before exiting
  tests: Ensure that loopback devices and zfs devices are cleaned up

 tests/pata_test.in           |  4 ++-
 tests/util/grub-fs-tester.in | 56 ++++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 9 deletions(-)

Range-diff:
1:  24b2a4bfd = 1:  313168dd7 tests: Do not remove image file on error in pata_test
2:  a64ebe41a = 2:  53df676e7 tests: Skip pata_test on i386-efi
3:  d2248490b = 3:  401b227cb tests: Remove $((BASE#NUM)) bashism in grub-fs-tester
4:  410461b20 ! 4:  264f914b1 tests: Ensure that mountpoints are unmounted before exiting
    @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
      XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
      
     +MOUNTS=
    -+umount_all() {
    -+    for MOUNT in $MOUNTS; do
    -+	umount "$MOUNT" &&
    -+	MOUNTS="$(echo ${MOUNTS} | sed "s|$MOUNT||g;")"
    ++cleanup() {
    ++    for i in $MOUNTS; do
    ++	umount "$i" || :
     +    done
     +}
     +trap umount_all EXIT INT
5:  61bd6959b ! 5:  eb4a20b84 tests: Ensure that loopback devices and zfs devices are cleaned up
    @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
      XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
      
      MOUNTS=
    --umount_all() {
     +LODEVICES=
    -+cleanup() {
    + cleanup() {
     +    if [ -n "$fs" -a -z "${fs##*zfs*}" -a -n "$FSLABEL" ]; then
     +	zpool list "$FSLABEL" 2>/dev/null &&
     +	while ! zpool export "$FSLABEL" ; do
    @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
     +	done
     +    fi
     +
    -     for MOUNT in $MOUNTS; do
    - 	umount "$MOUNT" &&
    - 	MOUNTS="$(echo ${MOUNTS} | sed "s|$MOUNT||g;")"
    +     for i in $MOUNTS; do
    + 	umount "$i" || :
          done
     +
     +    for lodev in $LODEVICES; do
-- 
2.27.0

[PATCH 1/5] tests: Do not remove image file on error in pata_test

[Glenn Washburn]

The image file can be useful in debugging an issue when the test fails.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 tests/pata_test.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/pata_test.in b/tests/pata_test.in
index 4fee0b0fb..27dccec19 100644
--- a/tests/pata_test.in
+++ b/tests/pata_test.in
@@ -47,7 +47,6 @@ tar cf "$imgfile" "$outfile"
 
 v=$(echo "nativedisk; source '($indisk)/$outfile';" | "${grubshell}" --qemu-opts="-$disk $imgfile")
 if [ "$v" != "Hello World" ]; then
-   rm "$imgfile"
    rm "$outfile"
    exit 1
 fi
-- 
2.27.0

[PATCH 2/5] tests: Skip pata_test on i386-efi

[Glenn Washburn]

In comparison to other i386 targets, on i386-efi the Q35 QEMU machine type
is used to do the testing to be able to make use of the EFI firmware in
QEMU. On the Q35 machine type there is no way to use ATA to communicate with
an IDE, only AHCI.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 tests/pata_test.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/pata_test.in b/tests/pata_test.in
index 27dccec19..31144a8fd 100644
--- a/tests/pata_test.in
+++ b/tests/pata_test.in
@@ -29,6 +29,9 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
     # PLATFORM: no ATA on ARC platforms (they use SCSI)
     *-arc)
 	exit 77;;
+    # QEMU: no ATA on Q35 machine type (they use AHCI)
+    i386-efi)
+	exit 77;;
     # FIXME: No native drivers are available for those
     powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
 	exit 77;;
-- 
2.27.0

[PATCH 3/5] tests: Remove $((BASE#NUM)) bashism in grub-fs-tester

[Glenn Washburn]

This bashism allows converting NUM in base BASE to decimal. Its not needed
because the only place its used is to convert from hexidecimal and this can
also be done with the more portable $((0xHEXNUM)) syntax.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 tests/util/grub-fs-tester.in | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index aa72b2174..a1f3f299b 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -459,7 +459,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    # FIXME: not so sure about AFFS
 		    # OS LIMITATION: minix2/minix3 could be formatted in a way to permit more.
 		x"minix3" | x"minix2" | x"hfs"| x"affs" | xaffs_intl | xreiserfs_old | xext2_old)
-		    BIGBLOCKCNT=$((16#7fffffff));;
+		    BIGBLOCKCNT=$((0x7fffffff));;
 
 		    # FS LIMITATION: redundant storage
 		    # We have only limited space. Mirroring multiplies it very effectively.
@@ -669,18 +669,18 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 	# mkfs.hfs and mkfs.hfsplus don't fill UUID.
 		x"hfsplus")
 		    "mkfs.hfsplus" -b $BLKSIZE -v "$FSLABEL" "${MOUNTDEVICE}"
-		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((16#468)) conv=notrunc count=8 ;;
+		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((0x468)) conv=notrunc count=8 ;;
 		x"hfsplus_wrap")
 		    "mkfs.hfsplus" -w -b $BLKSIZE -v "$FSLABEL" "${MOUNTDEVICE}"
-		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((16#468)) conv=notrunc count=8
+		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((0x468)) conv=notrunc count=8
 		    MOUNTFS="hfsplus";;
 		x"hfsplus_casesens")
 		    "mkfs.hfsplus" -s -b $BLKSIZE -v "$FSLABEL" "${MOUNTDEVICE}"
-		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((16#468)) conv=notrunc count=8
+		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((0x468)) conv=notrunc count=8
 		    MOUNTFS="hfsplus";;
 		x"hfs")
 		    "mkfs.hfs" -b $BLKSIZE -v "`echo $FSLABEL |recode utf8..macroman`" -h "${MOUNTDEVICE}"
-		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((16#474)) conv=notrunc count=8
+		    dd if=/dev/urandom of="${MOUNTDEVICE}" bs=1 seek=$((0x474)) conv=notrunc count=8
 		    MOUNTOPTS="iocharset=utf8,codepage=macroman,"
 		    ;;
 		x"vfat"*|xmsdos*)
-- 
2.27.0

[PATCH 4/5] tests: Ensure that mountpoints are unmounted before exiting

[Glenn Washburn]

When all tests complete successfully, filesystems mounted by grub-fs-tester
will be unmounted before exiting. However, on certain test failures the
tester will exit with a failure code and not unmount previously mounted
filesystems. Now keep track of mounts and umounts and run an exit handler
on exit or process interruption that will umount all mounts that haven't
already been unmounted.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 tests/util/grub-fs-tester.in | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index a1f3f299b..ebd7b1e03 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -13,6 +13,16 @@ tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` ||
 # FSLABEL. This is especially needed for the conversion to Joliet UCS-2.
 XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
 
+MOUNTS=
+cleanup() {
+    for i in $MOUNTS; do
+	umount "$i" || :
+    done
+}
+trap umount_all EXIT INT
+# This is for bash, dash and ash do not recognize ERR
+trap umount_all ERR || :
+
 # This wrapper is to ease insertion of valgrind or time statistics
 run_it () {
     LC_ALL=C "$GRUBFSTEST" "$@"
@@ -934,6 +944,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 			done
 			exit 99;
 		    fi
+		    MOUNTS="$MOUNTS $MNTPOINTRW"
 		    ;;
 	    esac
 	    case x"$fs" in
@@ -1058,11 +1069,13 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		x"bfs")
 		    sleep 1
 		    fusermount -u "$MNTPOINTRW"
+		    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRW||g;")"
 		    ;;
 		xlvm*)
 		    sleep 1
 		    for try in $(range 0 20 1); do
 			if umount "$MNTPOINTRW" ; then
+			    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRW||g;")"
 			    break;
 			fi
 			sleep 1;
@@ -1075,6 +1088,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    sleep 1
 		    for try in $(range 0 20 1); do
 			if umount "$MNTPOINTRW" ; then
+			    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRW||g;")"
 			    break;
 			fi
 			sleep 1;
@@ -1087,6 +1101,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    sleep 1
 		    for try in $(range 0 20 1); do
 			if umount "$MNTPOINTRW" ; then
+			    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRW||g;")"
 			    break;
 			fi
 			sleep 1;
@@ -1116,13 +1131,19 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		xlvm*)
 		    vgchange -a y grub_test
 		    sleep 1
-		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro ;;
+		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
+		    MOUNTS="$MOUNTS $MNTPOINTRO"
+		    ;;
 		xmdraid*)
 		    mdadm --assemble /dev/md/"${fs}_$NDEVICES" $LODEVICES
 		    sleep 1
-		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro ;;
+		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
+		    MOUNTS="$MOUNTS $MNTPOINTRO"
+		    ;;
 		*)
-		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro ;;
+		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
+		    MOUNTS="$MOUNTS $MNTPOINTRO"
+		    ;;
 	    esac
 
 	    run_grubfstest ls -- -la
@@ -1547,6 +1568,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    sleep 1
 		    umount "$MNTPOINTRO"  || true
 		    umount "$MNTPOINTRW" || true
+		    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRO||g;")"
+		    MOUNTS="$(echo ${MOUNTS} | sed "s|$MNTPOINTRW||g;")"
 	    esac
 	    sleep 1
 	    case x"$fs" in
-- 
2.27.0

[PATCH 5/5] tests: Ensure that loopback devices and zfs devices are cleaned up

[Glenn Washburn]

ZFS file systems are not unmounted using umount, but instead by exporting
them. So export the ZFS file system that has the same label as the one that
was created during the test, if such one exists. This is required to delete
the loopback device that uses the ZFS image file. Otherwise the added code
to delete all loopback devices setup during the test run will never be able
to finish because the loopback device can not be deleted while in use.

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

diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index ebd7b1e03..3bc3006de 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -14,14 +14,31 @@ tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"` ||
 XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
 
 MOUNTS=
+LODEVICES=
 cleanup() {
+    if [ -n "$fs" -a -z "${fs##*zfs*}" -a -n "$FSLABEL" ]; then
+	zpool list "$FSLABEL" 2>/dev/null &&
+	while ! zpool export "$FSLABEL" ; do
+	    sleep 1;
+	done
+    fi
+
     for i in $MOUNTS; do
 	umount "$i" || :
     done
+
+    for lodev in $LODEVICES; do
+	local i=600
+	while losetup -l -O NAME | grep -q "^$lodev\$"; do
+	    losetup -d "$lodev" || sleep 1
+	    [ "$((i--))" = "0" ] && break
+	done
+    done
+    return 0
 }
-trap umount_all EXIT INT
+trap cleanup EXIT INT
 # This is for bash, dash and ash do not recognize ERR
-trap umount_all ERR || :
+trap cleanup ERR || :
 
 # This wrapper is to ease insertion of valgrind or time statistics
 run_it () {
-- 
2.27.0

Re: [PATCH 0/5] Various test fixes and improvements

[Glenn Washburn]

Please ignore this patch series.

Glenn

On Sun,  6 Feb 2022 02:02:39 -0600
Glenn Washburn <development@efficientek.com> wrote:

> Updated with Daniel's suggestions.
> 
> Glenn
> 
> Glenn Washburn (5):
>   tests: Do not remove image file on error in pata_test
>   tests: Skip pata_test on i386-efi
>   tests: Remove $((BASE#NUM)) bashism in grub-fs-tester
>   tests: Ensure that mountpoints are unmounted before exiting
>   tests: Ensure that loopback devices and zfs devices are cleaned up
> 
>  tests/pata_test.in           |  4 ++-
>  tests/util/grub-fs-tester.in | 56 ++++++++++++++++++++++++++++++------
>  2 files changed, 51 insertions(+), 9 deletions(-)
> 
> Range-diff:
> 1:  24b2a4bfd = 1:  313168dd7 tests: Do not remove image file on error in pata_test
> 2:  a64ebe41a = 2:  53df676e7 tests: Skip pata_test on i386-efi
> 3:  d2248490b = 3:  401b227cb tests: Remove $((BASE#NUM)) bashism in grub-fs-tester
> 4:  410461b20 ! 4:  264f914b1 tests: Ensure that mountpoints are unmounted before exiting
>     @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
>       XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
>       
>      +MOUNTS=
>     -+umount_all() {
>     -+    for MOUNT in $MOUNTS; do
>     -+	umount "$MOUNT" &&
>     -+	MOUNTS="$(echo ${MOUNTS} | sed "s|$MOUNT||g;")"
>     ++cleanup() {
>     ++    for i in $MOUNTS; do
>     ++	umount "$i" || :
>      +    done
>      +}
>      +trap umount_all EXIT INT
> 5:  61bd6959b ! 5:  eb4a20b84 tests: Ensure that loopback devices and zfs devices are cleaned up
>     @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
>       XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
>       
>       MOUNTS=
>     --umount_all() {
>      +LODEVICES=
>     -+cleanup() {
>     + cleanup() {
>      +    if [ -n "$fs" -a -z "${fs##*zfs*}" -a -n "$FSLABEL" ]; then
>      +	zpool list "$FSLABEL" 2>/dev/null &&
>      +	while ! zpool export "$FSLABEL" ; do
>     @@ tests/util/grub-fs-tester.in: tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX
>      +	done
>      +    fi
>      +
>     -     for MOUNT in $MOUNTS; do
>     - 	umount "$MOUNT" &&
>     - 	MOUNTS="$(echo ${MOUNTS} | sed "s|$MOUNT||g;")"
>     +     for i in $MOUNTS; do
>     + 	umount "$i" || :
>           done
>      +
>      +    for lodev in $LODEVICES; do

[PATCH 0/5] Various test fixes and improvements

[Glenn Washburn]

I believe the patches are fairly self explantatory.

Glenn

Glenn Washburn (5):
  tests: Do not remove image file on error in pata_test
  tests: Skip pata_test on i386-efi
  tests: Remove $((BASE#NUM)) bashism in grub-fs-tester
  tests: Ensure that mountpoints are unmounted before exiting
  tests: Ensure that loopback devices and zfs devices are cleaned up

 tests/pata_test.in           |  4 ++-
 tests/util/grub-fs-tester.in | 57 +++++++++++++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 9 deletions(-)

-- 
2.27.0

Re: [PATCH 0/5] Various test fixes and improvements

[Daniel Kiper]

On Wed, Jan 12, 2022 at 09:59:38PM -0600, Glenn Washburn wrote:
> I believe the patches are fairly self explantatory.
>
> Glenn
>
> Glenn Washburn (5):
>   tests: Do not remove image file on error in pata_test
>   tests: Skip pata_test on i386-efi
>   tests: Remove $((BASE#NUM)) bashism in grub-fs-tester

For first three patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel