"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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;
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list