On Mon, Sep 12, 2016 at 02:12:10PM +0300, Nikolay Shirokovskiy wrote:
Read API call of logger daemon is used to get extra information from terminated qemu process. For this purpose we need to wait until qemu process finishes its writing to log. We need to:
1. Read until EOF. 2. Don't stop reading on hangup.
virtlogd will not receive EOF as both qemu and libvirtd keep logging pipe descriptor. Thus let's close it after qemu process startup is finished as it will not be used anymore. All following logging on behalf of qemu by libvirt is done either thru daemon API or by receiving new writing descriptor.
Beware, qemuDomainLogContextFree is unref actually. --- src/logging/log_handler.c | 39 +++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_domain.c | 4 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 2 ++ 4 files changed, 44 insertions(+), 2 deletions(-)
Any virtlogd changes should really be in separate commits from any QEMU changes. virtlogd is intended to be a reusable component of libvirt for multiple use cases, so any changes to it need to described & justifiable independently of any QEMU changes.
+static virLogHandlerLogFilePtr +virLogHandlerGetLogFileFromPath(virLogHandlerPtr handler, + const char* path) +{ + size_t i; + + for (i = 0; i < handler->nfiles; i++) { + if (STREQ(handler->files[i]->path, path)) + return handler->files[i]; + } + + return NULL; +}
@@ -167,11 +185,14 @@ virLogHandlerDomainLogFileEvent(int watch, goto error; }
+ if (len == 0) + goto error; + if (virRotatingFileWriterAppend(logfile->file, buf, len) != len) goto error;
if (events & VIR_EVENT_HANDLE_HANGUP) - goto error; + goto reread;
This patch hunk seems to be indendant of the other changes in this file. It is a straightfoward bugfix to ensure we full read all data before handling the hangup. So it should be a separate commit too.
virObjectUnlock(handler); return; @@ -179,6 +200,7 @@ virLogHandlerDomainLogFileEvent(int watch, error: handler->inhibitor(false, handler->opaque); virLogHandlerLogFileClose(handler, logfile); + virCondBroadcast(&handler->condition); virObjectUnlock(handler); }
@@ -198,6 +220,9 @@ virLogHandlerNew(bool privileged, if (!(handler = virObjectLockableNew(virLogHandlerClass))) goto error;
+ if (virCondInit(&handler->condition) < 0) + goto error; + handler->privileged = privileged; handler->max_size = max_size; handler->max_backups = max_backups; @@ -357,6 +382,7 @@ virLogHandlerDispose(void *obj) virLogHandlerLogFileFree(handler->files[i]); } VIR_FREE(handler->files); + virCondDestroy(&handler->condition); }
@@ -401,7 +427,8 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, pipefd[0] = -1; memcpy(file->domuuid, domuuid, VIR_UUID_BUFLEN); if (VIR_STRDUP(file->driver, driver) < 0 || - VIR_STRDUP(file->domname, domname) < 0) + VIR_STRDUP(file->domname, domname) < 0 || + VIR_STRDUP(file->path, path) < 0) goto error;
if ((file->file = virRotatingFileWriterNew(path, @@ -496,6 +523,14 @@ virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
virObjectLock(handler);
+ while (virLogHandlerGetLogFileFromPath(handler, path)) { + if (virCondWait(&handler->condition, &handler->parent.lock) < 0) { + virReportSystemError(errno, "%s", + _("failed to wait for EOF")); + goto error; + } + }
Ewww no - this code is not only intended for use from codepath where QEMU is shutdown. It absolutely should be able to read from a log where QEMU is still running. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|