Re: [libvirt] [[PATCH v2] 3/4] virtlogd: add flag to wait for log end on read
On 05.10.2016 18:08, Peter Krempa wrote:
On Mon, Sep 12, 2016 at 17:34:42 +0300, Nikolay Shirokovskiy wrote:
This is a pretty big change but you did not write anything to describe
or justify it.
> ---
> src/logging/log_handler.c | 38 ++++++++++++++++++++++++++++++++++++--
> src/logging/log_protocol.x | 5 +++++
> 2 files changed, 41 insertions(+), 2 deletions(-)
[...]
> @@ -492,10 +517,19 @@ virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
> char *data = NULL;
> ssize_t got;
>
> - virCheckFlags(0, NULL);
> + virCheckFlags(VIR_LOG_MANAGER_PROTOCOL_DOMAIN_READ_LOG_FILE_WAIT, NULL);
>
> virObjectLock(handler);
>
> + if (flags & VIR_LOG_MANAGER_PROTOCOL_DOMAIN_READ_LOG_FILE_WAIT) {
> + while (virLogHandlerGetLogFileFromPath(handler, path)) {
> + if (virCondWait(&handler->condition,
&handler->parent.lock) < 0) {
> + virReportSystemError(errno, "%s", _("failed to wait
for EOF"));
> + goto error;
> + }
> + }
E.g why do you need this? The qemu process is dead at the point when
libvirt attempts to read the log file so I don't see a reason to wait
here.
Peter
When we read log it can be written partially at that moment. Qemu is dead but
there is a chance the pipe that connects the process and the log daemon is
not drained. Thus if we want read everything the qemu process has written
we need to wait for EOF. Log file handler for path of interest
will disappear in case of EOF.
By the way patch can be simplified. No need to store path in _virLogHandlerLogFile
because it's child store this path already.
Should I send v2 of the patch?
Nikolay