[libvirt] [PATCH 0/5] Consolidate all code for process interaction

This series pulls a handful of functions out of command.c and util.c, into virprocess.c, in preparation for adding new process management functions.

From: "Daniel P. Berrange" <berrange@redhat.com> Changing naming to follow the convention of "object" followed by "action" Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_process.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/util.c | 2 +- src/util/util.h | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 488aa99..2eae3f2 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -219,7 +219,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) break; if ((errno == ENOENT || errno == ECONNREFUSED) && - virKillProcess(cpid, 0) == 0) { + virProcessKill(cpid, 0) == 0) { /* ENOENT : Socket may not have shown up yet * ECONNREFUSED : Leftover socket hasn't been removed yet */ continue; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fa3b7b2..aec7b6c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -283,7 +283,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) break; if ((errno == ENOENT || errno == ECONNREFUSED) && - virKillProcess(cpid, 0) == 0) { + virProcessKill(cpid, 0) == 0) { /* ENOENT : Socket may not have shown up yet * ECONNREFUSED : Leftover socket hasn't been removed yet */ continue; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ec312d1..637f7be 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3920,7 +3920,7 @@ qemuProcessKill(struct qemud_driver *driver, signum = 0; /* Just check for existence */ } - if (virKillProcess(vm->pid, signum) < 0) { + if (virProcessKill(vm->pid, signum) < 0) { if (errno != ESRCH) { char ebuf[1024]; VIR_WARN("Failed to terminate process %d with SIG%s: %s", diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 8fd18f9..6d0ef79 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1128,7 +1128,7 @@ static void umlShutdownVMDaemon(struct uml_driver *driver, if (!virDomainObjIsActive(vm)) return; - virKillProcess(vm->pid, SIGTERM); + virProcessKill(vm->pid, SIGTERM); VIR_FORCE_CLOSE(priv->monitor); diff --git a/src/util/util.c b/src/util/util.c index 3faadb2..8ad38ec 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2290,7 +2290,7 @@ check_and_return: } /* send signal to a single process */ -int virKillProcess(pid_t pid, int sig) +int virProcessKill(pid_t pid, int sig) { if (pid <= 1) { errno = ESRCH; diff --git a/src/util/util.h b/src/util/util.h index ad72f67..501373d 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -255,7 +255,7 @@ static inline int getgid (void) { return 0; } char *virGetHostname(virConnectPtr conn); -int virKillProcess(pid_t pid, int sig); +int virProcessKill(pid_t pid, int sig); char *virGetUserDirectory(void); char *virGetUserConfigDirectory(void); -- 1.7.11.2

On 09/25/2012 02:28 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Changing naming to follow the convention of "object" followed by "action"
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_process.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/util.c | 2 +- src/util/util.h | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
ACK. Mechanical, and nice lead-in to the later file motion. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> Change "Pid" to "Process" to align with the virProcessKill API naming prefix Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 +- src/libvirt_private.syms | 4 ++-- src/lxc/lxc_container.c | 4 ++-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetsocket.c | 2 +- src/util/command.c | 24 ++++++++++++------------ src/util/command.h | 6 +++--- src/util/util.c | 6 +++--- tests/testutils.c | 4 ++-- 9 files changed, 27 insertions(+), 27 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 349ffd6..8023b39 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -196,7 +196,7 @@ static int daemonForkIntoBackground(const char *argv0) VIR_FORCE_CLOSE(statuspipe[1]); /* We wait to make sure the first child forked successfully */ - if (virPidWait(pid, NULL) < 0) + if (virProcessWait(pid, NULL) < 0) goto error; /* If we get here, then the grandchild was spawned, so we diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b6068d..362621d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -166,8 +166,8 @@ virCommandTranslateStatus; virCommandWait; virCommandWriteArgLog; virFork; -virPidAbort; -virPidWait; +virProcessAbort; +virProcessWait; virRun; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4bb2aff..0c7e029 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -163,7 +163,7 @@ int lxcContainerHasReboot(void) virReportSystemError(errno, "%s", _("Unable to clone to check reboot support")); return -1; - } else if (virPidWait(cpid, &status) < 0) { + } else if (virProcessWait(cpid, &status) < 0) { return -1; } @@ -2001,7 +2001,7 @@ int lxcContainerAvailable(int features) VIR_DEBUG("clone call returned %s, container support is not enabled", virStrerror(errno, ebuf, sizeof(ebuf))); return -1; - } else if (virPidWait(cpid, NULL) < 0) { + } else if (virProcessWait(cpid, NULL) < 0) { return -1; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d2e8094..87601b3 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -214,7 +214,7 @@ static void virLXCControllerStopInit(virLXCControllerPtr ctrl) return; virLXCControllerCloseLoopDevices(ctrl, true); - virPidAbort(ctrl->initpid); + virProcessAbort(ctrl->initpid); ctrl->initpid = 0; } diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a96feda..e85fbf1 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -914,7 +914,7 @@ void virNetSocketDispose(void *obj) VIR_FORCE_CLOSE(sock->fd); VIR_FORCE_CLOSE(sock->errfd); - virPidAbort(sock->pid); + virProcessAbort(sock->pid); VIR_FREE(sock->localAddrStr); VIR_FREE(sock->remoteAddrStr); diff --git a/src/util/command.c b/src/util/command.c index 1adf7b9..ecfb015 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1691,7 +1691,7 @@ virCommandToString(virCommandPtr cmd) * @status: child exit status to translate * * Translate an exit status into a malloc'd string. Generic helper - * for virCommandRun(), virCommandWait(), virRun(), and virPidWait() + * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait() * status argument, as well as raw waitpid(). */ char * @@ -2164,7 +2164,7 @@ virCommandHook(void *data) * for pid. While cmd is still in scope, you may reap the child via * virCommandWait or virCommandAbort. But after virCommandFree, if * you have not yet reaped the child, then it continues to run until - * you call virPidWait or virPidAbort. + * you call virProcessWait or virProcessAbort. */ int virCommandRunAsync(virCommandPtr cmd, pid_t *pid) @@ -2257,7 +2257,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) /** - * virPidWait: + * virProcessWait: * @pid: child to wait on * @exitstatus: optional status collection * @@ -2268,7 +2268,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) * child must exit with status 0 for this to succeed. */ int -virPidWait(pid_t pid, int *exitstatus) +virProcessWait(pid_t pid, int *exitstatus) { int ret; int status; @@ -2338,13 +2338,13 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) return -1; } - /* If virPidWait reaps pid but then returns failure because + /* If virProcessWait reaps pid but then returns failure because * exitstatus was NULL, then a second virCommandWait would risk * calling waitpid on an unrelated process. Besides, that error * message is not as detailed as what we can provide. So, we - * guarantee that virPidWait only fails due to failure to wait, + * guarantee that virProcessWait only fails due to failure to wait, * and repeat the exitstatus check code ourselves. */ - ret = virPidWait(cmd->pid, exitstatus ? exitstatus : &status); + ret = virProcessWait(cmd->pid, exitstatus ? exitstatus : &status); if (ret == 0) { cmd->pid = -1; cmd->reap = false; @@ -2370,7 +2370,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) #ifndef WIN32 /** - * virPidAbort: + * virProcessAbort: * @pid: child process to kill * * Abort a child process if PID is positive and that child is still @@ -2380,7 +2380,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) * this does nothing. */ void -virPidAbort(pid_t pid) +virProcessAbort(pid_t pid) { int saved_errno; int ret; @@ -2444,13 +2444,13 @@ virCommandAbort(virCommandPtr cmd) { if (!cmd || cmd->pid == -1) return; - virPidAbort(cmd->pid); + virProcessAbort(cmd->pid); cmd->pid = -1; cmd->reap = false; } #else /* WIN32 */ void -virPidAbort(pid_t pid) +virProcessAbort(pid_t pid) { /* Not yet ported to mingw. Any volunteers? */ VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid); @@ -2617,7 +2617,7 @@ int virCommandHandshakeNotify(virCommandPtr cmd) * * Release all resources. The only exception is that if you called * virCommandRunAsync with a non-null pid, then the asynchronous child - * is not reaped, and you must call virPidWait() or virPidAbort() yourself. + * is not reaped, and you must call virProcessWait() or virProcessAbort() yourself. */ void virCommandFree(virCommandPtr cmd) diff --git a/src/util/command.h b/src/util/command.h index d1a1bf9..de17a43 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -151,8 +151,8 @@ int virCommandRun(virCommandPtr cmd, int virCommandRunAsync(virCommandPtr cmd, pid_t *pid) ATTRIBUTE_RETURN_CHECK; -int virPidWait(pid_t pid, - int *exitstatus) ATTRIBUTE_RETURN_CHECK; +int virProcessWait(pid_t pid, + int *exitstatus) ATTRIBUTE_RETURN_CHECK; int virCommandWait(virCommandPtr cmd, int *exitstatus) ATTRIBUTE_RETURN_CHECK; @@ -165,7 +165,7 @@ int virCommandHandshakeWait(virCommandPtr cmd) int virCommandHandshakeNotify(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; -void virPidAbort(pid_t pid); +void virProcessAbort(pid_t pid); void virCommandAbort(virCommandPtr cmd); diff --git a/src/util/util.c b/src/util/util.c index 8ad38ec..5bf82dc 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -729,10 +729,10 @@ virFileAccessibleAs(const char *path, int mode, } if (pid) { /* parent */ - if (virPidWait(pid, &status) < 0) { - /* virPidWait() already + if (virProcessWait(pid, &status) < 0) { + /* virProcessWait() already * reported error */ - return -1; + return -1; } if (!WIFEXITED(status)) { diff --git a/tests/testutils.c b/tests/testutils.c index b555e06..004731d 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -329,7 +329,7 @@ virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen) VIR_FORCE_CLOSE(pipefd[1]); len = virFileReadLimFD(pipefd[0], maxlen, buf); VIR_FORCE_CLOSE(pipefd[0]); - if (virPidWait(pid, NULL) < 0) + if (virProcessWait(pid, NULL) < 0) return -1; return len; @@ -699,7 +699,7 @@ int virtTestMain(int argc, } else { int i, status; for (i = 0 ; i < mp ; i++) { - if (virPidWait(workers[i], NULL) < 0) + if (virProcessWait(workers[i], NULL) < 0) ret = EXIT_FAILURE; } VIR_FREE(workers); -- 1.7.11.2

On 09/25/2012 02:28 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Change "Pid" to "Process" to align with the virProcessKill API naming prefix
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 +- src/libvirt_private.syms | 4 ++-- src/lxc/lxc_container.c | 4 ++-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetsocket.c | 2 +- src/util/command.c | 24 ++++++++++++------------ src/util/command.h | 6 +++--- src/util/util.c | 6 +++--- tests/testutils.c | 4 ++-- 9 files changed, 27 insertions(+), 27 deletions(-)
ACK - again, mechanical. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> The virCommand prefix was inappropriate because the API does not use any virCommandPtr object instance. This API closely related to waitpid/exit, so use virProcess as the prefix Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 2 +- src/libvirt_private.syms | 2 +- src/util/command.c | 16 ++++++++-------- src/util/command.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 0a082b9..889bdbe 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2832,7 +2832,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, authdismissed = (pkout && strstr(pkout, "dismissed=true")); if (status != 0) { - char *tmp = virCommandTranslateStatus(status); + char *tmp = virProcessTranslateStatus(status); VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d: %s"), action, (long long) callerPid, callerUid, NULLSTR(tmp)); VIR_FREE(tmp); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 362621d..4bb6fa3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -162,7 +162,7 @@ virCommandSetPreExecHook; virCommandSetWorkingDirectory; virCommandToString; virCommandTransferFD; -virCommandTranslateStatus; +virProcessTranslateStatus; virCommandWait; virCommandWriteArgLog; virFork; diff --git a/src/util/command.c b/src/util/command.c index ecfb015..4f0ba88 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1687,7 +1687,7 @@ virCommandToString(virCommandPtr cmd) /** - * virCommandTranslateStatus: + * virProcessTranslateStatus: * @status: child exit status to translate * * Translate an exit status into a malloc'd string. Generic helper @@ -1695,7 +1695,7 @@ virCommandToString(virCommandPtr cmd) * status argument, as well as raw waitpid(). */ char * -virCommandTranslateStatus(int status) +virProcessTranslateStatus(int status) { char *buf; if (WIFEXITED(status)) { @@ -2032,7 +2032,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) if (virCommandWait(cmd, exitstatus) < 0) ret = -1; - str = (exitstatus ? virCommandTranslateStatus(*exitstatus) + str = (exitstatus ? virProcessTranslateStatus(*exitstatus) : (char *) "status 0"); VIR_DEBUG("Result %s, stdout: '%s' stderr: '%s'", NULLSTR(str), @@ -2291,7 +2291,7 @@ virProcessWait(pid_t pid, int *exitstatus) if (exitstatus == NULL) { if (status != 0) { - char *st = virCommandTranslateStatus(status); + char *st = virProcessTranslateStatus(status); virReportError(VIR_ERR_INTERNAL_ERROR, _("Child process (%lld) unexpected %s"), (long long) pid, NULLSTR(st)); @@ -2350,7 +2350,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) cmd->reap = false; if (status) { char *str = virCommandToString(cmd); - char *st = virCommandTranslateStatus(status); + char *st = virProcessTranslateStatus(status); bool haveErrMsg = cmd->errbuf && *cmd->errbuf && (*cmd->errbuf)[0]; virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2398,7 +2398,7 @@ virProcessAbort(pid_t pid) while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR); if (ret == pid) { - tmp = virCommandTranslateStatus(status); + tmp = virProcessTranslateStatus(status); VIR_DEBUG("process has ended: %s", tmp); goto cleanup; } else if (ret == 0) { @@ -2408,7 +2408,7 @@ virProcessAbort(pid_t pid) while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR); if (ret == pid) { - tmp = virCommandTranslateStatus(status); + tmp = virProcessTranslateStatus(status); VIR_DEBUG("process has ended: %s", tmp); goto cleanup; } else if (ret == 0) { @@ -2417,7 +2417,7 @@ virProcessAbort(pid_t pid) while ((ret = waitpid(pid, &status, 0)) == -1 && errno == EINTR); if (ret == pid) { - tmp = virCommandTranslateStatus(status); + tmp = virProcessTranslateStatus(status); VIR_DEBUG("process has ended: %s", tmp); goto cleanup; } diff --git a/src/util/command.h b/src/util/command.h index de17a43..b91331c 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -141,7 +141,7 @@ void virCommandWriteArgLog(virCommandPtr cmd, char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; -char *virCommandTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK; +char *virProcessTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK; int virCommandExec(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; -- 1.7.11.2

On 09/25/2012 02:28 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virCommand prefix was inappropriate because the API does not use any virCommandPtr object instance. This API closely related to waitpid/exit, so use virProcess as the prefix
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 2 +- src/libvirt_private.syms | 2 +- src/util/command.c | 16 ++++++++-------- src/util/command.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-)
Well, it CAN be used on the status argument returned by virCommandRun, but that's a stretch. I like you rename. ACK.
+++ b/src/libvirt_private.syms @@ -162,7 +162,7 @@ virCommandSetPreExecHook; virCommandSetWorkingDirectory; virCommandToString; virCommandTransferFD; -virCommandTranslateStatus; +virProcessTranslateStatus; virCommandWait;
Now unsorted, but a later patch moves it anyways, so no need to sort just to avoid churn. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> There are a number of process related functions spread across multiple files. Start to consolidate them by creating a virprocess.{c,h} file Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/libvirt_private.syms | 3 ++ src/qemu/qemu_agent.c | 1 + src/qemu/qemu_monitor.c | 1 + src/qemu/qemu_process.c | 1 + src/uml/uml_driver.c | 1 + src/util/util.c | 57 -------------------------------- src/util/util.h | 2 -- src/util/virprocess.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 31 ++++++++++++++++++ 10 files changed, 123 insertions(+), 59 deletions(-) create mode 100644 src/util/virprocess.c create mode 100644 src/util/virprocess.h diff --git a/src/Makefile.am b/src/Makefile.am index 4ae741b..ae3d491 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -87,6 +87,7 @@ UTIL_SOURCES = \ util/virnodesuspend.c util/virnodesuspend.h \ util/virobject.c util/virobject.h \ util/virpidfile.c util/virpidfile.h \ + util/virprocess.c util/virprocess.h \ util/virtypedparam.c util/virtypedparam.h \ util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4bb6fa3..52c2cb2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1708,6 +1708,9 @@ virPidFileDelete; virPidFileDeletePath; +# virprocess.h +virProcessKill; + # virrandom.h virRandom; virRandomBits; diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 2eae3f2..ab6dc22 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -39,6 +39,7 @@ #include "virterror_internal.h" #include "json.h" #include "virfile.h" +#include "virprocess.h" #include "virtime.h" #include "virobject.h" diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aec7b6c..2796a61 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -36,6 +36,7 @@ #include "memory.h" #include "logging.h" #include "virfile.h" +#include "virprocess.h" #include "virobject.h" #ifdef WITH_DTRACE_PROBES diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 637f7be..3cd30af 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -61,6 +61,7 @@ #include "locking/domain_lock.h" #include "network/bridge_driver.h" #include "uuid.h" +#include "virprocess.h" #include "virtime.h" #include "virnetdevtap.h" #include "bitmap.h" diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 6d0ef79..c341fab 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -63,6 +63,7 @@ #include "configmake.h" #include "virnetdevtap.h" #include "virnodesuspend.h" +#include "virprocess.h" #include "viruri.h" #define VIR_FROM_THIS VIR_FROM_UML diff --git a/src/util/util.c b/src/util/util.c index 5bf82dc..062fbf8 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2289,63 +2289,6 @@ check_and_return: return result; } -/* send signal to a single process */ -int virProcessKill(pid_t pid, int sig) -{ - if (pid <= 1) { - errno = ESRCH; - return -1; - } - -#ifdef WIN32 - /* Mingw / Windows don't have many signals (AFAIK) */ - switch (sig) { - case SIGINT: - /* This does a Ctrl+C equiv */ - if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, pid)) { - errno = ESRCH; - return -1; - } - break; - - case SIGTERM: - /* Since TerminateProcess is closer to SIG_KILL, we do - * a Ctrl+Break equiv which is more pleasant like the - * good old unix SIGTERM/HUP - */ - if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid)) { - errno = ESRCH; - return -1; - } - break; - - default: - { - HANDLE proc; - proc = OpenProcess(PROCESS_TERMINATE, FALSE, pid); - if (!proc) { - errno = ESRCH; /* Not entirely accurate, but close enough */ - return -1; - } - - /* - * TerminateProcess is more or less equiv to SIG_KILL, in that - * a process can't trap / block it - */ - if (sig != 0 && !TerminateProcess(proc, sig)) { - errno = ESRCH; - return -1; - } - CloseHandle(proc); - } - } - return 0; -#else - return kill(pid, sig); -#endif -} - - #ifdef HAVE_GETPWUID_R enum { VIR_USER_ENT_DIRECTORY, diff --git a/src/util/util.h b/src/util/util.h index 501373d..5ab36ed 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -255,8 +255,6 @@ static inline int getgid (void) { return 0; } char *virGetHostname(virConnectPtr conn); -int virProcessKill(pid_t pid, int sig); - char *virGetUserDirectory(void); char *virGetUserConfigDirectory(void); char *virGetUserCacheDirectory(void); diff --git a/src/util/virprocess.c b/src/util/virprocess.c new file mode 100644 index 0000000..e7db68f --- /dev/null +++ b/src/util/virprocess.c @@ -0,0 +1,84 @@ +/* + * virprocess.c: interaction with processes + * + * Copyright (C) 2010-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + + +#include <config.h> + +#include <signal.h> +#include <errno.h> + +#include "virprocess.h" + +/* send signal to a single process */ +int virProcessKill(pid_t pid, int sig) +{ + if (pid <= 1) { + errno = ESRCH; + return -1; + } + +#ifdef WIN32 + /* Mingw / Windows don't have many signals (AFAIK) */ + switch (sig) { + case SIGINT: + /* This does a Ctrl+C equiv */ + if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, pid)) { + errno = ESRCH; + return -1; + } + break; + + case SIGTERM: + /* Since TerminateProcess is closer to SIG_KILL, we do + * a Ctrl+Break equiv which is more pleasant like the + * good old unix SIGTERM/HUP + */ + if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid)) { + errno = ESRCH; + return -1; + } + break; + + default: + { + HANDLE proc; + proc = OpenProcess(PROCESS_TERMINATE, FALSE, pid); + if (!proc) { + errno = ESRCH; /* Not entirely accurate, but close enough */ + return -1; + } + + /* + * TerminateProcess is more or less equiv to SIG_KILL, in that + * a process can't trap / block it + */ + if (sig != 0 && !TerminateProcess(proc, sig)) { + errno = ESRCH; + return -1; + } + CloseHandle(proc); + } + } + return 0; +#else + return kill(pid, sig); +#endif +} diff --git a/src/util/virprocess.h b/src/util/virprocess.h new file mode 100644 index 0000000..b1000c6 --- /dev/null +++ b/src/util/virprocess.h @@ -0,0 +1,31 @@ +/* + * virprocess.h: interaction with processes + * + * Copyright (C) 2010-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_PROCESS_H__ +# define __VIR_PROCESS_H__ + +# include <sys/types.h> + +# include "internal.h" + +int virProcessKill(pid_t pid, int sig); + +#endif /* __VIR_PROCESS_H__ */ -- 1.7.11.2

On 09/25/2012 02:28 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
There are a number of process related functions spread across multiple files. Start to consolidate them by creating a virprocess.{c,h} file
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/libvirt_private.syms | 3 ++ src/qemu/qemu_agent.c | 1 + src/qemu/qemu_monitor.c | 1 + src/qemu/qemu_process.c | 1 + src/uml/uml_driver.c | 1 + src/util/util.c | 57 -------------------------------- src/util/util.h | 2 -- src/util/virprocess.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 31 ++++++++++++++++++ 10 files changed, 123 insertions(+), 59 deletions(-) create mode 100644 src/util/virprocess.c create mode 100644 src/util/virprocess.h
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange@redhat.com> Continue consolidation of process functions by moving some helpers out of command.{c,h} into virprocess.{c,h} Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 1 + daemon/remote.c | 1 + po/POTFILES.in | 1 + src/libvirt_private.syms | 7 ++- src/lxc/lxc_container.c | 1 + src/lxc/lxc_controller.c | 1 + src/rpc/virnetsocket.c | 1 + src/util/command.c | 143 +------------------------------------------ src/util/command.h | 8 --- src/util/util.c | 1 + src/util/virprocess.c | 153 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 11 ++++ tests/testutils.c | 1 + 13 files changed, 177 insertions(+), 153 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 8023b39..0151124 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -36,6 +36,7 @@ #include "virterror_internal.h" #include "virfile.h" #include "virpidfile.h" +#include "virprocess.h" #define VIR_FROM_THIS VIR_FROM_QEMU diff --git a/daemon/remote.c b/daemon/remote.c index 889bdbe..e7fe128 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -46,6 +46,7 @@ #include "virfile.h" #include "virtypedparam.h" #include "virdbus.h" +#include "virprocess.h" #include "remote_protocol.h" #include "qemu_protocol.h" diff --git a/po/POTFILES.in b/po/POTFILES.in index 12a2b25..74d8dcc 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -157,6 +157,7 @@ src/util/virnetdevvportprofile.c src/util/virnetlink.c src/util/virnodesuspend.c src/util/virpidfile.c +src/util/virprocess.c src/util/virrandom.c src/util/virsocketaddr.c src/util/virterror.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 52c2cb2..4635a4d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -162,12 +162,9 @@ virCommandSetPreExecHook; virCommandSetWorkingDirectory; virCommandToString; virCommandTransferFD; -virProcessTranslateStatus; virCommandWait; virCommandWriteArgLog; virFork; -virProcessAbort; -virProcessWait; virRun; @@ -1709,7 +1706,11 @@ virPidFileDeletePath; # virprocess.h +virProcessAbort; virProcessKill; +virProcessTranslateStatus; +virProcessWait; + # virrandom.h virRandom; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 0c7e029..2789c17 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -63,6 +63,7 @@ #include "virfile.h" #include "command.h" #include "virnetdev.h" +#include "virprocess.h" #define VIR_FROM_THIS VIR_FROM_LXC diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 87601b3..d407e70 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -70,6 +70,7 @@ #include "processinfo.h" #include "nodeinfo.h" #include "virrandom.h" +#include "virprocess.h" #include "rpc/virnetserver.h" #define VIR_FROM_THIS VIR_FROM_LXC diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index e85fbf1..6ffa06e 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -44,6 +44,7 @@ #include "virfile.h" #include "event.h" #include "threads.h" +#include "virprocess.h" #include "passfd.h" diff --git a/src/util/command.c b/src/util/command.c index 4f0ba88..2657a1d 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -40,6 +40,7 @@ #include "logging.h" #include "virfile.h" #include "virpidfile.h" +#include "virprocess.h" #include "buf.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1686,31 +1687,6 @@ virCommandToString(virCommandPtr cmd) } -/** - * virProcessTranslateStatus: - * @status: child exit status to translate - * - * Translate an exit status into a malloc'd string. Generic helper - * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait() - * status argument, as well as raw waitpid(). - */ -char * -virProcessTranslateStatus(int status) -{ - char *buf; - if (WIFEXITED(status)) { - ignore_value(virAsprintf(&buf, _("exit status %d"), - WEXITSTATUS(status))); - } else if (WIFSIGNALED(status)) { - ignore_value(virAsprintf(&buf, _("fatal signal %d"), - WTERMSIG(status))); - } else { - ignore_value(virAsprintf(&buf, _("invalid value %d"), status)); - } - return buf; -} - - /* * Manage input and output to the child process. */ @@ -2257,55 +2233,6 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) /** - * virProcessWait: - * @pid: child to wait on - * @exitstatus: optional status collection - * - * Wait for a child process to complete. - * Return -1 on any error waiting for - * completion. Returns 0 if the command - * finished with the exit status set. If @exitstatus is NULL, then the - * child must exit with status 0 for this to succeed. - */ -int -virProcessWait(pid_t pid, int *exitstatus) -{ - int ret; - int status; - - if (pid <= 0) { - virReportSystemError(EINVAL, _("unable to wait for process %lld"), - (long long) pid); - return -1; - } - - /* Wait for intermediate process to exit */ - while ((ret = waitpid(pid, &status, 0)) == -1 && - errno == EINTR); - - if (ret == -1) { - virReportSystemError(errno, _("unable to wait for process %lld"), - (long long) pid); - return -1; - } - - if (exitstatus == NULL) { - if (status != 0) { - char *st = virProcessTranslateStatus(status); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Child process (%lld) unexpected %s"), - (long long) pid, NULLSTR(st)); - VIR_FREE(st); - return -1; - } - } else { - *exitstatus = status; - } - - return 0; -} - -/** * virCommandWait: * @cmd: command to wait on * @exitstatus: optional status collection @@ -2370,67 +2297,6 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) #ifndef WIN32 /** - * virProcessAbort: - * @pid: child process to kill - * - * Abort a child process if PID is positive and that child is still - * running, without issuing any errors or affecting errno. Designed - * for error paths where some but not all paths to the cleanup code - * might have started the child process. If @pid is 0 or negative, - * this does nothing. - */ -void -virProcessAbort(pid_t pid) -{ - int saved_errno; - int ret; - int status; - char *tmp = NULL; - - if (pid <= 0) - return; - - /* See if intermediate process has exited; if not, try a nice - * SIGTERM followed by a more severe SIGKILL. - */ - saved_errno = errno; - VIR_DEBUG("aborting child process %d", pid); - while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && - errno == EINTR); - if (ret == pid) { - tmp = virProcessTranslateStatus(status); - VIR_DEBUG("process has ended: %s", tmp); - goto cleanup; - } else if (ret == 0) { - VIR_DEBUG("trying SIGTERM to child process %d", pid); - kill(pid, SIGTERM); - usleep(10 * 1000); - while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && - errno == EINTR); - if (ret == pid) { - tmp = virProcessTranslateStatus(status); - VIR_DEBUG("process has ended: %s", tmp); - goto cleanup; - } else if (ret == 0) { - VIR_DEBUG("trying SIGKILL to child process %d", pid); - kill(pid, SIGKILL); - while ((ret = waitpid(pid, &status, 0)) == -1 && - errno == EINTR); - if (ret == pid) { - tmp = virProcessTranslateStatus(status); - VIR_DEBUG("process has ended: %s", tmp); - goto cleanup; - } - } - } - VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid); - -cleanup: - VIR_FREE(tmp); - errno = saved_errno; -} - -/** * virCommandAbort: * @cmd: command to abort * @@ -2450,13 +2316,6 @@ virCommandAbort(virCommandPtr cmd) } #else /* WIN32 */ void -virProcessAbort(pid_t pid) -{ - /* Not yet ported to mingw. Any volunteers? */ - VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid); -} - -void virCommandAbort(virCommandPtr cmd ATTRIBUTE_UNUSED) { /* Mingw lacks WNOHANG and kill(). But since we haven't ported diff --git a/src/util/command.h b/src/util/command.h index b91331c..177a0eb 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -140,9 +140,6 @@ void virCommandWriteArgLog(virCommandPtr cmd, char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; - -char *virProcessTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK; - int virCommandExec(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; int virCommandRun(virCommandPtr cmd, @@ -151,9 +148,6 @@ int virCommandRun(virCommandPtr cmd, int virCommandRunAsync(virCommandPtr cmd, pid_t *pid) ATTRIBUTE_RETURN_CHECK; -int virProcessWait(pid_t pid, - int *exitstatus) ATTRIBUTE_RETURN_CHECK; - int virCommandWait(virCommandPtr cmd, int *exitstatus) ATTRIBUTE_RETURN_CHECK; @@ -165,8 +159,6 @@ int virCommandHandshakeWait(virCommandPtr cmd) int virCommandHandshakeNotify(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; -void virProcessAbort(pid_t pid); - void virCommandAbort(virCommandPtr cmd); void virCommandFree(virCommandPtr cmd); diff --git a/src/util/util.c b/src/util/util.c index 062fbf8..28f9ae3 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -87,6 +87,7 @@ #include "command.h" #include "nonblocking.h" #include "passfd.h" +#include "virprocess.h" #ifndef NSIG # define NSIG 32 diff --git a/src/util/virprocess.c b/src/util/virprocess.c index e7db68f..958f5f7 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -24,8 +24,161 @@ #include <signal.h> #include <errno.h> +#include <sys/wait.h> #include "virprocess.h" +#include "virterror_internal.h" +#include "memory.h" +#include "logging.h" +#include "util.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/** + * virProcessTranslateStatus: + * @status: child exit status to translate + * + * Translate an exit status into a malloc'd string. Generic helper + * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait() + * status argument, as well as raw waitpid(). + */ +char * +virProcessTranslateStatus(int status) +{ + char *buf; + if (WIFEXITED(status)) { + ignore_value(virAsprintf(&buf, _("exit status %d"), + WEXITSTATUS(status))); + } else if (WIFSIGNALED(status)) { + ignore_value(virAsprintf(&buf, _("fatal signal %d"), + WTERMSIG(status))); + } else { + ignore_value(virAsprintf(&buf, _("invalid value %d"), status)); + } + return buf; +} + + +#ifndef WIN32 +/** + * virProcessAbort: + * @pid: child process to kill + * + * Abort a child process if PID is positive and that child is still + * running, without issuing any errors or affecting errno. Designed + * for error paths where some but not all paths to the cleanup code + * might have started the child process. If @pid is 0 or negative, + * this does nothing. + */ +void +virProcessAbort(pid_t pid) +{ + int saved_errno; + int ret; + int status; + char *tmp = NULL; + + if (pid <= 0) + return; + + /* See if intermediate process has exited; if not, try a nice + * SIGTERM followed by a more severe SIGKILL. + */ + saved_errno = errno; + VIR_DEBUG("aborting child process %d", pid); + while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && + errno == EINTR); + if (ret == pid) { + tmp = virProcessTranslateStatus(status); + VIR_DEBUG("process has ended: %s", tmp); + goto cleanup; + } else if (ret == 0) { + VIR_DEBUG("trying SIGTERM to child process %d", pid); + kill(pid, SIGTERM); + usleep(10 * 1000); + while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && + errno == EINTR); + if (ret == pid) { + tmp = virProcessTranslateStatus(status); + VIR_DEBUG("process has ended: %s", tmp); + goto cleanup; + } else if (ret == 0) { + VIR_DEBUG("trying SIGKILL to child process %d", pid); + kill(pid, SIGKILL); + while ((ret = waitpid(pid, &status, 0)) == -1 && + errno == EINTR); + if (ret == pid) { + tmp = virProcessTranslateStatus(status); + VIR_DEBUG("process has ended: %s", tmp); + goto cleanup; + } + } + } + VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid); + +cleanup: + VIR_FREE(tmp); + errno = saved_errno; +} +#else +void +virProcessAbort(pid_t pid) +{ + /* Not yet ported to mingw. Any volunteers? */ + VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid); +} +#endif + + +/** + * virProcessWait: + * @pid: child to wait on + * @exitstatus: optional status collection + * + * Wait for a child process to complete. + * Return -1 on any error waiting for + * completion. Returns 0 if the command + * finished with the exit status set. If @exitstatus is NULL, then the + * child must exit with status 0 for this to succeed. + */ +int +virProcessWait(pid_t pid, int *exitstatus) +{ + int ret; + int status; + + if (pid <= 0) { + virReportSystemError(EINVAL, _("unable to wait for process %lld"), + (long long) pid); + return -1; + } + + /* Wait for intermediate process to exit */ + while ((ret = waitpid(pid, &status, 0)) == -1 && + errno == EINTR); + + if (ret == -1) { + virReportSystemError(errno, _("unable to wait for process %lld"), + (long long) pid); + return -1; + } + + if (exitstatus == NULL) { + if (status != 0) { + char *st = virProcessTranslateStatus(status); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Child process (%lld) unexpected %s"), + (long long) pid, NULLSTR(st)); + VIR_FREE(st); + return -1; + } + } else { + *exitstatus = status; + } + + return 0; +} + /* send signal to a single process */ int virProcessKill(pid_t pid, int sig) diff --git a/src/util/virprocess.h b/src/util/virprocess.h index b1000c6..048a73c 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -26,6 +26,17 @@ # include "internal.h" +char * +virProcessTranslateStatus(int status); + +void +virProcessAbort(pid_t pid); + +int +virProcessWait(pid_t pid, int *exitstatus) + ATTRIBUTE_RETURN_CHECK; + int virProcessKill(pid_t pid, int sig); + #endif /* __VIR_PROCESS_H__ */ diff --git a/tests/testutils.c b/tests/testutils.c index 004731d..4d4f589 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -48,6 +48,7 @@ #include "command.h" #include "virrandom.h" #include "dirname.h" +#include "virprocess.h" #if TEST_OOM_TRACE # include <execinfo.h> -- 1.7.11.2

On 09/25/2012 02:28 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Continue consolidation of process functions by moving some helpers out of command.{c,h} into virprocess.{c,h}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 1 + daemon/remote.c | 1 + po/POTFILES.in | 1 + src/libvirt_private.syms | 7 ++- src/lxc/lxc_container.c | 1 + src/lxc/lxc_controller.c | 1 + src/rpc/virnetsocket.c | 1 + src/util/command.c | 143 +------------------------------------------ src/util/command.h | 8 --- src/util/util.c | 1 + src/util/virprocess.c | 153 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 11 ++++ tests/testutils.c | 1 + 13 files changed, 177 insertions(+), 153 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange
-
Eric Blake