On Wed, Dec 01, 2010 at 10:42:40PM +0100, Matthias Bolte wrote:
> 2010/11/24 Eric Blake <eblake@xxxxxxxxxx>:
> > +/*
> > + * 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
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list