[libvirt] [PATCH] libxl: open libxl log stream with libvirtd log_level

Jim Fehlig jfehlig at suse.com
Wed Sep 16 03:52:16 UTC 2015

On 09/15/2015 04:06 PM, John Ferlan wrote:
> On 09/15/2015 01:21 PM, Jim Fehlig wrote:
>> Daniel P. Berrange wrote:
>>> On Tue, Sep 15, 2015 at 10:57:50AM -0600, Jim Fehlig wrote:
>>>> Daniel P. Berrange wrote:
>>>>> On Tue, Sep 15, 2015 at 09:26:23AM -0600, Jim Fehlig wrote:
>>>>>> Instead of a hardcoded DEBUG log level, use the overall
>>>>>> daemon log level specified in libvirtd.conf when opening
>>>>>> a log stream with libxl. libxl is very verbose when DEBUG
>>>>>> log level is set, resulting in huge log files that can
>>>>>> potentially fill a disk. Control of libxl verbosity should
>>>>>> be placed in the administrator's hands.
>>>>>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>>>>>> ---
>>>>>>   src/libxl/libxl_conf.c | 18 +++++++++++++++++-
>>>>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>>> ACK, this makes sense as default behaviour. As a future enhancement
>>>>> you might also consider supporting a config setting in /etc/libvirt/libxl.conf
>>>>> to explicitly control the libxl library logging behaviour independantly.
>>>> I had actually thought of adding it there first, but then took this
>>>> approach assuming it would be more receptive upstream :-). Personally,
>>>> I'm on the fence. I like the idea of a single knob to control log level
>>>> throughout the daemon, making it a bit easier on admins. On the other
>>>> hand, individual knobs are more friendly to those pouring through logs.
>>>> I can add a knob in libxl.conf if preferred.
>>> After thinking about it some more, I could actually see value in
>>> create a dedicated virLogSource() instance, solely for libxl
>>> library messages. If we then created a virLogSourceGetPriority()
>>> method, you could query that to see if to turn on logging for
>>> the libxl library. That would ultimately allow you to turn on
>>> debug for just the libxl library if desired, eg
>>>   static virLogSource virLogLibXL = {
>>>       .name = "libxl.libxl_library",
>>>       ....
>>>   }
>>> LIBVIRT_LOG_FILTERS="1:libxl_library"
>> Ah, good idea. I'll look into it.
>>> Regardless we should just merge your current patch right now.
>> Thanks; done now.
>> Regards,
>> Jim
> Looks like compilations on Fedora are 'unhappy':
> libxl/libxl_conf.c: In function 'libxlDriverConfigNew':
> libxl/libxl_conf.c:1560:30: error: 'log_level' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
>           (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file,
>                                ^
> libxl/libxl_conf.c:1499:22: note: 'log_level' was declared here
>       xentoollog_level log_level;
>                        ^
> cc1: all warnings being treated as errors
> Makefile:8000: recipe for target
> 'libxl/libvirt_driver_libxl_impl_la-libxl_conf.lo' failed
> You can also see failures today in libvirt-daemon-build and
> libvirt-daemon-check from:
> https://ci.centos.org/view/libvirt-project/
> I wasn't sure if initializing log_level to XTL_DEBUG and then changing
> if virLogGetDefaultPriority returns something else would be better or
> going the "default:" route in the switch statement would be better.

I took the bit more explicit route and went with the former, worried that 
Coverity would be the next thing to complain :-).

I considered initializing log_level to XTL_WARN, matching VIR_LOG_DEFAULT, but 
in the end went with DEBUG. It matches the behavior before this change, and is 
reasonable default if we're already this confused while loading the driver.


More information about the libvir-list mailing list