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(a)redhat.com&gt; 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(a)redhat.com&gt; --- 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> <pre> -x:name (log message only) -x:+name (log message + stack trace)</pre> +x:name (log message only)</pre> <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: # # 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: # # 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: # # 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 */ }; static int virLogFiltersSerial = 1; @@ -127,7 +125,6 @@ static void virLogOutputToFd(virLogSourcePtr src, const char *funcname, const char *timestamp, virLogMetadataPtr metadata, - unsigned int flags, const char *rawstr, const char *str, void *data); @@ -510,19 +507,16 @@ virLogSourceUpdate(virLogSourcePtr source) virLogLock(); if (source->serial < virLogFiltersSerial) { unsigned int priority = virLogDefaultPriority; - unsigned int flags = 0; size_t i; for (i = 0; i < virLogNbFilters; i++) { if (fnmatch(virLogFilters[i]->match, source->name, 0) == 0) { priority = virLogFilters[i]->priority; - flags = virLogFilters[i]->flags; break; } } source->priority = priority; - source->flags = flags; source->serial = virLogFiltersSerial; } virLogUnlock(); @@ -591,7 +585,6 @@ virLogVMessage(virLogSourcePtr source, int ret; size_t i; int saved_errno = errno; - unsigned int filterflags = 0; if (virLogInitialize() < 0) return; @@ -611,7 +604,6 @@ virLogVMessage(virLogSourcePtr source, virLogSourceUpdate(source); if (priority < source->priority) goto cleanup; - filterflags = source->flags; /* * serialize the error message, add level and timestamp @@ -641,13 +633,13 @@ virLogVMessage(virLogSourcePtr source, if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputs[i]->f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, rawinitmsg, initmsg, virLogOutputs[i]->data); VIR_FREE(initmsg); if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputs[i]->f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, hoststr, initmsg, + timestamp, NULL, hoststr, initmsg, virLogOutputs[i]->data); VIR_FREE(hoststr); VIR_FREE(initmsg); @@ -655,7 +647,7 @@ virLogVMessage(virLogSourcePtr source, } virLogOutputs[i]->f(source, priority, filename, linenr, funcname, - timestamp, metadata, filterflags, + timestamp, metadata, str, msg, virLogOutputs[i]->data); } } @@ -667,13 +659,13 @@ virLogVMessage(virLogSourcePtr source, if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, rawinitmsg, initmsg, + timestamp, NULL, rawinitmsg, initmsg, (void *) STDERR_FILENO); VIR_FREE(initmsg); if (virLogHostnameString(&hoststr, &initmsg) >= 0) virLogOutputToFd(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, - timestamp, NULL, 0, hoststr, initmsg, + timestamp, NULL, hoststr, initmsg, (void *) STDERR_FILENO); VIR_FREE(hoststr); VIR_FREE(initmsg); @@ -681,7 +673,7 @@ virLogVMessage(virLogSourcePtr source, } virLogOutputToFd(source, priority, filename, linenr, funcname, - timestamp, metadata, filterflags, + timestamp, metadata, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); @@ -693,26 +685,6 @@ virLogVMessage(virLogSourcePtr source, } -static void -virLogStackTraceToFd(int fd) -{ - void *array[100]; - int size; - static bool doneWarning; - const char *msg = "Stack trace not available on this platform\n"; - -#define STRIP_DEPTH 3 - size = backtrace(array, ARRAY_CARDINALITY(array)); - if (size) { - backtrace_symbols_fd(array + STRIP_DEPTH, size - STRIP_DEPTH, fd); - ignore_value(safewrite(fd, "\n", 1)); - } else if (!doneWarning) { - ignore_value(safewrite(fd, msg, strlen(msg))); - doneWarning = true; - } -#undef STRIP_DEPTH -} - static void virLogOutputToFd(virLogSourcePtr source ATTRIBUTE_UNUSED, virLogPriority priority ATTRIBUTE_UNUSED, @@ -721,7 +693,6 @@ virLogOutputToFd(virLogSourcePtr source ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, const char *timestamp, virLogMetadataPtr metadata ATTRIBUTE_UNUSED, - unsigned int flags, const char *rawstr ATTRIBUTE_UNUSED, const char *str, void *data) @@ -737,9 +708,6 @@ virLogOutputToFd(virLogSourcePtr source ATTRIBUTE_UNUSED, ignore_value(safewrite(fd, msg, strlen(msg))); VIR_FREE(msg); - - if (flags & VIR_LOG_STACK_TRACE) - virLogStackTraceToFd(fd); } @@ -827,13 +795,10 @@ virLogOutputToSyslog(virLogSourcePtr source ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, const char *timestamp ATTRIBUTE_UNUSED, virLogMetadataPtr metadata ATTRIBUTE_UNUSED, - unsigned int flags, const char *rawstr ATTRIBUTE_UNUSED, const char *str, void *data ATTRIBUTE_UNUSED) { - virCheckFlags(VIR_LOG_STACK_TRACE,); - syslog(virLogPrioritySyslog(priority), "%s", str); } @@ -975,12 +940,10 @@ virLogOutputToJournald(virLogSourcePtr source, const char *funcname, const char *timestamp ATTRIBUTE_UNUSED, virLogMetadataPtr metadata, - unsigned int flags, const char *rawstr, const char *str ATTRIBUTE_UNUSED, void *data) { - virCheckFlags(VIR_LOG_STACK_TRACE,); int buffd = -1; int journalfd = (intptr_t) data; struct msghdr mh; @@ -1168,8 +1131,6 @@ virLogGetFilters(void) virLogLock(); for (i = 0; i < virLogNbFilters; i++) { const char *sep = ":"; - if (virLogFilters[i]->flags & VIR_LOG_STACK_TRACE) - sep = ":+"; virBufferAsprintf(&filterbuf, "%d%s%s ", virLogFilters[i]->priority, sep, @@ -1395,7 +1356,6 @@ virLogOutputNew(virLogOutputFunc f, * virLogFilterNew: * @match: the pattern to match * @priority: the priority to give to messages matching the pattern - * @flags: extra flags, see virLogFilterFlags enum * * Allocates and returns a new log filter object. The object has to be later * defined, so that the pattern will be taken into account when executing the @@ -1409,15 +1369,12 @@ virLogOutputNew(virLogOutputFunc f, */ virLogFilterPtr virLogFilterNew(const char *match, - virLogPriority priority, - unsigned int flags) + virLogPriority priority) { virLogFilterPtr ret = NULL; char *mdup = NULL; size_t mlen = strlen(match); - virCheckFlags(VIR_LOG_STACK_TRACE, NULL); - if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) { virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"), priority); @@ -1441,7 +1398,6 @@ virLogFilterNew(const char *match, ret->match = mdup; ret->priority = priority; - ret->flags = flags; return ret; } @@ -1661,9 +1617,6 @@ virLogParseOutput(const char *src) * * The format of @src should be 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 @@ -1689,7 +1642,6 @@ virLogParseFilter(const char *src) size_t count = 0; virLogPriority prio; char **tokens = NULL; - unsigned int flags = 0; char *match = NULL; VIR_DEBUG("filter=%s", src); @@ -1711,7 +1663,9 @@ virLogParseFilter(const char *src) match = tokens[1]; if (match[0] == '+') { - flags |= VIR_LOG_STACK_TRACE; + /* '+' used to indicate printing a stack trace, + * but we dropped that feature, so just chomp + * that leading '+' */ match++; } @@ -1723,7 +1677,7 @@ virLogParseFilter(const char *src) goto cleanup; } - if (!(ret = virLogFilterNew(match, prio, flags))) + if (!(ret = virLogFilterNew(match, prio))) goto cleanup; cleanup: diff --git a/src/util/virlog.h b/src/util/virlog.h index a30b760fb4..e27cd6475d 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -64,7 +64,6 @@ struct _virLogSource { const char *name; unsigned int priority; unsigned int serial; - unsigned int flags; }; /* @@ -77,7 +76,6 @@ struct _virLogSource { .name = "" n "", \ .priority = VIR_LOG_ERROR, \ .serial = 0, \ - .flags = 0, \ } /* @@ -144,7 +142,6 @@ typedef virLogFilter *virLogFilterPtr; * @funcname: the function emitting the message * @timestamp: zero terminated string with timestamp of the message * @metadata: NULL or metadata array, terminated by an item with NULL key - * @flags: flags associated with the message * @rawstr: the unformatted message to log, zero terminated * @str: the message to log, preformatted and zero terminated * @data: extra output logging data @@ -158,7 +155,6 @@ typedef void (*virLogOutputFunc) (virLogSourcePtr src, const char *funcname, const char *timestamp, virLogMetadataPtr metadata, - unsigned int flags, const char *rawstr, const char *str, void *data); @@ -171,10 +167,6 @@ typedef void (*virLogOutputFunc) (virLogSourcePtr src, */ typedef void (*virLogCloseFunc) (void *data); -typedef enum { - VIR_LOG_STACK_TRACE = (1 << 0), -} virLogFilterFlags; - int virLogGetNbFilters(void); int virLogGetNbOutputs(void); char *virLogGetFilters(void); @@ -224,8 +216,7 @@ virLogOutputPtr virLogOutputNew(virLogOutputFunc f, virLogDestination dest, const char *name) ATTRIBUTE_NONNULL(1); virLogFilterPtr virLogFilterNew(const char *match, - virLogPriority priority, - unsigned int flags) ATTRIBUTE_NONNULL(1); + virLogPriority priority) ATTRIBUTE_NONNULL(1); int virLogFindOutput(virLogOutputPtr *outputs, size_t noutputs, virLogDestination dest, const void *opaque); int virLogDefineOutputs(virLogOutputPtr *outputs, diff --git a/tests/testutils.c b/tests/testutils.c index 1b663f9d5d..92eb780617 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -740,13 +740,11 @@ virtTestLogOutput(virLogSourcePtr source ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, const char *timestamp, virLogMetadataPtr metadata ATTRIBUTE_UNUSED, - unsigned int flags, const char *rawstr ATTRIBUTE_UNUSED, const char *str, void *data) { struct virtTestLogData *log = data; - virCheckFlags(VIR_LOG_STACK_TRACE,); virBufferAsprintf(&log->buf, "%s: %s", timestamp, str); } diff --git a/tests/virtestmock.c b/tests/virtestmock.c index df8cac6441..1261393ec2 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -22,7 +22,6 @@ #include <unistd.h> #include <sys/types.h> #include <fcntl.h> -#include <execinfo.h> #include <sys/file.h> #include <sys/stat.h> #include <sys/socket.h> -- 2.21.0