logging: change log protocol to be more reusable

The current virtlogd RPC protocol provides the ability to
handle log files associated with QEMU stdout/err. The log
protocol messages take the virt driver, domain name and
use that to form a log file path. This is quite restrictive
as it prevents us re-using the same RPC protocol messages
for logging to char device backends where the filename
can be arbitrarily user specified. It is also bad because
it means we have 2 separate locations which have to decide
on logfile name.

This change alters the RPC protocol so that we pass the
desired log file path along when opening the log file
initially. Now the virt driver is exclusively in charge
of deciding the log filename

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/src/logging/log_daemon_dispatch.c b/src/logging/log_daemon_dispatch.c
index 160ab00a4bc8a349b0d3641278ef6301943b6c56..da7c41463283c4e294a8a312e488924952c9dd39 100644 (file)
--- a/src/logging/log_daemon_dispatch.c
+++ b/src/logging/log_daemon_dispatch.c
@@ -55,6 +55,7 @@ virLogManagerProtocolDispatchDomainOpenLogFile(virNetServerPtr server ATTRIBUTE_
                                              args->driver,
                                              (unsigned char *)args->dom.uuid,
                                              args->dom.name,
+                                             args->path,
                                              &inode, &offset)) < 0)
         goto cleanup;
 
@@ -87,9 +88,7 @@ virLogManagerProtocolDispatchDomainGetLogFilePosition(virNetServerPtr server ATT
     ino_t inode;
 
     if (virLogHandlerDomainGetLogFilePosition(virLogDaemonGetHandler(logDaemon),
-                                              args->driver,
-                                              (unsigned char *)args->dom.uuid,
-                                              args->dom.name,
+                                              args->path,
                                               &inode, &offset) < 0)
         goto cleanup;
 
@@ -125,9 +124,7 @@ virLogManagerProtocolDispatchDomainReadLogFile(virNetServerPtr server ATTRIBUTE_
     }
 
     if ((data = virLogHandlerDomainReadLogFile(virLogDaemonGetHandler(logDaemon),
-                                               args->driver,
-                                               (unsigned char *)args->dom.uuid,
-                                               args->dom.name,
+                                               args->path,
                                                args->pos.inode,
                                                args->pos.offset,
                                                args->maxlen)) == NULL)

diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index c9a602ddb4baaae9ae9f028583a85d2f9431356e..55da48ed46a8f887c7595fc339686f8987ca9ad9 100644 (file)
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -351,59 +351,23 @@ virLogHandlerDispose(void *obj)
 }
 
 
-static char *
-virLogHandlerGetLogFilePathForDomain(virLogHandlerPtr handler,
-                                     const char *driver,
-                                     const unsigned char *domuuid ATTRIBUTE_UNUSED,
-                                     const char *domname)
-{
-    char *path;
-    if (handler->privileged) {
-        if (virAsprintf(&path,
-                        LOCALSTATEDIR "/log/libvirt/%s/%s.log",
-                        driver, domname) < 0)
-            return NULL;
-    } else {
-        char *cachedir;
-
-        cachedir = virGetUserCacheDirectory();
-        if (!cachedir)
-            return NULL;
-
-        if (virAsprintf(&path,
-                        "%s/%s/log/%s.log", cachedir, driver, domname) < 0) {
-            VIR_FREE(cachedir);
-            return NULL;
-        }
-
-    }
-    return path;
-}
-
-
 int
 virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler,
                                const char *driver,
                                const unsigned char *domuuid,
                                const char *domname,
+                               const char *path,
                                ino_t *inode,
                                off_t *offset)
 {
     size_t i;
     virLogHandlerLogFilePtr file = NULL;
     int pipefd[2] = { -1, -1 };
-    char *path;
 
     virObjectLock(handler);
 
     handler->inhibitor(true, handler->opaque);
 
-    if (!(path = virLogHandlerGetLogFilePathForDomain(handler,
-                                                      driver,
-                                                      domuuid,
-                                                      domname)))
-        goto error;
-
     for (i = 0; i < handler->nfiles; i++) {
         if (STREQ(virRotatingFileWriterGetPath(handler->files[i]->file),
                   path)) {
@@ -449,8 +413,6 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler,
         goto error;
     }
 
-    VIR_FREE(path);
-
     *inode = virRotatingFileWriterGetINode(file->file);
     *offset = virRotatingFileWriterGetOffset(file->file);
 
@@ -458,7 +420,6 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler,
     return pipefd[1];
 
  error:
-    VIR_FREE(path);
     VIR_FORCE_CLOSE(pipefd[0]);
     VIR_FORCE_CLOSE(pipefd[1]);
     handler->inhibitor(false, handler->opaque);
@@ -470,25 +431,16 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler,
 
 int
 virLogHandlerDomainGetLogFilePosition(virLogHandlerPtr handler,
-                                      const char *driver,
-                                      const unsigned char *domuuid,
-                                      const char *domname,
+                                      const char *path,
                                       ino_t *inode,
                                       off_t *offset)
 {
-    char *path;
     virLogHandlerLogFilePtr file = NULL;
     int ret = -1;
     size_t i;
 
     virObjectLock(handler);
 
-    if (!(path = virLogHandlerGetLogFilePathForDomain(handler,
-                                                      driver,
-                                                      domuuid,
-                                                      domname)))
-        goto cleanup;
-
     for (i = 0; i < handler->nfiles; i++) {
         if (STREQ(virRotatingFileWriterGetPath(handler->files[i]->file),
                   path)) {
@@ -499,8 +451,8 @@ virLogHandlerDomainGetLogFilePosition(virLogHandlerPtr handler,
 
     if (!file) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("No open log file for domain %s"),
-                       domname);
+                       _("No open log file %s"),
+                       path);
         goto cleanup;
     }
 
@@ -510,7 +462,6 @@ virLogHandlerDomainGetLogFilePosition(virLogHandlerPtr handler,
     ret = 0;
 
  cleanup:
-    VIR_FREE(path);
     virObjectUnlock(handler);
     return ret;
 }
@@ -518,26 +469,17 @@ virLogHandlerDomainGetLogFilePosition(virLogHandlerPtr handler,
 
 char *
 virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
-                               const char *driver,
-                               const unsigned char *domuuid,
-                               const char *domname,
+                               const char *path,
                                ino_t inode,
                                off_t offset,
                                size_t maxlen)
 {
-    char *path;
     virRotatingFileReaderPtr file = NULL;
     char *data = NULL;
     ssize_t got;
 
     virObjectLock(handler);
 
-    if (!(path = virLogHandlerGetLogFilePathForDomain(handler,
-                                                      driver,
-                                                      domuuid,
-                                                      domname)))
-        goto error;
-
     if (!(file = virRotatingFileReaderNew(path, DEFAULT_MAX_BACKUP)))
         goto error;
 
@@ -554,11 +496,9 @@ virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
 
     virRotatingFileReaderFree(file);
     virObjectUnlock(handler);
-    VIR_FREE(path);
     return data;
 
  error:
-    VIR_FREE(path);
     VIR_FREE(data);
     virRotatingFileReaderFree(file);
     virObjectUnlock(handler);

diff --git a/src/logging/log_handler.h b/src/logging/log_handler.h
index e41ac7fa068a4f13523bb680223a8c40171e0471..6a130722129e4887c19277fdad497bd1f0c6db69 100644 (file)
--- a/src/logging/log_handler.h
+++ b/src/logging/log_handler.h
@@ -47,20 +47,17 @@ int virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler,
                                    const char *driver,
                                    const unsigned char *domuuid,
                                    const char *domname,
+                                   const char *path,
                                    ino_t *inode,
                                    off_t *offset);
 
 int virLogHandlerDomainGetLogFilePosition(virLogHandlerPtr handler,
-                                          const char *driver,
-                                          const unsigned char *domuuid,
-                                          const char *domname,
+                                          const char *path,
                                           ino_t *inode,
                                           off_t *offset);
 
 char *virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
-                                     const char *driver,
-                                     const unsigned char *domuuid,
-                                     const char *domname,
+                                     const char *path,
                                      ino_t inode,
                                      off_t offset,
                                      size_t maxlen);

diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index 7a4c7e2b79ec6b85f216bd42cafff6afeb26b46e..290e2c814646bf5874f62647923d07843ea7b163 100644 (file)
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -156,6 +156,7 @@ virLogManagerDomainOpenLogFile(virLogManagerPtr mgr,
                                const char *driver,
                                const unsigned char *domuuid,
                                const char *domname,
+                               const char *path,
                                unsigned int flags,
                                ino_t *inode,
                                off_t *offset)
@@ -172,6 +173,7 @@ virLogManagerDomainOpenLogFile(virLogManagerPtr mgr,
     args.driver = (char *)driver;
     memcpy(args.dom.uuid, domuuid, VIR_UUID_BUFLEN);
     args.dom.name = (char *)domname;
+    args.path = (char *)path;
     args.flags = flags;
 
     if (virNetClientProgramCall(mgr->program,
@@ -211,9 +213,7 @@ virLogManagerDomainOpenLogFile(virLogManagerPtr mgr,
 
 int
 virLogManagerDomainGetLogFilePosition(virLogManagerPtr mgr,
-                                      const char *driver,
-                                      const unsigned char *domuuid,
-                                      const char *domname,
+                                      const char *path,
                                       unsigned int flags,
                                       ino_t *inode,
                                       off_t *offset)
@@ -225,9 +225,7 @@ virLogManagerDomainGetLogFilePosition(virLogManagerPtr mgr,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    args.driver = (char *)driver;
-    memcpy(args.dom.uuid, domuuid, VIR_UUID_BUFLEN);
-    args.dom.name = (char *)domname;
+    args.path = (char *)path;
     args.flags = flags;
 
     if (virNetClientProgramCall(mgr->program,
@@ -250,9 +248,7 @@ virLogManagerDomainGetLogFilePosition(virLogManagerPtr mgr,
 
 char *
 virLogManagerDomainReadLogFile(virLogManagerPtr mgr,
-                               const char *driver,
-                               const unsigned char *domuuid,
-                               const char *domname,
+                               const char *path,
                                ino_t inode,
                                off_t offset,
                                size_t maxlen,
@@ -267,9 +263,7 @@ virLogManagerDomainReadLogFile(virLogManagerPtr mgr,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    args.driver = (char *)driver;
-    memcpy(args.dom.uuid, domuuid, VIR_UUID_BUFLEN);
-    args.dom.name = (char *)domname;
+    args.path = (char *)path;
     args.flags = flags;
     args.pos.inode = inode;
     args.pos.offset = offset;

diff --git a/src/logging/log_manager.h b/src/logging/log_manager.h
index 3dfe516faed097cb5f02d8ac140d614f8bbc7d3f..d3b9d29e10c20b32e60556474031438729989096 100644 (file)
--- a/src/logging/log_manager.h
+++ b/src/logging/log_manager.h
@@ -37,22 +37,19 @@ int virLogManagerDomainOpenLogFile(virLogManagerPtr mgr,
                                    const char *driver,
                                    const unsigned char *domuuid,
                                    const char *domname,
+                                   const char *path,
                                    unsigned int flags,
                                    ino_t *inode,
                                    off_t *offset);
 
 int virLogManagerDomainGetLogFilePosition(virLogManagerPtr mgr,
-                                          const char *driver,
-                                          const unsigned char *domuuid,
-                                          const char *domname,
+                                          const char *path,
                                           unsigned int flags,
                                           ino_t *inode,
                                           off_t *offset);
 
 char *virLogManagerDomainReadLogFile(virLogManagerPtr mgr,
-                                     const char *driver,
-                                     const unsigned char *domuuid,
-                                     const char *domname,
+                                     const char *path,
                                      ino_t inode,
                                      off_t offset,
                                      size_t maxlen,

diff --git a/src/logging/log_protocol.x b/src/logging/log_protocol.x
index de57c69dfabc73b11b3a6aa09acac37bcc27066f..a07334ff8638a58445da2513bc3ca6fcfb6f82ca 100644 (file)
--- a/src/logging/log_protocol.x
+++ b/src/logging/log_protocol.x
@@ -35,6 +35,7 @@ typedef struct virLogManagerProtocolLogFilePosition virLogManagerProtocolLogFile
 struct virLogManagerProtocolDomainOpenLogFileArgs {
     virLogManagerProtocolNonNullString driver;
     virLogManagerProtocolDomain dom;
+    virLogManagerProtocolNonNullString path;
     unsigned int flags;
 };
 
@@ -43,8 +44,7 @@ struct virLogManagerProtocolDomainOpenLogFileRet {
 };
 
 struct virLogManagerProtocolDomainGetLogFilePositionArgs {
-    virLogManagerProtocolNonNullString driver;
-    virLogManagerProtocolDomain dom;
+    virLogManagerProtocolNonNullString path;
     unsigned int flags;
 };
 
@@ -53,8 +53,7 @@ struct virLogManagerProtocolDomainGetLogFilePositionRet {
 };
 
 struct virLogManagerProtocolDomainReadLogFileArgs {
-    virLogManagerProtocolNonNullString driver;
-    virLogManagerProtocolDomain dom;
+    virLogManagerProtocolNonNullString path;
     virLogManagerProtocolLogFilePosition pos;
     unsigned hyper maxlen;
     unsigned int flags;

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d60cf73a2915a8412ca4ab62b6123808b9eddc2e..40e1f18110ea08146cdc36bf5174310a0e846843 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -85,8 +85,7 @@ struct _qemuDomainLogContext {
     int readfd; /* Only used if manager == NULL */
     off_t pos;
     ino_t inode; /* Only used if manager != NULL */
-    unsigned char uuid[VIR_UUID_BUFLEN]; /* Only used if manager != NULL */
-    char *name; /* Only used if manager != NULL */
+    char *path;
     virLogManagerPtr manager;
 };
 
@@ -2285,7 +2284,6 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     qemuDomainLogContextPtr ctxt = NULL;
-    char *logfile = NULL;
 
     if (VIR_ALLOC(ctxt) < 0)
         goto error;
@@ -2295,37 +2293,33 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
     ctxt->readfd = -1;
     virAtomicIntSet(&ctxt->refs, 1);
 
+    if (virAsprintf(&ctxt->path, "%s/%s.log", cfg->logDir, vm->def->name) < 0)
+        goto error;
+
     if (cfg->stdioLogD) {
         ctxt->manager = virLogManagerNew(virQEMUDriverIsPrivileged(driver));
         if (!ctxt->manager)
             goto error;
 
-        if (VIR_STRDUP(ctxt->name, vm->def->name) < 0)
-            goto error;
-
-        memcpy(ctxt->uuid, vm->def->uuid, VIR_UUID_BUFLEN);
-
         ctxt->writefd = virLogManagerDomainOpenLogFile(ctxt->manager,
                                                        "qemu",
                                                        vm->def->uuid,
                                                        vm->def->name,
+                                                       ctxt->path,
                                                        0,
                                                        &ctxt->inode,
                                                        &ctxt->pos);
         if (ctxt->writefd < 0)
             goto error;
     } else {
-        if (virAsprintf(&logfile, "%s/%s.log", cfg->logDir, vm->def->name) < 0)
-            goto error;
-
-        if ((ctxt->writefd = open(logfile, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
+        if ((ctxt->writefd = open(ctxt->path, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) {
             virReportSystemError(errno, _("failed to create logfile %s"),
-                                 logfile);
+                                 ctxt->path);
             goto error;
         }
         if (virSetCloseExec(ctxt->writefd) < 0) {
             virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
-                                 logfile);
+                                 ctxt->path);
             goto error;
         }
 
@@ -2336,33 +2330,32 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
             !virQEMUDriverIsPrivileged(driver) &&
             ftruncate(ctxt->writefd, 0) < 0) {
             virReportSystemError(errno, _("failed to truncate %s"),
-                                 logfile);
+                                 ctxt->path);
             goto error;
         }
 
         if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) {
-            if ((ctxt->readfd = open(logfile, O_RDONLY, S_IRUSR | S_IWUSR)) < 0) {
+            if ((ctxt->readfd = open(ctxt->path, O_RDONLY, S_IRUSR | S_IWUSR)) < 0) {
                 virReportSystemError(errno, _("failed to open logfile %s"),
-                                     logfile);
+                                     ctxt->path);
                 goto error;
             }
             if (virSetCloseExec(ctxt->readfd) < 0) {
                 virReportSystemError(errno, _("failed to set close-on-exec flag on %s"),
-                                     logfile);
+                                     ctxt->path);
                 goto error;
             }
         }
 
         if ((ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END)) < 0) {
             virReportSystemError(errno, _("failed to seek in log file %s"),
-                                 logfile);
+                                 ctxt->path);
             goto error;
         }
     }
 
  cleanup:
     virObjectUnref(cfg);
-    VIR_FREE(logfile);
     return ctxt;
 
  error:
@@ -2415,9 +2408,7 @@ ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt,
     size_t buflen;
     if (ctxt->manager) {
         buf = virLogManagerDomainReadLogFile(ctxt->manager,
-                                             "qemu",
-                                             ctxt->uuid,
-                                             ctxt->name,
+                                             ctxt->path,
                                              ctxt->inode,
                                              ctxt->pos,
                                              1024 * 128,
@@ -2466,9 +2457,7 @@ void qemuDomainLogContextMarkPosition(qemuDomainLogContextPtr ctxt)
 {
     if (ctxt->manager)
         virLogManagerDomainGetLogFilePosition(ctxt->manager,
-                                              "qemu",
-                                              ctxt->uuid,
-                                              ctxt->name,
+                                              ctxt->path,
                                               0,
                                               &ctxt->inode,
                                               &ctxt->pos);
@@ -2497,7 +2486,7 @@ void qemuDomainLogContextFree(qemuDomainLogContextPtr ctxt)
         return;
 
     virLogManagerFree(ctxt->manager);
-    VIR_FREE(ctxt->name);
+    VIR_FREE(ctxt->path);
     VIR_FORCE_CLOSE(ctxt->writefd);
     VIR_FORCE_CLOSE(ctxt->readfd);
     VIR_FREE(ctxt);