From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 883CFC48BC2 for ; Sun, 27 Jun 2021 13:55:06 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8B04A61C2E for ; Sun, 27 Jun 2021 13:55:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8B04A61C2E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0CD3782C57; Sun, 27 Jun 2021 15:55:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="BezPrOk+"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 664BF82C5A; Sun, 27 Jun 2021 15:54:59 +0200 (CEST) Received: from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com [IPv6:2a00:1450:4864:20::52a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5DB1B82C24 for ; Sun, 27 Jun 2021 15:54:56 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=alpernebiyasak@gmail.com Received: by mail-ed1-x52a.google.com with SMTP id w13so15216788edc.0 for ; Sun, 27 Jun 2021 06:54:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=IfS0VvxBL0D2fG9lb5kcLBgbpbOIbUwhyQcvRn9yRV8=; b=BezPrOk+pV62lZj6rtzyeyxqiuF3DVCnlnGrJF7+MsPznqAsLadtfVMclL8i8oqFER Jjyj51nZyORlOIDDVHCgDDhz5OTZKj/cFZfxiG3KOk7MjlXcEr89TZ3iSIFw8KZ9hAOx 91absNIq2n5wXy4cVkDD6fvi4au5WIcxmbnRmptuRoJT5M8/CQrXmrjhKeaUk6qjp11v 3zLiP/EAApoD+KhNo44sjMv9PqbDzH3gTNjk/RZlcIst1SOjZp7ao0hgotPtBKNj4xtK yHtWGsMRFvCnWFHEqfqjD3zlUVAkWgjd1syg0Sms4MYrJ3fXXBHxLrm/5hoRdRc6wpED qd6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=IfS0VvxBL0D2fG9lb5kcLBgbpbOIbUwhyQcvRn9yRV8=; b=f9VYS4XnGXMmEOEarrFk05yKbPsZhCYbBUpOwd0LWDglqONsDAvKI26tLWX7kUqkBb pS6NCk4CBM19PKC71jp85j1jB4NnGhMFHj+KXbktfwJmm96VD8plW9gYWgvMPKVZ5FuM 9e4wkS2vJPUyklgq8jHpx/7riszNw583rHX9JTV0CKg6aKAgds7e38P4hlzbDBXDst0R 6oRNDT+dpZ1xE/eq5wgWD/6ma3xSd6KgYH/an1Hnm5ZNMPnW1TLCwwXjCU82TO5SkGDv XFszyDe3eYzqfgmAbjAWDpaXeDRSnlhCI8tLmG4W0pnlfj8OQWW0Qodpy+XAvMwpycz7 MyYA== X-Gm-Message-State: AOAM530Ei+TOmxv4Pg0o9Z5aN/0Wey09R5U9vIR6n/MH1RFO0dCTqPZ7 esedWJdUDpsB6/JgBWZFauM= X-Google-Smtp-Source: ABdhPJwAChB1u8Uzu+WvaILgSfGcb5Zj290UjQdcZnD00TlnzHiydut+nIJtXwY0M9hzEw2S0Oe8Xg== X-Received: by 2002:aa7:da96:: with SMTP id q22mr28046286eds.307.1624802096051; Sun, 27 Jun 2021 06:54:56 -0700 (PDT) Received: from [192.168.0.74] ([178.233.26.119]) by smtp.gmail.com with ESMTPSA id o9sm3249102edc.91.2021.06.27.06.54.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 27 Jun 2021 06:54:55 -0700 (PDT) Subject: Re: [PATCH 2/2] test/py: Wait for guestmount worker to exit after running guestunmount To: Simon Glass Cc: U-Boot Mailing List , Heinrich Schuchardt , Stephen Warren , Andy Shevchenko , Tom Rini References: <20210604190447.45342-1-alpernebiyasak@gmail.com> <20210604190447.45342-2-alpernebiyasak@gmail.com> From: Alper Nebi Yasak Message-ID: Date: Sun, 27 Jun 2021 16:54:51 +0300 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 26/06/2021 21:29, Simon Glass wrote: > On Fri, 4 Jun 2021 at 13:05, Alper Nebi Yasak 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 >> --- >> >> 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 > >> 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? Guestmount overwrites the pid file without complaining, and the worker deletes the file on its own when it terminates after the guestunmount call. I think it's OK to ignore an existing pid file if the mount point is clean. If not clean, that's the actual problem and both guestmount and mount will fail, causing the tests to be skipped. We could clean mount points automatically before attempting to mount. >> 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 >>