From a48539c0139747c07ddb31600d0a3a803f2278be Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 12 Nov 2015 13:54:04 +0000 Subject: [PATCH] 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 --- src/qemu/qemu_domain.c | 12 ----- src/qemu/qemu_domain.h | 2 - src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 47 ++++++++++---------- src/qemu/qemu_monitor.h | 8 +++- src/qemu/qemu_process.c | 94 ++++++++++++++++++++------------------- src/qemu/qemu_process.h | 4 -- 7 files changed, 79 insertions(+), 90 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 14200aedd8..a6a0eedaeb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2410,24 +2410,12 @@ int qemuDomainLogContextGetWriteFD(qemuDomainLogContextPtr ctxt) } -int qemuDomainLogContextGetReadFD(qemuDomainLogContextPtr ctxt) -{ - return ctxt->readfd; -} - - void qemuDomainLogContextMarkPosition(qemuDomainLogContextPtr ctxt) { ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END); } -off_t qemuDomainLogContextGetPosition(qemuDomainLogContextPtr ctxt) -{ - return ctxt->pos; -} - - void qemuDomainLogContextRef(qemuDomainLogContextPtr ctxt) { virAtomicIntInc(&ctxt->refs); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 010647b3f2..14892fd7de 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -366,9 +366,7 @@ int qemuDomainLogContextWrite(qemuDomainLogContextPtr ctxt, ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt, char **msg); int qemuDomainLogContextGetWriteFD(qemuDomainLogContextPtr ctxt); -int qemuDomainLogContextGetReadFD(qemuDomainLogContextPtr ctxt); void qemuDomainLogContextMarkPosition(qemuDomainLogContextPtr ctxt); -off_t qemuDomainLogContextGetPosition(qemuDomainLogContextPtr ctxt); void qemuDomainLogContextRef(qemuDomainLogContextPtr ctxt); void qemuDomainLogContextFree(qemuDomainLogContextPtr ctxt); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5814bf328b..4519aefbec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5969,7 +5969,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, cleanup: virPortAllocatorRelease(driver->migrationPorts, port); if (priv->mon) - qemuMonitorSetDomainLog(priv->mon, -1, -1); + qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); VIR_FREE(priv->origname); virDomainObjEndAPI(&vm); if (mig) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 15c82cf034..bc5ed3363e 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")); @@ -924,7 +922,7 @@ qemuMonitorClose(qemuMonitorPtr mon) PROBE(QEMU_MONITOR_CLOSE, "mon=%p refs=%d", mon, mon->parent.parent.u.s.refs); - qemuMonitorSetDomainLog(mon, -1, -1); + qemuMonitorSetDomainLog(mon, NULL, NULL, NULL); if (mon->fd >= 0) { if (mon->watch) { @@ -3657,21 +3655,22 @@ 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 + * @destroy: optional callback to free @opaque */ -int -qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd, off_t pos) +void +qemuMonitorSetDomainLog(qemuMonitorPtr mon, + qemuMonitorReportDomainLogError func, + void *opaque, + virFreeCallback destroy) { - VIR_FORCE_CLOSE(mon->logfd); - if (logfd >= 0 && - (mon->logfd = dup(logfd)) < 0) { - virReportSystemError(errno, "%s", _("failed to duplicate log fd")); - return -1; - } - mon->logpos = pos; + if (mon->logDestroy && mon->logOpaque) + mon->logDestroy(mon->logOpaque); - return 0; + mon->logFunc = func; + mon->logOpaque = opaque; + mon->logDestroy = destroy; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e9c55d61f8..6be0108ead 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -892,7 +892,13 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon, int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, char ***aliases); -int qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd, off_t pos); +typedef void (*qemuMonitorReportDomainLogError)(qemuMonitorPtr mon, + const char *msg, + void *opaque); +void qemuMonitorSetDomainLog(qemuMonitorPtr mon, + qemuMonitorReportDomainLogError func, + void *opaque, + virFreeCallback destroy); int qemuMonitorGetGuestCPU(qemuMonitorPtr mon, virArch arch, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aa7e99cfe3..192730cb27 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,37 +1644,22 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, /** * qemuProcessReadLog: Read log file of a qemu VM - * @fd: File descriptor of the log file - * @off: Offset to start reading from + * @logCtxt: the domain log context * @msg: pointer to buffer to store the read messages in * * 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) { - VIR_FREE(buf); - 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; @@ -1674,24 +1677,24 @@ qemuProcessReadLog(int fd, off_t offset, char **msg) } filter_next = NULL; /* silence false coverity warning */ - 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; } -int -qemuProcessReportLogError(int logfd, - off_t offset, +static int +qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt, const char *msgprefix) { char *logmsg = NULL; - if (qemuProcessReadLog(logfd, offset, &logmsg) < 0) + if (qemuProcessReadLog(logCtxt, &logmsg) < 0) return -1; virResetLastError(); @@ -1702,6 +1705,16 @@ qemuProcessReportLogError(int logfd, } +static void +qemuProcessMonitorReportLogError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *msg, + void *opaque) +{ + qemuDomainLogContextPtr logCtxt = opaque; + qemuProcessReportLogError(logCtxt, msg); +} + + static int qemuProcessLookupPTYs(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, @@ -1911,16 +1924,9 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, int ret = -1; virHashTablePtr info = NULL; qemuDomainObjPrivatePtr priv; - int logfd = -1; - off_t pos = -1; - - if (logCtxt) { - logfd = qemuDomainLogContextGetReadFD(logCtxt); - pos = qemuDomainLogContextGetPosition(logCtxt); - } VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); - if (qemuConnectMonitor(driver, vm, asyncJob, logfd, pos) < 0) + if (qemuConnectMonitor(driver, vm, asyncJob, logCtxt) < 0) goto cleanup; /* Try to get the pty path mappings again via the monitor. This is much more @@ -1948,8 +1954,8 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, cleanup: virHashFree(info); - if (pos != (off_t)-1 && kill(vm->pid, 0) == -1 && errno == ESRCH) { - qemuProcessReportLogError(logfd, pos, + if (logCtxt && kill(vm->pid, 0) == -1 && errno == ESRCH) { + qemuProcessReportLogError(logCtxt, _("process exited while connecting to monitor")); ret = -1; } @@ -3516,7 +3522,7 @@ qemuProcessReconnect(void *opaque) VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); /* XXX check PID liveliness & EXE path */ - if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, -1, -1) < 0) + if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, NULL) < 0) goto error; /* Failure to connect to agent shouldn't be fatal */ @@ -4826,12 +4832,8 @@ qemuProcessLaunch(virConnectPtr conn, VIR_DEBUG("Waiting for handshake from child"); if (virCommandHandshakeWait(cmd) < 0) { /* Read errors from child that occurred between fork and exec. */ - int logfd = qemuDomainLogContextGetReadFD(logCtxt); - off_t pos = qemuDomainLogContextGetPosition(logCtxt); - if (logfd >= 0) { - qemuProcessReportLogError(logfd, pos, - _("Process exited prior to exec")); - } + qemuProcessReportLogError(logCtxt, + _("Process exited prior to exec")); goto cleanup; } @@ -5116,7 +5118,7 @@ qemuProcessStart(virConnectPtr conn, /* Keep watching qemu log for errors during incoming migration, otherwise * unset reporting errors from qemu log. */ if (!incoming) - qemuMonitorSetDomainLog(priv->mon, -1, -1); + qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); ret = 0; @@ -5131,7 +5133,7 @@ qemuProcessStart(virConnectPtr conn, if (migrateFrom) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; if (priv->mon) - qemuMonitorSetDomainLog(priv->mon, -1, -1); + qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stopFlags); goto cleanup; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index ff0623eb1e..85e3a06817 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -140,10 +140,6 @@ int qemuProcessAutoDestroyRemove(virQEMUDriverPtr driver, bool qemuProcessAutoDestroyActive(virQEMUDriverPtr driver, virDomainObjPtr vm); -int qemuProcessReportLogError(int fd, - off_t offset, - const char *msgprefix); - int qemuProcessSetSchedParams(int id, pid_t pid, size_t nsp, virDomainThreadSchedParamPtr sp); -- GitLab