public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
To: u-boot@lists.denx.de
Cc: Simon Glass <sjg@chromium.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Stephen Warren <swarren@nvidia.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Tom Rini <trini@konsulko.com>,
	Alper Nebi Yasak <alpernebiyasak@gmail.com>
Subject: [PATCH 2/2] test/py: Wait for guestmount worker to exit after running guestunmount
Date: Fri,  4 Jun 2021 22:04:46 +0300	[thread overview]
Message-ID: <20210604190447.45342-2-alpernebiyasak@gmail.com> (raw)
In-Reply-To: <20210604190447.45342-1-alpernebiyasak@gmail.com>

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>
---

 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 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')
         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


  reply	other threads:[~2021-06-04 19:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 19:04 [PATCH 1/2] test/py: Use loop mounts if guestmount fails in filesystem tests Alper Nebi Yasak
2021-06-04 19:04 ` Alper Nebi Yasak [this message]
2021-06-26 18:29   ` [PATCH 2/2] test/py: Wait for guestmount worker to exit after running guestunmount Simon Glass
2021-06-27 13:54     ` Alper Nebi Yasak
2021-07-06  0:58   ` Tom Rini
2021-06-26 18:29 ` [PATCH 1/2] test/py: Use loop mounts if guestmount fails in filesystem tests Simon Glass
2021-06-27 12:34   ` Alper Nebi Yasak
2021-07-06  0:58 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210604190447.45342-2-alpernebiyasak@gmail.com \
    --to=alpernebiyasak@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=sjg@chromium.org \
    --cc=swarren@nvidia.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox