Re: [libvirt] [PATCH v2 06/13] qemu: unify code for reporting errors from QEMU log files

There are two pretty similar functions qemuProcessReadLog and qemuProcessReadChildErrors. Both read from the QEMU log file and try to strip out libvirt messages. The latter than reports

an error, while the former lets the callers report an error. Re-write qemuProcessReadLog so that it uses a single read into a dynamically allocated buffer. Then introduce a new qemuProcessReportLogError that calls qemuProcessReadLog and reports an error. Convert all callers to use qemuProcessReportLogError. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 24 +----- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 58 +++----------- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_process.c | 200 ++++++++++++++++++---------------------------- src/qemu/qemu_process.h | 4 +- 7 files changed, 95 insertions(+), 197 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 69a0f97..c09e9dc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1549,7 +1549,7 @@ static qemuMonitorCallbacks monitorCallbacks = { static int qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, - int logfd) + int logfd, off_t pos) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; @@ -1575,8 +1575,8 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, &monitorCallbacks, driver); - if (mon) - ignore_value(qemuMonitorSetDomainLog(mon, logfd)); + if (mon && logfd != -1 && pos != -1) + ignore_value(qemuMonitorSetDomainLog(mon, logfd, pos)); virObjectLock(vm); virObjectUnref(vm); @@ -1630,111 +1630,76 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, /** * qemuProcessReadLog: Read log file of a qemu VM * @fd: File descriptor of the log file - * @buf: buffer to store the read messages - * @buflen: allocated space available in @buf + * @msg: pointer to buffer to store the read messages in

* @off: Offset to start reading from - * @skipchar: Skip messages about created character devices * * Reads log of a qemu VM. Skips messages not produced by qemu or irrelevant - * messages. Returns length of the message stored in @buf, or -1 on error. + * messages. Returns returns 0 on success or -1 on error */ -int -qemuProcessReadLog(int fd, char *buf, int buflen, int off, bool skipchar) +static int +qemuProcessReadLog(int fd, off_t offset, char **msg) { - char *filter_next = buf; - ssize_t bytes; + char *buf; + size_t buflen = 1024 * 128; + ssize_t got; char *eol; + char *filter_next; - while (off < buflen - 1) { - bytes = saferead(fd, buf + off, buflen - off - 1); - if (bytes < 0) - return -1; - - off += bytes; - buf[off] = '\0'; + /* Best effort jump to start of messages */ + ignore_value(lseek(fd, offset, SEEK_SET)); - if (bytes == 0) - break; + if (VIR_ALLOC_N(buf, buflen) < 0) + return -1; - /* Filter out debug messages from intermediate libvirt process */ - while ((eol = strchr(filter_next, '\n'))) { - *eol = '\0'; - if (virLogProbablyLogMessage(filter_next) || - (skipchar && - STRPREFIX(filter_next, "char device redirected to"))) { - memmove(filter_next, eol + 1, off - (eol - buf)); - off -= eol + 1 - filter_next; - } else { - filter_next = eol + 1; - *eol = '\n'; - } - } + got = saferead(fd, buf, buflen - 1); + if (got < 0) { + virReportSystemError(errno, "%s", + _("Unable to read from log file"));

+ return -1; }

- return off; -} - -/* - * Read domain log and probably overwrite error if there's one in - * the domain log file. This function exists to cover the small - * window between fork() and exec() during which child may fail - * by libvirt's hand, e.g. placing onto a NUMA node failed. - */ -static int -qemuProcessReadChildErrors(virQEMUDriverPtr driver, - virDomainObjPtr vm, - off_t originalOff) -{ - int ret = -1; - int logfd; - off_t off = 0; - ssize_t bytes; - char buf[1024] = {0}; - char *eol, *filter_next = buf; - - if ((logfd = qemuDomainOpenLog(driver, vm, originalOff)) < 0) - goto cleanup; - - while (off < sizeof(buf) - 1) { - bytes = saferead(logfd, buf + off, sizeof(buf) - off - 1); - if (bytes < 0) { - VIR_WARN("unable to read from log file: %s", - virStrerror(errno, buf, sizeof(buf))); - goto cleanup; - } + buf[got] = '\0'; - off += bytes; - buf[off] = '\0'; - - if (bytes == 0) - break; - - while ((eol = strchr(filter_next, '\n'))) { - *eol = '\0'; - if (STRPREFIX(filter_next, "libvirt: ")) { - filter_next = eol + 1; - *eol = '\n'; - break; - } else { - memmove(filter_next, eol + 1, off - (eol - buf)); - off -= eol + 1 - filter_next; - } + /* Filter out debug messages from intermediate libvirt process */ + filter_next = buf; + while ((eol = strchr(filter_next, '\n'))) { + *eol = '\0'; + if (virLogProbablyLogMessage(filter_next) || + STRPREFIX(filter_next, "char device redirected to")) { + size_t skip = (eol + 1) - filter_next; + memmove(filter_next, eol + 1, (got - skip) + 1); + got -= skip; + } else { + filter_next = eol + 1; + *eol = '\n'; } }

- if (off > 0) { - /* Found an error in the log. Report it */ - virResetLastError(); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Process exited prior to exec: %s"), - buf); + if (buf[got - 1] == '\n') { + buf[got - 1] = '\0'; + got--; } + VIR_SHRINK_N(buf, buflen, buflen - got - 1); + *msg = buf; + return 0; +} - ret = 0; - cleanup: - VIR_FORCE_CLOSE(logfd); - return ret;