diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 14200aedd82f133fddeac8dd948f1daca4d44735..a6a0eedaeb254208fc2c670b07aa7d883c9b186c 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 010647b3f27e39a8d68ad1318793c3d5a7cd1b96..14892fd7dea2923f18c200ebf930b214507f3b30 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 5814bf328ba3105d41505e64a9405ee3cda31e91..4519aefbeca5c2968d725165c38e267837f83d86 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 15c82cf0341328e5d5ab398963573b959e77a60f..bc5ed3363e75d5ed169c10f626bb56853e74af26 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 e9c55d61f8a886741e1b2ccac87d2572b443ba2c..6be0108ead8d086722be50796f11d23ed50bbde2 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 aa7e99cfe3fd3b5e9ad30238415395470741d93a..192730cb276fce72063c62bfe1071386bde552bc 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 ff0623eb1eee57afbb32b18af132f2f798bd0c7d..85e3a068171fc46616388c235f066ebcfe992fcc 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);