And drop the now unused virExecDaemonize Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/libvirt_private.syms | 1 - src/remote/remote_driver.c | 18 ++++++----- src/util/util.c | 73 +------------------------------------------- src/util/util.h | 9 ----- 4 files changed, 11 insertions(+), 90 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7e5b1d7..4c4f65d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -909,7 +909,6 @@ virEnumToString; virEventAddHandle; virEventRemoveHandle; virExec; -virExecDaemonize; virExecWithHook; virFileAbsPath; virFileDeletePid; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 09736d9..1072f9f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -83,6 +83,7 @@ #include "event.h" #include "ignore-value.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_REMOTE @@ -326,8 +327,8 @@ static int remoteForkDaemon(void) { const char *daemonPath = remoteFindDaemonPath(); - const char *const daemonargs[] = { daemonPath, "--timeout=30", NULL }; - pid_t pid; + virCommandPtr cmd = NULL; + int ret; if (!daemonPath) { remoteError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -335,13 +336,14 @@ remoteForkDaemon(void) return -1; } - if (virExecDaemonize(daemonargs, NULL, NULL, - &pid, -1, NULL, NULL, - VIR_EXEC_CLEAR_CAPS, - NULL, NULL, NULL) < 0) - return -1; + cmd = virCommandNewArgList(daemonPath, "--timeout", "30", NULL); + virCommandClearCaps(cmd); + virCommandDaemonize(cmd); - return 0; + ret = virCommandRun(cmd, NULL); + virCommandFree(cmd); + + return ret; } #endif diff --git a/src/util/util.c b/src/util/util.c index 4c50fcb..1d0c2cc 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -443,8 +443,7 @@ cleanup: * @flags possible combination of the following: * VIR_EXEC_NONE : Default function behavior * VIR_EXEC_NONBLOCK : Set child process output fd's as non-blocking - * VIR_EXEC_DAEMON : Daemonize the child process (don't use directly, - * use virExecDaemonize wrapper) + * VIR_EXEC_DAEMON : Daemonize the child process * @hook optional virExecHook function to call prior to exec * @data data to pass to the hook function * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) @@ -789,57 +788,6 @@ virExec(const char *const*argv, flags, NULL, NULL, NULL); } -/* - * See __virExec for explanation of the arguments. - * - * This function will wait for the intermediate process (between the caller - * and the daemon) to exit. retpid will be the pid of the daemon, which can - * be checked for example to see if the daemon crashed immediately. - * - * Returns 0 on success - * -1 if initial fork failed (will have a reported error) - * -2 if intermediate process failed - * (won't have a reported error. pending on where the failure - * occured and when in the process occured, the error output - * could have gone to stderr or the passed errfd). - */ -int virExecDaemonize(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) { - int ret; - int childstat = 0; - - ret = virExecWithHook(argv, envp, keepfd, retpid, - infd, outfd, errfd, - flags | VIR_EXEC_DAEMON, - hook, data, pidfile); - - /* __virExec should have set an error */ - if (ret != 0) - return -1; - - /* Wait for intermediate process to exit */ - while (waitpid(*retpid, &childstat, 0) == -1 && - errno == EINTR); - - if (childstat != 0) { - char *str = virCommandTranslateStatus(childstat); - virUtilError(VIR_ERR_INTERNAL_ERROR, - _("Intermediate daemon process status unexpected: %s"), - NULLSTR(str)); - VIR_FREE(str); - ret = -2; - } - - return ret; -} - /** * @argv NULL terminated argv to run * @status optional variable to return exit status in @@ -984,25 +932,6 @@ virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, } int -virExecDaemonize(const char *const*argv ATTRIBUTE_UNUSED, - const char *const*envp ATTRIBUTE_UNUSED, - const fd_set *keepfd ATTRIBUTE_UNUSED, - pid_t *retpid ATTRIBUTE_UNUSED, - int infd ATTRIBUTE_UNUSED, - int *outfd ATTRIBUTE_UNUSED, - int *errfd ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED, - virExecHook hook ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - char *pidfile ATTRIBUTE_UNUSED) -{ - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virExecDaemonize is not implemented for WIN32")); - - return -1; -} - -int virFork(pid_t *pid) { *pid = -1; diff --git a/src/util/util.h b/src/util/util.h index 9d8df06..482294f 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -58,15 +58,6 @@ int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK; * after fork() but before execve() */ typedef int (*virExecHook)(void *data); -int virExecDaemonize(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) ATTRIBUTE_RETURN_CHECK; int virExecWithHook(const char *const*argv, const char *const*envp, const fd_set *keepfd, -- 1.7.4.4

All callers were expecting argv logging, so the split is unneeded. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/util.c | 86 ++++++++++++++++++++++-------------------------------- 1 files changed, 35 insertions(+), 51 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 1d0c2cc..f860392 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -448,8 +448,8 @@ cleanup: * @data data to pass to the hook function * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) */ -static int -__virExec(const char *const*argv, +int +virExecWithHook(const char *const*argv, const char *const*envp, const fd_set *keepfd, pid_t *retpid, @@ -468,6 +468,26 @@ __virExec(const char *const*argv, int tmpfd; const char *binary = NULL; int forkRet; + char *argv_str = NULL; + char *envp_str = NULL; + + if ((argv_str = virArgvToString(argv)) == NULL) { + virReportOOMError(); + return -1; + } + + if (envp) { + if ((envp_str = virArgvToString(envp)) == NULL) { + VIR_FREE(argv_str); + virReportOOMError(); + return -1; + } + VIR_DEBUG("%s %s", envp_str, argv_str); + VIR_FREE(envp_str); + } else { + VIR_DEBUG0(argv_str); + } + VIR_FREE(argv_str); if (argv[0][0] != '/') { if (!(binary = virFindFileInPath(argv[0]))) { @@ -732,48 +752,12 @@ __virExec(const char *const*argv, return -1; } -int -virExecWithHook(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) -{ - char *argv_str; - char *envp_str; - - if ((argv_str = virArgvToString(argv)) == NULL) { - virReportOOMError(); - return -1; - } - - if (envp) { - if ((envp_str = virArgvToString(envp)) == NULL) { - VIR_FREE(argv_str); - virReportOOMError(); - return -1; - } - VIR_DEBUG("%s %s", envp_str, argv_str); - VIR_FREE(envp_str); - } else { - VIR_DEBUG0(argv_str); - } - VIR_FREE(argv_str); - - return __virExec(argv, envp, keepfd, retpid, infd, outfd, errfd, - flags, hook, data, pidfile); -} - /* - * See __virExec for explanation of the arguments. + * See virExecWithHook for explanation of the arguments. * - * Wrapper function for __virExec, with a simpler set of parameters. - * Used to insulate the numerous callers from changes to __virExec argument - * list. + * Wrapper function for virExecWithHook, with a simpler set of parameters. + * Used to insulate the numerous callers from changes to virExecWithHook + * argument list. */ int virExec(const char *const*argv, @@ -813,15 +797,8 @@ virRunWithHook(const char *const*argv, int errfd = -1, outfd = -1; char *outbuf = NULL; char *errbuf = NULL; - char *argv_str = NULL; - - if ((argv_str = virArgvToString(argv)) == NULL) { - virReportOOMError(); - goto error; - } - VIR_DEBUG0(argv_str); - if ((execret = __virExec(argv, NULL, NULL, + if ((execret = virExecWithHook(argv, NULL, NULL, &childpid, -1, &outfd, &errfd, VIR_EXEC_NONE, hook, data, NULL)) < 0) { ret = execret; @@ -852,10 +829,18 @@ virRunWithHook(const char *const*argv, errno = EINVAL; if (exitstatus) { char *str = virCommandTranslateStatus(exitstatus); + char *argv_str = virArgvToString(argv); + if (!argv_str) { + virReportOOMError(); + goto error; + } + virUtilError(VIR_ERR_INTERNAL_ERROR, _("'%s' exited unexpectedly: %s"), argv_str, NULLSTR(str)); + VIR_FREE(str); + VIR_FREE(argv_str); goto error; } } else { @@ -867,7 +852,6 @@ virRunWithHook(const char *const*argv, error: VIR_FREE(outbuf); VIR_FREE(errbuf); - VIR_FREE(argv_str); VIR_FORCE_CLOSE(outfd); VIR_FORCE_CLOSE(errfd); return ret; -- 1.7.4.4

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/storage/storage_backend.c | 31 ++++++------------------------- 1 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 2acbf90..63ba686 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -607,28 +607,19 @@ enum { static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) { - const char *const qemuarg[] = { qemuimg, "-h", NULL }; - const char *const qemuenv[] = { "LC_ALL=C", NULL }; - pid_t child = 0; - int status; - int newstdout = -1; char *help = NULL; - enum { MAX_HELP_OUTPUT_SIZE = 1024*8 }; char *start; char *end; char *tmp; int ret = -1; + virCommandPtr cmd = virCommandNewArgList(qemuimg, "-h", NULL); - if (virExec(qemuarg, qemuenv, NULL, - &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) - goto cleanup; + virCommandAddEnvString(cmd, "LC_ALL=C"); + virCommandSetOutputBuffer(cmd, &help); + virCommandClearCaps(cmd); - if (virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help) < 0) { - virReportSystemError(errno, - _("Unable to read '%s -h' output"), - qemuimg); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; - } start = strstr(help, " create "); end = strstr(start, "\n"); @@ -641,18 +632,8 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) ret = QEMU_IMG_BACKING_FORMAT_NONE; cleanup: + virCommandFree(cmd); VIR_FREE(help); - VIR_FORCE_CLOSE(newstdout); - if (child) { - while (waitpid(child, &status, 0) == -1 && errno == EINTR); - if (status) { - tmp = virCommandTranslateStatus(status); - VIR_WARN("Unexpected status, qemu probably failed: %s", - NULLSTR(tmp)); - VIR_FREE(tmp); - } - } - return ret; } -- 1.7.4.4

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/storage/storage_backend.c | 101 ++++++++++++++--------------------------- 1 files changed, 35 insertions(+), 66 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 63ba686..5f9ff8f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1363,8 +1363,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, void *data, int *outexit) { - int fd = -1, exitstatus, err, failed = 1; - pid_t child = 0; + int fd = -1, err, ret = -1; FILE *list = NULL; regex_t *reg; regmatch_t *vars = NULL; @@ -1372,6 +1371,9 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, int maxReg = 0, i, j; int totgroups = 0, ngroup = 0, maxvars = 0; char **groups; + virCommandPtr cmd = NULL; + const char * const *tmp; + pid_t pid = -1; /* Compile all regular expressions */ if (VIR_ALLOC_N(reg, nregex) < 0) { @@ -1408,10 +1410,14 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, goto cleanup; } + cmd = virCommandNew(prog[0]); + tmp = prog; + while (*(++tmp)) { + virCommandAddArg(cmd, *tmp); + } - /* Run the program and capture its output */ - if (virExec(prog, NULL, NULL, - &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { + virCommandSetOutputFD(cmd, &fd); + if (virCommandRunAsync(cmd, &pid) < 0) { goto cleanup; } @@ -1460,9 +1466,11 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, } } - failed = 0; + ret = 0; +cleanup: + if (pid > 0 && virCommandWait(cmd, outexit) < 0) + ret = -1; - cleanup: if (groups) { for (j = 0 ; j < totgroups ; j++) VIR_FREE(groups[j]); @@ -1478,29 +1486,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, VIR_FORCE_FCLOSE(list); VIR_FORCE_CLOSE(fd); - while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); - - /* Don't bother checking exit status if we already failed */ - if (failed) - return -1; - - if (err == -1) { - virReportSystemError(errno, - _("failed to wait for command '%s'"), - prog[0]); - return -1; - } else { - if (WIFEXITED(exitstatus)) { - if (outexit != NULL) - *outexit = WEXITSTATUS(exitstatus); - } else { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("command did not exit cleanly")); - return -1; - } - } - - return 0; + return ret; } /* @@ -1522,13 +1508,14 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, void *data) { size_t n_tok = 0; - int fd = -1, exitstatus; - pid_t child = 0; + int fd = -1; FILE *fp = NULL; char **v; - int err = -1; - int w_err; + int ret = -1; int i; + const char * const *tmp; + virCommandPtr cmd = NULL; + pid_t pid = -1; if (n_columns == 0) return -1; @@ -1540,9 +1527,14 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, for (i = 0; i < n_columns; i++) v[i] = NULL; - /* Run the program and capture its output */ - if (virExec(prog, NULL, NULL, - &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { + cmd = virCommandNew(prog[0]); + tmp = prog; + while (*(++tmp)) { + virCommandAddArg(cmd, *tmp); + } + + virCommandSetOutputFD(cmd, &fd); + if (virCommandRunAsync(cmd, &pid) < 0) { goto cleanup; } @@ -1578,47 +1570,24 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, } if (feof (fp)) - err = 0; + ret = 0; else virReportSystemError(errno, _("read error on pipe to '%s'"), prog[0]); cleanup: + if (pid > 0 && virCommandWait(cmd, NULL) < 0) + ret = -1; + for (i = 0; i < n_columns; i++) VIR_FREE(v[i]); VIR_FREE(v); + virCommandFree(cmd); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(fd); - while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) - /* empty */ ; - - /* Don't bother checking exit status if we already failed */ - if (err < 0) - return -1; - - if (w_err == -1) { - virReportSystemError(errno, - _("failed to wait for command '%s'"), - prog[0]); - return -1; - } else { - if (WIFEXITED(exitstatus)) { - if (WEXITSTATUS(exitstatus) != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("non-zero exit status from command %d"), - WEXITSTATUS(exitstatus)); - return -1; - } - } else { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("command did not exit cleanly")); - return -1; - } - } - - return 0; + return ret; } #else /* WIN32 */ -- 1.7.4.4

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/openvz/openvz_conf.c | 37 ++++++++++--------------------- src/openvz/openvz_driver.c | 52 ++++++++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 88cd4c8..239cd48 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -74,53 +74,40 @@ strtoI(const char *str) static int -openvzExtractVersionInfo(const char *cmd, int *retversion) +openvzExtractVersionInfo(const char *cmdstr, int *retversion) { - const char *const vzarg[] = { cmd, "--help", NULL }; - const char *const vzenv[] = { "LC_ALL=C", NULL }; - pid_t child; - int newstdout = -1; - int ret = -1, status; + int ret = -1; unsigned long version; + char *help = NULL; char *tmp; + virCommandPtr cmd = virCommandNewArgList(cmdstr, "--help", NULL); if (retversion) *retversion = 0; - if (virExec(vzarg, vzenv, NULL, - &child, -1, &newstdout, NULL, VIR_EXEC_NONE) < 0) - return -1; + virCommandAddEnvString(cmd, "LC_ALL=C"); + virCommandSetOutputBuffer(cmd, &help); - char *help = NULL; - int len = virFileReadLimFD(newstdout, 4096, &help); - if (len < 0) - goto cleanup2; + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; tmp = help; /* expected format: vzctl version <major>.<minor>.<micro> */ if ((tmp = STRSKIP(tmp, "vzctl version ")) == NULL) - goto cleanup2; + goto cleanup; if (virParseVersionString(tmp, &version) < 0) - goto cleanup2; + goto cleanup; if (retversion) *retversion = version; ret = 0; -cleanup2: +cleanup: + virCommandFree(cmd); VIR_FREE(help); - if (VIR_CLOSE(newstdout) < 0) - ret = -1; - -rewait: - if (waitpid(child, &status, 0) != child) { - if (errno == EINTR) - goto rewait; - ret = -1; - } return ret; } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index f5fae2d..d65f618 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -59,6 +59,7 @@ #include "bridge.h" #include "files.h" #include "logging.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -1379,19 +1380,15 @@ static int openvzListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int veid; pid_t pid; int outfd = -1; + int rc = -1; int ret; char buf[32]; char *endptr; - const char *cmd[] = {VZLIST, "-ovpsid", "-H" , NULL}; - - ret = virExec(cmd, NULL, NULL, - &pid, -1, &outfd, NULL, VIR_EXEC_NONE); - if (ret == -1) { - openvzError(VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZLIST); - VIR_FORCE_CLOSE(outfd); - return -1; - } + virCommandPtr cmd = virCommandNewArgList(VZLIST, "-ovpsid", "-H" , NULL); + + virCommandSetOutputFD(cmd, &outfd); + if (virCommandRunAsync(cmd, &pid) < 0) + goto cleanup; while (got < nids) { ret = openvz_readline(outfd, buf, 32); @@ -1405,13 +1402,20 @@ static int openvzListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, ids[got] = veid; got ++; } - waitpid(pid, NULL, 0); + + if (virCommandWait(cmd, NULL) < 0) + goto cleanup; if (VIR_CLOSE(outfd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); - return -1; + goto cleanup; } - return got; + + rc = got; +cleanup: + VIR_FORCE_CLOSE(outfd); + virCommandFree(cmd); + return rc; } static int openvzNumDomains(virConnectPtr conn) { @@ -1429,20 +1433,18 @@ static int openvzListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, char **const names, int nnames) { int got = 0; int veid, outfd = -1, ret; + int rc = -1; pid_t pid; char vpsname[32]; char buf[32]; char *endptr; - const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S", NULL}; + virCommandPtr cmd = virCommandNewArgList(VZLIST, + "-ovpsid", "-H", "-S", NULL); /* the -S options lists only stopped domains */ - ret = virExec(cmd, NULL, NULL, - &pid, -1, &outfd, NULL, VIR_EXEC_NONE); - if (ret == -1) { - openvzError(VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZLIST); + virCommandSetOutputFD(cmd, &outfd); + if (virCommandRunAsync(cmd, &pid) < 0) goto out; - } while (got < nnames) { ret = openvz_readline(outfd, buf, 32); @@ -1460,18 +1462,22 @@ static int openvzListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, } got ++; } - waitpid(pid, NULL, 0); + + if (virCommandWait(cmd, NULL) < 0) + goto out; + if (VIR_CLOSE(outfd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); goto out; } - return got; + rc = got; out: VIR_FORCE_CLOSE(outfd); + virCommandFree(cmd); for ( ; got >= 0 ; got--) VIR_FREE(names[got]); - return -1; + return rc; } static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) -- 1.7.4.4

Seems reasonable to have all command wrappers in the same place Signed-off-by: Cole Robinson <crobinso@redhat.com> --- cfg.mk | 2 +- src/libvirt_private.syms | 5 +- src/lxc/veth.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 1 + src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/util/command.c | 590 +++++++++++++++++++++++++++++ src/util/command.h | 14 + src/util/ebtables.c | 2 +- src/util/pci.c | 2 +- src/util/util.c | 579 +---------------------------- src/util/util.h | 24 -- src/vmware/vmware_driver.c | 1 + 13 files changed, 616 insertions(+), 610 deletions(-) diff --git a/cfg.mk b/cfg.mk index 9ee0dd0..09e361a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -621,7 +621,7 @@ exclude_file_name_regexp--sc_prohibit_doubled_word = ^po/ exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^docs/api_extension/|^tests/qemuhelpdata/|\.(gif|ico|png)$$) -_src2=src/(util/util|libvirt|lxc/lxc_controller) +_src2=src/(util/command|libvirt|lxc/lxc_controller) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^docs|^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc900e7..ecf6ab9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -130,6 +130,8 @@ virCommandTransferFD; virCommandTranslateStatus; virCommandWait; virCommandWriteArgLog; +virFork; +virRun; # conf.h @@ -908,7 +910,6 @@ virEnumFromString; virEnumToString; virEventAddHandle; virEventRemoveHandle; -virExecWithHook; virFileAbsPath; virFileDeletePid; virFileExists; @@ -930,7 +931,6 @@ virFileStripSuffix; virFileWaitForDevices; virFileWriteStr; virFindFileInPath; -virFork; virFormatMacAddr; virGenerateMacAddr; virGetGroupID; @@ -949,7 +949,6 @@ virParseVersionString; virPipeReadUntilEOF; virRandom; virRandomInitialize; -virRun; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/lxc/veth.c b/src/lxc/veth.c index a00aa23..1a96f82 100644 --- a/src/lxc/veth.c +++ b/src/lxc/veth.c @@ -21,7 +21,7 @@ #include "internal.h" #include "logging.h" #include "memory.h" -#include "util.h" +#include "command.h" #include "virterror_internal.h" #define VIR_FROM_THIS VIR_FROM_LXC diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 14ce019..b4cd198 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -39,6 +39,7 @@ #include "nwfilter_gentech_driver.h" #include "nwfilter_ebiptables_driver.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_NWFILTER diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0a6b074..d940055 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -41,7 +41,7 @@ #include "storage_backend_fs.h" #include "storage_conf.h" #include "storage_file.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "xml.h" #include "files.h" diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 7809324..c18cd57 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -34,7 +34,7 @@ #include "virterror_internal.h" #include "storage_backend_logical.h" #include "storage_conf.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "logging.h" #include "files.h" diff --git a/src/util/command.c b/src/util/command.c index fa6425d..5e65fc7 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -26,6 +26,12 @@ #include <stdlib.h> #include <sys/stat.h> #include <sys/wait.h> +#include <sys/types.h> +#include <fcntl.h> + +#if HAVE_CAPNG +# include <cap-ng.h> +#endif #include "command.h" #include "memory.h" @@ -34,6 +40,7 @@ #include "logging.h" #include "files.h" #include "buf.h" +#include "ignore-value.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -41,6 +48,14 @@ virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) +/* Flags for virExecWithHook */ +enum { + VIR_EXEC_NONE = 0, + VIR_EXEC_NONBLOCK = (1 << 0), + VIR_EXEC_DAEMON = (1 << 1), + VIR_EXEC_CLEAR_CAPS = (1 << 2), +}; + enum { /* Internal-use extension beyond public VIR_EXEC_ flags */ VIR_EXEC_RUN_SYNC = 0x40000000, @@ -84,6 +99,581 @@ struct _virCommand { bool reap; }; +#ifndef WIN32 + +int virSetInherit(int fd, bool inherit) { + int flags; + if ((flags = fcntl(fd, F_GETFD)) < 0) + return -1; + if (inherit) + flags &= ~FD_CLOEXEC; + else + flags |= FD_CLOEXEC; + if ((fcntl(fd, F_SETFD, flags)) < 0) + return -1; + return 0; +} + + +# if HAVE_CAPNG +static int virClearCapabilities(void) +{ + int ret; + + capng_clear(CAPNG_SELECT_BOTH); + + if ((ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) { + virCommandError(VIR_ERR_INTERNAL_ERROR, + _("cannot clear process capabilities %d"), ret); + return -1; + } + + return 0; +} +# else +static int virClearCapabilities(void) +{ +// VIR_WARN0("libcap-ng support not compiled in, unable to clear capabilities"); + return 0; +} +# endif + + +/* virFork() - fork a new process while avoiding various race/deadlock + conditions + + @pid - a pointer to a pid_t that will receive the return value from + fork() + + on return from virFork(), if *pid < 0, the fork failed and there is + no new process. Otherwise, just like fork(), if *pid == 0, it is the + child process returning, and if *pid > 0, it is the parent. + + Even if *pid >= 0, if the return value from virFork() is < 0, it + indicates a failure that occurred in the parent or child process + after the fork. In this case, the child process should call + _exit(EXIT_FAILURE) after doing any additional error reporting. + + */ +int virFork(pid_t *pid) { +# ifdef HAVE_PTHREAD_SIGMASK + sigset_t oldmask, newmask; +# endif + struct sigaction sig_action; + int saved_errno, ret = -1; + + *pid = -1; + + /* + * Need to block signals now, so that child process can safely + * kill off caller's signal handlers without a race. + */ +# ifdef HAVE_PTHREAD_SIGMASK + sigfillset(&newmask); + if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { + saved_errno = errno; + virReportSystemError(errno, + "%s", _("cannot block signals")); + goto cleanup; + } +# endif + + /* Ensure we hold the logging lock, to protect child processes + * from deadlocking on another thread's inherited mutex state */ + virLogLock(); + + *pid = fork(); + saved_errno = errno; /* save for caller */ + + /* Unlock for both parent and child process */ + virLogUnlock(); + + if (*pid < 0) { +# ifdef HAVE_PTHREAD_SIGMASK + /* attempt to restore signal mask, but ignore failure, to + avoid obscuring the fork failure */ + ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); +# endif + virReportSystemError(saved_errno, + "%s", _("cannot fork child process")); + goto cleanup; + } + + if (*pid) { + + /* parent process */ + +# ifdef HAVE_PTHREAD_SIGMASK + /* Restore our original signal mask now that the child is + safely running */ + if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { + saved_errno = errno; /* save for caller */ + virReportSystemError(errno, "%s", _("cannot unblock signals")); + goto cleanup; + } +# endif + ret = 0; + + } else { + + /* child process */ + + int logprio; + int i; + + /* Remove any error callback so errors in child now + get sent to stderr where they stand a fighting chance + of being seen / logged */ + virSetErrorFunc(NULL, NULL); + virSetErrorLogPriorityFunc(NULL); + + /* Make sure any hook logging is sent to stderr, since child + * process may close the logfile FDs */ + logprio = virLogGetDefaultPriority(); + virLogReset(); + virLogSetDefaultPriority(logprio); + + /* Clear out all signal handlers from parent so nothing + unexpected can happen in our child once we unblock + signals */ + sig_action.sa_handler = SIG_DFL; + sig_action.sa_flags = 0; + sigemptyset(&sig_action.sa_mask); + + for (i = 1; i < NSIG; i++) { + /* Only possible errors are EFAULT or EINVAL + The former wont happen, the latter we + expect, so no need to check return value */ + + sigaction(i, &sig_action, NULL); + } + +# ifdef HAVE_PTHREAD_SIGMASK + /* Unmask all signals in child, since we've no idea + what the caller's done with their signal mask + and don't want to propagate that to children */ + sigemptyset(&newmask); + if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) { + saved_errno = errno; /* save for caller */ + virReportSystemError(errno, "%s", _("cannot unblock signals")); + goto cleanup; + } +# endif + ret = 0; + } + +cleanup: + if (ret < 0) + errno = saved_errno; + return ret; +} + +/* + * @argv argv to exec + * @envp optional environment to use for exec + * @keepfd options fd_ret to keep open for child process + * @retpid optional pointer to store child process pid + * @infd optional file descriptor to use as child input, otherwise /dev/null + * @outfd optional pointer to communicate output fd behavior + * outfd == NULL : Use /dev/null + * *outfd == -1 : Use a new fd + * *outfd != -1 : Use *outfd + * @errfd optional pointer to communcate error fd behavior. See outfd + * @flags possible combination of the following: + * VIR_EXEC_NONE : Default function behavior + * VIR_EXEC_NONBLOCK : Set child process output fd's as non-blocking + * VIR_EXEC_DAEMON : Daemonize the child process + * @hook optional virExecHook function to call prior to exec + * @data data to pass to the hook function + * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) + */ +static int +virExecWithHook(const char *const*argv, + const char *const*envp, + const fd_set *keepfd, + pid_t *retpid, + int infd, int *outfd, int *errfd, + int flags, + virExecHook hook, + void *data, + char *pidfile) +{ + pid_t pid; + int null, i, openmax; + int pipeout[2] = {-1,-1}; + int pipeerr[2] = {-1,-1}; + int childout = -1; + int childerr = -1; + int tmpfd; + const char *binary = NULL; + int forkRet; + char *argv_str = NULL; + char *envp_str = NULL; + + if ((argv_str = virArgvToString(argv)) == NULL) { + virReportOOMError(); + return -1; + } + + if (envp) { + if ((envp_str = virArgvToString(envp)) == NULL) { + VIR_FREE(argv_str); + virReportOOMError(); + return -1; + } + VIR_DEBUG("%s %s", envp_str, argv_str); + VIR_FREE(envp_str); + } else { + VIR_DEBUG0(argv_str); + } + VIR_FREE(argv_str); + + if (argv[0][0] != '/') { + if (!(binary = virFindFileInPath(argv[0]))) { + virReportSystemError(ENOENT, + _("Cannot find '%s' in path"), + argv[0]); + return -1; + } + } else { + binary = argv[0]; + } + + if ((null = open("/dev/null", O_RDWR)) < 0) { + virReportSystemError(errno, + _("cannot open %s"), + "/dev/null"); + goto cleanup; + } + + if (outfd != NULL) { + if (*outfd == -1) { + if (pipe(pipeout) < 0) { + virReportSystemError(errno, + "%s", _("cannot create pipe")); + goto cleanup; + } + + if ((flags & VIR_EXEC_NONBLOCK) && + virSetNonBlock(pipeout[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set non-blocking file descriptor flag")); + goto cleanup; + } + + if (virSetCloseExec(pipeout[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set close-on-exec file descriptor flag")); + goto cleanup; + } + + childout = pipeout[1]; + } else { + childout = *outfd; + } + } else { + childout = null; + } + + if (errfd != NULL) { + if (*errfd == -1) { + if (pipe(pipeerr) < 0) { + virReportSystemError(errno, + "%s", _("Failed to create pipe")); + goto cleanup; + } + + if ((flags & VIR_EXEC_NONBLOCK) && + virSetNonBlock(pipeerr[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set non-blocking file descriptor flag")); + goto cleanup; + } + + if (virSetCloseExec(pipeerr[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set close-on-exec file descriptor flag")); + goto cleanup; + } + + childerr = pipeerr[1]; + } else { + childerr = *errfd; + } + } else { + childerr = null; + } + + forkRet = virFork(&pid); + + if (pid < 0) { + goto cleanup; + } + + if (pid) { /* parent */ + VIR_FORCE_CLOSE(null); + if (outfd && *outfd == -1) { + VIR_FORCE_CLOSE(pipeout[1]); + *outfd = pipeout[0]; + } + if (errfd && *errfd == -1) { + VIR_FORCE_CLOSE(pipeerr[1]); + *errfd = pipeerr[0]; + } + + if (forkRet < 0) { + goto cleanup; + } + + *retpid = pid; + + if (binary != argv[0]) + VIR_FREE(binary); + + return 0; + } + + /* child */ + + if (forkRet < 0) { + /* The fork was sucessful, but after that there was an error + * in the child (which was already logged). + */ + goto fork_error; + } + + openmax = sysconf (_SC_OPEN_MAX); + for (i = 3; i < openmax; i++) + if (i != infd && + i != null && + i != childout && + i != childerr && + (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd))) { + tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } + + if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stdin file handle")); + goto fork_error; + } + if (childout > 0 && + dup2(childout, STDOUT_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stdout file handle")); + goto fork_error; + } + if (childerr > 0 && + dup2(childerr, STDERR_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stderr file handle")); + goto fork_error; + } + + if (infd != STDIN_FILENO) + VIR_FORCE_CLOSE(infd); + VIR_FORCE_CLOSE(null); + if (childout > STDERR_FILENO) { + tmpfd = childout; /* preserve childout value */ + VIR_FORCE_CLOSE(tmpfd); + } + if (childerr > STDERR_FILENO && + childerr != childout) { + VIR_FORCE_CLOSE(childerr); + } + + /* Initialize full logging for a while */ + virLogSetFromEnv(); + + /* Daemonize as late as possible, so the parent process can detect + * the above errors with wait* */ + if (flags & VIR_EXEC_DAEMON) { + if (setsid() < 0) { + virReportSystemError(errno, + "%s", _("cannot become session leader")); + goto fork_error; + } + + if (chdir("/") < 0) { + virReportSystemError(errno, + "%s", _("cannot change to root directory")); + goto fork_error; + } + + pid = fork(); + if (pid < 0) { + virReportSystemError(errno, + "%s", _("cannot fork child process")); + goto fork_error; + } + + if (pid > 0) { + if (pidfile && virFileWritePidPath(pidfile,pid)) { + kill(pid, SIGTERM); + usleep(500*1000); + kill(pid, SIGTERM); + virReportSystemError(errno, + _("could not write pidfile %s for %d"), + pidfile, pid); + goto fork_error; + } + _exit(0); + } + } + + if (hook) { + /* virFork reset all signal handlers to the defaults. + * This is good for the child process, but our hook + * risks running something that generates SIGPIPE, + * so we need to temporarily block that again + */ + struct sigaction waxon, waxoff; + waxoff.sa_handler = SIG_IGN; + waxoff.sa_flags = 0; + sigemptyset(&waxoff.sa_mask); + memset(&waxon, 0, sizeof(waxon)); + if (sigaction(SIGPIPE, &waxoff, &waxon) < 0) { + virReportSystemError(errno, "%s", + _("Could not disable SIGPIPE")); + goto fork_error; + } + + if ((hook)(data) != 0) { + VIR_DEBUG0("Hook function failed."); + goto fork_error; + } + + if (sigaction(SIGPIPE, &waxon, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Could not re-enable SIGPIPE")); + goto fork_error; + } + } + + /* The steps above may need todo something privileged, so + * we delay clearing capabilities until the last minute */ + if ((flags & VIR_EXEC_CLEAR_CAPS) && + virClearCapabilities() < 0) + goto fork_error; + + /* Close logging again to ensure no FDs leak to child */ + virLogReset(); + + if (envp) + execve(binary, (char **) argv, (char**)envp); + else + execv(binary, (char **) argv); + + virReportSystemError(errno, + _("cannot execute binary %s"), + argv[0]); + + fork_error: + virDispatchError(NULL); + _exit(EXIT_FAILURE); + + cleanup: + /* This is cleanup of parent process only - child + should never jump here on error */ + + if (binary != argv[0]) + VIR_FREE(binary); + + /* NB we don't virCommandError() on any failures here + because the code which jumped hre already raised + an error condition which we must not overwrite */ + VIR_FORCE_CLOSE(pipeerr[0]); + VIR_FORCE_CLOSE(pipeerr[1]); + VIR_FORCE_CLOSE(pipeout[0]); + VIR_FORCE_CLOSE(pipeout[1]); + VIR_FORCE_CLOSE(null); + return -1; +} + +/** + * @argv NULL terminated argv to run + * @status optional variable to return exit status in + * + * Run a command without using the shell. + * + * If status is NULL, then return 0 if the command run and + * exited with 0 status; Otherwise return -1 + * + * If status is not-NULL, then return 0 if the command ran. + * The status variable is filled with the command exit status + * and should be checked by caller for success. Return -1 + * only if the command could not be run. + */ +int +virRun(const char *const*argv, int *status) +{ + virCommandPtr cmd = virCommandNew(argv[0]); + const char * const *tmp; + int ret; + + tmp = argv; + while (*(++tmp)) { + virCommandAddArg(cmd, *tmp); + } + + ret = virCommandRun(cmd, status); + virCommandFree(cmd); + return ret; +} + +#else /* WIN32 */ + +int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED) +{ + return -1; +} + +virRun(const char *const *argv ATTRIBUTE_UNUSED, + int *status) +{ + if (status) + *status = ENOTSUP; + else + virCommandError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virRun is not implemented for WIN32")); + return -1; +} + +static int +virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, + const char *const*envp ATTRIBUTE_UNUSED, + const fd_set *keepfd ATTRIBUTE_UNUSED, + pid_t *retpid ATTRIBUTE_UNUSED, + int infd ATTRIBUTE_UNUSED, + int *outfd ATTRIBUTE_UNUSED, + int *errfd ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + char *pidfile ATTRIBUTE_UNUSED) +{ + /* XXX: Some day we can implement pieces of virCommand/virExec on + * top of _spawn() or CreateProcess(), but we can't implement + * everything, since mingw completely lacks fork(), so we cannot + * run hook code in the child. */ + virCommandError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExec is not implemented for WIN32")); + return -1; +} + +int +virFork(pid_t *pid) +{ + *pid = -1; + errno = ENOTSUP; + + return -1; +} + +#endif /* WIN32 */ + + /* * Create a new command for named binary */ diff --git a/src/util/command.h b/src/util/command.h index b16bc27..e690fda 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -29,6 +29,20 @@ typedef struct _virCommand virCommand; typedef virCommand *virCommandPtr; +/* This will execute in the context of the first child + * after fork() but before execve() */ +typedef int (*virExecHook)(void *data); + +/* + * Fork wrapper with extra error checking + */ +int virFork(pid_t *pid) ATTRIBUTE_RETURN_CHECK; + +/* + * Simple synchronous command wrapper + */ +int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; + /* * Create a new command for named binary */ diff --git a/src/util/ebtables.c b/src/util/ebtables.c index 27dce5d..d1afff0 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -41,7 +41,7 @@ #include "internal.h" #include "ebtables.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "virterror_internal.h" #include "logging.h" diff --git a/src/util/pci.c b/src/util/pci.c index d7f74f9..091d76a 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -35,7 +35,7 @@ #include "logging.h" #include "memory.h" -#include "util.h" +#include "command.h" #include "virterror_internal.h" #include "files.h" diff --git a/src/util/util.c b/src/util/util.c index f83b2d0..9b8d70d 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -34,11 +34,11 @@ #include <errno.h> #include <poll.h> #include <time.h> -#include <sys/types.h> -#include <sys/stat.h> #include <sys/ioctl.h> #include <sys/wait.h> #include <sys/time.h> +#include <sys/stat.h> +#include <sys/types.h> #if HAVE_MMAP # include <sys/mman.h> #endif @@ -69,7 +69,6 @@ #include "virterror_internal.h" #include "logging.h" #include "event.h" -#include "ignore-value.h" #include "buf.h" #include "util.h" #include "memory.h" @@ -255,585 +254,11 @@ int virSetNonBlock(int fd) { return virSetBlocking(fd, false); } - int virSetCloseExec(int fd) { return virSetInherit(fd, false); } -#ifndef WIN32 - -int virSetInherit(int fd, bool inherit) { - int flags; - if ((flags = fcntl(fd, F_GETFD)) < 0) - return -1; - if (inherit) - flags &= ~FD_CLOEXEC; - else - flags |= FD_CLOEXEC; - if ((fcntl(fd, F_SETFD, flags)) < 0) - return -1; - return 0; -} - - -# if HAVE_CAPNG -static int virClearCapabilities(void) -{ - int ret; - - capng_clear(CAPNG_SELECT_BOTH); - - if ((ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) { - virUtilError(VIR_ERR_INTERNAL_ERROR, - _("cannot clear process capabilities %d"), ret); - return -1; - } - - return 0; -} -# else -static int virClearCapabilities(void) -{ -// VIR_WARN0("libcap-ng support not compiled in, unable to clear capabilities"); - return 0; -} -# endif - - -/* virFork() - fork a new process while avoiding various race/deadlock conditions - - @pid - a pointer to a pid_t that will receive the return value from - fork() - - on return from virFork(), if *pid < 0, the fork failed and there is - no new process. Otherwise, just like fork(), if *pid == 0, it is the - child process returning, and if *pid > 0, it is the parent. - - Even if *pid >= 0, if the return value from virFork() is < 0, it - indicates a failure that occurred in the parent or child process - after the fork. In this case, the child process should call - _exit(EXIT_FAILURE) after doing any additional error reporting. - - */ -int virFork(pid_t *pid) { -# ifdef HAVE_PTHREAD_SIGMASK - sigset_t oldmask, newmask; -# endif - struct sigaction sig_action; - int saved_errno, ret = -1; - - *pid = -1; - - /* - * Need to block signals now, so that child process can safely - * kill off caller's signal handlers without a race. - */ -# ifdef HAVE_PTHREAD_SIGMASK - sigfillset(&newmask); - if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { - saved_errno = errno; - virReportSystemError(errno, - "%s", _("cannot block signals")); - goto cleanup; - } -# endif - - /* Ensure we hold the logging lock, to protect child processes - * from deadlocking on another thread's inherited mutex state */ - virLogLock(); - - *pid = fork(); - saved_errno = errno; /* save for caller */ - - /* Unlock for both parent and child process */ - virLogUnlock(); - - if (*pid < 0) { -# ifdef HAVE_PTHREAD_SIGMASK - /* attempt to restore signal mask, but ignore failure, to - avoid obscuring the fork failure */ - ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); -# endif - virReportSystemError(saved_errno, - "%s", _("cannot fork child process")); - goto cleanup; - } - - if (*pid) { - - /* parent process */ - -# ifdef HAVE_PTHREAD_SIGMASK - /* Restore our original signal mask now that the child is - safely running */ - if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { - saved_errno = errno; /* save for caller */ - virReportSystemError(errno, "%s", _("cannot unblock signals")); - goto cleanup; - } -# endif - ret = 0; - - } else { - - /* child process */ - - int logprio; - int i; - - /* Remove any error callback so errors in child now - get sent to stderr where they stand a fighting chance - of being seen / logged */ - virSetErrorFunc(NULL, NULL); - virSetErrorLogPriorityFunc(NULL); - - /* Make sure any hook logging is sent to stderr, since child - * process may close the logfile FDs */ - logprio = virLogGetDefaultPriority(); - virLogReset(); - virLogSetDefaultPriority(logprio); - - /* Clear out all signal handlers from parent so nothing - unexpected can happen in our child once we unblock - signals */ - sig_action.sa_handler = SIG_DFL; - sig_action.sa_flags = 0; - sigemptyset(&sig_action.sa_mask); - - for (i = 1; i < NSIG; i++) { - /* Only possible errors are EFAULT or EINVAL - The former wont happen, the latter we - expect, so no need to check return value */ - - sigaction(i, &sig_action, NULL); - } - -# ifdef HAVE_PTHREAD_SIGMASK - /* Unmask all signals in child, since we've no idea - what the caller's done with their signal mask - and don't want to propagate that to children */ - sigemptyset(&newmask); - if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) { - saved_errno = errno; /* save for caller */ - virReportSystemError(errno, "%s", _("cannot unblock signals")); - goto cleanup; - } -# endif - ret = 0; - } - -cleanup: - if (ret < 0) - errno = saved_errno; - return ret; -} - -/* - * @argv argv to exec - * @envp optional environment to use for exec - * @keepfd options fd_ret to keep open for child process - * @retpid optional pointer to store child process pid - * @infd optional file descriptor to use as child input, otherwise /dev/null - * @outfd optional pointer to communicate output fd behavior - * outfd == NULL : Use /dev/null - * *outfd == -1 : Use a new fd - * *outfd != -1 : Use *outfd - * @errfd optional pointer to communcate error fd behavior. See outfd - * @flags possible combination of the following: - * VIR_EXEC_NONE : Default function behavior - * VIR_EXEC_NONBLOCK : Set child process output fd's as non-blocking - * VIR_EXEC_DAEMON : Daemonize the child process - * @hook optional virExecHook function to call prior to exec - * @data data to pass to the hook function - * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) - */ -int -virExecWithHook(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) -{ - pid_t pid; - int null, i, openmax; - int pipeout[2] = {-1,-1}; - int pipeerr[2] = {-1,-1}; - int childout = -1; - int childerr = -1; - int tmpfd; - const char *binary = NULL; - int forkRet; - char *argv_str = NULL; - char *envp_str = NULL; - - if ((argv_str = virArgvToString(argv)) == NULL) { - virReportOOMError(); - return -1; - } - - if (envp) { - if ((envp_str = virArgvToString(envp)) == NULL) { - VIR_FREE(argv_str); - virReportOOMError(); - return -1; - } - VIR_DEBUG("%s %s", envp_str, argv_str); - VIR_FREE(envp_str); - } else { - VIR_DEBUG0(argv_str); - } - VIR_FREE(argv_str); - - if (argv[0][0] != '/') { - if (!(binary = virFindFileInPath(argv[0]))) { - virReportSystemError(ENOENT, - _("Cannot find '%s' in path"), - argv[0]); - return -1; - } - } else { - binary = argv[0]; - } - - if ((null = open("/dev/null", O_RDWR)) < 0) { - virReportSystemError(errno, - _("cannot open %s"), - "/dev/null"); - goto cleanup; - } - - if (outfd != NULL) { - if (*outfd == -1) { - if (pipe(pipeout) < 0) { - virReportSystemError(errno, - "%s", _("cannot create pipe")); - goto cleanup; - } - - if ((flags & VIR_EXEC_NONBLOCK) && - virSetNonBlock(pipeout[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set non-blocking file descriptor flag")); - goto cleanup; - } - - if (virSetCloseExec(pipeout[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set close-on-exec file descriptor flag")); - goto cleanup; - } - - childout = pipeout[1]; - } else { - childout = *outfd; - } - } else { - childout = null; - } - - if (errfd != NULL) { - if (*errfd == -1) { - if (pipe(pipeerr) < 0) { - virReportSystemError(errno, - "%s", _("Failed to create pipe")); - goto cleanup; - } - - if ((flags & VIR_EXEC_NONBLOCK) && - virSetNonBlock(pipeerr[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set non-blocking file descriptor flag")); - goto cleanup; - } - - if (virSetCloseExec(pipeerr[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set close-on-exec file descriptor flag")); - goto cleanup; - } - - childerr = pipeerr[1]; - } else { - childerr = *errfd; - } - } else { - childerr = null; - } - - forkRet = virFork(&pid); - - if (pid < 0) { - goto cleanup; - } - - if (pid) { /* parent */ - VIR_FORCE_CLOSE(null); - if (outfd && *outfd == -1) { - VIR_FORCE_CLOSE(pipeout[1]); - *outfd = pipeout[0]; - } - if (errfd && *errfd == -1) { - VIR_FORCE_CLOSE(pipeerr[1]); - *errfd = pipeerr[0]; - } - - if (forkRet < 0) { - goto cleanup; - } - - *retpid = pid; - - if (binary != argv[0]) - VIR_FREE(binary); - - return 0; - } - - /* child */ - - if (forkRet < 0) { - /* The fork was sucessful, but after that there was an error - * in the child (which was already logged). - */ - goto fork_error; - } - - openmax = sysconf (_SC_OPEN_MAX); - for (i = 3; i < openmax; i++) - if (i != infd && - i != null && - i != childout && - i != childerr && - (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd))) { - tmpfd = i; - VIR_FORCE_CLOSE(tmpfd); - } - - if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stdin file handle")); - goto fork_error; - } - if (childout > 0 && - dup2(childout, STDOUT_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stdout file handle")); - goto fork_error; - } - if (childerr > 0 && - dup2(childerr, STDERR_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stderr file handle")); - goto fork_error; - } - - if (infd != STDIN_FILENO) - VIR_FORCE_CLOSE(infd); - VIR_FORCE_CLOSE(null); - if (childout > STDERR_FILENO) { - tmpfd = childout; /* preserve childout value */ - VIR_FORCE_CLOSE(tmpfd); - } - if (childerr > STDERR_FILENO && - childerr != childout) { - VIR_FORCE_CLOSE(childerr); - } - - /* Initialize full logging for a while */ - virLogSetFromEnv(); - - /* Daemonize as late as possible, so the parent process can detect - * the above errors with wait* */ - if (flags & VIR_EXEC_DAEMON) { - if (setsid() < 0) { - virReportSystemError(errno, - "%s", _("cannot become session leader")); - goto fork_error; - } - - if (chdir("/") < 0) { - virReportSystemError(errno, - "%s", _("cannot change to root directory")); - goto fork_error; - } - - pid = fork(); - if (pid < 0) { - virReportSystemError(errno, - "%s", _("cannot fork child process")); - goto fork_error; - } - - if (pid > 0) { - if (pidfile && virFileWritePidPath(pidfile,pid)) { - kill(pid, SIGTERM); - usleep(500*1000); - kill(pid, SIGTERM); - virReportSystemError(errno, - _("could not write pidfile %s for %d"), - pidfile, pid); - goto fork_error; - } - _exit(0); - } - } - - if (hook) { - /* virFork reset all signal handlers to the defaults. - * This is good for the child process, but our hook - * risks running something that generates SIGPIPE, - * so we need to temporarily block that again - */ - struct sigaction waxon, waxoff; - waxoff.sa_handler = SIG_IGN; - waxoff.sa_flags = 0; - sigemptyset(&waxoff.sa_mask); - memset(&waxon, 0, sizeof(waxon)); - if (sigaction(SIGPIPE, &waxoff, &waxon) < 0) { - virReportSystemError(errno, "%s", - _("Could not disable SIGPIPE")); - goto fork_error; - } - - if ((hook)(data) != 0) { - VIR_DEBUG0("Hook function failed."); - goto fork_error; - } - - if (sigaction(SIGPIPE, &waxon, NULL) < 0) { - virReportSystemError(errno, "%s", - _("Could not re-enable SIGPIPE")); - goto fork_error; - } - } - - /* The steps above may need todo something privileged, so - * we delay clearing capabilities until the last minute */ - if ((flags & VIR_EXEC_CLEAR_CAPS) && - virClearCapabilities() < 0) - goto fork_error; - - /* Close logging again to ensure no FDs leak to child */ - virLogReset(); - - if (envp) - execve(binary, (char **) argv, (char**)envp); - else - execv(binary, (char **) argv); - - virReportSystemError(errno, - _("cannot execute binary %s"), - argv[0]); - - fork_error: - virDispatchError(NULL); - _exit(EXIT_FAILURE); - - cleanup: - /* This is cleanup of parent process only - child - should never jump here on error */ - - if (binary != argv[0]) - VIR_FREE(binary); - - /* NB we don't virUtilError() on any failures here - because the code which jumped hre already raised - an error condition which we must not overwrite */ - VIR_FORCE_CLOSE(pipeerr[0]); - VIR_FORCE_CLOSE(pipeerr[1]); - VIR_FORCE_CLOSE(pipeout[0]); - VIR_FORCE_CLOSE(pipeout[1]); - VIR_FORCE_CLOSE(null); - return -1; -} - -/** - * @argv NULL terminated argv to run - * @status optional variable to return exit status in - * - * Run a command without using the shell. - * - * If status is NULL, then return 0 if the command run and - * exited with 0 status; Otherwise return -1 - * - * If status is not-NULL, then return 0 if the command ran. - * The status variable is filled with the command exit status - * and should be checked by caller for success. Return -1 - * only if the command could not be run. - */ -int -virRun(const char *const*argv, int *status) -{ - virCommandPtr cmd = virCommandNew(argv[0]); - const char * const *tmp; - int ret; - - tmp = argv; - while (*(++tmp)) { - virCommandAddArg(cmd, *tmp); - } - - ret = virCommandRun(cmd, status); - virCommandFree(cmd); - return ret; -} - -#else /* WIN32 */ - -int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED) -{ - return -1; -} - -virRun(const char *const *argv ATTRIBUTE_UNUSED, - int *status) -{ - if (status) - *status = ENOTSUP; - else - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virRun is not implemented for WIN32")); - return -1; -} - -int -virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, - const char *const*envp ATTRIBUTE_UNUSED, - const fd_set *keepfd ATTRIBUTE_UNUSED, - pid_t *retpid ATTRIBUTE_UNUSED, - int infd ATTRIBUTE_UNUSED, - int *outfd ATTRIBUTE_UNUSED, - int *errfd ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED, - virExecHook hook ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - char *pidfile ATTRIBUTE_UNUSED) -{ - /* XXX: Some day we can implement pieces of virCommand/virExec on - * top of _spawn() or CreateProcess(), but we can't implement - * everything, since mingw completely lacks fork(), so we cannot - * run hook code in the child. */ - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virExec is not implemented for WIN32")); - return -1; -} - -int -virFork(pid_t *pid) -{ - *pid = -1; - errno = ENOTSUP; - - return -1; -} - -#endif /* WIN32 */ - int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf) { diff --git a/src/util/util.h b/src/util/util.h index 3e95cae..68a8431 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -42,37 +42,13 @@ ssize_t safewrite(int fd, const void *buf, size_t count) int safezero(int fd, int flags, off_t offset, off_t len) ATTRIBUTE_RETURN_CHECK; -enum { - VIR_EXEC_NONE = 0, - VIR_EXEC_NONBLOCK = (1 << 0), - VIR_EXEC_DAEMON = (1 << 1), - VIR_EXEC_CLEAR_CAPS = (1 << 2), -}; - int virSetBlocking(int fd, bool blocking) ATTRIBUTE_RETURN_CHECK; int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK; int virSetInherit(int fd, bool inherit) ATTRIBUTE_RETURN_CHECK; int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK; -/* This will execute in the context of the first child - * after fork() but before execve() */ -typedef int (*virExecHook)(void *data); - -int virExecWithHook(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - int *retpid, - int infd, - int *outfd, - int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) ATTRIBUTE_RETURN_CHECK; -int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf); -int virFork(pid_t *pid); int virSetUIDGID(uid_t uid, gid_t gid); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index a3a13c1..9a7fbf1 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -29,6 +29,7 @@ #include "files.h" #include "memory.h" #include "uuid.h" +#include "command.h" #include "vmx.h" #include "vmware_conf.h" #include "vmware_driver.h" -- 1.7.4.4