11:19 a.m.
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>
---
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(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 75f78fe..2417cb7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2378,24 +2378,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 bc84e99..ac49377 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -365,9 +365,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 475f97d..3a1c068 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5849,7 +5849,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 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)
{
- 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 a2a9a77..29e211f 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 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;
}
-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();
@@ -1700,6 +1704,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,
@@ -1909,16 +1923,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
@@ -1946,8 +1953,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;
}
@@ -3498,7 +3505,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 */
@@ -4606,12 +4613,8 @@ int qemuProcessStart(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 error;
}
@@ -4834,7 +4837,7 @@ int qemuProcessStart(virConnectPtr conn,
/* Keep watching qemu log for errors during incoming migration, otherwise
* unset reporting errors from qemu log. */
if (!migrateFrom)
- qemuMonitorSetDomainLog(priv->mon, -1, -1);
+ qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
ret = 0;
@@ -4853,7 +4856,7 @@ int qemuProcessStart(virConnectPtr conn,
* if we failed to initialize the now running VM. kill it off and
* pretend we never started it */
if (priv->mon)
- qemuMonitorSetDomainLog(priv->mon, -1, -1);
+ qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
/* Must close log now to allow ProcessSto to re-open it */
qemuDomainLogContextFree(logCtxt);
logCtxt = NULL;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 2df5c4d..640c498 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -100,10 +100,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);
--
2.5.0