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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
 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@xxxxxxxxxx>
---
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@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux