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