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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list