]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
Remove global log buffer feature entirely
authorDaniel P. Berrange <berrange@redhat.com>
Mon, 3 Mar 2014 14:54:33 +0000 (14:54 +0000)
committerDaniel P. Berrange <berrange@redhat.com>
Tue, 18 Mar 2014 14:29:22 +0000 (14:29 +0000)
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 <berrange@redhat.com>
14 files changed:
daemon/libvirtd-config.c
daemon/libvirtd-config.h
daemon/libvirtd.c
daemon/libvirtd.conf
docs/logging.html.in
src/libvirt_private.syms
src/locking/lock_daemon.c
src/locking/lock_daemon_config.c
src/locking/lock_daemon_config.h
src/locking/virtlockd.conf
src/rpc/virnetserver.c
src/util/virlog.c
src/util/virlog.h
tests/libvirtdconftest.c

index e8d1b4d50d63a9d48248a0d2662ccc4987277f49..cff2a7ed1c5feb3b95615561aa0f96f5d04febca 100644 (file)
@@ -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);
index 8d3d085ae25515164dec8faa2658511afd96e73d..c9969955b809cb988452be1842d5c64488d765e8 100644 (file)
@@ -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;
index 4ee4316fb448b937d8f3ba8acf83793fcc760c4e..4179147e50dc84ff67ed29f2bff967427bb4075e 100644 (file)
@@ -662,8 +662,6 @@ daemonSetupLogging(struct daemonConfig *config,
 
     virLogSetFromEnv();
 
-    virLogSetBufferSize(config->log_buffer_size);
-
     if (virLogGetNbFilters() == 0)
         virLogParseFilters(config->log_filters);
 
index 64c215d6c3b25cbb8bceb17b1438072b78f6dc7b..aeba11d77c2445e3b382d6d91cad547abddb192d 100644 (file)
 #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
 
 
index a8cb53846e03006b4adfd5b065b76f42e19c06d6..d8d8f1e89933eb9c6218f86932adccd0a871b210 100644 (file)
           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>
index aaaac2f73bccfb5ff9d545c98f594c9ca46d04ce..3baf766bf823724a97f5f17b272744b258316517 100644 (file)
@@ -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;
index 35d3090eab9a67ff46f5461faaa748bf385a96b8..dbec7161b0c951f5dca76789ddb90f4cc413171f 100644 (file)
@@ -478,8 +478,6 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
 
     virLogSetFromEnv();
 
-    virLogSetBufferSize(config->log_buffer_size);
-
     if (virLogGetNbFilters() == 0)
         virLogParseFilters(config->log_filters);
 
index e90d87908b56430eef09c77053256ed0000221a6..42abc565581acf3c7af35cfcf37a6de8838070a1 100644 (file)
@@ -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;
index e75d4a9ed3ab1477acb966c519f7426240fe10b2..1c79a4e591b3d894a760fa1589c6b93989ce441e 100644 (file)
@@ -33,7 +33,6 @@ struct _virLockDaemonConfig {
     int log_level;
     char *log_filters;
     char *log_outputs;
-    int log_buffer_size;
     int max_clients;
 };
 
index 652e15604d5cd3a9a336972760dca136b3a42308..4c935d0a2c1d2db1ce7ad959ababba8f95c0dc4e 100644 (file)
 #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
index e672755c124fec3d9dd4b08972283d7fbc453f01..cea2b2317b200eb2ae0a7f07bf60aacd6b24d5cf 100644 (file)
@@ -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:
index 21b82da6678a72eb01a3f5d9d6d63031094d2138..5d4d3c7ee3b25d53d70c6d3846358ce743308e21 100644 (file)
@@ -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
 
 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) {
index c73867ae749c7f997917244880bae45816d4087b..5b38891b9a7ca53e48db94e4bc542b193fb31966 100644 (file)
@@ -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);
 
index 41c3e37419af5fb1b8847f3671ad9f105de7bcf1..bc22087b32cf81c9d789105bc9b968b980773600 100644 (file)
@@ -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;
     }