[PATCH] command: avoid hanging on daemon processes

[Eric Blake <eblake@xxxxxxxxxx>]

* src/util/command.c (virCommandRun): Don't capture output on
daemons.
* tests/commandtest.c (test18): Expose the bug.
Reported by Laine Stump.
---

Even though 'test4' in commandtest created a daemon, the daemon
exits rather quickly, so that no one noticed the problem.  And
the existing qemu daemon use of virCommand was supplying log
file descriptors, rather than letting virCommand capture output.

I've verified that with just the tests/ changes, that the testsuite
fails (termination due to SIGALRM).

 src/util/command.c  |   24 ++++++++++++++----------
 tests/commandtest.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index f9d475e..abd2dc4 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -1008,17 +1008,21 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
     }

     /* If caller hasn't requested capture of stdout/err, then capture
-     * it ourselves so we can log it.
+     * it ourselves so we can log it.  But the intermediate child for
+     * a daemon has no expected output, and we don't want our
+     * capturing pipes passed on to the daemon grandchild.
      */
-    if (!cmd->outfdptr) {
-        cmd->outfdptr = &cmd->outfd;
-        cmd->outbuf = &outbuf;
-        string_io = true;
-    }
-    if (!cmd->errfdptr) {
-        cmd->errfdptr = &cmd->errfd;
-        cmd->errbuf = &errbuf;
-        string_io = true;
+    if (!(cmd->flags & VIR_EXEC_DAEMON)) {
+        if (!cmd->outfdptr) {
+            cmd->outfdptr = &cmd->outfd;
+            cmd->outbuf = &outbuf;
+            string_io = true;
+        }
+        if (!cmd->errfdptr) {
+            cmd->errfdptr = &cmd->errfd;
+            cmd->errbuf = &errbuf;
+            string_io = true;
+        }
     }

     if (virCommandRunAsync(cmd, NULL) < 0) {
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 333dd4d..38a7816 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -668,6 +668,47 @@ cleanup:
     return ret;
 }

+/*
+ * Run long-running daemon, to ensure no hang.
+ */
+static int test18(const void *unused ATTRIBUTE_UNUSED)
+{
+    virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL);
+    char *pidfile = virFilePid(abs_builddir, "commandhelper");
+    pid_t pid;
+    int ret = -1;
+
+    if (!pidfile)
+        goto cleanup;
+
+    virCommandSetPidFile(cmd, pidfile);
+    virCommandDaemonize(cmd);
+
+    alarm(5);
+    if (virCommandRun(cmd, NULL) < 0) {
+        virErrorPtr err = virGetLastError();
+        printf("Cannot run child %s\n", err->message);
+        goto cleanup;
+    }
+    alarm(0);
+
+    if (virFileReadPid(abs_builddir, "commandhelper", &pid) != 0) {
+        printf("cannot read pidfile\n");
+        goto cleanup;
+    }
+    while (kill(pid, SIGINT) != -1)
+        usleep(100*1000);
+
+    ret = 0;
+
+cleanup:
+    virCommandFree(cmd);
+    unlink(pidfile);
+    VIR_FREE(pidfile);
+    return ret;
+}
+
+
 static int
 mymain(int argc, char **argv)
 {
@@ -732,6 +773,7 @@ mymain(int argc, char **argv)
     DO_TEST(test15);
     DO_TEST(test16);
     DO_TEST(test17);
+    DO_TEST(test18);

     return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
-- 
1.7.3.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list