From 99f5303c8b857320b00a63dca81d1f6f7c75c636 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Fri, 4 Jun 2021 22:04:46 +0300
Subject: [PATCH] test/py: Wait for guestmount worker to exit after running
 guestunmount

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 <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 test/py/tests/test_fs/conftest.py | 13 ++++++++++-
 test/py/u_boot_utils.py           | 36 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
index 0189298f914..b638284e076 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']
@@ -210,7 +211,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
@@ -239,6 +240,16 @@ def umount_fs(mount_point):
     if fuse_mounted:
         call('sync')
         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 939d82eec12..e816c7fbb6a 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)
+    )