From 9ed545185f895d515aedc6d449e4bf0b2351ab79 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 22 Mar 2011 16:22:37 -0600 Subject: [PATCH] command: add virCommandAbort for cleanup paths Sometimes, an asynchronous helper is started (such as a compressor or iohelper program), but a later error means that we want to abort that child. Make this easier. Note that since daemons and virCommandRunAsync can't mix, the only time virCommandFree can reap a process is if someone did virCommandRunAsync for a non-daemon and didn't stash the pid. * src/util/command.h (virCommandAbort): New prototype. * src/util/command.c (_virCommand): Add new field. (virCommandRunAsync, virCommandWait): Track whether pid was used. (virCommandFree): Reap child if caller did not request pid. (virCommandAbort): New function. * src/libvirt_private.syms (command.h): Export it. * tests/commandtest.c (test19): New test. --- src/libvirt_private.syms | 1 + src/util/command.c | 66 ++++++++++++++++++++++++++++++++++++++++ src/util/command.h | 12 +++++++- tests/commandtest.c | 45 +++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 62539cf690..11fd5c7d02 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -91,6 +91,7 @@ virCgroupSetMemSwapHardLimit; # command.h +virCommandAbort; virCommandAddArg; virCommandAddArgBuffer; virCommandAddArgFormat; diff --git a/src/util/command.c b/src/util/command.c index 3a8ffaea77..905256e308 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -81,6 +81,7 @@ struct _virCommand { pid_t pid; char *pidfile; + bool reap; }; /* @@ -1222,6 +1223,8 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) if (ret == 0 && pid) *pid = cmd->pid; + else + cmd->reap = true; return ret; } @@ -1267,6 +1270,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) } cmd->pid = -1; + cmd->reap = false; if (exitstatus == NULL) { if (status != 0) { @@ -1287,6 +1291,65 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) } +/* + * 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) +{ + int saved_errno; + int ret; + int status; + char *tmp = NULL; + + if (!cmd || cmd->pid == -1) + 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 && + errno == EINTR); + if (ret == cmd->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); + usleep(10 * 1000); + while ((ret = waitpid(cmd->pid, &status, WNOHANG)) == -1 && + errno == EINTR); + if (ret == cmd->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 && + errno == EINTR); + if (ret == cmd->pid) { + tmp = virCommandTranslateStatus(status); + VIR_DEBUG("process has ended: %s", tmp); + goto cleanup; + } + } + } + VIR_DEBUG("failed to reap child %d, abandoning it", cmd->pid); + +cleanup: + VIR_FREE(tmp); + cmd->pid = -1; + cmd->reap = false; + errno = saved_errno; +} + /* * Release all resources */ @@ -1320,5 +1383,8 @@ virCommandFree(virCommandPtr cmd) VIR_FREE(cmd->pidfile); + if (cmd->reap) + virCommandAbort(cmd); + VIR_FREE(cmd); } diff --git a/src/util/command.h b/src/util/command.h index e4027e5877..ff8ccf5f91 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -275,7 +275,17 @@ int virCommandWait(virCommandPtr cmd, int *exitstatus) ATTRIBUTE_RETURN_CHECK; /* - * Release all resources + * 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); + +/* + * 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 waitpid() yourself. */ void virCommandFree(virCommandPtr cmd); diff --git a/tests/commandtest.c b/tests/commandtest.c index dc2f8a124b..c313a2cec3 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -696,6 +696,14 @@ static int test18(const void *unused ATTRIBUTE_UNUSED) printf("cannot read pidfile\n"); goto cleanup; } + + virCommandFree(cmd); + cmd = NULL; + if (kill(pid, 0) != 0) { + printf("daemon should still be running\n"); + goto cleanup; + } + while (kill(pid, SIGINT) != -1) usleep(100*1000); @@ -708,6 +716,42 @@ cleanup: return ret; } +/* + * Asynchronously run long-running daemon, to ensure no hang. + */ +static int test19(const void *unused ATTRIBUTE_UNUSED) +{ + virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); + pid_t pid; + int ret = -1; + + alarm(5); + if (virCommandRunAsync(cmd, &pid) < 0) { + virErrorPtr err = virGetLastError(); + printf("Cannot run child %s\n", err->message); + goto cleanup; + } + + if (kill(pid, 0) != 0) { + printf("Child should still be running"); + goto cleanup; + } + + virCommandAbort(cmd); + + if (kill(pid, 0) == 0) { + printf("Child should be aborted"); + goto cleanup; + } + + alarm(0); + + ret = 0; + +cleanup: + virCommandFree(cmd); + return ret; +} static int mymain(int argc, char **argv) @@ -781,6 +825,7 @@ mymain(int argc, char **argv) DO_TEST(test16); DO_TEST(test17); DO_TEST(test18); + DO_TEST(test19); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- GitLab