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

[Daniel Kiper]

On Sun, Feb 06, 2022 at 04:00:12PM -0600, Glenn Washburn wrote:
> 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 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> index 5c722209d..3bc3006de 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -14,10 +14,27 @@ 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

Why 600? How did you come up with this number? Do not we have better
mechanism to break this loop? If not please add a comment here.

And if we want to be perfect I would do "local i" at the beginning of
this function in the previous patch. Then we should initialize i before
this loop.

> +     while losetup -l -O NAME | grep -q "^$lodev\$"; do
> +         losetup -d "$lodev" || sleep 1
> +         [ "$((i--))" = "0" ] && break

This is a number not a string. Then: [ $((i--)) -eq 0 ] && break

Daniel