[libvirt] [PATCH 0/3] qemu: Fix error reporting from QEMU log
Jiri Denemark (3): qemu: Properly skip "char device redirected to" in QEMU log vierror: Define VIR_ERROR_MAX_LENGTH macro qemu: Use the end of QEMU log for reporting errors src/qemu/qemu_process.c | 36 +++++++++++++++++++++++++++++++----- src/util/virerror.c | 6 +++--- src/util/virerror.h | 2 ++ 3 files changed, 36 insertions(+), 8 deletions(-) -- 2.15.0
When reading QEMU log for reporting it as an error message, we want to skip "char device redirected to" line. However, this string is not printed at the beginning of a line, which means STRPREFIX will never find it. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6d242b1b51..3da297c16f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1847,7 +1847,7 @@ qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, char **msg) while ((eol = strchr(filter_next, '\n'))) { *eol = '\0'; if (virLogProbablyLogMessage(filter_next) || - STRPREFIX(filter_next, "char device redirected to")) { + strstr(filter_next, "char device redirected to")) { size_t skip = (eol + 1) - filter_next; memmove(filter_next, eol + 1, buf + got - eol); got -= skip; -- 2.15.0
And use it instead of a magic 1024 constant. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virerror.c | 6 +++--- src/util/virerror.h | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 1f15c5dbbe..91022c3b63 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1480,7 +1480,7 @@ void virReportErrorHelper(int domcode, { int save_errno = errno; va_list args; - char errorMessage[1024]; + char errorMessage[VIR_ERROR_MAX_LENGTH]; const char *virerr; if (fmt) { @@ -1541,8 +1541,8 @@ void virReportSystemErrorFull(int domcode, const char *fmt, ...) { int save_errno = errno; - char strerror_buf[1024]; - char msgDetailBuf[1024]; + char strerror_buf[VIR_ERROR_MAX_LENGTH]; + char msgDetailBuf[VIR_ERROR_MAX_LENGTH]; const char *errnoDetail = virStrerror(theerrno, strerror_buf, sizeof(strerror_buf)); diff --git a/src/util/virerror.h b/src/util/virerror.h index fba18ba589..cf434f45fc 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -24,6 +24,8 @@ # include "internal.h" +# define VIR_ERROR_MAX_LENGTH 1024 + extern virErrorFunc virErrorHandler; extern void *virUserData; -- 2.15.0
When QEMU dies, we read its output stored in a log file and use it for reporting a hopefully useful error. However, virReportError will trim the message to (VIR_ERROR_MAX_LENGTH - 1) characters, which means the end of the log (which likely contains the error message we want to report) may get lost. We should trim the beginning of the log instead. https://bugzilla.redhat.com/show_bug.cgi?id=1335534 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3da297c16f..214f41dd85 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1827,17 +1827,24 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, * qemuProcessReadLog: Read log file of a qemu VM * @logCtxt: the domain log context * @msg: pointer to buffer to store the read messages in + * @max: maximum length of the message returned in @msg * * Reads log of a qemu VM. Skips messages not produced by qemu or irrelevant - * messages. Returns returns 0 on success or -1 on error + * messages. If @max is not zero, @msg will contain at most @max characters + * from the end of the log and @msg will start after a new line if possible. + * + * Returns 0 on success or -1 on error */ static int -qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, char **msg) +qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, + char **msg, + size_t max) { char *buf; ssize_t got; char *eol; char *filter_next; + size_t skip; if ((got = qemuDomainLogContextRead(logCtxt, &buf)) < 0) return -1; @@ -1848,7 +1855,7 @@ qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, char **msg) *eol = '\0'; if (virLogProbablyLogMessage(filter_next) || strstr(filter_next, "char device redirected to")) { - size_t skip = (eol + 1) - filter_next; + skip = (eol + 1) - filter_next; memmove(filter_next, eol + 1, buf + got - eol); got -= skip; } else { @@ -1863,6 +1870,19 @@ qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, char **msg) buf[got - 1] = '\0'; got--; } + + if (max > 0 && got > max) { + skip = got - max; + + if (buf[skip - 1] != '\n' && + (eol = strchr(buf + skip, '\n')) && + !virStringIsEmpty(eol + 1)) + skip = eol + 1 - buf; + + memmove(buf, buf + skip, got - skip + 1); + got -= skip; + } + ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); *msg = buf; return 0; @@ -1874,8 +1894,14 @@ qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt, const char *msgprefix) { char *logmsg = NULL; + size_t max; - if (qemuProcessReadLog(logCtxt, &logmsg) < 0) + max = VIR_ERROR_MAX_LENGTH - 1; + max -= strlen(msgprefix); + /* The length of the formatting string minus two '%s' */ + max -= strlen(_("%s: %s")) - 4; + + if (qemuProcessReadLog(logCtxt, &logmsg, max) < 0) return -1; virResetLastError(); -- 2.15.0
On Wed, Nov 22, 2017 at 01:36:53PM +0100, Jiri Denemark wrote:
Jiri Denemark (3): qemu: Properly skip "char device redirected to" in QEMU log vierror: Define VIR_ERROR_MAX_LENGTH macro qemu: Use the end of QEMU log for reporting errors
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Jiri Denemark -
Pavel Hrdina