On Wed, Dec 01, 2010 at 10:42:40PM +0100, Matthias Bolte wrote: > 2010/11/24 Eric Blake <[email protected]>: > > +/* > > + * Manage input and output to the child process. > > + */ > > +static int > > +virCommandProcessIO(virCommandPtr cmd) > > +{ > > + int infd = -1, outfd = -1, errfd = -1; > > + size_t inlen = 0, outlen = 0, errlen = 0; > > + size_t inoff = 0; > > + > > + /* With an input buffer, feed data to child > > + * via pipe */ > > + if (cmd->inbuf) { > > + inlen = strlen(cmd->inbuf); > > + infd = cmd->inpipe; > > + } > > + > > + /* With out/err buffer, the outfd/errfd > > + * have been filled with an FD for us */ > > + if (cmd->outbuf) > > + outfd = cmd->outfd; > > + if (cmd->errbuf) > > + errfd = cmd->errfd; > > + > > > +/* > > + * Run the command and wait for completion. > > + * Returns -1 on any error executing the > > + * command. Returns 0 if the command executed, > > + * with the exit status set > > + */ > > +int > > +virCommandRun(virCommandPtr cmd, int *exitstatus) > > +{ > > + int ret = 0; > > + char *outbuf = NULL; > > + char *errbuf = NULL; > > + int infd[2]; > > + > > + if (!cmd || cmd->has_error == -1) { > > + virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("invalid use of command API")); > > + return -1; > > + } > > + if (cmd->has_error == ENOMEM) { > > + virReportOOMError(); > > + return -1; > > + } > > + > > + /* If we have an input buffer, we need > > + * a pipe to feed the data to the child */ > > + if (cmd->inbuf) { > > + if (pipe(infd) < 0) { > > + virReportSystemError(errno, "%s", > > + _("unable to open pipe")); > > + return -1; > > + } > > + cmd->infd = infd[0]; > > + cmd->inpipe = infd[1]; > > + } > > + > > + if (virCommandRunAsync(cmd, NULL) < 0) { > > + if (cmd->inbuf) { > > + int tmpfd = infd[0]; > > + if (VIR_CLOSE(infd[0]) < 0) > > + VIR_DEBUG("ignoring failed close on fd %d", tmpfd); > > + tmpfd = infd[1]; > > + if (VIR_CLOSE(infd[1]) < 0) > > + VIR_DEBUG("ignoring failed close on fd %d", tmpfd); > > + } > > + return -1; > > + } > > + > > + /* If caller hasn't requested capture of > > + * stdout/err, then capture it ourselves > > + * so we can log it > > + */ > > + if (!cmd->outbuf && > > + !cmd->outfdptr) { > > + cmd->outfd = -1; > > + cmd->outfdptr = &cmd->outfd; > > + cmd->outbuf = &outbuf; > > + } > > + if (!cmd->errbuf && > > + !cmd->errfdptr) { > > + cmd->errfd = -1; > > + cmd->errfdptr = &cmd->errfd; > > + cmd->errbuf = &errbuf; > > + } > > + > > + if (cmd->inbuf || cmd->outbuf || cmd->errbuf) > > + ret = virCommandProcessIO(cmd); > > In virCommandProcessIO you say that cmd->{out|err}fd is a valid FD > (created in virExecWithHook called by virCommandRunAsync) when > cmd->{out|err}buf is not NULL. As far as I can tell from the code that > is true when the caller had requested to capture stdout and stderr. > But in case that caller didn't do this then you setup buffers to > capture stdout and stderr for logging. In that case > virCommandProcessIO will be called with cmd->{out|err}buf being > non-NULL but cmd->{out|err}fd being invalid as you explicitly set them > to -1 in the two if blocks before the call to virCommandProcessIO.
Those two blocks setting errfd/outfd to -1 are not executed when outbuf/errbuf are non-NULL, so errfd/outfd are still pointing to the pipe() FDs when virCommandProcesIO is run. > > diff --git a/tests/commandtest.c b/tests/commandtest.c > > new file mode 100644 > > index 0000000..46d6ae5 > > --- /dev/null > > +++ b/tests/commandtest.c > > > + > > +#ifndef __linux__ > > What's Linux specific in this test? Shouldn't the command API and this > test be working on all systems that support fork/exec? It should have been #ifndef WIN32 actually. Daniel -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
