On Fri, Apr 20, 2018 at 05:56:39PM +0100, Daniel P. Berrangé wrote:
> Strongly recommend against use of the log_levels setting since it
> creates overly verbose logs and has a serious performance impact.
>
> Describe the log filter syntax better and mention use of shell
> glob syntax. Also provide more realistic example of good settings
> to use. The libvirtd example is biased towards QEMU, but when the
> drivers split off each daemon can get its own more appropriate
> example.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
[...]
Besides couple of ultra minor things, the core change looks good to me.
> +# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
> +#
> +# WARNING: It outputs too much information to practically read.
> +# WARNING: The "log_filters" setting is recommended instead.
> +#
> +# WARNING: Journald may employ rate limiting of the messages logged
> +# WARNING: and thus lock up the libvirt daemon. To use the debug
> +# WARNING: level with journald you have to specify it explicitly in
> +# WARNING: 'log_outputs', otherwise only information level messages
> +# WARNING: will be logged.
I see you said in your reply to Erik that you're going to reword the
above. That said ...
> +# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
> #log_level = 3
... the above bit reads slightly confusing, as it implies using
'log_level = 3' is discouraged, while (I think) you mean that use of
'log_level' in general is discouraged. Unless I'm misreading it.
[...]
> -# Multiple filter can be defined in a single @filters, they just need to be
> -# separated by spaces.
> +# Multiple filters can be defined in a single @filters, they just need to be
> +# separated by spaces. Note that libvirt performs "first" match, i.e. if
> +# there are concurrent filters, the first one that matches will be applied,
> +# given the order in log_filters.
> +#
> +# For the virtlockd daemon, a typical need is to capture information
> +# from the locking code and some of the utility code. Some utility
> +# code is very verbose and is generally not desired. Taking the QEMU
> +# hypervisor as an example, a suitable filter string for debugging
> +# might be to turn off object, json & event logging, but enable the
OCD comment (applies to all occurrences of this block of text): Either
s/json/JSON to read as a sentence. Or I'd put them in single quotes:
'object', 'json' & 'event', 'util' to indicate they're special.
> +# rest of the util code:
> #
> -# e.g. to only get warning or errors from the remote layer and only errors
> -# from the event layer:
> -#log_filters="3:remote 4:event"
> +#log_filters="1:locking 4:object 4:json 4:event 1:util"
>
> # Logging outputs:
> # An output is one of the places to save logging information
> # The format for an output can be:
> -# x:stderr
> +# level:stderr
> # output goes to stderr
> -# x:syslog:name
> +# level:syslog:name
> # use syslog for the output and use the given name as the ident
> -# x:file:file_path
> +# level:file:file_path
> # output to a file, with the given filepath
> -# In all case the x prefix is the minimal level, acting as a filter
> +# level:journald
> +# output to journald logging system
> +# In all cases 'level' is the minimal priority, acting as a filter
> # 1: DEBUG
> # 2: INFO
> # 3: WARNING
> # 4: ERROR
> #
> -# Multiple output can be defined, they just need to be separated by spaces.
> -# e.g. to log all warnings and errors to syslog under the virtlockd ident:
> +# Multiple outputs can be defined, they just need to be separated by spaces.
> +# e.g. to log all warnings and errors to syslog under the libvirtd ident:
Super nit, pre-existing: s/e.g./E.g./ (Since it's the start of a sentence)
> #log_outputs="3:syslog:virtlockd"
> #
>
> diff --git a/src/logging/test_virtlogd.aug.in b/src/logging/test_virtlogd.aug.in
> index 744f3246af..a29e7e3730 100644
> --- a/src/logging/test_virtlogd.aug.in
> +++ b/src/logging/test_virtlogd.aug.in
> @@ -3,7 +3,7 @@ module Test_virtlogd =
>
> test Virtlogd.lns get conf =
> { "log_level" = "3" }
> - { "log_filters" = "3:remote 4:event" }
> + { "log_filters" = "1:logging 4:object 4:json 4:event 1:util" }
> { "log_outputs" = "3:syslog:virtlogd" }
> { "max_clients" = "1024" }
> { "admin_max_clients" = "5" }
> diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf
> index c22b7737ef..f2078a730c 100644
> --- a/src/logging/virtlogd.conf
> +++ b/src/logging/virtlogd.conf
> @@ -8,49 +8,80 @@
>
> # Logging level: 4 errors, 3 warnings, 2 information, 1 debug
> # basically 1 will log everything possible
> +#
> +# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
> +#
> +# WARNING: It outputs too much information to practically read.
> +# WARNING: The "log_filters" setting is recommended instead.
> +#
> +# WARNING: Journald may employ rate limiting of the messages logged
> +# WARNING: and thus lock up the libvirt daemon. To use the debug
> +# WARNING: level with journald you have to specify it explicitly in
> +# WARNING: 'log_outputs', otherwise only information level messages
> +# WARNING: will be logged.
> +#
> +# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
> #log_level = 3
Here, too. I'll interpret the above as: "Using 'log_level' is strongly
discouraged". So probably: s/USE OF/USE OF `log_level`/
[...]
> +#
> +# where 'match' is a string which is matched against the category
> +# given in the VIR_LOG_INIT() at the top of each libvirt source
> +# file, e.g., "remote", "qemu", or "util.json". The 'match' in the
> +# filter matches using shell wildcard syntax (see 'man glob(7)').
> +# The 'match' is always treated a substring match. IOW a match
> +# string 'foo' is equivalent to '*foo*'.
> +#
> +# If 'match' contains the optional "+" prefix, it tells libvirt
> +# to log stack trace for each message matching name.
> +#
> +# 'level' is the minimal level where matching messages should
> +# be logged:
> +#
> # 1: DEBUG
> # 2: INFO
> # 3: WARNING
> @@ -370,22 +383,27 @@
> # there are concurrent filters, the first one that matches will be applied,
> # given the order in log_filters.
> #
> -# e.g. to only get warning or errors from the remote layer and only errors
> -# from the event layer:
> -#log_filters="3:remote 4:event"
> +# A typical need is to capture information from a hypervisor driver,
> +# public API entrypoints and some of the utility code. Some utility
Nit: s/entrypoint/entry points/
Thanks for the patch.
[...]
--
/kashyap
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
