From 79d9d2432ff6142102c4e9489d79741b6baf54c7 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 11 May 2009 13:34:37 +0000 Subject: [PATCH] Add helper function virExecDaemonize 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. --- ChangeLog | 7 ++++++ 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 ++++++++ 8 files changed, 92 insertions(+), 57 deletions(-) diff --git a/ChangeLog b/ChangeLog index c4e9739ce7..07681e2fc2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +Mon May 11 09:29:52 EDT 2009 Cole Robinson + + * src/libvirt_private.syms src/util.[ch]: Add a helper function + virExecDaemonize + * src/proxy_internal.c src/qemu_driver.c src/uml_driver.c + src/remote_driver.c: Use the new helper. + Mon May 11 11:54:53 CEST 2009 Daniel Veillard * src/vbox/vbox_tmpl.c: "Host only" and "Internal" network support diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 599ec0b196..ce6386968e 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 56e8db89fa..9e7ab56f70 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, 0, + 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 7a46b1c06b..bafdd73f78 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, + 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 24226e5023..4f69876dad 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, 0, + 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 0457f13c86..b72c540831 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, + 0, 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 c5edc9d96e..829e1783ba 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 4bed0778d8..bc29b169f2 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,