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: NDaniel P. Berrange <berrange@redhat.com>

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);

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);
 

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) {

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,
+            mon->logFunc(mon,
                          _("early end of file from monitor, "
-                                        "possible problem"));
-            VIR_FORCE_CLOSE(mon->logfd);
+                           "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;
 }
 
 

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,

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,
+        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;
 }

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);