On Fri, 4 Jun 2021 at 13:05, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote:
>
> Some filesystem tests are failing when their image is prepared with
> guestmount, but succeeding if loop mounts are used instead. The reason
> seems to be a race condition the guestmount(1) manual page explains:
>
> When guestunmount(1)/fusermount(1) exits, guestmount may still be
> running and cleaning up the mountpoint. The disk image will not be
> fully finalized.
>
> This means that scripts like the following have a nasty race condition:
>
> guestmount -a disk.img -i /mnt
> # copy things into /mnt
> guestunmount /mnt
> # immediately try to use 'disk.img' ** UNSAFE **
>
> The solution is to use the --pid-file option to write the guestmount
> PID to a file, then after guestunmount spin waiting for this PID to
> exit.
>
> The Python standard library has an os.waitpid() function for waiting a
> child to terminate, but it cannot wait on non-child processes. Implement
> a utility function that can do this by polling the process repeatedly
> for a given duration, optionally killing the process if it won't
> terminate on its own. Apply the suggested solution with this utility
> function, which makes the failing tests succeed again.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com>
> ---
>
> test/py/tests/test_fs/conftest.py | 13 ++++++++++-
> test/py/u_boot_utils.py | 36 +++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
>
Reviewed-by: Simon Glass <s...@chromium.org>
> diff --git a/test/py/tests/test_fs/conftest.py
> b/test/py/tests/test_fs/conftest.py
> index e3c461635f8e..6b1ff05a8143 100644
> --- a/test/py/tests/test_fs/conftest.py
> +++ b/test/py/tests/test_fs/conftest.py
> @@ -8,6 +8,7 @@
> import re
> from subprocess import call, check_call, check_output, CalledProcessError
> from fstest_defs import *
> +import u_boot_utils as util
>
> supported_fs_basic = ['fat16', 'fat32', 'ext4']
> supported_fs_ext = ['fat16', 'fat32']
> @@ -206,7 +207,7 @@ def mount_fs(fs_type, device, mount_point):
> global fuse_mounted
>
> try:
> - check_call('guestmount -a %s -m /dev/sda %s'
> + check_call('guestmount --pid-file guestmount.pid -a %s -m /dev/sda
> %s'
> % (device, mount_point), shell=True)
> fuse_mounted = True
> return
> @@ -235,6 +236,16 @@ def umount_fs(mount_point):
> if fuse_mounted:
> call('sync')
should you remove guestmount.pid first in case it is there from a
previous crash?
> call('guestunmount %s' % mount_point, shell=True)
> +
> + try:
> + with open("guestmount.pid", "r") as pidfile:
> + pid = int(pidfile.read())
> + util.waitpid(pid, kill=True)
> + os.remove("guestmount.pid")
> +
> + except FileNotFoundError:
> + pass
> +
> else:
> call('sudo umount %s' % mount_point, shell=True)
>
> diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
> index 939d82eec12a..e816c7fbb6a3 100644
> --- a/test/py/u_boot_utils.py
> +++ b/test/py/u_boot_utils.py
> @@ -8,6 +8,7 @@
> import os
> import os.path
> import pytest
> +import signal
> import sys
> import time
> import re
> @@ -339,3 +340,38 @@ def crc32(u_boot_console, address, count):
> assert m, 'CRC32 operation failed.'
>
> return m.group(1)
> +
> +def waitpid(pid, timeout=60, kill=False):
> + """Wait a process to terminate by its PID
> +
> + This is an alternative to a os.waitpid(pid, 0) call that works on
> + processes that aren't children of the python process.
> +
> + Args:
> + pid: PID of a running process.
> + timeout: Time in seconds to wait.
> + kill: Whether to forcibly kill the process after timeout.
> +
> + Returns:
> + True, if the process ended on its own.
> + False, if the process was killed by this function.
> +
> + Raises:
> + TimeoutError, if the process is still running after timeout.
> + """
> + try:
> + for _ in range(timeout):
> + os.kill(pid, 0)
> + time.sleep(1)
> +
> + if kill:
> + os.kill(pid, signal.SIGKILL)
> + return False
> +
> + except ProcessLookupError:
> + return True
> +
> + raise TimeoutError(
> + "Process with PID {} did not terminate after {} seconds."
> + .format(pid, timeout)
> + )
> --
> 2.32.0.rc2
>
