[libvirt] [PATCH] log: fix false oom error message

Erik Skultety eskultet at redhat.com
Mon Sep 12 14:00:33 UTC 2016


On 12/09/16 12:56, Nikolay Shirokovskiy wrote:
> In case filter is new one an oom error message is reported.
> This bug is introduced in 51b2606f.

oops

 However this error does
> not get into the log in the default configuration. To trigger
> it one need to specify log outputs in LIBVIRT_LOG_OUTPUTS
> variable.

Well, in my understanding, this is only true when the daemon is starting
and the error isn't reported because the outputs are initialized once
the filters' init routine is finished. So if you're preparing a test for
example where you include our internal APIs, then at least the error
would be reported. That's the reason why I'm saying that the statement
about not reporting the error is only true for a daemon restart. Anyhow,
it is a bug that should be fixed but I'm still wondering if we should do
something similar to virLogDefineOutput, which btw also lacks reporting
virReportOOMError() when an allocation fails, since my refactor series
[1] is going to remove these bits of code completely.

[1] https://www.redhat.com/archives/libvir-list/2016-August/msg00907.html

> ---
>  src/util/virlog.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 06f9a60..f5b88b6 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -335,11 +335,12 @@ virLogDefineFilter(const char *match,
>          goto cleanup;
>  
>      virLogFiltersSerial++;
> +    ret = virLogNbFilters - 1;
>   cleanup:
>      virLogUnlock();
>      if (ret < 0)
>          virReportOOMError();
> -    return virLogNbFilters;
> +    return ret;
>  }
>  
>  /**
> 

ACK to the patch though, I slightly adjusted the CM and pushed.
Thanks,
Erik




More information about the libvir-list mailing list