[libvirt] [PATCH 1/3] command: introduce virPidWait, virPidAbort
Eric Blake
eblake at redhat.com
Tue Jul 12 19:36:49 UTC 2011
When using virCommandRunAsync and saving the pid for later, it
is useful to be able to reap that pid in the same way that it
would have been auto-reaped by virCommand if we had passed
NULL for the pid argument in the first place.
* src/util/command.c (virPidWait, virPidAbort): New functions,
created from...
(virCommandWait, virCommandAbort): ...bodies of these.
(includes): Drop duplicate <stdlib.h>. Ensure that our pid_t
assumptions hold.
(virCommandRunAsync): Improve documentation.
* src/util/command.h (virPidWait, virPidAbort): New prototypes.
* src/libvirt_private.syms: Export them.
* docs/internals/command.html.in: Document them.
---
docs/internals/command.html.in | 17 +++++
src/libvirt_private.syms | 2 +
src/util/command.c | 140 ++++++++++++++++++++++++++++------------
src/util/command.h | 28 ++++++++
4 files changed, 146 insertions(+), 41 deletions(-)
diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index 27dcf9c..8a194ec 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -497,6 +497,23 @@
error if not.
</p>
+ <p>
+ There are two approaches to child process cleanup, determined by
+ how long you want to keep the virCommand object in scope.
+ </p>
+
+ <p>1. If the virCommand object will outlast the child process,
+ then pass NULL for the pid argument, and the child process will
+ automatically be reaped at virCommandFree, unless you reap it
+ sooner via virCommandWait or virCommandAbort.
+ </p>
+
+ <p>2. If the child process must exist on at least one code path
+ after virCommandFree, then pass a pointer for the pid argument.
+ Later, to clean up the child, call virPidWait or virPidAbort.
+ Before virCommandFree, you can still use virCommandWait or
+ virCommandAbort to reap the process.
+ </p>
<h3><a name="release">Releasing resources</a></h3>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3237d18..f95d341 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -134,6 +134,8 @@ virCommandTranslateStatus;
virCommandWait;
virCommandWriteArgLog;
virFork;
+virPidAbort;
+virPidWait;
virRun;
diff --git a/src/util/command.c b/src/util/command.c
index eae58b2..c194fb1 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -41,8 +41,7 @@
#include "files.h"
#include "buf.h"
#include "ignore-value.h"
-
-#include <stdlib.h>
+#include "verify.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -50,6 +49,9 @@
virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
+/* We have quite a bit of changes to make if this doesn't hold. */
+verify(sizeof(pid_t) <= sizeof(int));
+
/* Flags for virExecWithHook */
enum {
VIR_EXEC_NONE = 0,
@@ -1897,6 +1899,48 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
/*
+ * 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
+ */
+int
+virPidWait(pid_t pid, int *exitstatus)
+{
+ int ret;
+ int status;
+
+ if (pid <= 0) {
+ virReportSystemError(EINVAL, _("unable to wait for process %d"), 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 %d"), pid);
+ return -1;
+ }
+
+ if (exitstatus == NULL) {
+ if (status != 0) {
+ char *st = virCommandTranslateStatus(status);
+ virCommandError(VIR_ERR_INTERNAL_ERROR,
+ _("Child process (%d) status unexpected: %s"),
+ pid, NULLSTR(st));
+ VIR_FREE(st);
+ return -1;
+ }
+ } else {
+ *exitstatus = status;
+ }
+
+ return 0;
+}
+
+/*
* Wait for the async command to complete.
* Return -1 on any error waiting for
* completion. Returns 0 if the command
@@ -1906,7 +1950,7 @@ int
virCommandWait(virCommandPtr cmd, int *exitstatus)
{
int ret;
- int status;
+ int status = 0;
if (!cmd ||cmd->has_error == ENOMEM) {
virReportOOMError();
@@ -1924,22 +1968,17 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
return -1;
}
-
- /* Wait for intermediate process to exit */
- while ((ret = waitpid(cmd->pid, &status, 0)) == -1 &&
- errno == EINTR);
-
- if (ret == -1) {
- virReportSystemError(errno, _("unable to wait for process %d"),
- cmd->pid);
- return -1;
- }
-
- cmd->pid = -1;
- cmd->reap = false;
-
- if (exitstatus == NULL) {
- if (status != 0) {
+ /* If virPidWait 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,
+ * and repeat the exitstatus check code ourselves. */
+ ret = virPidWait(cmd->pid, exitstatus ? exitstatus : &status);
+ if (ret == 0) {
+ cmd->pid = -1;
+ cmd->reap = false;
+ if (status) {
char *str = virCommandToString(cmd);
char *st = virCommandTranslateStatus(status);
virCommandError(VIR_ERR_INTERNAL_ERROR,
@@ -1949,75 +1988,94 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
VIR_FREE(st);
return -1;
}
- } else {
- *exitstatus = status;
}
- return 0;
+ return ret;
}
#ifndef WIN32
/*
- * Abort an async command if it is 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.
+ * 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.
*/
void
-virCommandAbort(virCommandPtr cmd)
+virPidAbort(pid_t pid)
{
int saved_errno;
int ret;
int status;
char *tmp = NULL;
- if (!cmd || cmd->pid == -1)
+ 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", cmd->pid);
- while ((ret = waitpid(cmd->pid, &status, WNOHANG)) == -1 &&
+ VIR_DEBUG("aborting child process %d", pid);
+ while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
errno == EINTR);
- if (ret == cmd->pid) {
+ if (ret == pid) {
tmp = virCommandTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
} else if (ret == 0) {
- VIR_DEBUG("trying SIGTERM to child process %d", cmd->pid);
- kill(cmd->pid, SIGTERM);
+ VIR_DEBUG("trying SIGTERM to child process %d", pid);
+ kill(pid, SIGTERM);
usleep(10 * 1000);
- while ((ret = waitpid(cmd->pid, &status, WNOHANG)) == -1 &&
+ while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
errno == EINTR);
- if (ret == cmd->pid) {
+ if (ret == pid) {
tmp = virCommandTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
} else if (ret == 0) {
- VIR_DEBUG("trying SIGKILL to child process %d", cmd->pid);
- kill(cmd->pid, SIGKILL);
- while ((ret = waitpid(cmd->pid, &status, 0)) == -1 &&
+ VIR_DEBUG("trying SIGKILL to child process %d", pid);
+ kill(pid, SIGKILL);
+ while ((ret = waitpid(pid, &status, 0)) == -1 &&
errno == EINTR);
- if (ret == cmd->pid) {
+ if (ret == pid) {
tmp = virCommandTranslateStatus(status);
VIR_DEBUG("process has ended: %s", tmp);
goto cleanup;
}
}
}
- VIR_DEBUG("failed to reap child %d, abandoning it", cmd->pid);
+ VIR_DEBUG("failed to reap child %d, abandoning it", pid);
cleanup:
VIR_FREE(tmp);
+ errno = saved_errno;
+}
+
+/*
+ * Abort an async command if it is 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.
+ */
+void
+virCommandAbort(virCommandPtr cmd)
+{
+ if (!cmd || cmd->pid == -1)
+ return;
+ virPidAbort(cmd->pid);
cmd->pid = -1;
cmd->reap = false;
- errno = saved_errno;
}
#else /* WIN32 */
void
+virPidAbort(pid_t pid)
+{
+ /* Not yet ported to mingw. Any volunteers? */
+ VIR_DEBUG("failed to reap child %d, abandoning it", 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 e9f4227..c8a04f1 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -292,11 +292,31 @@ int virCommandRun(virCommandPtr cmd,
* Run the command asynchronously
* Returns -1 on any error executing the
* command. Returns 0 if the command executed.
+ *
+ * There are two approaches to child process cleanup.
+ * 1. Use auto-cleanup, by passing NULL for pid. The child will be
+ * auto-reaped by virCommandFree, unless you reap it earlier via
+ * virCommandWait or virCommandAbort. Good for where cmd is in
+ * scope for the duration of the child process.
+ * 2. Use manual cleanup, by passing the address of a pid_t variable
+ * 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.
*/
int virCommandRunAsync(virCommandPtr cmd,
pid_t *pid) ATTRIBUTE_RETURN_CHECK;
/*
+ * 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.
+ */
+int virPidWait(pid_t pid,
+ int *exitstatus) ATTRIBUTE_RETURN_CHECK;
+
+/*
* Wait for the async command to complete.
* Return -1 on any error waiting for
* completion. Returns 0 if the command
@@ -328,6 +348,14 @@ int virCommandHandshakeNotify(virCommandPtr cmd)
ATTRIBUTE_RETURN_CHECK;
/*
+ * 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.
+ */
+void virPidAbort(pid_t pid);
+
+/*
* Abort an async command if it is running, without issuing
* any errors or affecting errno. Designed for error paths
* where some but not all paths to the cleanup code might
--
1.7.4.4
More information about the libvir-list
mailing list