8:51 a.m.
Commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18)
missed updating a relative pathname in "tests/mount-local". This causes
the test case to hang. Furthermore, due to an earlier error handling
bug, the symptoms of the hang are somewhat chaotic. This small series
intends to fix both issues.
Thanks,
Laszlo
Laszlo Ersek (2):
tests/mount-local: exit child immediately when exec fails
tests/mount-local: fix relative pathname of FUSE client executable
tests/mount-local/test-parallel-mount-local.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.19.1.3.g30247aa5d201
8:51 a.m.
New subject: [PATCH 1/2] tests/mount-local: exit child immediately when exec fails
Each worker thread of "test-parallel-mount-local" performs the following
steps (among others):
(1) it starts an appliance dedicated to that thread, using a private
scratch disk image,
(2) exports a dedicated FUSE mount point on the host, exposing the file
system on the appliance's disk,
(3) launches a child process for manipulating the particular FUSE mount
point on the host,
(4) enters a FUSE request processing loop, translating requests between
the host kernel (coming in via the FUSE mount point) and the
appliance.
Items to note:
- The child process from step (3) consists of a single thread of execution
(see fork() in POSIX): a duplicate of the parent process's respective
worker thread.
- The child process from step (3) blocks on any FUSE mount point access on
the host until the worker thread in the parent process starts processing
FUSE requests, in step (4).
- The FUSE request processing in step (4), in the worker thread living in
the parent process, terminates if and only if the child process unmounts
the FUSE mount point originating from (2).
Should the exec call in step (3) fail for any reason, the child currently
jumps to the "error" label. This is wrong: under the error label, we call
guestfs_close() on the appliance -- but the appliance is owned by the
parent process's worker thread, not the child. What happens is that the
child kills off the appliance while the parent's worker thread is in the
FUSE request processing loop (4).
The "error" label was never meant to be reached by the child process -- if
exec fails for any reason, exit the child immediately. The parent will
remain in the FUSE request processing loop (4) forever, but no state will
be corrupted. For example, using another (interactive) session on the
host, the FUSE mount points can be interacted with, and if all of them are
manually unmounted, the FUSE request processing (4) completes in every
worker thread.
This patch does not fix the primary issue with
"test-parallel-mount-local", but removes "chaos" from the symptoms.
The
next patch will fix the actual regression in this test case.
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
tests/mount-local/test-parallel-mount-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index d3db6914fc4c..5f00e328a39c 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -223,7 +223,7 @@ start_thread (void *statevp)
execlp ("./test-parallel-mount-local",
"test-parallel-mount-local", "--test", state->mp,
NULL);
perror ("execlp");
- goto error;
+ _exit (EXIT_FAILURE);
}
/* Run the FUSE main loop. We don't really want to see libguestfs
--
2.19.1.3.g30247aa5d201
8:51 a.m.
New subject: [PATCH 2/2] tests/mount-local: fix relative pathname of FUSE client executable
In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
the working directory relative to which "test-parallel-mount-local" would
be launched (by the test machinery) changed from "tests/mount-local" to
just "tests".
While the relative pathname of the "guestunmount" executable was updated
inside "test-parallel-mount-local" accordingly, the relative pathname of
the FUSE client ("test-parallel-mount-local" itself, just invoked with
"--test") was not. This issue guarantees that the exec call fails in the
child, and so the test case always hangs.
Because we had removed "mount-local" from the end of the working
directory, prepend it now to the relative pathname of the FUSE client
executable.
Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
tests/mount-local/test-parallel-mount-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index 5f00e328a39c..c33ecf5b2680 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -220,7 +220,7 @@ start_thread (void *statevp)
if (pid == 0) { /* child */
setpgid (0, 0); /* so we don't get ^C from parent */
- execlp ("./test-parallel-mount-local",
+ execlp ("mount-local/test-parallel-mount-local",
"test-parallel-mount-local", "--test", state->mp,
NULL);
perror ("execlp");
_exit (EXIT_FAILURE);
--
2.19.1.3.g30247aa5d201
8:56 a.m.
New subject: [PATCH 2/2] tests/mount-local: fix relative pathname of FUSE client executable
On Thu, Sep 02, 2021 at 03:51:24PM +0200, Laszlo Ersek wrote:
In commit 6d32773e8118 ("tests: Run the tests in
parallel.", 2021-03-18),
the working directory relative to which "test-parallel-mount-local" would
be launched (by the test machinery) changed from "tests/mount-local" to
just "tests".
While the relative pathname of the "guestunmount" executable was updated
inside "test-parallel-mount-local" accordingly, the relative pathname of
the FUSE client ("test-parallel-mount-local" itself, just invoked with
"--test") was not. This issue guarantees that the exec call fails in the
child, and so the test case always hangs.
Because we had removed "mount-local" from the end of the working
directory, prepend it now to the relative pathname of the FUSE client
executable.
Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
tests/mount-local/test-parallel-mount-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index 5f00e328a39c..c33ecf5b2680 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -220,7 +220,7 @@ start_thread (void *statevp)
if (pid == 0) { /* child */
setpgid (0, 0); /* so we don't get ^C from parent */
- execlp ("./test-parallel-mount-local",
+ execlp ("mount-local/test-parallel-mount-local",
"test-parallel-mount-local", "--test", state->mp,
NULL);
perror ("execlp");
_exit (EXIT_FAILURE);
ACK series.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
5:11 a.m.
New subject: [PATCH 2/2] tests/mount-local: fix relative pathname of FUSE client executable
On 09/02/21 15:56, Richard W.M. Jones wrote:
On Thu, Sep 02, 2021 at 03:51:24PM +0200, Laszlo Ersek wrote:
> In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
> the working directory relative to which "test-parallel-mount-local" would
> be launched (by the test machinery) changed from "tests/mount-local" to
> just "tests".
>
> While the relative pathname of the "guestunmount" executable was updated
> inside "test-parallel-mount-local" accordingly, the relative pathname of
> the FUSE client ("test-parallel-mount-local" itself, just invoked with
> "--test") was not. This issue guarantees that the exec call fails in the
> child, and so the test case always hangs.
>
> Because we had removed "mount-local" from the end of the working
> directory, prepend it now to the relative pathname of the FUSE client
> executable.
>
> Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
> ---
> tests/mount-local/test-parallel-mount-local.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
> index 5f00e328a39c..c33ecf5b2680 100644
> --- a/tests/mount-local/test-parallel-mount-local.c
> +++ b/tests/mount-local/test-parallel-mount-local.c
> @@ -220,7 +220,7 @@ start_thread (void *statevp)
>
> if (pid == 0) { /* child */
> setpgid (0, 0); /* so we don't get ^C from parent */
> - execlp ("./test-parallel-mount-local",
> + execlp ("mount-local/test-parallel-mount-local",
> "test-parallel-mount-local", "--test",
state->mp, NULL);
> perror ("execlp");
> _exit (EXIT_FAILURE);
ACK series.
Pushed as commit range 13a1ae6da6e8..0a2f7621a089.
I added "Acked-by: Richard W.M. Jones <rjones(a)redhat.com>" to both
commit messages, with Rich's permission.
Thanks!
Laszlo
1057
days inactive
1058
days old
4 comments
2 participants
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones