[libvirt] [PATCH] util: drop support for stack traces with logging

Thu Oct 10 12:55:30 UTC 2019

On Thu, Oct 10, 2019 at 01:05:43PM +0100, Daniel P. Berrangé wrote:
>The log filters have supported the use of a "+" before the source match
>string to request that a stack trace be emitted for every log message:
>
>  commit 548563956e484e0e43e9a66a89bdda0f95930108
>  Author: Daniel P. Berrange <berrange at redhat.com>
>  Date:   Wed May 9 15:18:56 2012 +0100
>
>    Allow stack traces to be included with log messages
>
>    Sometimes it is useful to see the callpath for log messages.
>    This change enhances the log filter syntax so that stack traces
>    can be show by setting '1:+NAME' instead of '1:NAME'.
>
>With the huge & ever increasing number of logging statements per file,
>this will be incredibly verbose and have a major performance penalty.
>This makes the feature impractical to use widely and as such it is not
>worth the code maint cost.
>
>Removing this seldom used feature allows us to drop the 'execinfo'
>module in gnulib which provides the backtrace() function which doesn't
>exist on non-Linux.
>
>Users who want to get stack traces of parts of libvirt can use GDB,
>or systemtap for live tracing with minimal perf impact.
>
>Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>---
> bootstrap.conf              |  1 -
> docs/logging.html.in        |  7 ++--
> src/locking/virtlockd.conf  |  4 ---
> src/logging/virtlogd.conf   |  4 ---
> src/remote/libvirtd.conf.in |  4 ---
> src/util/virlog.c           | 68 ++++++-------------------------------
> src/util/virlog.h           | 11 +-----
> tests/testutils.c           |  2 --
> tests/virtestmock.c         |  1 -
> 9 files changed, 14 insertions(+), 88 deletions(-)
>
>diff --git a/bootstrap.conf b/bootstrap.conf
>index 358d783a6b..55997b018f 100644
>--- a/bootstrap.conf
>+++ b/bootstrap.conf
>@@ -35,7 +35,6 @@ connect
> configmake
> dirname-lgpl
> environ
>-execinfo
> fclose
> fcntl
> fcntl-h
>diff --git a/docs/logging.html.in b/docs/logging.html.in
>index be2fd4ab5b..d62220a9c1 100644
>--- a/docs/logging.html.in
>+++ b/docs/logging.html.in
>@@ -102,16 +102,13 @@
>        variables.</p>
>     <p>The format for a filter is one of:</p>

d/ one of/

>     <pre>
>-x:name  (log message only)
>-x:+name (log message + stack trace)</pre>
>+x:name  (log message only)</pre>

I guess "log message only" no longer makes sense here either

>     <p>where <code>name</code> 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., <code>remote</code>, <code>qemu</code>,
>     or <code>util.json</code> (the name in the filter can be a
>     substring of the full category name, in order to match multiple
>-    similar categories), the optional <code>+</code> prefix tells
>-    libvirt to log stack trace for each message
>-    matching <code>name</code>, and <code>x</code> is the minimal
>+    similar categories), and <code>x</code> is the minimal
>     level where matching messages should be logged:</p>
>     <ul>
>       <li>1: DEBUG</li>
>diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf
>index b65110fc3e..b75eb3b279 100644
>--- a/src/locking/virtlockd.conf
>+++ b/src/locking/virtlockd.conf
>@@ -26,7 +26,6 @@
> # of logs. The format for a filter is one of:
> #

d/ one of/

> #    level:match
>-#    level:+match
> #
> # where 'match' is a string which is matched against the category
> # given in the VIR_LOG_INIT() at the top of each libvirt source
>@@ -35,9 +34,6 @@
> # The 'match' is always treated as 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:
> #
>diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf
>index bc41edbc6b..9f2d36c382 100644
>--- a/src/logging/virtlogd.conf
>+++ b/src/logging/virtlogd.conf
>@@ -26,7 +26,6 @@
> # of logs. The format for a filter is one of:
> #

d/ one of/

> #    level:match
>-#    level:+match
> #
> # where 'match' is a string which is matched against the category
> # given in the VIR_LOG_INIT() at the top of each libvirt source
>@@ -35,9 +34,6 @@
> # The 'match' is always treated as 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:
> #
>diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
>index fdef97f371..50e3a00854 100644
>--- a/src/remote/libvirtd.conf.in
>+++ b/src/remote/libvirtd.conf.in
>@@ -372,7 +372,6 @@
> # of logs. The format for a filter is one of:
> #

d/ one of/

> #    level:match
>-#    level:+match
> #
> # where 'match' is a string which is matched against the category
> # given in the VIR_LOG_INIT() at the top of each libvirt source
>@@ -381,9 +380,6 @@
> # The 'match' is always treated as 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:
> #
>diff --git a/src/util/virlog.c b/src/util/virlog.c
>index 4c76fbc5a4..611475c8d7 100644
>--- a/src/util/virlog.c
>+++ b/src/util/virlog.c
>@@ -28,7 +28,6 @@
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <unistd.h>
>-#include <execinfo.h>
> #include <regex.h>
> #include <sys/uio.h>
> #if HAVE_SYSLOG_H
>@@ -88,7 +87,6 @@ VIR_ENUM_IMPL(virLogDestination,
> struct _virLogFilter {
>     char *match;
>     virLogPriority priority;
>-    unsigned int flags; /* bitwise OR of virLogFilterFlags */

The 'flags' removal could've been a follow-up commit.

> };
>
> static int virLogFiltersSerial = 1;
>@@ -1661,9 +1617,6 @@ virLogParseOutput(const char *src)
>  *
>  * The format of @src should be one of the following:

d/ one of the following/

>  *    x:name - filter affecting all modules which match 'name'
>- *    x:+name
>- *
>- *      '+' - hints the logger to also include a stack trace for every message
>  *      'name' - match string which either matches a name of a directory in
>  *               libvirt's source tree which in turn affects all modules in
>  *               that directory or it can matches a specific module within a

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191010/34b19fc5/attachment-0001.sig>