Signed-off-by: Shi Lei <shilei.massclouds@gmx.com> --- docs/internals/command.html.in | 68 +++++++ src/libvirt_private.syms | 2 + src/util/vircommand.c | 110 ++++++++++- src/util/vircommand.h | 4 + src/util/virprocess.c | 107 ++++++++--- src/util/virprocess.h | 4 + tests/commandtest.c | 411 ++++++++++++++++++++++++++++++++++++++--- 7 files changed, 651 insertions(+), 55 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 43f51a4..e064019 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -530,6 +530,74 @@ if (WEXITSTATUS(status)...) { virCommandAbort to reap the process. </p> + <h3><a id="timeout">Set timeout for commands</a></h3> + + <p> + With <code>virCommandSetTimeout</code>, commands can be set + timeout in milliseconds. It can be used under one of these conditions: + </p> + + <p> + 1. In synchronous mode, using <code>virCommandRun</code> NOT in + daemon mode. By default, <code>virCommandRun</code> waits for the + command to complete & exit and then checks its exit status. If the + caller uses <code>virCommandSetTimeout</code> before it, the command will be + aborted internally when timeout and there is no need to use + <code>virCommandAbort</code> to reap the child process. + </p> + +<pre> +int timeout = 3000; /* in milliseconds */ +virCommandSetTimeout(cmd, timeout); +if (virCommandRun(cmd, NULL) < 0) { + if (virCommandGetErr(cmd) == ETIME) + ... do something for timeout, e.g. report error ... + + return -1; +} +</pre> + + <p> + The argument <code>timeout</code> of the <code>virCommandSetTimeout</code> + accepts a positive value. When timeout, <code>virCommandRun</code> return + -1 and the caller can use <code>virCommandGetErr</code> to check errorcode. And + <code>ETIME</code> means command timeout. + </p> + <p> + <strong>Note:</strong> <code>virCommandSetTimeout</code> cannot mix with + <code>virCommandDaemonize</code>. Or <code>virCommandRun</code> fails directly. + </p> + + <p> + 2. In asynchronous mode, the caller can also call <code>virCommandSetTimeout</code> + before <code>virCommandRunAsync</code> to limit the running time of the command. + The caller uses <code>virCommandWait</code> to wait for the child process to complete + & exit. <code>virCommandWait</code> returns -1 if timeout and the caller can use + <code>virCommandGetErr</code> to check the reason. + </p> + +<pre> +char *outbuf = NULL; +char *errbuf = NULL; +int timeout = 3000; /* in milliseconds */ +virCommandSetTimeout(cmd, timeout); + +virCommandSetOutputBuffer(cmd, &outbuf); +virCommandSetErrorBuffer(cmd, &errbuf); +virCommandDoAsyncIO(cmd); +if (virCommandRunAsync(cmd, NULL) < 0) + return -1; + +... do something ... + +if (virCommandWait(cmd, NULL) < 0) { + if (virCommandGetErr(cmd) == ETIME) + ... do something for timeout, e.g. report error ... + + return -1; +} +</pre> + <h3><a id="release">Releasing resources</a></h3> <p> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 32ed5a0..7a44d19 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1604,6 +1604,7 @@ virCommandDaemonize; virCommandDoAsyncIO; virCommandExec; virCommandFree; +virCommandGetErr; virCommandGetGID; virCommandGetUID; virCommandHandshakeNotify; @@ -1638,6 +1639,7 @@ virCommandSetOutputFD; virCommandSetPidFile; virCommandSetPreExecHook; virCommandSetSELinuxLabel; +virCommandSetTimeout; virCommandSetUID; virCommandSetUmask; virCommandSetWorkingDirectory; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 8be3fdf..1966648 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -53,6 +53,7 @@ #include "virbuffer.h" #include "virthread.h" #include "virstring.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -136,6 +137,8 @@ struct _virCommand { char *appArmorProfile; #endif int mask; + + int timeout; }; /* See virCommandSetDryRun for description for this variable */ @@ -906,6 +909,8 @@ virCommandNewArgs(const char *const*args) cmd->uid = -1; cmd->gid = -1; + cmd->timeout = -1; + virCommandAddArgSet(cmd, args); return cmd; @@ -1077,6 +1082,13 @@ virCommandGetUID(virCommandPtr cmd) } +int +virCommandGetErr(virCommandPtr cmd) +{ + return cmd ? cmd->has_error : -1; +} + + void virCommandSetGID(virCommandPtr cmd, gid_t gid) { @@ -2016,6 +2028,8 @@ virCommandProcessIO(virCommandPtr cmd) size_t inlen = 0, outlen = 0, errlen = 0; size_t inoff = 0; int ret = 0; + int timeout = -1; + unsigned long long start; if (dryRunBuffer || dryRunCallback) { VIR_DEBUG("Dry run requested, skipping I/O processing"); @@ -2041,6 +2055,11 @@ virCommandProcessIO(virCommandPtr cmd) if (VIR_REALLOC_N(*cmd->errbuf, 1) < 0) ret = -1; } + if (cmd->timeout > 0) { + timeout = cmd->timeout; + if (virTimeMillisNow(&start) < 0) + ret = -1; + } if (ret == -1) goto cleanup; ret = -1; @@ -2069,17 +2088,45 @@ virCommandProcessIO(virCommandPtr cmd) nfds++; } - if (nfds == 0) + if (nfds == 0) { + /* + * Timeout can be changed during loops. + * Set cmd->timeout with rest of timeout, + * and virCommandWait may uses it later + */ + if (timeout != -1 && timeout != cmd->timeout) + cmd->timeout = timeout; break; + } + + ret = poll(fds, nfds, timeout); + if (ret < 0) { + if (errno == EAGAIN || errno == EINTR) { + if (cmd->timeout > 0) { + unsigned long long now; + if (virTimeMillisNow(&now) < 0) + goto cleanup; - if (poll(fds, nfds, -1) < 0) { - if (errno == EAGAIN || errno == EINTR) + if (timeout > (int)(now - start)) + timeout -= (int)(now - start); + else + timeout = 0; + } continue; + } virReportSystemError(errno, "%s", _("unable to poll on child")); goto cleanup; } + if (ret == 0) { + /* Timeout to abort command and return failure */ + virCommandAbort(cmd); + cmd->has_error = ETIME; + ret = -1; + goto cleanup; + } + for (i = 0; i < nfds; i++) { if (fds[i].revents & (POLLIN | POLLHUP | POLLERR) && (fds[i].fd == errfd || fds[i].fd == outfd)) { @@ -2126,6 +2173,7 @@ virCommandProcessIO(virCommandPtr cmd) done = write(cmd->inpipe, cmd->inbuf + inoff, inlen - inoff); + if (done < 0) { if (errno == EPIPE) { VIR_DEBUG("child closed stdin early, ignoring EPIPE " @@ -2348,7 +2396,9 @@ virCommandDoAsyncIOHelper(void *opaque) virCommandPtr cmd = opaque; if (virCommandProcessIO(cmd) < 0) { /* If something went wrong, save errno or -1*/ - cmd->has_error = errno ? errno : -1; + /* except ETIME which means timeout */ + if (cmd->has_error != ETIME) + cmd->has_error = errno ? errno : -1; } } @@ -2438,6 +2488,13 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) _("creation of pid file requires daemonized command")); goto cleanup; } + if (cmd->timeout > 0) { + if (cmd->flags & VIR_EXEC_DAEMON) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("daemonized command cannot use virCommandSetTimeout")); + goto cleanup; + } + } str = virCommandToString(cmd); if (dryRunBuffer || dryRunCallback) { @@ -2531,6 +2588,11 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) virReportOOMError(); return -1; } + if (cmd->has_error == ETIME) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timeout waiting for child io")); + return -1; + } if (cmd->has_error) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid use of command API")); @@ -2559,7 +2621,17 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) * message is not as detailed as what we can provide. So, we * guarantee that virProcessWait only fails due to failure to wait, * and repeat the exitstatus check code ourselves. */ - ret = virProcessWait(cmd->pid, &status, true); + if (cmd->timeout >= 0) { + ret = virProcessWaitTimeout(cmd->pid, &status, true, &(cmd->timeout)); + if (ret == 1) { + virCommandAbort(cmd); + cmd->has_error = ETIME; + ret = -1; + } + } else { + ret = virProcessWait(cmd->pid, &status, true); + } + if (cmd->flags & VIR_EXEC_ASYNC_IO) { cmd->flags &= ~VIR_EXEC_ASYNC_IO; virThreadJoin(cmd->asyncioThread); @@ -3163,3 +3235,31 @@ virCommandRunNul(virCommandPtr cmd ATTRIBUTE_UNUSED, return -1; } #endif /* WIN32 */ + + +/** + * virCommandSetTimeout: + * @cmd: the command to set timeout + * @timeout: the number of milliseconds for timeout. + * it should be a positive value. + * + * Set timeout for the command. When timeout, abort the command. + * By default, command without calling this function has no timeout. + * Note: virCommandSetTimeout should be used under one of the conditions: + * 1) virCommandRun WITHOUT setting VIR_EXEC_DAEMON + * 2) virCommandRunAsync and virCommandWait + */ +void +virCommandSetTimeout(virCommandPtr cmd, int timeout) +{ + if (!cmd || cmd->has_error) + return; + + if (timeout <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timeout can't be zero or negative")); + return; + } + + cmd->timeout = timeout; +} diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 90bcc6c..99785af 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -77,6 +77,8 @@ void virCommandSetGID(virCommandPtr cmd, gid_t gid); void virCommandSetUID(virCommandPtr cmd, uid_t uid); +int virCommandGetErr(virCommandPtr cmd) ATTRIBUTE_NONNULL(1); + void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes); void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs); void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files); @@ -219,6 +221,8 @@ int virCommandRunNul(virCommandPtr cmd, virCommandRunNulFunc func, void *data); +void virCommandSetTimeout(virCommandPtr cmd, int timeout); + VIR_DEFINE_AUTOPTR_FUNC(virCommand, virCommandFree) #endif /* __VIR_COMMAND_H__ */ diff --git a/src/util/virprocess.c b/src/util/virprocess.c index ecea27a..e290c95 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -61,6 +61,7 @@ #include "virutil.h" #include "virstring.h" #include "vircommand.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -211,32 +212,12 @@ virProcessAbort(pid_t pid) #endif -/** - * virProcessWait: - * @pid: child to wait on - * @exitstatus: optional status collection - * @raw: whether to pass non-normal status back to caller - * - * Wait for a child process to complete. If @pid is -1, do nothing, but - * return -1 (useful for error cleanup, and assumes an earlier message was - * already issued). All other pids issue an error message on failure. - * - * If @exitstatus is NULL, then the child must exit normally with status 0. - * Otherwise, if @raw is false, the child must exit normally, and - * @exitstatus will contain the final exit status (no need for the caller - * to use WEXITSTATUS()). If @raw is true, then the result of waitpid() is - * returned in @exitstatus, and the caller must use WIFEXITED() and friends - * to decipher the child's status. - * - * Returns 0 on a successful wait. Returns -1 on any error waiting for - * completion, or if the command completed with a status that cannot be - * reflected via the choice of @exitstatus and @raw. - */ -int -virProcessWait(pid_t pid, int *exitstatus, bool raw) +static int +virProcessWaitHelper(pid_t pid, int *exitstatus, bool raw, const int *timeout) { int ret; int status; + unsigned long long start; VIR_AUTOFREE(char *) st = NULL; if (pid <= 0) { @@ -246,9 +227,32 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) return -1; } + if (virTimeMillisNow(&start) < 0) + return -1; + /* Wait for intermediate process to exit */ - while ((ret = waitpid(pid, &status, 0)) == -1 && - errno == EINTR); + if (timeout && *timeout >= 0) { + for (;;) { + while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && + errno == EINTR); + + if (ret == 0) { + unsigned long long now; + if (virTimeMillisNow(&now) < 0) + return -1; + + if ((int)(now - start) >= *timeout) + return 1; /* Timeout */ + + usleep(100 * 1000); + continue; + } + + break; + } + } else { + while ((ret = waitpid(pid, &status, 0)) == -1 && errno == EINTR); + } if (ret == -1) { virReportSystemError(errno, _("unable to wait for process %lld"), @@ -278,6 +282,59 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) } +/** + * virProcessWait: + * @pid: child to wait on + * @exitstatus: optional status collection + * @raw: whether to pass non-normal status back to caller + * + * Wait for a child process to complete. If @pid is -1, do nothing, but + * return -1 (useful for error cleanup, and assumes an earlier message was + * already issued). All other pids issue an error message on failure. + * + * If @exitstatus is NULL, then the child must exit normally with status 0. + * Otherwise, if @raw is false, the child must exit normally, and + * @exitstatus will contain the final exit status (no need for the caller + * to use WEXITSTATUS()). If @raw is true, then the result of waitpid() is + * returned in @exitstatus, and the caller must use WIFEXITED() and friends + * to decipher the child's status. + * + * Returns 0 on a successful wait. Returns -1 on any error waiting for + * completion, or if the command completed with a status that cannot be + * reflected via the choice of @exitstatus and @raw. + */ +int +virProcessWait(pid_t pid, int *exitstatus, bool raw) +{ + return virProcessWaitHelper(pid, exitstatus, raw, NULL); +} + + +/** + * virProcessWaitTimeout: + * @pid: child to wait on + * @exitstatus: optional status collection + * @raw: whether to pass non-normal status back to caller + * @timeout: pointer to timeout. + * + * Just like virProcessWait, but it has timeout. + * Expect @timeout to be altered by outside, so passes pointer of it. + * + * Returns 0 on a successful wait. + * Returns 1 on timeout. + * Returns -1 on any error waiting for completion. + */ +int +virProcessWaitTimeout(pid_t pid, int *exitstatus, bool raw, const int *timeout) +{ + if (!timeout || *timeout < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("timeout invalid")); + return -1; + } + return virProcessWaitHelper(pid, exitstatus, raw, timeout); +} + + /* 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 3c5a882..bc4abd1 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -52,6 +52,10 @@ int virProcessWait(pid_t pid, int *exitstatus, bool raw) ATTRIBUTE_RETURN_CHECK; +int +virProcessWaitTimeout(pid_t pid, int *exitstatus, bool raw, const int *timeout) + ATTRIBUTE_RETURN_CHECK; + int virProcessKill(pid_t pid, int sig); int virProcessKillPainfully(pid_t pid, bool force); diff --git a/tests/commandtest.c b/tests/commandtest.c index 744a387..26c6b58 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -51,6 +51,10 @@ struct _virCommandTestData { bool running; }; +struct testdata { + int timeout; /* milliseconds */ +}; + #ifdef WIN32 int @@ -113,17 +117,22 @@ static int checkoutput(const char *testname, return ret; } + /* * Run program, no args, inherit all ENV, keep CWD. * Only stdin/out/err open * No slot for return status must log error. */ -static int test0(const void *unused ATTRIBUTE_UNUSED) +static int test0(const void *opaque) { virCommandPtr cmd; int ret = -1; + const struct testdata *data = opaque; cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist"); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) == 0) goto cleanup; @@ -138,18 +147,23 @@ static int test0(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Run program, no args, inherit all ENV, keep CWD. * Only stdin/out/err open * Capturing return status must not log error. */ -static int test1(const void *unused ATTRIBUTE_UNUSED) +static int test1(const void *opaque) { virCommandPtr cmd; int ret = -1; int status; + const struct testdata *data = opaque; cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist"); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, &status) < 0) goto cleanup; if (status != EXIT_ENOENT) @@ -167,14 +181,18 @@ static int test1(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Run program (twice), no args, inherit all ENV, keep CWD. * Only stdin/out/err open */ -static int test2(const void *unused ATTRIBUTE_UNUSED) +static int test2(const void *opaque) { - virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); int ret; + virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; + if (data) + virCommandSetTimeout(cmd, data->timeout); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); @@ -198,13 +216,15 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) return checkoutput("test2", NULL); } + /* * Run program, no args, inherit all ENV, keep CWD. * stdin/out/err + two extra FD open */ -static int test3(const void *unused ATTRIBUTE_UNUSED) +static int test3(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; int newfd1 = dup(STDERR_FILENO); int newfd2 = dup(STDERR_FILENO); int newfd3 = dup(STDERR_FILENO); @@ -214,6 +234,9 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -282,12 +305,16 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) * Run program, no args, inherit filtered ENV, keep CWD. * Only stdin/out/err open */ -static int test5(const void *unused ATTRIBUTE_UNUSED) +static int test5(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; virCommandAddEnvPassCommon(cmd); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -304,13 +331,17 @@ static int test5(const void *unused ATTRIBUTE_UNUSED) * Run program, no args, inherit filtered ENV, keep CWD. * Only stdin/out/err open */ -static int test6(const void *unused ATTRIBUTE_UNUSED) +static int test6(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL); virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -327,14 +358,18 @@ static int test6(const void *unused ATTRIBUTE_UNUSED) * Run program, no args, inherit filtered ENV, keep CWD. * Only stdin/out/err open */ -static int test7(const void *unused ATTRIBUTE_UNUSED) +static int test7(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; virCommandAddEnvPassCommon(cmd); virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL); virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -346,19 +381,24 @@ static int test7(const void *unused ATTRIBUTE_UNUSED) return checkoutput("test7", NULL); } + /* * Run program, no args, inherit filtered ENV, keep CWD. * Only stdin/out/err open */ -static int test8(const void *unused ATTRIBUTE_UNUSED) +static int test8(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; virCommandAddEnvString(cmd, "USER=bogus"); virCommandAddEnvString(cmd, "LANG=C"); virCommandAddEnvPair(cmd, "USER", "also bogus"); virCommandAddEnvPair(cmd, "USER", "test"); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -375,9 +415,10 @@ static int test8(const void *unused ATTRIBUTE_UNUSED) * Run program, some args, inherit all ENV, keep CWD. * Only stdin/out/err open */ -static int test9(const void *unused ATTRIBUTE_UNUSED) +static int test9(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; const char* const args[] = { "arg1", "arg2", NULL }; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -396,6 +437,9 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) return -1; } + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -412,15 +456,19 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) * Run program, some args, inherit all ENV, keep CWD. * Only stdin/out/err open */ -static int test10(const void *unused ATTRIBUTE_UNUSED) +static int test10(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; const char *const args[] = { "-version", "-log=bar.log", NULL, }; virCommandAddArgSet(cmd, args); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -432,17 +480,22 @@ static int test10(const void *unused ATTRIBUTE_UNUSED) return checkoutput("test10", NULL); } + /* * Run program, some args, inherit all ENV, keep CWD. * Only stdin/out/err open */ -static int test11(const void *unused ATTRIBUTE_UNUSED) +static int test11(const void *opaque) { const char *args[] = { abs_builddir "/commandhelper", "-version", "-log=bar.log", NULL, }; virCommandPtr cmd = virCommandNewArgs(args); + const struct testdata *data = opaque; + + if (data) + virCommandSetTimeout(cmd, data->timeout); if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); @@ -455,16 +508,21 @@ static int test11(const void *unused ATTRIBUTE_UNUSED) return checkoutput("test11", NULL); } + /* * Run program, no args, inherit all ENV, keep CWD. * Only stdin/out/err open. Set stdin data */ -static int test12(const void *unused ATTRIBUTE_UNUSED) +static int test12(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; virCommandSetInputBuffer(cmd, "Hello World\n"); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); @@ -476,13 +534,15 @@ static int test12(const void *unused ATTRIBUTE_UNUSED) return checkoutput("test12", NULL); } + /* * Run program, no args, inherit all ENV, keep CWD. * Only stdin/out/err open. Set stdin data */ -static int test13(const void *unused ATTRIBUTE_UNUSED) +static int test13(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; char *outactual = NULL; const char *outexpect = "BEGIN STDOUT\n" "Hello World\n" @@ -492,6 +552,9 @@ static int test13(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, "Hello World\n"); virCommandSetOutputBuffer(cmd, &outactual); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -515,13 +578,15 @@ static int test13(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Run program, no args, inherit all ENV, keep CWD. * Only stdin/out/err open. Set stdin data */ -static int test14(const void *unused ATTRIBUTE_UNUSED) +static int test14(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; char *outactual = NULL; const char *outexpect = "BEGIN STDOUT\n" "Hello World\n" @@ -544,6 +609,9 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetOutputBuffer(cmd, &outactual); virCommandSetErrorBuffer(cmd, &erractual); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -557,6 +625,9 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, "Hello World\n"); virCommandSetOutputBuffer(cmd, &jointactual); virCommandSetErrorBuffer(cmd, &jointactual); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -592,9 +663,10 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) * Run program, no args, inherit all ENV, change CWD. * Only stdin/out/err open */ -static int test15(const void *unused ATTRIBUTE_UNUSED) +static int test15(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; char *cwd = NULL; int ret = -1; @@ -603,6 +675,9 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) virCommandSetWorkingDirectory(cmd, cwd); virCommandSetUmask(cmd, 002); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -617,17 +692,22 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Don't run program; rather, log what would be run. */ -static int test16(const void *unused ATTRIBUTE_UNUSED) +static int test16(const void *opaque) { virCommandPtr cmd = virCommandNew("true"); + const struct testdata *data = opaque; char *outactual = NULL; const char *outexpect = "A=B C='D E' true F 'G H'"; int ret = -1; int fd = -1; + if (data) + virCommandSetTimeout(cmd, data->timeout); + virCommandAddEnvPair(cmd, "A", "B"); virCommandAddEnvPair(cmd, "C", "D E"); virCommandAddArg(cmd, "F"); @@ -662,16 +742,21 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Test string handling when no output is present. */ -static int test17(const void *unused ATTRIBUTE_UNUSED) +static int test17(const void *opaque) { virCommandPtr cmd = virCommandNew("true"); + const struct testdata *data = opaque; int ret = -1; char *outbuf; char *errbuf = NULL; + if (data) + virCommandSetTimeout(cmd, data->timeout); + virCommandSetOutputBuffer(cmd, &outbuf); if (outbuf != NULL) { puts("buffer not sanitized at registration"); @@ -718,6 +803,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Run long-running daemon, to ensure no hang. */ @@ -766,15 +852,20 @@ static int test18(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Asynchronously run long-running daemon, to ensure no hang. */ -static int test19(const void *unused ATTRIBUTE_UNUSED) +static int test19(const void *opaque) { + const struct testdata *data = opaque; virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); pid_t pid; int ret = -1; + if (data) + virCommandSetTimeout(cmd, data->timeout); + alarm(5); if (virCommandRunAsync(cmd, &pid) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); @@ -802,14 +893,16 @@ static int test19(const void *unused ATTRIBUTE_UNUSED) return ret; } + /* * Run program, no args, inherit all ENV, keep CWD. * Ignore huge stdin data, to provoke SIGPIPE or EPIPE in parent. */ -static int test20(const void *unused ATTRIBUTE_UNUSED) +static int test20(const void *opaque) { virCommandPtr cmd = virCommandNewArgList(abs_builddir "/commandhelper", "--close-stdin", NULL); + const struct testdata *data = opaque; char *buf; int ret = -1; @@ -825,6 +918,9 @@ static int test20(const void *unused ATTRIBUTE_UNUSED) goto cleanup; virCommandSetInputBuffer(cmd, buf); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRun(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -837,6 +933,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED) return ret; } + static const char *const newenv[] = { "PATH=/usr/bin:/bin", "HOSTNAME=test", @@ -849,9 +946,11 @@ static const char *const newenv[] = { NULL }; -static int test21(const void *unused ATTRIBUTE_UNUSED) + +static int test21(const void *opaque) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; int ret = -1; const char *wrbuf = "Hello world\n"; char *outbuf = NULL, *errbuf = NULL; @@ -867,6 +966,9 @@ static int test21(const void *unused ATTRIBUTE_UNUSED) virCommandSetErrorBuffer(cmd, &errbuf); virCommandDoAsyncIO(cmd); + if (data) + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRunAsync(cmd, NULL) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; @@ -896,14 +998,17 @@ static int test21(const void *unused ATTRIBUTE_UNUSED) return ret; } -static int -test22(const void *unused ATTRIBUTE_UNUSED) + +static int test22(const void *opaque) { int ret = -1; virCommandPtr cmd; int status = -1; + const struct testdata *data = opaque; cmd = virCommandNewArgList("/bin/sh", "-c", "exit 3", NULL); + if (data) + virCommandSetTimeout(cmd, data->timeout); if (virCommandRun(cmd, &status) < 0) { printf("Cannot run child %s\n", virGetLastErrorMessage()); @@ -949,8 +1054,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) } -static int -test23(const void *unused ATTRIBUTE_UNUSED) +static int test23(const void *unused ATTRIBUTE_UNUSED) { /* Not strictly a virCommand test, but this is the easiest place * to test this lower-level interface. It takes a double fork to @@ -1006,6 +1110,7 @@ test23(const void *unused ATTRIBUTE_UNUSED) return ret; } + static int test24(const void *unused ATTRIBUTE_UNUSED) { char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); @@ -1138,6 +1243,211 @@ static int test25(const void *unused ATTRIBUTE_UNUSED) } +/* + * virCommandSetTimeout cannot mix with daemon + */ +static int test26(const void *opaque) +{ + const char *expect_msg = + "internal error: daemonized command cannot use virCommandSetTimeout"; + virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + const struct testdata *data = opaque; + char *pidfile = virPidFileBuildPath(abs_builddir, "commandhelper"); + int ret = -1; + + if (!data) + goto cleanup; + if (!pidfile) + goto cleanup; + + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + virCommandSetTimeout(cmd, data->timeout); + + if (virCommandRun(cmd, NULL) != -1 || + STRNEQ(virGetLastErrorMessage(), expect_msg)) { + printf("virCommandSetTimeout mixes with virCommandDaemonize %s\n", + virGetLastErrorMessage()); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + if (pidfile) + unlink(pidfile); + VIR_FREE(pidfile); + return ret; +} + + +/* + * virCommandRunAsync without async string io when time-out + */ +static int test27(const void *opaque) +{ + int ret = -1; + virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); + pid_t pid; + const struct testdata *data = opaque; + if (!data) + goto cleanup; + + virCommandSetTimeout(cmd, data->timeout); + if (virCommandRunAsync(cmd, &pid) < 0) { + printf("Cannot run child %s\n", virGetLastErrorMessage()); + goto cleanup; + } + + if (virCommandWait(cmd, NULL) != -1 || + virCommandGetErr(cmd) != ETIME) { + printf("Timeout doesn't work %s:%d\n", + virGetLastErrorMessage(), + virCommandGetErr(cmd)); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + + +/* + * synchronous mode: abort command when time-out + */ +static int test28(const void *opaque) +{ + const char *expect_msg1 = "internal error: timeout waiting for child io"; + const char *expect_msg2 = "internal error: invalid use of command API"; + virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); + const struct testdata *data = opaque; + if (!data) { + printf("opaque arg NULL\n"); + virCommandFree(cmd); + return -1; + } + + virCommandSetTimeout(cmd, data->timeout); + + if (virCommandRun(cmd, NULL) != -1 || + virCommandGetErr(cmd) != ETIME || + STRNEQ(virGetLastErrorMessage(), expect_msg1)) { + printf("Timeout doesn't work %s (first)\n", virGetLastErrorMessage()); + virCommandFree(cmd); + return -1; + } + + if (virCommandRun(cmd, NULL) != -1 || + virCommandGetErr(cmd) != ETIME || + STRNEQ(virGetLastErrorMessage(), expect_msg2)) { + printf("Timeout doesn't work %s (second)\n", virGetLastErrorMessage()); + virCommandFree(cmd); + return -1; + } + + virCommandFree(cmd); + return 0; +} + + +/* + * asynchronous mode with async string io: + * abort command when time-out + */ +static int test29(const void *opaque) +{ + int ret = -1; + const char *wrbuf = "Hello world\n"; + char *outbuf = NULL; + char *errbuf = NULL; + const char *expect_msg = + "Error while processing command's IO: Timer expired:"; + virCommandPtr cmd = virCommandNewArgList("sleep", "100", NULL); + const struct testdata *data = opaque; + if (!data) { + printf("opaque arg NULL\n"); + ret = -1; + goto cleanup; + } + + virCommandSetTimeout(cmd, data->timeout); + + virCommandSetInputBuffer(cmd, wrbuf); + virCommandSetOutputBuffer(cmd, &outbuf); + virCommandSetErrorBuffer(cmd, &errbuf); + virCommandDoAsyncIO(cmd); + + if (virCommandRunAsync(cmd, NULL) < 0) { + printf("Cannot run child %s\n", virGetLastErrorMessage()); + ret = -1; + goto cleanup; + } + + if (virCommandWait(cmd, NULL) != -1 || + virCommandGetErr(cmd) != ETIME || + STRPREFIX(virGetLastErrorMessage(), expect_msg)) { + printf("Timeout doesn't work %s:%d\n", + virGetLastErrorMessage(), + virCommandGetErr(cmd)); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(outbuf); + VIR_FREE(errbuf); + virCommandFree(cmd); + return ret; +} + + +/* + * asynchronous mode only with input buffer + * abort command when time-out + */ +static int test30(const void *opaque) +{ + int ret = -1; + const char *wrbuf = "Hello world\n"; + const char *expect_msg = + "Error while processing command's IO: Timer expired:"; + virCommandPtr cmd = virCommandNewArgList("sleep", "10", NULL); + const struct testdata *data = opaque; + if (!data) { + printf("opaque arg NULL\n"); + ret = -1; + goto cleanup; + } + + virCommandSetTimeout(cmd, data->timeout); + + virCommandSetInputBuffer(cmd, wrbuf); + virCommandDoAsyncIO(cmd); + + if (virCommandRunAsync(cmd, NULL) < 0) { + printf("Cannot run child %s\n", virGetLastErrorMessage()); + ret = -1; + goto cleanup; + } + + if (virCommandWait(cmd, NULL) != -1 || + virCommandGetErr(cmd) != ETIME || + STRPREFIX(virGetLastErrorMessage(), expect_msg)) { + printf("Timeout doesn't work %s:%d\n", + virGetLastErrorMessage(), + virCommandGetErr(cmd)); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + + static void virCommandThreadWorker(void *opaque) { virCommandTestDataPtr test = opaque; @@ -1161,6 +1471,7 @@ static void virCommandThreadWorker(void *opaque) return; } + static void virCommandTestFreeTimer(int timer ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) @@ -1168,6 +1479,7 @@ virCommandTestFreeTimer(int timer ATTRIBUTE_UNUSED, /* nothing to be done here */ } + static int mymain(void) { @@ -1176,6 +1488,7 @@ mymain(void) virCommandTestDataPtr test = NULL; int timer = -1; int virinitret; + struct testdata data; if (virThreadInitialize() < 0) return EXIT_FAILURE; @@ -1292,6 +1605,54 @@ mymain(void) DO_TEST(test24); DO_TEST(test25); + + /* tests for virCommandSetTimeout */ + /* 1) NO time-out */ + data.timeout = 3*1000; + +# define DO_TEST_SET_TIMEOUT(NAME) \ + if (virTestRun("Command Exec " #NAME " test", \ + NAME, &data) < 0) \ + ret = -1 + + /* Exclude test4, test18 and test24 for they're in daemon mode. + * Exclude test25, there's no meaning to set timeout + */ + DO_TEST_SET_TIMEOUT(test0); + DO_TEST_SET_TIMEOUT(test1); + DO_TEST_SET_TIMEOUT(test2); + DO_TEST_SET_TIMEOUT(test3); + DO_TEST_SET_TIMEOUT(test5); + DO_TEST_SET_TIMEOUT(test6); + DO_TEST_SET_TIMEOUT(test7); + DO_TEST_SET_TIMEOUT(test8); + DO_TEST_SET_TIMEOUT(test9); + DO_TEST_SET_TIMEOUT(test10); + DO_TEST_SET_TIMEOUT(test11); + DO_TEST_SET_TIMEOUT(test12); + DO_TEST_SET_TIMEOUT(test13); + DO_TEST_SET_TIMEOUT(test14); + DO_TEST_SET_TIMEOUT(test15); + DO_TEST_SET_TIMEOUT(test16); + DO_TEST_SET_TIMEOUT(test17); + DO_TEST_SET_TIMEOUT(test19); + DO_TEST_SET_TIMEOUT(test20); + DO_TEST_SET_TIMEOUT(test21); + DO_TEST_SET_TIMEOUT(test22); + DO_TEST_SET_TIMEOUT(test23); + + /* 2) unsupported usage */ + /* Failure: virCommandSetTimeout mixes with daemon */ + DO_TEST_SET_TIMEOUT(test26); + + /* 3) when time-out */ + data.timeout = 100; + DO_TEST_SET_TIMEOUT(test27); /* timeout in async mode without async string io */ + DO_TEST_SET_TIMEOUT(test28); /* timeout in sync mode */ + DO_TEST_SET_TIMEOUT(test29); /* timeout in async mode with async string io */ + DO_TEST_SET_TIMEOUT(test30); /* timeout in async mode with only input buffer */ + + virMutexLock(&test->lock); if (test->running) { test->quit = true; -- 2.7.4