From c0c8c1d7bb53e0420f8573ea961ef7e19b7fff85 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 3 Mar 2014 14:54:33 +0000 Subject: [PATCH] Remove global log buffer feature entirely 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. The log_buffer_size config parameter is left in the augeas lens to prevent breakage for users on upgrade. It is however completely ignored hereafter. Signed-off-by: Daniel P. Berrange --- daemon/libvirtd-config.c | 3 - daemon/libvirtd-config.h | 1 - daemon/libvirtd.c | 2 - daemon/libvirtd.conf | 10 +- docs/logging.html.in | 10 -- src/libvirt_private.syms | 2 - src/locking/lock_daemon.c | 2 - src/locking/lock_daemon_config.c | 2 - src/locking/lock_daemon_config.h | 1 - src/locking/virtlockd.conf | 10 +- src/rpc/virnetserver.c | 45 ------ src/util/virlog.c | 264 +------------------------------ src/util/virlog.h | 2 - tests/libvirtdconftest.c | 3 + 14 files changed, 16 insertions(+), 341 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index e8d1b4d50d..cff2a7ed1c 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -268,8 +268,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; @@ -433,7 +431,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 8d3d085ae2..c9969955b8 100644 --- a/daemon/libvirtd-config.h +++ b/daemon/libvirtd-config.h @@ -74,7 +74,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.c b/daemon/libvirtd.c index 4ee4316fb4..4179147e50 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -662,8 +662,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 64c215d6c3..aeba11d77c 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -349,11 +349,11 @@ #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 debug buffer size: +# +# This configuration option is no longer used, since the global +# log buffer functionality has been removed. Please configure +# suitable log_outputs/log_filters settings to obtain logs. #log_buffer_size = 64 diff --git a/docs/logging.html.in b/docs/logging.html.in index a8cb53846e..d8d8f1e899 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. -

Note that the logging module saves all logs to a debug buffer - 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).

Configuring logging in the library @@ -245,9 +239,5 @@ log_outputs="1:file:/var/log/libvirt/libvirtd.log"

in libvirtd.conf and restart the daemon will allow to gather a copious amount of debugging traces for the operations done in those areas.

-

On the other hand to deactivate the logbuffer in the daemon - for stable high load servers, set

- 
log_buffer_size=0
-

in the libvirtd.conf.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aaaac2f73b..3baf766bf8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1448,7 +1448,6 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h virLogDefineFilter; virLogDefineOutput; -virLogEmergencyDumpAll; virLogGetDefaultPriority; virLogGetFilters; virLogGetNbFilters; @@ -1462,7 +1461,6 @@ virLogParseOutputs; virLogPriorityFromSyslog; virLogProbablyLogMessage; virLogReset; -virLogSetBufferSize; virLogSetDefaultPriority; virLogSetFromEnv; virLogUnlock; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 35d3090eab..dbec7161b0 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 e90d87908b..42abc56558 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 e75d4a9ed3..1c79a4e591 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/virtlockd.conf b/src/locking/virtlockd.conf index 652e15604d..4c935d0a2c 100644 --- a/src/locking/virtlockd.conf +++ b/src/locking/virtlockd.conf @@ -52,11 +52,11 @@ #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 debug buffer size: +# +# This configuration option is no longer used, since the global +# log buffer functionality has been removed. Please configure +# suitable log_outputs/log_filters settings to obtain logs. #log_buffer_size = 64 # The maximum number of concurrent client connections to allow diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index e672755c12..cea2b2317b 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -341,34 +341,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, @@ -429,23 +401,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 21b82da667..5d4d3c7ee3 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #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,73 +196,12 @@ virLogOnceInit(void) } virLogUnlock(); - if (pbm) - VIR_WARN("%s", pbm); return 0; } 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: * @@ -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 @@ -840,19 +589,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 c73867ae74..5b38891b9a 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); diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c index 41c3e37419..bc22087b32 100644 --- a/tests/libvirtdconftest.c +++ b/tests/libvirtdconftest.c @@ -225,6 +225,9 @@ mymain(void) VIR_DEBUG("Initial config [%s]", filedata); for (i = 0; params[i] != 0; i++) { const struct testCorruptData data = { params, filedata, filename, i }; + /* Skip now ignored config param */ + if (STRPREFIX(filedata + params[i], "log_buffer_size")) + continue; if (virtTestRun("Test corruption", testCorrupt, &data) < 0) ret = -1; } -- 2.39.5