[libvirt] [PATCH] Log virRun command output, return with error

[Daniel P. Berrange]

On Wed, Sep 24, 2008 at 12:24:18PM +0100, Richard W.M. Jones wrote:
> On Tue, Sep 23, 2008 at 04:56:45PM -0400, Cole Robinson wrote:
> >      if ((ret = virExec(conn, argv, NULL, NULL,
> > -                       &childpid, -1, NULL, NULL, VIR_EXEC_NONE)) < 0)
> > +                       &childpid, -1, &outfd, &errfd, VIR_EXEC_NONE)) < 0)
> >          return ret;
> >  
> >      while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR);
> > @@ -418,16 +430,31 @@ virRun(virConnectPtr conn,
> >          return -1;
> >      }
> >  
> > +    /* Log command output */
> > +    if (outfd) {
> > +        ret = saferead(outfd, out, sizeof(out)-1);
> > +        err[ret < 0 ? 0 : ret] = '\0';
> > +        if (*out)
> > +            DEBUG("Command stdout: %s", out);
> > +    }
> > +    if (errfd) {
> > +        ret = saferead(errfd, err, sizeof(err)-1);
> > +        err[ret < 0 ? 0 : ret] = '\0';
> > +        if (*err)
> > +            DEBUG("Command stderr: %s", err);
> > +    }
> > +
> 
> I'm sure this works in tests, but I don't think it'll work reliably
> all the time.
> 
> What happens if the command pauses for some time before sending output
> to stdout/stderr?  You need to integrate these in the event loop,
> which is going to make everything a lot more complicated.

The virRun command is synchronous so its blocking the caller no matter
what.

We do however need to process more than justa fixed size buffer, and
in doing so you can't rely on reading all of stdout, followed by all
or stderr. You need to read which has data sent to it as it happens
otherwise the process could block on a full pipe for stderr(), while
you're waiting for end-of-file on stdout.

So we do need to call poll() here to determine which FD has more data
available. Don't need a full event loop though.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|