The virt driver, dom name and uuid associated with a log file are important pieces of metadata to keep around for sake of future enhancements to virtlogd. Currently we discard them after opening the log file, but we should preserve them, even across restarts. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/logging/log_handler.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index a4f0395..23c33da 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -30,6 +30,7 @@ #include "virstring.h" #include "virlog.h" #include "virrotatingfile.h" +#include "viruuid.h" #include <unistd.h> #include <fcntl.h> @@ -51,6 +52,10 @@ struct _virLogHandlerLogFile { virRotatingFileWriterPtr file; int watch; int pipefd; /* Read from QEMU via this */ + + char *driver; + unsigned char domuuid[VIR_UUID_BUFLEN]; + char *domname; }; struct _virLogHandler { @@ -93,6 +98,9 @@ virLogHandlerLogFileFree(virLogHandlerLogFilePtr file) if (file->watch != -1) virEventRemoveHandle(file->watch); + + VIR_FREE(file->driver); + VIR_FREE(file->domname); VIR_FREE(file); } @@ -204,6 +212,8 @@ virLogHandlerLogFilePostExecRestart(virLogHandlerPtr handler, { virLogHandlerLogFilePtr file; const char *path; + const char *domuuid; + const char *tmp; if (VIR_ALLOC(file) < 0) return NULL; @@ -216,6 +226,33 @@ virLogHandlerLogFilePostExecRestart(virLogHandlerPtr handler, goto error; } + if ((tmp = virJSONValueObjectGetString(object, "driver")) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing file driver in JSON document")); + goto error; + } + if (VIR_STRDUP(file->driver, tmp) < 0) + goto error; + + if ((tmp = virJSONValueObjectGetString(object, "domname")) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing file domname in JSON document")); + goto error; + } + if (VIR_STRDUP(file->domname, tmp) < 0) + goto error; + + if ((domuuid = virJSONValueObjectGetString(object, "domuuid")) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing file domuuid in JSON document")); + goto error; + } + if (virUUIDParse(domuuid, file->domuuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed file domuuid in JSON document")); + goto error; + } + if ((file->file = virRotatingFileWriterNew(path, DEFAULT_FILE_SIZE, DEFAULT_MAX_BACKUP, @@ -388,6 +425,10 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, file->watch = -1; file->pipefd = pipefd[0]; pipefd[0] = -1; + memcpy(file->domuuid, domuuid, VIR_UUID_BUFLEN); + if (VIR_STRDUP(file->driver, driver) < 0 || + VIR_STRDUP(file->domname, domname) < 0) + goto error; if ((file->file = virRotatingFileWriterNew(path, DEFAULT_FILE_SIZE, @@ -531,6 +572,7 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler) virJSONValuePtr ret = virJSONValueNewObject(); virJSONValuePtr files; size_t i; + char domuuid[VIR_UUID_STRING_BUFLEN]; if (!ret) return NULL; @@ -561,6 +603,18 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler) virRotatingFileWriterGetPath(handler->files[i]->file)) < 0) goto error; + if (virJSONValueObjectAppendString(file, "driver", + handler->files[i]->driver) < 0) + goto error; + + if (virJSONValueObjectAppendString(file, "domname", + handler->files[i]->domname) < 0) + goto error; + + virUUIDFormat(handler->files[i]->domuuid, domuuid); + if (virJSONValueObjectAppendString(file, "domuuid", domuuid) < 0) + goto error; + if (virSetInherit(handler->files[i]->pipefd, true) < 0) { virReportSystemError(errno, "%s", _("Cannot disable close-on-exec flag")); -- 2.5.0

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(a)redhat.com&gt; --- src/logging/log_daemon_dispatch.c | 9 ++--- src/logging/log_handler.c | 70 +++------------------------------------ src/logging/log_handler.h | 9 ++--- src/logging/log_manager.c | 18 ++++------ src/logging/log_manager.h | 9 ++--- src/logging/log_protocol.x | 7 ++-- src/qemu/qemu_domain.c | 43 +++++++++--------------- 7 files changed, 39 insertions(+), 126 deletions(-) diff --git a/src/logging/log_daemon_dispatch.c b/src/logging/log_daemon_dispatch.c index 160ab00..da7c414 100644 --- 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 23c33da..b24826c 100644 --- 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 ATTRIBUTE_UNUSED, 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 e41ac7f..6a13072 100644 --- 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 7a4c7e2..290e2c8 100644 --- 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 3dfe516..d3b9d29 100644 --- 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 de57c69..a07334f 100644 --- 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 4d6c595..438f7a1 100644 --- 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); -- 2.5.0