Commit 94c465d0 refactored the logging setup phase but introduced an issue, where the daemon ignores verbose mode when there are no outputs defined and the default must be used. The problem is that the default output was determined too early, thus ignoring the potential '--verbose' option taking effect. This patch postpones the creation of the default output to the very last moment when nothing else can change. Since the default output is only created during the init phase, it's safe to leave the pointer as NULL for a while, but it will be set eventually, thus not affecting runtime. Patch also adjusts both the other daemons. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457193 Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/libvirtd.c | 21 +++++++++++++-------- src/locking/lock_daemon.c | 21 +++++++++++++-------- src/logging/log_daemon.c | 21 +++++++++++++-------- src/util/virlog.c | 7 +++++++ 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7e5d7af69..589b32192 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -615,19 +615,15 @@ daemonSetupLogging(struct daemonConfig *config, * Libvirtd's order of precedence is: * cmdline > environment > config * - * The default output is applied only if there was no setting from either - * the config or the environment. Because we don't have a way to determine - * if the log level has been set, we must process variables in the opposite + * Given the precedence, we must process the variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); - if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0) - return -1; - - /* In case the config is empty, the filters become empty and outputs will - * be set to default + /* In case the config is empty, both filters and outputs will become empty, + * however we can't start with empty outputs, thus we'll need to define and + * setup a default one. */ ignore_value(virLogSetFilters(config->log_filters)); ignore_value(virLogSetOutputs(config->log_outputs)); @@ -641,6 +637,15 @@ daemonSetupLogging(struct daemonConfig *config, if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); + /* Define the default output. This is only applied if there was no setting + * from either the config or the environment. + */ + if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0) + return -1; + + if (virLogGetNbOutputs() == 0) + virLogSetOutputs(virLogGetDefaultOutput()); + return 0; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 12485e966..6fbbf4b3d 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -460,19 +460,15 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, * Libvirtd's order of precedence is: * cmdline > environment > config * - * The default output is applied only if there was no setting from either - * the config or the environment. Because we don't have a way to determine - * if the log level has been set, we must process variables in the opposite + * Given the precedence, we must process the variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); - if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0) - return -1; - - /* In case the config is empty, the filters become empty and outputs will - * be set to default + /* In case the config is empty, both filters and outputs will become empty, + * however we can't start with empty outputs, thus we'll need to define and + * setup a default one. */ ignore_value(virLogSetFilters(config->log_filters)); ignore_value(virLogSetOutputs(config->log_outputs)); @@ -486,6 +482,15 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); + /* Define the default output. This is only applied if there was no setting + * from either the config or the environment. + */ + if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0) + return -1; + + if (virLogGetNbOutputs() == 0) + virLogSetOutputs(virLogGetDefaultOutput()); + return 0; } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index d878efa63..5a136c59d 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -388,19 +388,15 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, * Libvirtd's order of precedence is: * cmdline > environment > config * - * The default output is applied only if there was no setting from either - * the config or the environment. Because we don't have a way to determine - * if the log level has been set, we must process variables in the opposite + * Given the precedence, we must process the variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); - if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0) - return -1; - - /* In case the config is empty, the filters become empty and outputs will - * be set to default + /* In case the config is empty, both filters and outputs will become empty, + * however we can't start with empty outputs, thus we'll need to define and + * setup a default one. */ ignore_value(virLogSetFilters(config->log_filters)); ignore_value(virLogSetOutputs(config->log_outputs)); @@ -414,6 +410,15 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); + /* Define the default output. This is only applied if there was no setting + * from either the config or the environment. + */ + if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0) + return -1; + + if (virLogGetNbOutputs() == 0) + virLogSetOutputs(virLogGetDefaultOutput()); + return 0; } diff --git a/src/util/virlog.c b/src/util/virlog.c index 2228cf645..d45a451a7 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1840,6 +1840,13 @@ virLogSetOutputs(const char *src) if (src && *src) outputstr = src; + /* This can only happen during daemon init when the default output is not + * determined yet. It's safe to do, since it's the only place setting the + * default output. + */ + if (!outputstr) + return 0; + if ((noutputs = virLogParseOutputs(outputstr, &outputs)) < 0) goto cleanup; -- 2.13.3