[libvirt] [PATCH 0/4] Introduce wildcard to log filters
First, 2 tiny fixes along the way, then the actual change, which lets you do the following for example during runtime: virt-admin daemon-log-filters "1:* 4:remote 4:json" which previously, given the default config setting log_level=3, translated into virt-admin daemon-log-filters \ "1:access 1:admin 1:conf 1:cpu 1:qemu 1:interface 1:<etc-it's-long>" ...of course, "3:*" is equivalent to "log_level=3". Erik Skultety (4): virlog: Fix a typo in virLogParseFilter's error msg libvirtd.conf: Document that we do a 'first' match on log filters util: virlog: Introduce wildcard to log filters news: Update the news file with the log filter wildcard improvement docs/news.xml | 12 ++++++++++++ src/remote/libvirtd.conf | 6 +++++- src/util/virlog.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- src/util/virlog.h | 1 + 4 files changed, 62 insertions(+), 3 deletions(-) -- 2.14.3
This was some copy-paste leftover. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virlog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index dd927f0ba..5810643e1 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1699,7 +1699,7 @@ virLogParseFilter(const char *src) if (virStrToLong_uip(tokens[0], NULL, 10, &prio) < 0 || (prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) { virReportError(VIR_ERR_INVALID_ARG, - _("Invalid priority '%s' for output '%s'"), + _("Invalid priority '%s' for filter '%s'"), tokens[0], src); goto cleanup; } -- 2.14.3
On Tue, Apr 03, 2018 at 10:45:46AM +0200, Erik Skultety wrote:
This was some copy-paste leftover.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virlog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK Jano
When applying the log filters, one has to define the more specific filters before the generic ones, because the first filter that matches will be applied. However, we've been missing this information in the config, so it always has been a trial-error scenario figuring out that e.g. '4:util 1:util.pci' doesn't actually enable verbose logging on the src/util/virpci.c module because 4:util will be matched first. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/remote/libvirtd.conf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf index 4aed7e746..9c0080dc0 100644 --- a/src/remote/libvirtd.conf +++ b/src/remote/libvirtd.conf @@ -366,7 +366,9 @@ # 4: ERROR # # Multiple filters can be defined in a single @filters, they just need to be -# separated by spaces. +# 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. # # e.g. to only get warning or errors from the remote layer and only errors # from the event layer: -- 2.14.3
On Tue, Apr 03, 2018 at 10:45:47AM +0200, Erik Skultety wrote:
When applying the log filters, one has to define the more specific filters before the generic ones, because the first filter that matches will be applied. However, we've been missing this information in the config, so it always has been a trial-error scenario figuring out that e.g. '4:util 1:util.pci' doesn't actually enable verbose logging on the src/util/virpci.c module because 4:util will be matched first.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/remote/libvirtd.conf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
ACK Jano
Since the introduction of log tuning capabilities to virt-admin by @06b91785, this has been a much needed missing improvement on the way to deprecate the global 'log_level'. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/remote/libvirtd.conf | 4 +++- src/util/virlog.c | 44 +++++++++++++++++++++++++++++++++++++++++++- src/util/virlog.h | 1 + 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf index 9c0080dc0..7d823cf1a 100644 --- a/src/remote/libvirtd.conf +++ b/src/remote/libvirtd.conf @@ -368,7 +368,9 @@ # 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. +# given the order in log_filters with the exception of a wildcard filter, since +# that's only taken into account if no other filter has matched, so +# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions. # # e.g. to only get warning or errors from the remote layer and only errors # from the event layer: diff --git a/src/util/virlog.c b/src/util/virlog.c index 5810643e1..81a9dc439 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -497,6 +497,38 @@ virLogHostnameString(char **rawmsg, return 0; } +/* virLogFiltersFind: + * @filters: haystack + * @nfilters: haystack size + * @key1: primary string 'needle' + * @key2: secondary integer 'needle' + * + * Performs "first match" search on the input set of filters, using @key1 + * (string) and @key2 (integer) as primary and secondary keys respectively. + * Secondary key is only considered if primary key wasn't provided. + * + * Returns a pointer to the matched object or NULL if no match was found. + */ +static virLogFilterPtr +virLogFiltersFind(virLogFilterPtr *filters, + size_t nfilters, + const char *key1, + unsigned int key2) +{ + size_t i; + + if (!key1 && key2 == 0) + return NULL; + + for (i = 0; i < nfilters; i++) { + if ((key1 && STREQ(key1, filters[i]->match)) || + filters[i]->flags == key2) + return filters[i]; + } + + return NULL; +} + static void virLogSourceUpdate(virLogSourcePtr source) @@ -1409,7 +1441,8 @@ virLogFilterNew(const char *match, virLogFilterPtr ret = NULL; char *mdup = NULL; - virCheckFlags(VIR_LOG_STACK_TRACE, NULL); + virCheckFlags(VIR_LOG_STACK_TRACE | + VIR_LOG_WILDCARD, NULL); if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) { virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"), @@ -1528,6 +1561,8 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs) int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) { + virLogFilterPtr rc; + if (virLogInitialize() < 0) return -1; @@ -1535,6 +1570,10 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) virLogResetFilters(); virLogFilters = filters; virLogNbFilters = nfilters; + + /* if there's a wildcard filter, update default priority */ + if ((rc = virLogFiltersFind(filters, nfilters, NULL, VIR_LOG_WILDCARD))) + virLogDefaultPriority = rc->priority; virLogUnlock(); return 0; @@ -1710,6 +1749,9 @@ virLogParseFilter(const char *src) match++; } + if (STREQ(match, "*")) + flags |= VIR_LOG_WILDCARD; + /* match string cannot comprise just from a single '+' */ if (!*match) { virReportError(VIR_ERR_INVALID_ARG, diff --git a/src/util/virlog.h b/src/util/virlog.h index 8973f83b2..07b91f0b0 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -174,6 +174,7 @@ typedef void (*virLogCloseFunc) (void *data); typedef enum { VIR_LOG_STACK_TRACE = (1 << 0), + VIR_LOG_WILDCARD = (1 << 1) } virLogFlags; int virLogGetNbFilters(void); -- 2.14.3
On Tue, Apr 03, 2018 at 10:45:48AM +0200, Erik Skultety wrote:
Since the introduction of log tuning capabilities to virt-admin by @06b91785, this has been a much needed missing improvement on the way to deprecate the global 'log_level'.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/remote/libvirtd.conf | 4 +++- src/util/virlog.c | 44 +++++++++++++++++++++++++++++++++++++++++++- src/util/virlog.h | 1 + 3 files changed, 47 insertions(+), 2 deletions(-)
ACK
diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf index 9c0080dc0..7d823cf1a 100644 --- a/src/remote/libvirtd.conf +++ b/src/remote/libvirtd.conf @@ -368,7 +368,9 @@ # 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. +# given the order in log_filters with the exception of a wildcard filter, since +# that's only taken into account if no other filter has matched, so +# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions. #
Also, "4:* 1:util.pci 1:*" is equivalent.
# e.g. to only get warning or errors from the remote layer and only errors # from the event layer:
Jano
On Fri, Apr 06, 2018 at 03:00:14PM +0200, Ján Tomko wrote:
On Tue, Apr 03, 2018 at 10:45:48AM +0200, Erik Skultety wrote:
Since the introduction of log tuning capabilities to virt-admin by @06b91785, this has been a much needed missing improvement on the way to deprecate the global 'log_level'.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/remote/libvirtd.conf | 4 +++- src/util/virlog.c | 44 +++++++++++++++++++++++++++++++++++++++++++- src/util/virlog.h | 1 + 3 files changed, 47 insertions(+), 2 deletions(-)
ACK
diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf index 9c0080dc0..7d823cf1a 100644 --- a/src/remote/libvirtd.conf +++ b/src/remote/libvirtd.conf @@ -368,7 +368,9 @@ # 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. +# given the order in log_filters with the exception of a wildcard filter, since +# that's only taken into account if no other filter has matched, so +# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions. #
Also, "4:* 1:util.pci 1:*" is equivalent.
Sure, but if someone really tries that, 1) it doesn't make any sense 2) a question on what the expected outcome they think should be is relevant, therefore I'm not going to mention it. Thanks, Erik
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 87f52e83e..6dab4844d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,18 @@ supported. In fact, kernel has been supporting this since 4.10. </description> </change> + <change> + <summary> + logging: Introduce wildcard to log filters + </summary> + <description> + In an effort to deprecate the log_level variable in favour of the log + filters, introduce a wildcard filter, e.g. '1:*'. This change is most + noticeable during runtime using virt-admin, where the only way to + mimic the log_level's functionality has been to specify a number of + filters which can a bit painful. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.14.3
On Tue, Apr 03, 2018 at 10:45:49AM +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 87f52e83e..6dab4844d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,18 @@ supported. In fact, kernel has been supporting this since 4.10. </description> </change> + <change> + <summary> + logging: Introduce wildcard to log filters + </summary> + <description> + In an effort to deprecate the log_level variable in favour of the log + filters, introduce a wildcard filter, e.g. '1:*'. This change is most + noticeable during runtime using virt-admin, where the only way to + mimic the log_level's functionality has been to specify a number of + filters which can a bit painful.
can be ACK Jano
+ </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.14.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 10:45:45AM +0200, Erik Skultety wrote:
First, 2 tiny fixes along the way, then the actual change, which lets you do the following for example during runtime: virt-admin daemon-log-filters "1:* 4:remote 4:json"
which previously, given the default config setting log_level=3, translated into virt-admin daemon-log-filters \ "1:access 1:admin 1:conf 1:cpu 1:qemu 1:interface 1:<etc-it's-long>"
...of course, "3:*" is equivalent to "log_level=3".
Erik Skultety (4): virlog: Fix a typo in virLogParseFilter's error msg libvirtd.conf: Document that we do a 'first' match on log filters util: virlog: Introduce wildcard to log filters news: Update the news file with the log filter wildcard improvement
docs/news.xml | 12 ++++++++++++ src/remote/libvirtd.conf | 6 +++++- src/util/virlog.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- src/util/virlog.h | 1 + 4 files changed, 62 insertions(+), 3 deletions(-)
I fixed patch 4 as suggested and pushed. Thanks, Erik
participants (2)
-
Erik Skultety -
Ján Tomko