2:52 p.m.
Wraps __virExec with the VIR_EXEC_DAEMON flag. Waits on the intermediate
process to ensure we don't end up with any zombies, and differentiates between
original process errors and intermediate process errors.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/libvirt_private.syms | 2 +-
src/proxy_internal.c | 16 ++------------
src/qemu_driver.c | 40 ++++++++++++++++---------------------
src/remote_internal.c | 15 ++-----------
src/uml_driver.c | 11 ++-------
src/util.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
src/util.h | 9 ++++++++
7 files changed, 85 insertions(+), 57 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 599ec0b..ce63869 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -320,7 +320,7 @@ virEnumToString;
virEventAddHandle;
virEventRemoveHandle;
virExec;
-virExecWithHook;
+virExecDaemonize;
virSetCloseExec;
virSetNonBlock;
virFormatMacAddr;
diff --git a/src/proxy_internal.c b/src/proxy_internal.c
index 56e8db8..86be004 100644
--- a/src/proxy_internal.c
+++ b/src/proxy_internal.c
@@ -143,7 +143,6 @@ static int
virProxyForkServer(void)
{
const char *proxyPath = virProxyFindServerPath();
- int ret, status;
pid_t pid;
const char *proxyarg[2];
@@ -157,20 +156,11 @@ virProxyForkServer(void)
proxyarg[0] = proxyPath;
proxyarg[1] = NULL;
- if (virExec(NULL, proxyarg, NULL, NULL,
- &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+ if (virExecDaemonize(NULL, proxyarg, NULL, NULL,
+ &pid, -1, NULL, NULL, VIR_EXEC_DAEMON,
+ NULL, NULL) < 0)
VIR_ERROR0("Failed to fork libvirt_proxy\n");
- /*
- * do a waitpid on the intermediate process to avoid zombies.
- */
-retry_wait:
- ret = waitpid(pid, &status, 0);
- if (ret < 0) {
- if (errno == EINTR)
- goto retry_wait;
- }
-
return (0);
}
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f9fe2ba..cc510b2 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1434,38 +1434,32 @@ static int qemudStartVMDaemon(virConnectPtr conn,
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
- ret = virExecWithHook(conn, argv, progenv, &keepfd, &child,
- stdin_fd, &vm->logfile, &vm->logfile,
- VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON,
- qemudSecurityHook, &hookData);
+ ret = virExecDaemonize(conn, argv, progenv, &keepfd, &child,
+ stdin_fd, &vm->logfile, &vm->logfile,
+ VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON,
+ qemudSecurityHook, &hookData);
/* wait for qemu process to to show up */
if (ret == 0) {
int retries = 100;
- int childstat;
- while (waitpid(child, &childstat, 0) == -1 &&
- errno == EINTR);
+ while (retries) {
+ if ((ret = virFileReadPid(driver->stateDir,
+ vm->def->name, &vm->pid)) == 0)
+ break;
- if (childstat == 0) {
- while (retries) {
- if ((ret = virFileReadPid(driver->stateDir, vm->def->name,
&vm->pid)) == 0)
- break;
- usleep(100*1000);
- retries--;
- }
- if (ret) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Domain %s didn't show up\n"),
vm->def->name);
- ret = -1;
- }
- } else {
+ usleep(100*1000);
+ retries--;
+ }
+ if (ret) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- "%s", _("Unable to daemonize QEMU
process"));
+ _("Domain %s didn't show up\n"),
vm->def->name);
ret = -1;
}
- vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
- }
+ } else
+ ret = -1;
+
+ vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 24226e5..8a389bc 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -284,7 +284,6 @@ remoteForkDaemon(virConnectPtr conn)
{
const char *daemonPath = remoteFindDaemonPath();
const char *const daemonargs[] = { daemonPath, "--timeout=30", NULL };
- int ret, status;
pid_t pid;
if (!daemonPath) {
@@ -292,18 +291,10 @@ remoteForkDaemon(virConnectPtr conn)
return -1;
}
- if (virExec(NULL, daemonargs, NULL, NULL,
- &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+ if (virExecDaemonize(NULL, daemonargs, NULL, NULL,
+ &pid, -1, NULL, NULL, VIR_EXEC_DAEMON,
+ NULL, NULL) < 0)
return -1;
- /*
- * do a waitpid on the intermediate process to avoid zombies.
- */
- retry_wait:
- ret = waitpid(pid, &status, 0);
- if (ret < 0) {
- if (errno == EINTR)
- goto retry_wait;
- }
return 0;
}
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 0457f13..8673095 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -821,16 +821,11 @@ static int umlStartVMDaemon(virConnectPtr conn,
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
- ret = virExec(conn, argv, progenv, &keepfd, &pid,
- -1, &logfd, &logfd,
- VIR_EXEC_DAEMON);
+ ret = virExecDaemonize(conn, argv, progenv, &keepfd, &pid,
+ -1, &logfd, &logfd,
+ VIR_EXEC_DAEMON, NULL, NULL);
close(logfd);
- /* Cleanup intermediate proces */
- if (waitpid(pid, NULL, 0) != pid)
- umlLog(VIR_LOG_WARN, _("failed to wait on process: %d: %s\n"),
- pid, virStrerror(errno, ebuf, sizeof ebuf));
-
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
VIR_FREE(argv);
diff --git a/src/util.c b/src/util.c
index 47a1cd3..07484af 100644
--- a/src/util.c
+++ b/src/util.c
@@ -591,6 +591,55 @@ virExec(virConnectPtr conn,
flags, NULL, NULL);
}
+/*
+ * See __virExec for explanation of the arguments.
+ *
+ * This function will wait for the intermediate process (between the caller
+ * and the daemon) to exit. retpid will be the pid of the daemon, which can
+ * be checked for example to see if the daemon crashed immediately.
+ *
+ * Returns 0 on success
+ * -1 if initial fork failed (will have a reported error)
+ * -2 if intermediate process failed
+ * (won't have a reported error. pending on where the failure
+ * occured and when in the process occured, the error output
+ * could have gone to stderr or the passed errfd).
+ */
+int virExecDaemonize(virConnectPtr conn,
+ const char *const*argv,
+ const char *const*envp,
+ const fd_set *keepfd,
+ pid_t *retpid,
+ int infd, int *outfd, int *errfd,
+ int flags,
+ virExecHook hook,
+ void *data) {
+ int ret;
+ int childstat = 0;
+
+ ret = virExecWithHook(conn, argv, envp, keepfd, retpid,
+ infd, outfd, errfd,
+ flags |= VIR_EXEC_DAEMON,
+ hook, data);
+
+ /* __virExec should have set an error */
+ if (ret != 0)
+ return -1;
+
+ /* Wait for intermediate process to exit */
+ while (waitpid(*retpid, &childstat, 0) == -1 &&
+ errno == EINTR);
+
+ if (childstat != 0) {
+ ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Intermediate daemon process exited with status %d."),
+ WEXITSTATUS(childstat));
+ ret = -2;
+ }
+
+ return ret;
+}
+
static int
virPipeReadUntilEOF(virConnectPtr conn, int outfd, int errfd,
char **outbuf, char **errbuf) {
diff --git a/src/util.h b/src/util.h
index 4bed077..bc29b16 100644
--- a/src/util.h
+++ b/src/util.h
@@ -46,6 +46,15 @@ int virSetCloseExec(int fd);
* after fork() but before execve() */
typedef int (*virExecHook)(void *data);
+int virExecDaemonize(virConnectPtr conn,
+ const char *const*argv,
+ const char *const*envp,
+ const fd_set *keepfd,
+ pid_t *retpid,
+ int infd, int *outfd, int *errfd,
+ int flags,
+ virExecHook hook,
+ void *data);
int virExecWithHook(virConnectPtr conn,
const char *const*argv,
const char *const*envp,
--
1.6.0.6