From a6ecbf836c1e70cdf05a1ad6b78c86c5aef4dca3 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 30 May 2018 17:57:23 +0200 Subject: [PATCH 1/3] pid1: preserve current value of log level across re-{load,execution} To make debugging easier, this patches allows one to change the log level and do reload/reexec without modifying configuration permanently, which makes debugging easier. Indeed if one changed the log max level at runtime (via the bus or via signals), the change was lost on the next daemon reload/reexecution. In order to restore the original value back (set via system.conf, environment variables or any other means), the empty string in the "LogLevel" property is now supported as well as sending SIGRTMIN+23 signal. --- man/systemd.xml | 17 ++++++++------ src/core/dbus-manager.c | 18 +++++++++++---- src/core/main.c | 18 ++++++++++----- src/core/manager.c | 49 +++++++++++++++++++++++++++++++++++++---- src/core/manager.h | 6 +++++ 5 files changed, 88 insertions(+), 20 deletions(-) diff --git a/man/systemd.xml b/man/systemd.xml index b409ba7c015..3e3e8678bc1 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -749,15 +749,18 @@ SIGRTMIN+22 + + Sets the service manager's log level to debug, in a fashion equivalent to + systemd.log_level=debug on the kernel command line. + + + SIGRTMIN+23 - Sets the log level to debug - (or info on - SIGRTMIN+23), as controlled via - systemd.log_level=debug (or - systemd.log_level=info on - SIGRTMIN+23) on the kernel command - line. + Restores the log level to its configured value. The configured value is derived from – in order + of priority – the value specified with systemd.log-level= on the kernel command line, or the + value specified with in the configuration file, or the built-in default of + info. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 09daf268b7f..d97fe4087cd 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -158,6 +158,7 @@ static int property_set_log_level( void *userdata, sd_bus_error *error) { + Manager *m = userdata; const char *t; int r; @@ -168,10 +169,19 @@ static int property_set_log_level( if (r < 0) return r; - r = log_set_max_level_from_string(t); - if (r == 0) - log_info("Setting log level to %s.", t); - return r; + if (isempty(t)) + manager_restore_original_log_level(m); + else { + int level; + + level = log_level_from_string(t); + if (level < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid log level '%s'", t); + + manager_override_log_level(m, level); + } + + return 0; } static int property_get_progress( diff --git a/src/core/main.c b/src/core/main.c index 1e75ad7203d..d044427d745 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -525,10 +525,10 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return 0; \ } -DEFINE_SETTER(config_parse_level2, log_set_max_level_from_string, "log level") -DEFINE_SETTER(config_parse_target, log_set_target_from_string, "target") -DEFINE_SETTER(config_parse_color, log_show_color_from_string, "color" ) -DEFINE_SETTER(config_parse_location, log_show_location_from_string, "location") +DEFINE_SETTER(config_parse_level2, log_set_max_level_from_string, "log level"); +DEFINE_SETTER(config_parse_target, log_set_target_from_string, "target"); +DEFINE_SETTER(config_parse_color, log_show_color_from_string, "color" ); +DEFINE_SETTER(config_parse_location, log_show_location_from_string, "location"); static int config_parse_cpu_affinity2( const char *unit, @@ -1640,20 +1640,28 @@ static int invoke_main_loop( switch (m->exit_code) { - case MANAGER_RELOAD: + case MANAGER_RELOAD: { + int saved_log_level; + log_info("Reloading."); + saved_log_level = m->log_level_overridden ? log_get_max_level() : -1; + r = parse_config_file(); if (r < 0) log_warning_errno(r, "Failed to parse config file, ignoring: %m"); set_manager_defaults(m); + if (saved_log_level >= 0) + manager_override_log_level(m, saved_log_level); + r = manager_reload(m); if (r < 0) log_warning_errno(r, "Failed to reload, ignoring: %m"); break; + } case MANAGER_REEXECUTE: diff --git a/src/core/manager.c b/src/core/manager.c index e1ce9229a32..92d5a0fff6f 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -74,6 +74,7 @@ #include "string-util.h" #include "strv.h" #include "strxcpyx.h" +#include "syslog-util.h" #include "terminal-util.h" #include "time-util.h" #include "transaction.h" @@ -735,6 +736,7 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) { m->default_timeout_start_usec = DEFAULT_TIMEOUT_USEC; m->default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC; m->default_restart_usec = DEFAULT_RESTART_USEC; + m->original_log_level = -1; #if ENABLE_EFI if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) @@ -2626,13 +2628,11 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t break; case 22: - log_set_max_level(LOG_DEBUG); - log_info("Setting log level to debug."); + manager_override_log_level(m, LOG_DEBUG); break; case 23: - log_set_max_level(LOG_INFO); - log_info("Setting log level to info."); + manager_restore_original_log_level(m); break; case 24: @@ -3027,6 +3027,9 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { fprintf(f, "taint-logged=%s\n", yes_no(m->taint_logged)); fprintf(f, "service-watchdogs=%s\n", yes_no(m->service_watchdogs)); + if (m->log_level_overridden) + fprintf(f, "log-level-override=%i\n", log_get_max_level()); + for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { /* The userspace and finish timestamps only apply to the host system, hence only serialize them there */ if (in_initrd() && IN_SET(q, MANAGER_TIMESTAMP_USERSPACE, MANAGER_TIMESTAMP_FINISH)) @@ -3210,6 +3213,15 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { else m->service_watchdogs = b; + } else if ((val = startswith(l, "log-level-override="))) { + int level; + + level = log_level_from_string(val); + if (level < 0) + log_notice("Failed to parse log-level-override value '%s', ignoring.", val); + else + manager_override_log_level(m, level); + } else if (startswith(l, "env=")) { r = deserialize_environment(&m->environment, l); if (r == -ENOMEM) @@ -4448,6 +4460,35 @@ void manager_unref_console(Manager *m) { m->no_console_output = false; /* unset no_console_output flag, since the console is definitely free now */ } +void manager_override_log_level(Manager *m, int level) { + _cleanup_free_ char *s = NULL; + assert(m); + + if (!m->log_level_overridden) { + m->original_log_level = log_get_max_level(); + m->log_level_overridden = true; + } + + (void) log_level_to_string_alloc(level, &s); + log_info("Setting log level to %s.", strna(s)); + + log_set_max_level(level); +} + +void manager_restore_original_log_level(Manager *m) { + _cleanup_free_ char *s = NULL; + assert(m); + + if (!m->log_level_overridden) + return; + + (void) log_level_to_string_alloc(m->original_log_level, &s); + log_info("Restoring log level to original (%s).", strna(s)); + + log_set_max_level(m->original_log_level); + m->log_level_overridden = false; +} + static const char *const manager_state_table[_MANAGER_STATE_MAX] = { [MANAGER_INITIALIZING] = "initializing", [MANAGER_STARTING] = "starting", diff --git a/src/core/manager.h b/src/core/manager.h index 8868e9c158c..fb28316b8e5 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -302,6 +302,9 @@ struct Manager { uint64_t default_tasks_max; usec_t default_timer_accuracy_usec; + int original_log_level; + bool log_level_overridden:1; + struct rlimit *rlimit[_RLIMIT_MAX]; /* non-zero if we are reloading or reexecuting, */ @@ -465,6 +468,9 @@ char *manager_taint_string(Manager *m); void manager_ref_console(Manager *m); void manager_unref_console(Manager *m); +void manager_override_log_level(Manager *m, int level); +void manager_restore_original_log_level(Manager *m); + const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; From bda7d78ba178e70c2279261a77be83f755ad0e49 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 1 Jun 2018 18:21:03 +0200 Subject: [PATCH 2/3] pid1: preserve current value of log target across re-{load,execution} To make debugging easier, this patches allows one to change the log target and do reload/reexec without modifying configuration permanently, which makes debugging easier. Indeed if one changed the log target at runtime (via the bus or via signals), the change was lost on the next reload/reexecution. In order to restore back the default value (set via system.conf, environment variables or any other means ), the empty string in the "LogTarget" property is now supported as well as sending SIGTRMIN+26 signal. --- man/systemd.xml | 23 +++++++++++---------- src/core/dbus-manager.c | 15 +++++++++++++- src/core/main.c | 7 +++++++ src/core/manager.c | 45 +++++++++++++++++++++++++++++++++++------ src/core/manager.h | 5 +++++ 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/man/systemd.xml b/man/systemd.xml index 3e3e8678bc1..65c7fc2bb12 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -772,20 +772,21 @@ SIGRTMIN+26 + + Restores the log target to its configured value. The configured value is derived from – in + order of priority – the value specified with systemd.log-target= on the kernel command line, + or the value specified with in the configuration file, or the built-in + default. + + + SIGRTMIN+27 SIGRTMIN+28 - Sets the log target to - journal-or-kmsg (or - console on - SIGRTMIN+27, kmsg on - SIGRTMIN+28), as controlled via - systemd.log_target=journal-or-kmsg (or - systemd.log_target=console on - SIGRTMIN+27 or - systemd.log_target=kmsg on - SIGRTMIN+28) on the kernel command - line. + Sets the log target to console on SIGRTMIN+27 (or + kmsg on SIGRTMIN+28), in a fashion equivalent to + systemd.log_target=console (or systemd.log_target=kmsg on + SIGRTMIN+28) on the kernel command line. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index d97fe4087cd..016741e40b5 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -114,6 +114,7 @@ static int property_set_log_target( void *userdata, sd_bus_error *error) { + Manager *m = userdata; const char *t; int r; @@ -124,7 +125,19 @@ static int property_set_log_target( if (r < 0) return r; - return log_set_target_from_string(t); + if (isempty(t)) + manager_restore_original_log_target(m); + else { + LogTarget target; + + target = log_target_from_string(t); + if (target < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid log target '%s'", t); + + manager_override_log_target(m, target); + } + + return 0; } static int property_get_log_level( diff --git a/src/core/main.c b/src/core/main.c index d044427d745..ad1ae591e1e 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1641,11 +1641,16 @@ static int invoke_main_loop( switch (m->exit_code) { case MANAGER_RELOAD: { + LogTarget saved_log_target; int saved_log_level; log_info("Reloading."); + /* First, save any overriden log level/target, then parse the configuration file, which might + * change the log level to new settings. */ + saved_log_level = m->log_level_overridden ? log_get_max_level() : -1; + saved_log_target = m->log_target_overridden ? log_get_target() : _LOG_TARGET_INVALID; r = parse_config_file(); if (r < 0) @@ -1655,6 +1660,8 @@ static int invoke_main_loop( if (saved_log_level >= 0) manager_override_log_level(m, saved_log_level); + if (saved_log_target >= 0) + manager_override_log_target(m, saved_log_target); r = manager_reload(m); if (r < 0) diff --git a/src/core/manager.c b/src/core/manager.c index 92d5a0fff6f..a6b2edcb83f 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -737,6 +737,7 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) { m->default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC; m->default_restart_usec = DEFAULT_RESTART_USEC; m->original_log_level = -1; + m->original_log_target = _LOG_TARGET_INVALID; #if ENABLE_EFI if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) @@ -2646,18 +2647,15 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t case 26: case 29: /* compatibility: used to be mapped to LOG_TARGET_SYSLOG_OR_KMSG */ - log_set_target(LOG_TARGET_JOURNAL_OR_KMSG); - log_notice("Setting log target to journal-or-kmsg."); + manager_restore_original_log_target(m); break; case 27: - log_set_target(LOG_TARGET_CONSOLE); - log_notice("Setting log target to console."); + manager_override_log_target(m, LOG_TARGET_CONSOLE); break; case 28: - log_set_target(LOG_TARGET_KMSG); - log_notice("Setting log target to kmsg."); + manager_override_log_target(m, LOG_TARGET_KMSG); break; default: @@ -3029,6 +3027,8 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { if (m->log_level_overridden) fprintf(f, "log-level-override=%i\n", log_get_max_level()); + if (m->log_target_overridden) + fprintf(f, "log-target-override=%s\n", log_target_to_string(log_get_target())); for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { /* The userspace and finish timestamps only apply to the host system, hence only serialize them there */ @@ -3222,6 +3222,15 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { else manager_override_log_level(m, level); + } else if ((val = startswith(l, "log-target-override="))) { + LogTarget target; + + target = log_target_from_string(val); + if (target < 0) + log_notice("Failed to parse log-target-override value '%s', ignoring.", val); + else + manager_override_log_target(m, target); + } else if (startswith(l, "env=")) { r = deserialize_environment(&m->environment, l); if (r == -ENOMEM) @@ -4489,6 +4498,30 @@ void manager_restore_original_log_level(Manager *m) { m->log_level_overridden = false; } +void manager_override_log_target(Manager *m, LogTarget target) { + assert(m); + + if (!m->log_target_overridden) { + m->original_log_target = log_get_target(); + m->log_target_overridden = true; + } + + log_info("Setting log target to %s.", log_target_to_string(target)); + log_set_target(target); +} + +void manager_restore_original_log_target(Manager *m) { + assert(m); + + if (!m->log_target_overridden) + return; + + log_info("Restoring log target to original %s.", log_target_to_string(m->original_log_target)); + + log_set_target(m->original_log_target); + m->log_target_overridden = false; +} + static const char *const manager_state_table[_MANAGER_STATE_MAX] = { [MANAGER_INITIALIZING] = "initializing", [MANAGER_STARTING] = "starting", diff --git a/src/core/manager.h b/src/core/manager.h index fb28316b8e5..4555a4ff152 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -303,7 +303,9 @@ struct Manager { usec_t default_timer_accuracy_usec; int original_log_level; + LogTarget original_log_target; bool log_level_overridden:1; + bool log_target_overridden:1; struct rlimit *rlimit[_RLIMIT_MAX]; @@ -471,6 +473,9 @@ void manager_unref_console(Manager *m); void manager_override_log_level(Manager *m, int level); void manager_restore_original_log_level(Manager *m); +void manager_override_log_target(Manager *m, LogTarget target); +void manager_restore_original_log_target(Manager *m); + const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; From b5752d23521fddb3753291f58d1dbd4427cc2445 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jun 2018 18:47:13 +0200 Subject: [PATCH 3/3] main: simplify arg_system initialization a bit For both branches of the if check it's the first line, hence let's just do it before. --- src/core/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index ad1ae591e1e..f396a61e124 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2188,6 +2188,9 @@ int main(int argc, char *argv[]) { log_set_upgrade_syslog_to_journal(true); if (getpid_cached() == 1) { + /* When we run as PID 1 force system mode */ + arg_system = true; + /* Disable the umask logic */ umask(0); @@ -2204,7 +2207,6 @@ int main(int argc, char *argv[]) { if (detect_container() <= 0) { /* Running outside of a container as PID 1 */ - arg_system = true; log_set_target(LOG_TARGET_KMSG); log_open(); @@ -2243,7 +2245,6 @@ int main(int argc, char *argv[]) { } else { /* Running inside a container, as PID 1 */ - arg_system = true; log_set_target(LOG_TARGET_CONSOLE); log_open();