[libvirt] [PATCH 4/4] log: update docs for daemons to improve user understanding

[Kashyap Chamarthy]

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 at redhat.com>
> ---

[...]

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