Re: [libvirt] [PATCH v2 12/13] qemu: convert monitor to use qemuDomainLogContextPtr indirectly

Currently the QEMU monitor is given an FD to the logfile. This won't work in the future with virtlogd, so it needs to use the qemuDomainLogContextPtr instead, but it shouldn't directly access that object either. So define a callback that the monitor can use for reporting errors from the log file. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 12 ------ src/qemu/qemu_domain.h | 2 - src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 51 ++++++++++++++------------ src/qemu/qemu_monitor.h | 8 +++- src/qemu/qemu_process.c | 93 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.h | 4 -- 7 files changed, 84 insertions(+), 88 deletions(-)

index d3f0c09..bd5d9ca 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -94,9 +94,10 @@ struct _qemuMonitor { char *balloonpath; bool ballooninit; - /* Log file fd of the qemu process to dig for usable info */ - int logfd; - off_t logpos; + /* Log file context of the qemu process to dig for usable info */ + qemuMonitorReportDomainLogError logFunc; + void *logOpaque; + virFreeCallback logDestroy; }; /** @@ -315,7 +316,6 @@ qemuMonitorDispose(void *obj) VIR_FREE(mon->buffer); virJSONValueFree(mon->options); VIR_FREE(mon->balloonpath); - VIR_FORCE_CLOSE(mon->logfd); } @@ -706,18 +706,17 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) } if (error || eof) { - if (hangup && mon->logfd != -1) { + if (hangup && mon->logFunc != NULL) { /* Check if an error message from qemu is available and if so, use * it to overwrite the actual message. It's done only in early * startup phases or during incoming migration when the message * from qemu is certainly more interesting than a * "connection reset by peer" message. */ - qemuProcessReportLogError(mon->logfd, - mon->logpos, - _("early end of file from monitor, " - "possible problem")); - VIR_FORCE_CLOSE(mon->logfd); + mon->logFunc(mon, + _("early end of file from monitor, " + "possible problem"), + mon->logOpaque);

virCopyLastError(&mon->lastError); virResetLastError(); } @@ -802,7 +801,6 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, if (!(mon = virObjectLockableNew(qemuMonitorClass))) return NULL; - mon->logfd = -1; if (virCondInit(&mon->notify) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize monitor condition")); @@ -932,6 +930,13 @@ qemuMonitorClose(qemuMonitorPtr mon) VIR_FORCE_CLOSE(mon->fd); } + if (mon->logDestroy && mon->logOpaque) { + mon->logDestroy(mon->logOpaque); + mon->logOpaque = NULL; + mon->logDestroy = NULL; + mon->logFunc = NULL; + } + /* In case another thread is waiting for its monitor command to be * processed, we need to wake it up with appropriate error set. */ @@ -3646,21 +3651,21 @@ qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, * early startup errors of qemu. * * @mon: Monitor object to set the log file reading on - * @logfd: File descriptor of the already open log file - * @pos: position to read errors from + * @func: the callback to report errors + * @opaque: data to pass to @func

*/ -int -qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd, off_t pos) +void +qemuMonitorSetDomainLog(qemuMonitorPtr mon, + qemuMonitorReportDomainLogError func, + void *opaque, + virFreeCallback destroy)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 132b3eb..4a2e2c1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1544,9 +1544,22 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainMigrationStatus = qemuProcessHandleMigrationStatus, }; +static void +qemuProcessMonitorReportLogError(qemuMonitorPtr mon, + const char *msg, + void *opaque); + + +static void +qemuProcessMonitorLogFree(void *opaque) +{ + qemuDomainLogContextPtr logCtxt = opaque;

+ qemuDomainLogContextFree(logCtxt); +} + static int qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, - int logfd, off_t pos) + qemuDomainLogContextPtr logCtxt) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; @@ -1572,8 +1585,13 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, &monitorCallbacks, driver); - if (mon && logfd != -1 && pos != -1) - ignore_value(qemuMonitorSetDomainLog(mon, logfd, pos)); + if (mon && logCtxt) { + qemuDomainLogContextRef(logCtxt); + qemuMonitorSetDomainLog(mon, + qemuProcessMonitorReportLogError, + logCtxt, + qemuProcessMonitorLogFree); + } virObjectLock(vm); virObjectUnref(vm); @@ -1626,36 +1644,22 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, /** * qemuProcessReadLog: Read log file of a qemu VM - * @fd: File descriptor of the log file + * @logCtxt: the domain log context * @msg: pointer to buffer to store the read messages in - * @off: Offset to start reading from * * Reads log of a qemu VM. Skips messages not produced by qemu or irrelevant * messages. Returns returns 0 on success or -1 on error */ static int -qemuProcessReadLog(int fd, off_t offset, char **msg) +qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, char **msg) { char *buf; - size_t buflen = 1024 * 128; ssize_t got; char *eol; char *filter_next; - /* Best effort jump to start of messages */ - ignore_value(lseek(fd, offset, SEEK_SET)); - - if (VIR_ALLOC_N(buf, buflen) < 0) - return -1; - - got = saferead(fd, buf, buflen - 1); - if (got < 0) { - virReportSystemError(errno, "%s", - _("Unable to read from log file")); + if ((got = qemuDomainLogContextRead(logCtxt, &buf)) < 0) return -1; - } - - buf[got] = '\0'; /* Filter out debug messages from intermediate libvirt process */ filter_next = buf; @@ -1672,24 +1676,24 @@ qemuProcessReadLog(int fd, off_t offset, char **msg) } } - if (buf[got - 1] == '\n') { + if (got > 0 && + buf[got - 1] == '\n') { buf[got - 1] = '\0'; got--; } - VIR_SHRINK_N(buf, buflen, buflen - got - 1); + ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); *msg = buf;

return 0; }