On Mon, Aug 06, 2012 at 08:07:32PM -0400, Laine Stump wrote: > This patch is in response to: > > https://bugzilla.redhat.com/show_bug.cgi?id=818467 > > If a caller to virCommandRun doesn't ask for the exitstatus of the > program it's running, the virCommand functions assume that they should > log an error message and return failure if the exit code isn't > 0. However, only the commandline and exit status are logged, while > potentially useful information sent by the program to stderr is > discarded. > > Fortunately, virCommandRun is already checking if the caller had asked > for stderr to be saved and, if not, sets things up to save it in > *cmd->errbuf. This makes it fairly simple for virCommandWait to > include *cmd->errbuf in the error log (there are still other callers > that don't setup errbuf, and even virCommandRun won't set it up if the > command is being daemonized, so we have to check that it's non-zero). > --- > > Note that the minor change to the first virReportError string was made > because I noticed that virCommandTranslateStatus already puts the word > "status" in its string. The new message is less awkward. > > src/util/command.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/src/util/command.c b/src/util/command.c > index 334ca89..7755572 100644 > --- a/src/util/command.c > +++ b/src/util/command.c > @@ -2269,7 +2269,7 @@ virPidWait(pid_t pid, int *exitstatus) > if (status != 0) { > char *st = virCommandTranslateStatus(status); > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Child process (%lld) status unexpected: %s"), > + _("Child process (%lld) unexpected %s"), > (long long) pid, NULLSTR(st)); > VIR_FREE(st); > return -1; > @@ -2327,9 +2327,13 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) > if (status) { > char *str = virCommandToString(cmd); > char *st = virCommandTranslateStatus(status); > + bool haveErrMsg = cmd->errbuf && *cmd->errbuf && (*cmd->errbuf)[0]; > + > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Child process (%s) status unexpected: %s"), > - str ? str : cmd->args[0], NULLSTR(st)); > + _("Child process (%s) unexpected %s%s%s"), > + str ? str : cmd->args[0], NULLSTR(st), > + haveErrMsg ? ": " : "", > + haveErrMsg ? *cmd->errbuf : ""); > VIR_FREE(str); > VIR_FREE(st); > return -1; ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list