A earlier commit changed the global log buffer so that it only
records messages that are explicitly requested via the log
filters setting. This removes the performance burden, and
improves the signal/noise ratio for messages in the global
buffer. At the same time though, it is somewhat pointless, since
all the recorded log messages are already going to be sent to an
explicit log output like syslog, stderr or the journal. The
global log buffer is thus just duplicating this data on stderr
upon crash.
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
daemon/libvirtd-config.c | 3 -
daemon/libvirtd-config.h | 1 -
daemon/libvirtd.aug | 1 -
daemon/libvirtd.c | 2 -
daemon/libvirtd.conf | 7 -
daemon/test_libvirtd.aug.in | 1 -
docs/logging.html.in | 10 --
src/locking/lock_daemon.c | 2 -
src/locking/lock_daemon_config.c | 2 -
src/locking/lock_daemon_config.h | 1 -
src/locking/test_virtlockd.aug.in | 2 -
src/locking/virtlockd.aug | 1 -
src/locking/virtlockd.conf | 7 -
src/rpc/virnetserver.c | 45 -------
src/util/virlog.c | 264 +-------------------------------------
src/util/virlog.h | 2 -
16 files changed, 3 insertions(+), 348 deletions(-)
diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index c68c6f4..95134ab 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -267,8 +267,6 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
data->max_requests = 20;
data->max_client_requests = 5;
- data->log_buffer_size = 64;
-
data->audit_level = 1;
data->audit_logging = 0;
@@ -431,7 +429,6 @@ daemonConfigLoadOptions(struct daemonConfig *data,
GET_CONF_INT(conf, filename, log_level);
GET_CONF_STR(conf, filename, log_filters);
GET_CONF_STR(conf, filename, log_outputs);
- GET_CONF_INT(conf, filename, log_buffer_size);
GET_CONF_INT(conf, filename, keepalive_interval);
GET_CONF_INT(conf, filename, keepalive_count);
diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
index a24d5d2..b8558ea 100644
--- a/daemon/libvirtd-config.h
+++ b/daemon/libvirtd-config.h
@@ -73,7 +73,6 @@ struct daemonConfig {
int log_level;
char *log_filters;
char *log_outputs;
- int log_buffer_size;
int audit_level;
int audit_logging;
diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
index 70fce5c..73843ed 100644
--- a/daemon/libvirtd.aug
+++ b/daemon/libvirtd.aug
@@ -64,7 +64,6 @@ module Libvirtd =
let logging_entry = int_entry "log_level"
| str_entry "log_filters"
| str_entry "log_outputs"
- | int_entry "log_buffer_size"
let auditing_entry = int_entry "audit_level"
| bool_entry "audit_logging"
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index f2f1a83..a805a2c 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -661,8 +661,6 @@ daemonSetupLogging(struct daemonConfig *config,
virLogSetFromEnv();
- virLogSetBufferSize(config->log_buffer_size);
-
if (virLogGetNbFilters() == 0)
virLogParseFilters(config->log_filters);
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 073c178..3b864ce 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -345,13 +345,6 @@
#log_outputs="3:syslog:libvirtd"
#
-# Log debug buffer size: default 64
-# The daemon keeps an internal debug log buffer which will be dumped in case
-# of crash or upon receiving a SIGUSR2 signal. This setting allows to override
-# the default buffer size in kilobytes.
-# If value is 0 or less the debug log buffer is deactivated
-#log_buffer_size = 64
-
##################################################################
#
diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
index a7e8515..ac4cdd7 100644
--- a/daemon/test_libvirtd.aug.in
+++ b/daemon/test_libvirtd.aug.in
@@ -44,7 +44,6 @@ module Test_libvirtd =
{ "log_level" = "3" }
{ "log_filters" = "3:remote 4:event" }
{ "log_outputs" = "3:syslog:libvirtd" }
- { "log_buffer_size" = "64" }
{ "audit_level" = "2" }
{ "audit_logging" = "1" }
{ "host_uuid" = "00000000-0000-0000-0000-000000000000" }
diff --git a/docs/logging.html.in b/docs/logging.html.in
index a8cb538..d8d8f1e 100644
--- a/docs/logging.html.in
+++ b/docs/logging.html.in
@@ -38,12 +38,6 @@
all messages to a debugging file but only allow errors to be
logged through syslog.</li>
</ul>
- <p>Note that the logging module saves all logs to a <b>debug buffer</b>
- filled in a round-robin fashion as to keep a full log of the
- recent logs including all debug. The debug buffer can be resized
- or deactivated in the daemon using the log_buffer_size variable,
- default is 64 kB. This can be used when debugging the library
- (see the virLogBuffer variable content).</p>
<h2>
<a name="log_config">Configuring logging in the library</a>
@@ -245,9 +239,5 @@ log_outputs="1:file:/var/log/libvirt/libvirtd.log"</pre>
<p>in libvirtd.conf and restart the daemon will allow to
gather a copious amount of debugging traces for the operations done
in those areas.</p>
- <p>On the other hand to deactivate the logbuffer in the daemon
- for stable high load servers, set</p>
- <pre>log_buffer_size=0</pre>
- <p>in the libvirtd.conf.</p>
</body>
</html>
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 46b2dd3..54d98d4 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -478,8 +478,6 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
virLogSetFromEnv();
- virLogSetBufferSize(config->log_buffer_size);
-
if (virLogGetNbFilters() == 0)
virLogParseFilters(config->log_filters);
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index c7d9d97..38f1a8d 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -115,7 +115,6 @@ virLockDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
if (VIR_ALLOC(data) < 0)
return NULL;
- data->log_buffer_size = 64;
data->max_clients = 1024;
return data;
@@ -141,7 +140,6 @@ virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data,
GET_CONF_INT(conf, filename, log_level);
GET_CONF_STR(conf, filename, log_filters);
GET_CONF_STR(conf, filename, log_outputs);
- GET_CONF_INT(conf, filename, log_buffer_size);
GET_CONF_INT(conf, filename, max_clients);
return 0;
diff --git a/src/locking/lock_daemon_config.h b/src/locking/lock_daemon_config.h
index e75d4a9..1c79a4e 100644
--- a/src/locking/lock_daemon_config.h
+++ b/src/locking/lock_daemon_config.h
@@ -33,7 +33,6 @@ struct _virLockDaemonConfig {
int log_level;
char *log_filters;
char *log_outputs;
- int log_buffer_size;
int max_clients;
};
diff --git a/src/locking/test_virtlockd.aug.in b/src/locking/test_virtlockd.aug.in
index 799818e..e2e3409 100644
--- a/src/locking/test_virtlockd.aug.in
+++ b/src/locking/test_virtlockd.aug.in
@@ -2,11 +2,9 @@ module Test_virtlockd =
let conf = "log_level = 3
log_filters=\"3:remote 4:event\"
log_outputs=\"3:syslog:libvirtd\"
-log_buffer_size = 64
"
test Virtlockd.lns get conf =
{ "log_level" = "3" }
{ "log_filters" = "3:remote 4:event" }
{ "log_outputs" = "3:syslog:libvirtd" }
- { "log_buffer_size" = "64" }
diff --git a/src/locking/virtlockd.aug b/src/locking/virtlockd.aug
index ec8d2b5..7d0d32a 100644
--- a/src/locking/virtlockd.aug
+++ b/src/locking/virtlockd.aug
@@ -27,7 +27,6 @@ module Virtlockd =
let logging_entry = int_entry "log_level"
| str_entry "log_filters"
| str_entry "log_outputs"
- | int_entry "log_buffer_size"
| int_entry "max_clients"
(* Each enty in the config is one of the following three ... *)
diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf
index 652e156..7a00873 100644
--- a/src/locking/virtlockd.conf
+++ b/src/locking/virtlockd.conf
@@ -52,13 +52,6 @@
#log_outputs="3:syslog:virtlockd"
#
-# Log debug buffer size: default 64
-# The daemon keeps an internal debug log buffer which will be dumped in case
-# of crash or upon receiving a SIGUSR2 signal. This setting allows to override
-# the default buffer size in kilobytes.
-# If value is 0 or less the debug log buffer is deactivated
-#log_buffer_size = 64
-
# The maximum number of concurrent client connections to allow
# over all sockets combined.
# Each running virtual machine will require one open connection
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 3a0e715..0ce89bf 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -326,34 +326,6 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc,
}
-static void
-virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
- void *context ATTRIBUTE_UNUSED)
-{
- struct sigaction sig_action;
- int origerrno;
-
- origerrno = errno;
- virLogEmergencyDumpAll(sig);
-
- /*
- * If the signal is fatal, avoid looping over this handler
- * by deactivating it
- */
-#ifdef SIGUSR2
- if (sig != SIGUSR2) {
-#endif
- memset(&sig_action, 0, sizeof(sig_action));
- sig_action.sa_handler = SIG_DFL;
- sigaction(sig, &sig_action, NULL);
- raise(sig);
-#ifdef SIGUSR2
- }
-#endif
- errno = origerrno;
-}
-
-
virNetServerPtr virNetServerNew(size_t min_workers,
size_t max_workers,
size_t priority_workers,
@@ -412,23 +384,6 @@ virNetServerPtr virNetServerNew(size_t min_workers,
sig_action.sa_handler = SIG_IGN;
sigaction(SIGPIPE, &sig_action, NULL);
- /*
- * catch fatal errors to dump a log, also hook to USR2 for dynamic
- * debugging purposes or testing
- */
- sig_action.sa_sigaction = virNetServerFatalSignal;
- sig_action.sa_flags = SA_SIGINFO;
- sigaction(SIGFPE, &sig_action, NULL);
- sigaction(SIGSEGV, &sig_action, NULL);
- sigaction(SIGILL, &sig_action, NULL);
- sigaction(SIGABRT, &sig_action, NULL);
-#ifdef SIGBUS
- sigaction(SIGBUS, &sig_action, NULL);
-#endif
-#ifdef SIGUSR2
- sigaction(SIGUSR2, &sig_action, NULL);
-#endif
-
return srv;
error:
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 048d2c4..f8d0a7a 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -30,7 +30,6 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
-#include <signal.h>
#include <execinfo.h>
#include <regex.h>
#if HAVE_SYSLOG_H
@@ -62,15 +61,6 @@
VIR_LOG_INIT("util.log");
-/*
- * A logging buffer to keep some history over logs
- */
-
-static int virLogSize = 64 * 1024;
-static char *virLogBuffer = NULL;
-static int virLogLen = 0;
-static int virLogStart = 0;
-static int virLogEnd = 0;
static regex_t *virLogRegex = NULL;
@@ -194,29 +184,10 @@ virLogPriorityString(virLogPriority lvl)
static int
virLogOnceInit(void)
{
- const char *pbm = NULL;
-
if (virMutexInit(&virLogMutex) < 0)
return -1;
virLogLock();
- if (VIR_ALLOC_N_QUIET(virLogBuffer, virLogSize + 1) < 0) {
- /*
- * The debug buffer is not a critical component, allow startup
- * even in case of failure to allocate it in case of a
- * configuration mistake.
- */
- virLogSize = 64 * 1024;
- if (VIR_ALLOC_N_QUIET(virLogBuffer, virLogSize + 1) < 0) {
- pbm = "Failed to allocate debug buffer: deactivating debug log\n";
- virLogSize = 0;
- } else {
- pbm = "Failed to allocate debug buffer: reduced to 64 kB\n";
- }
- }
- virLogLen = 0;
- virLogStart = 0;
- virLogEnd = 0;
virLogDefaultPriority = VIR_LOG_DEFAULT;
if (VIR_ALLOC_QUIET(virLogRegex) >= 0) {
@@ -225,8 +196,6 @@ virLogOnceInit(void)
}
virLogUnlock();
- if (pbm)
- VIR_WARN("%s", pbm);
return 0;
}
@@ -234,65 +203,6 @@ VIR_ONCE_GLOBAL_INIT(virLog)
/**
- * virLogSetBufferSize:
- * @size: size of the buffer in kilobytes or <= 0 to deactivate
- *
- * Dynamically set the size or deactivate the logging buffer used to keep
- * a trace of all recent debug output. Note that the content of the buffer
- * is lost if it gets reallocated.
- *
- * Return -1 in case of failure or 0 in case of success
- */
-int
-virLogSetBufferSize(int size)
-{
- int ret = 0;
- int oldsize;
- char *oldLogBuffer;
- const char *pbm = NULL;
-
- if (size < 0)
- size = 0;
-
- if (virLogInitialize() < 0)
- return -1;
-
- if (size * 1024 == virLogSize)
- return ret;
-
- virLogLock();
-
- oldsize = virLogSize;
- oldLogBuffer = virLogBuffer;
-
- if (INT_MAX / 1024 <= size) {
- pbm = "Requested log size of %d kB too large\n";
- ret = -1;
- goto error;
- }
-
- virLogSize = size * 1024;
- if (VIR_ALLOC_N_QUIET(virLogBuffer, virLogSize + 1) < 0) {
- pbm = "Failed to allocate debug buffer of %d kB\n";
- virLogBuffer = oldLogBuffer;
- virLogSize = oldsize;
- ret = -1;
- goto error;
- }
- VIR_FREE(oldLogBuffer);
- virLogLen = 0;
- virLogStart = 0;
- virLogEnd = 0;
-
-error:
- virLogUnlock();
- if (pbm)
- VIR_ERROR(pbm, size);
- return ret;
-}
-
-
-/**
* virLogReset:
*
* Reset the logging module to its default initial state
@@ -308,172 +218,11 @@ virLogReset(void)
virLogLock();
virLogResetFilters();
virLogResetOutputs();
- virLogLen = 0;
- virLogStart = 0;
- virLogEnd = 0;
virLogDefaultPriority = VIR_LOG_DEFAULT;
virLogUnlock();
return 0;
}
-
-/*
- * Store a string in the ring buffer
- */
-static void
-virLogStr(const char *str)
-{
- int tmp;
- int len;
-
- if ((str == NULL) || (virLogBuffer == NULL) || (virLogSize <= 0))
- return;
- len = strlen(str);
- if (len >= virLogSize)
- return;
-
- /*
- * copy the data and reset the end, we cycle over the end of the buffer
- */
- if (virLogEnd + len >= virLogSize) {
- tmp = virLogSize - virLogEnd;
- memcpy(&virLogBuffer[virLogEnd], str, tmp);
- memcpy(&virLogBuffer[0], &str[tmp], len - tmp);
- virLogEnd = len - tmp;
- } else {
- memcpy(&virLogBuffer[virLogEnd], str, len);
- virLogEnd += len;
- }
- virLogBuffer[virLogEnd] = 0;
- /*
- * Update the log length, and if full move the start index
- */
- virLogLen += len;
- if (virLogLen > virLogSize) {
- tmp = virLogLen - virLogSize;
- virLogLen = virLogSize;
- virLogStart += tmp;
- if (virLogStart >= virLogSize)
- virLogStart -= virLogSize;
- }
-}
-
-
-static void
-virLogDumpAllFD(const char *msg, int len)
-{
- size_t i;
- bool found = false;
-
- if (len <= 0)
- len = strlen(msg);
-
- for (i = 0; i < virLogNbOutputs; i++) {
- if (virLogOutputs[i].f == virLogOutputToFd) {
- int fd = (intptr_t) virLogOutputs[i].data;
-
- if (fd >= 0) {
- ignore_value(safewrite(fd, msg, len));
- found = true;
- }
- }
- }
- if (!found)
- ignore_value(safewrite(STDERR_FILENO, msg, len));
-}
-
-
-/**
- * virLogEmergencyDumpAll:
- * @signum: the signal number
- *
- * Emergency function called, possibly from a signal handler.
- * It need to output the debug ring buffer through the log
- * output which are safe to use from a signal handler.
- * In case none is found it is emitted to standard error.
- */
-void
-virLogEmergencyDumpAll(int signum)
-{
- int len;
- int oldLogStart, oldLogLen;
-
- switch (signum) {
-#ifdef SIGFPE
- case SIGFPE:
- virLogDumpAllFD("Caught signal Floating-point exception", -1);
- break;
-#endif
-#ifdef SIGSEGV
- case SIGSEGV:
- virLogDumpAllFD("Caught Segmentation violation", -1);
- break;
-#endif
-#ifdef SIGILL
- case SIGILL:
- virLogDumpAllFD("Caught illegal instruction", -1);
- break;
-#endif
-#ifdef SIGABRT
- case SIGABRT:
- virLogDumpAllFD("Caught abort signal", -1);
- break;
-#endif
-#ifdef SIGBUS
- case SIGBUS:
- virLogDumpAllFD("Caught bus error", -1);
- break;
-#endif
-#ifdef SIGUSR2
- case SIGUSR2:
- virLogDumpAllFD("Caught User-defined signal 2", -1);
- break;
-#endif
- default:
- virLogDumpAllFD("Caught unexpected signal", -1);
- break;
- }
- if ((virLogBuffer == NULL) || (virLogSize <= 0)) {
- virLogDumpAllFD(" internal log buffer deactivated\n", -1);
- return;
- }
-
- virLogDumpAllFD(" dumping internal log buffer:\n", -1);
- virLogDumpAllFD("\n\n ====== start of log =====\n\n", -1);
-
- /*
- * Since we can't lock the buffer safely from a signal handler
- * we mark it as empty in case of concurrent access, and proceed
- * with the data, at worse we will output something a bit weird
- * if another thread start logging messages at the same time.
- * Note that virLogStr() uses virLogEnd for the computations and
- * writes to the buffer and only then updates virLogLen and virLogStart
- * so it's best to reset it first.
- */
- oldLogStart = virLogStart;
- oldLogLen = virLogLen;
- virLogEnd = 0;
- virLogLen = 0;
- virLogStart = 0;
-
- while (oldLogLen > 0) {
- if (oldLogStart + oldLogLen < virLogSize) {
- virLogBuffer[oldLogStart + oldLogLen] = 0;
- virLogDumpAllFD(&virLogBuffer[oldLogStart], oldLogLen);
- oldLogStart += oldLogLen;
- oldLogLen = 0;
- } else {
- len = virLogSize - oldLogStart;
- virLogBuffer[virLogSize] = 0;
- virLogDumpAllFD(&virLogBuffer[oldLogStart], len);
- oldLogLen -= len;
- oldLogStart = 0;
- }
- }
- virLogDumpAllFD("\n\n ====== end of log =====\n\n", -1);
-}
-
-
/**
* virLogSetDefaultPriority:
* @priority: the default priority level
@@ -838,19 +587,12 @@ virLogVMessage(virLogSourcePtr source,
if (virTimeStringNowRaw(timestamp) < 0)
timestamp[0] = '\0';
+ virLogLock();
+
/*
- * Log based on defaults, first store in the history buffer,
- * then if emit push the message on the outputs defined, if none
+ * Push the message to the outputs defined, if none exist then
* use stderr.
- * NOTE: the locking is a single point of contention for multiple
- * threads, but avoid intermixing. Maybe set up locks per output
- * to improve paralellism.
*/
- virLogLock();
- virLogStr(timestamp);
- virLogStr(": ");
- virLogStr(msg);
-
for (i = 0; i < virLogNbOutputs; i++) {
if (priority >= virLogOutputs[i].priority) {
if (virLogOutputs[i].logVersion) {
diff --git a/src/util/virlog.h b/src/util/virlog.h
index b2d5edc..f62dd01 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -200,8 +200,6 @@ extern void virLogVMessage(virLogSourcePtr source,
virLogMetadataPtr metadata,
const char *fmt,
va_list vargs) ATTRIBUTE_FMT_PRINTF(7, 0);
-extern int virLogSetBufferSize(int size);
-extern void virLogEmergencyDumpAll(int signum);
bool virLogProbablyLogMessage(const char *str);
--
1.8.5.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list