[libvirt] PATCH: 1/5: Improved error reporting

[Daniel P. Berrange]

On Tue, Aug 19, 2008 at 12:44:16PM +0200, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > There are several system calls in the virExec function for which we don't
> > or can't report errors. This patch does a couple of things to improve the
> > situation. It moves the code for setting non-block/close-exec to before the
> > fork() so we can report errors for it. It removes the 'dom' and 'net' params
> > from the ReportError function since we deprecated those long ago and all
> > callers simply pass in NULL. It resets the 'virErrorHandler' callback to
> > NULL in the child, so that errors raised will get reported to stderr
> > instead of invoking a callback which is likely no longer valid in the child
> > process. It reports failures to exec the binary and dup  file descriptors.
> > All errors in the child will end up on stderr, so they will at least be
> > visible on the parent's console, or a logfile if one was setup for the
> > child. Previously there would just be silent failure.
> 
> Looks fine.
> ACK
> 
> one "would be nice" comment:
> 
> > diff -r 4f44b07c47c1 src/util.c
> ...
> >      if (status == NULL) {
> >          errno = EINVAL;
> > -        return (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) == 0) ? 0 : -1;
> > +        if (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) == 0)
> > +            return 0;
> > +
> > +        ReportError(conn, VIR_ERR_INTERNAL_ERROR,
> > +                    _("%s exited with non-zero status %s"),
> 
> Since this code is going to be used in so many contexts, now,
> it'd be nice to report which signal (if any) and the precise
> exit status value.

Yes, good idea - i'll add that when i commit.

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 :|