[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] util: include stderr in log message when an external command fails



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;
-- 
1.7.11.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]