diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 62539cf6906ad9006373afcfb1a59ffa4ff8ce7c..11fd5c7d0215d010f8703cbb95a94dac1881db52 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 3a8ffaea77beb42e9d1b52f4bebb9b25f75fc51a..905256e308fbda6b17a8485dffb009ce7d173a29 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 e4027e58771b16f856839798a4d19b20a841e1d1..ff8ccf5f91f43f74c3b7b1ed7ec57e8b57a8829d 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 dc2f8a124b53a16786fdeb5a6eddacedd418548e..c313a2cec348e0f08ffe9c635b7dfb4c04180828 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); }