External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-24 14:45:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

util: eliminate extra args from virExec

Browse Source 
All args except "cmd" in the call to virExec are now redundant, since
they can all be found in cmd, so remove the args and reference the
data directly in cmd. One exception to this is that "infd" was being
modified within virExec, and modifying the original in cmd caused make
check failures, so cmd->infd is copied to a local, and the local is
used during virExec().
This commit is contained in:
Laine Stump 2013-01-29 13:47:18 -05:00
parent b6decc57b1
commit 5f2ce53984
1 changed files with 49 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
src/util/vircommand.c
View File

@ -377,40 +377,18 @@ prepareStdFd(int fd, int std)
} }
/* /*
* @argv argv to exec * virExec:
* @envp optional environment to use for exec * @cmd virCommandPtr containing all information about the program to
* @keepfd options fd_ret to keep open for child process * exec.
* @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
* @cmd virCommandPtr to pass to virCommandHook
* @pidfile path to use as pidfile for daemonized process (needs DAEMON flag)
* @capabilities capabilities to keep
*/ */
static int static int
virExec(virCommandPtr cmd, virExec(virCommandPtr cmd)
const char *const*argv,
const char *const*envp,
const int *keepfd,
int keepfd_size,
pid_t *retpid,
int infd, int *outfd, int *errfd,
unsigned int flags,
char *pidfile,
unsigned long long capabilities)
{ {
pid_t pid; pid_t pid;
int null = -1, i, openmax; int null = -1, i, openmax;
int pipeout[2] = {-1,-1}; int pipeout[2] = {-1,-1};
int pipeerr[2] = {-1,-1}; int pipeerr[2] = {-1,-1};
int childin = cmd->infd;
int childout = -1; int childout = -1;
int childerr = -1; int childerr = -1;
int tmpfd; int tmpfd;
@ -418,41 +396,41 @@ virExec(virCommandPtr cmd,
int forkRet; int forkRet;
struct sigaction waxon, waxoff; struct sigaction waxon, waxoff;
if (argv[0][0] != '/') { if (cmd->args[0][0] != '/') {
if (!(binary = virFindFileInPath(argv[0]))) { if (!(binary = virFindFileInPath(cmd->args[0]))) {
virReportSystemError(ENOENT, virReportSystemError(ENOENT,
_("Cannot find '%s' in path"), _("Cannot find '%s' in path"),
argv[0]); cmd->args[0]);
return -1; return -1;
} }
} else { } else {
binary = argv[0]; binary = cmd->args[0];
} }
if (infd < 0) { if (childin < 0) {
if (getDevNull(&null) < 0) if (getDevNull(&null) < 0)
goto cleanup; goto cleanup;
infd = null; childin = null;
} }
if (outfd != NULL) { if (cmd->outfdptr != NULL) {
if (*outfd == -1) { if (*cmd->outfdptr == -1) {
if (pipe2(pipeout, O_CLOEXEC) < 0) { if (pipe2(pipeout, O_CLOEXEC) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
"%s", _("cannot create pipe")); "%s", _("cannot create pipe"));
goto cleanup; goto cleanup;
} }
if ((flags & VIR_EXEC_NONBLOCK) && if ((cmd->flags & VIR_EXEC_NONBLOCK) &&
virSetNonBlock(pipeout[0]) == -1) { virSetNonBlock(pipeout[0]) == -1) {
virReportSystemError(errno, virReportSystemError(errno, "%s",
"%s", _("Failed to set non-blocking file descriptor flag")); _("Failed to set non-blocking file descriptor flag"));
goto cleanup; goto cleanup;
} }
childout = pipeout[1]; childout = pipeout[1];
} else { } else {
childout = *outfd; childout = *cmd->outfdptr;
} }
} else { } else {
if (getDevNull(&null) < 0) if (getDevNull(&null) < 0)
@ -460,26 +438,26 @@ virExec(virCommandPtr cmd,
childout = null; childout = null;
} }
if (errfd != NULL) { if (cmd->errfdptr != NULL) {
if (errfd == outfd) { if (cmd->errfdptr == cmd->outfdptr) {
childerr = childout; childerr = childout;
} else if (*errfd == -1) { } else if (*cmd->errfdptr == -1) {
if (pipe2(pipeerr, O_CLOEXEC) < 0) { if (pipe2(pipeerr, O_CLOEXEC) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
"%s", _("Failed to create pipe")); "%s", _("Failed to create pipe"));
goto cleanup; goto cleanup;
} }
if ((flags & VIR_EXEC_NONBLOCK) && if ((cmd->flags & VIR_EXEC_NONBLOCK) &&
virSetNonBlock(pipeerr[0]) == -1) { virSetNonBlock(pipeerr[0]) == -1) {
virReportSystemError(errno, virReportSystemError(errno, "%s",
"%s", _("Failed to set non-blocking file descriptor flag")); _("Failed to set non-blocking file descriptor flag"));
goto cleanup; goto cleanup;
} }
childerr = pipeerr[1]; childerr = pipeerr[1];
} else { } else {
childerr = *errfd; childerr = *cmd->errfdptr;
} }
} else { } else {
if (getDevNull(&null) < 0) if (getDevNull(&null) < 0)
@ -499,18 +477,18 @@ virExec(virCommandPtr cmd,
} }
VIR_FORCE_CLOSE(null); VIR_FORCE_CLOSE(null);
if (outfd && *outfd == -1) { if (cmd->outfdptr && *cmd->outfdptr == -1) {
VIR_FORCE_CLOSE(pipeout[1]); VIR_FORCE_CLOSE(pipeout[1]);
*outfd = pipeout[0]; *cmd->outfdptr = pipeout[0];
} }
if (errfd && *errfd == -1) { if (cmd->errfdptr && *cmd->errfdptr == -1) {
VIR_FORCE_CLOSE(pipeerr[1]); VIR_FORCE_CLOSE(pipeerr[1]);
*errfd = pipeerr[0]; *cmd->errfdptr = pipeerr[0];
} }
*retpid = pid; cmd->pid = pid;
if (binary != argv[0]) if (binary != cmd->args[0])
VIR_FREE(binary); VIR_FREE(binary);
return 0; return 0;
@ -527,9 +505,10 @@ virExec(virCommandPtr cmd,
openmax = sysconf(_SC_OPEN_MAX); openmax = sysconf(_SC_OPEN_MAX);
for (i = 3; i < openmax; i++) { for (i = 3; i < openmax; i++) {
if (i == infd || i == childout || i == childerr) if (i == childin || i == childout || i == childerr)
continue; continue;
if (!keepfd || !virCommandFDIsSet(i, keepfd, keepfd_size)) { if (!cmd->preserve ||
!virCommandFDIsSet(i, cmd->preserve, cmd->preserve_size)) {
tmpfd = i; tmpfd = i;
VIR_MASS_CLOSE(tmpfd); VIR_MASS_CLOSE(tmpfd);
} else if (virSetInherit(i, true) < 0) { } else if (virSetInherit(i, true) < 0) {
@ -538,7 +517,7 @@ virExec(virCommandPtr cmd,
} }
} }
if (prepareStdFd(infd, STDIN_FILENO) < 0) { if (prepareStdFd(childin, STDIN_FILENO) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
"%s", _("failed to setup stdin file handle")); "%s", _("failed to setup stdin file handle"));
goto fork_error; goto fork_error;
@ -554,9 +533,9 @@ virExec(virCommandPtr cmd,
goto fork_error; goto fork_error;
} }
if (infd != STDIN_FILENO && infd != null && infd != childerr && if (childin != STDIN_FILENO && childin != null &&
infd != childout) childin != childerr && childin != childout)
VIR_FORCE_CLOSE(infd); VIR_FORCE_CLOSE(childin);
if (childout > STDERR_FILENO && childout != null && childout != childerr) if (childout > STDERR_FILENO && childout != null && childout != childerr)
VIR_FORCE_CLOSE(childout); VIR_FORCE_CLOSE(childout);
if (childerr > STDERR_FILENO && childerr != null) if (childerr > STDERR_FILENO && childerr != null)
@ -568,7 +547,7 @@ virExec(virCommandPtr cmd,
/* Daemonize as late as possible, so the parent process can detect /* Daemonize as late as possible, so the parent process can detect
* the above errors with wait* */ * the above errors with wait* */
if (flags & VIR_EXEC_DAEMON) { if (cmd->flags & VIR_EXEC_DAEMON) {
if (setsid() < 0) { if (setsid() < 0) {
virReportSystemError(errno, virReportSystemError(errno,
"%s", _("cannot become session leader")); "%s", _("cannot become session leader"));
@ -589,13 +568,13 @@ virExec(virCommandPtr cmd,
} }
if (pid > 0) { if (pid > 0) {
if (pidfile && (virPidFileWritePath(pidfile,pid) < 0)) { if (cmd->pidfile && (virPidFileWritePath(cmd->pidfile, pid) < 0)) {
kill(pid, SIGTERM); kill(pid, SIGTERM);
usleep(500*1000); usleep(500*1000);
kill(pid, SIGTERM); kill(pid, SIGTERM);
virReportSystemError(errno, virReportSystemError(errno,
_("could not write pidfile %s for %d"), _("could not write pidfile %s for %d"),
pidfile, pid); cmd->pidfile, pid);
goto fork_error; goto fork_error;
} }
_exit(0); _exit(0);
@ -630,21 +609,21 @@ virExec(virCommandPtr cmd,
/* The steps above may need todo something privileged, so /* The steps above may need todo something privileged, so
* we delay clearing capabilities until the last minute */ * we delay clearing capabilities until the last minute */
if (capabilities || (flags & VIR_EXEC_CLEAR_CAPS)) if (cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS))
if (virSetCapabilities(capabilities) < 0) if (virSetCapabilities(cmd->capabilities) < 0)
goto fork_error; goto fork_error;
/* Close logging again to ensure no FDs leak to child */ /* Close logging again to ensure no FDs leak to child */
virLogReset(); virLogReset();
if (envp) if (cmd->env)
execve(binary, (char **) argv, (char**)envp); execve(binary, cmd->args, cmd->env);
else else
execv(binary, (char **) argv); execv(binary, cmd->args);
virReportSystemError(errno, virReportSystemError(errno,
_("cannot execute binary %s"), _("cannot execute binary %s"),
argv[0]); cmd->args[0]);
fork_error: fork_error:
virDispatchError(NULL); virDispatchError(NULL);
@ -654,7 +633,7 @@ virExec(virCommandPtr cmd,
/* This is cleanup of parent process only - child /* This is cleanup of parent process only - child
should never jump here on error */ should never jump here on error */
if (binary != argv[0]) if (binary != cmd->args[0])
VIR_FREE(binary); VIR_FREE(binary);
/* NB we don't virReportError() on any failures here /* NB we don't virReportError() on any failures here
@ -709,18 +688,7 @@ virRun(const char *const *argv ATTRIBUTE_UNUSED,
} }
static int static int
virExec(virCommandPtr cmd ATTRIBUTE_UNUSED, virExec(virCommandPtr cmd ATTRIBUTE_UNUSED)
const char *const*argv ATTRIBUTE_UNUSED,
const char *const*envp ATTRIBUTE_UNUSED,
const int *keepfd ATTRIBUTE_UNUSED,
int keepfd_size ATTRIBUTE_UNUSED,
pid_t *retpid ATTRIBUTE_UNUSED,
int infd ATTRIBUTE_UNUSED,
int *outfd ATTRIBUTE_UNUSED,
int *errfd ATTRIBUTE_UNUSED,
int flags_unused ATTRIBUTE_UNUSED,
char *pidfile ATTRIBUTE_UNUSED,
unsigned long long capabilities ATTRIBUTE_UNUSED)
{ {
/* XXX: Some day we can implement pieces of virCommand/virExec on /* XXX: Some day we can implement pieces of virCommand/virExec on
* top of _spawn() or CreateProcess(), but we can't implement * top of _spawn() or CreateProcess(), but we can't implement
@ -2196,19 +2164,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
VIR_DEBUG("About to run %s", str ? str : cmd->args[0]); VIR_DEBUG("About to run %s", str ? str : cmd->args[0]);
VIR_FREE(str); VIR_FREE(str);
ret = virExec(cmd, ret = virExec(cmd);
(const char *const *)cmd->args,
(const char *const *)cmd->env,
cmd->preserve,
cmd->preserve_size,
&cmd->pid,
cmd->infd,
cmd->outfdptr,
cmd->errfdptr,
cmd->flags,
cmd->pidfile,
cmd->capabilities);
VIR_DEBUG("Command result %d, with PID %d", VIR_DEBUG("Command result %d, with PID %d",
ret, (int)cmd->pid); ret, (int)cmd->pid);