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

[Jim Meyering]

"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.

> +                    argv[0]);
> +        return -1;