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

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