"Daniel P. Berrange" <berrange(a)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;