From 2cc856ac890cb19c46684590df972c07f00cb23f Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:30:48 +0200
Subject: [PATCH 01/24] main: minor coding style update

---
 src/core/main.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index ce45f2ded2..3b06001a8c 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1115,14 +1115,19 @@ static int help(void) {
         return 0;
 }
 
-static int prepare_reexecute(Manager *m, FILE **_f, FDSet **_fds, bool switching_root) {
+static int prepare_reexecute(
+                Manager *m,
+                FILE **ret_f,
+                FDSet **ret_fds,
+                bool switching_root) {
+
         _cleanup_fdset_free_ FDSet *fds = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(m);
-        assert(_f);
-        assert(_fds);
+        assert(ret_f);
+        assert(ret_fds);
 
         r = manager_open_serialization(m, &f);
         if (r < 0)
@@ -1151,8 +1156,8 @@ static int prepare_reexecute(Manager *m, FILE **_f, FDSet **_fds, bool switching
         if (r < 0)
                 return log_error_errno(r, "Failed to disable O_CLOEXEC for serialization fds: %m");
 
-        *_f = TAKE_PTR(f);
-        *_fds = TAKE_PTR(fds);
+        *ret_f = TAKE_PTR(f);
+        *ret_fds = TAKE_PTR(fds);
 
         return 0;
 }

From 899987456c8a14865f29cb7c62d95eb34f9736cc Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:32:35 +0200
Subject: [PATCH 02/24] manager: add explanatory comment regarding ManagerState

---
 src/core/manager.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/core/manager.h b/src/core/manager.h
index 23e8e48a4d..81ff52aa4b 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -23,6 +23,8 @@ typedef struct Unit Unit;
 
 typedef struct Manager Manager;
 
+/* An externally visible state. We don't actually maintain this as state variable, but derive it from various fields
+ * when requested */
 typedef enum ManagerState {
         MANAGER_INITIALIZING,
         MANAGER_STARTING,

From af41e5086d5ef78b033f639c63f957995eed9370 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:42:19 +0200
Subject: [PATCH 03/24] =?UTF-8?q?core:=20rename=20ManagerExitCode=20?=
 =?UTF-8?q?=E2=86=92=20ManagerObjective?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

"ExitCode" is a bit of a misnomer in two ways: it suggests this was
about the "exit code" concept that exit()/waitid() deal with, but really
isn't. Moreover, it's not event just about exiting either, but more
often about reloading/reexecing or rebooting. Let's hence pick a new
name for this that is a bit more correct.

I initially thought about naming this the "state", but that'd be a
misnomer too, as the value really encodes a "goal" more than a current
state. Also we already have the externally visible ManagerState.

No actual changes in behaviour, just the rename.
---
 src/core/automount.c        |  2 +-
 src/core/dbus-manager.c     | 16 ++++++++--------
 src/core/emergency-action.c |  6 +++---
 src/core/main.c             | 16 ++++++++--------
 src/core/manager.c          | 22 +++++++++++-----------
 src/core/manager.h          | 16 ++++++++--------
 6 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/src/core/automount.c b/src/core/automount.c
index 21b3887e67..70232f358d 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -85,7 +85,7 @@ static void unmount_autofs(Automount *a) {
         a->pipe_fd = safe_close(a->pipe_fd);
 
         /* If we reload/reexecute things we keep the mount point around */
-        if (!IN_SET(UNIT(a)->manager->exit_code, MANAGER_RELOAD, MANAGER_REEXECUTE)) {
+        if (!IN_SET(UNIT(a)->manager->objective, MANAGER_RELOAD, MANAGER_REEXECUTE)) {
 
                 automount_send_ready(a, a->tokens, -EHOSTDOWN);
                 automount_send_ready(a, a->expire_tokens, -EHOSTDOWN);
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 1d5237614e..480a143091 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1334,7 +1334,7 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *
         if (r < 0)
                 return r;
 
-        m->exit_code = MANAGER_RELOAD;
+        m->objective = MANAGER_RELOAD;
 
         return 1;
 }
@@ -1363,7 +1363,7 @@ static int method_reexecute(sd_bus_message *message, void *userdata, sd_bus_erro
         /* We don't send a reply back here, the client should
          * just wait for us disconnecting. */
 
-        m->exit_code = MANAGER_REEXECUTE;
+        m->objective = MANAGER_REEXECUTE;
         return 1;
 }
 
@@ -1383,7 +1383,7 @@ static int method_exit(sd_bus_message *message, void *userdata, sd_bus_error *er
          * systemd-shutdown if it cannot do the exit() because it isn't a
          * container. */
 
-        m->exit_code = MANAGER_EXIT;
+        m->objective = MANAGER_EXIT;
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -1402,7 +1402,7 @@ static int method_reboot(sd_bus_message *message, void *userdata, sd_bus_error *
         if (!MANAGER_IS_SYSTEM(m))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Reboot is only supported for system managers.");
 
-        m->exit_code = MANAGER_REBOOT;
+        m->objective = MANAGER_REBOOT;
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -1421,7 +1421,7 @@ static int method_poweroff(sd_bus_message *message, void *userdata, sd_bus_error
         if (!MANAGER_IS_SYSTEM(m))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Powering off is only supported for system managers.");
 
-        m->exit_code = MANAGER_POWEROFF;
+        m->objective = MANAGER_POWEROFF;
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -1440,7 +1440,7 @@ static int method_halt(sd_bus_message *message, void *userdata, sd_bus_error *er
         if (!MANAGER_IS_SYSTEM(m))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Halt is only supported for system managers.");
 
-        m->exit_code = MANAGER_HALT;
+        m->objective = MANAGER_HALT;
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -1459,7 +1459,7 @@ static int method_kexec(sd_bus_message *message, void *userdata, sd_bus_error *e
         if (!MANAGER_IS_SYSTEM(m))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "KExec is only supported for system managers.");
 
-        m->exit_code = MANAGER_KEXEC;
+        m->objective = MANAGER_KEXEC;
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -1549,7 +1549,7 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
         free(m->switch_root_init);
         m->switch_root_init = ri;
 
-        m->exit_code = MANAGER_SWITCH_ROOT;
+        m->objective = MANAGER_SWITCH_ROOT;
 
         return sd_bus_reply_method_return(message, NULL);
 }
diff --git a/src/core/emergency-action.c b/src/core/emergency-action.c
index 5420db8a87..6f0b0a8f33 100644
--- a/src/core/emergency-action.c
+++ b/src/core/emergency-action.c
@@ -41,7 +41,7 @@ int emergency_action(
                  * in user mode */
 
                 log_warning("Exiting: %s", reason);
-                m->exit_code = MANAGER_EXIT;
+                m->objective = MANAGER_EXIT;
                 return -ECANCELED;
         }
 
@@ -59,7 +59,7 @@ int emergency_action(
                 log_and_status(m, "Forcibly rebooting", reason);
 
                 (void) update_reboot_parameter_and_warn(reboot_arg);
-                m->exit_code = MANAGER_REBOOT;
+                m->objective = MANAGER_REBOOT;
 
                 break;
 
@@ -85,7 +85,7 @@ int emergency_action(
 
         case EMERGENCY_ACTION_POWEROFF_FORCE:
                 log_and_status(m, "Forcibly powering off", reason);
-                m->exit_code = MANAGER_POWEROFF;
+                m->objective = MANAGER_POWEROFF;
                 break;
 
         case EMERGENCY_ACTION_POWEROFF_IMMEDIATE:
diff --git a/src/core/main.c b/src/core/main.c
index 3b06001a8c..2a2fe89c28 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1673,7 +1673,7 @@ static int invoke_main_loop(
                         return log_emergency_errno(r, "Failed to run main loop: %m");
                 }
 
-                switch (m->exit_code) {
+                switch (m->objective) {
 
                 case MANAGER_RELOAD: {
                         LogTarget saved_log_target;
@@ -1764,19 +1764,19 @@ static int invoke_main_loop(
                 case MANAGER_POWEROFF:
                 case MANAGER_HALT:
                 case MANAGER_KEXEC: {
-                        static const char * const table[_MANAGER_EXIT_CODE_MAX] = {
-                                [MANAGER_EXIT] = "exit",
-                                [MANAGER_REBOOT] = "reboot",
+                        static const char * const table[_MANAGER_OBJECTIVE_MAX] = {
+                                [MANAGER_EXIT]     = "exit",
+                                [MANAGER_REBOOT]   = "reboot",
                                 [MANAGER_POWEROFF] = "poweroff",
-                                [MANAGER_HALT] = "halt",
-                                [MANAGER_KEXEC] = "kexec"
+                                [MANAGER_HALT]     = "halt",
+                                [MANAGER_KEXEC]    = "kexec",
                         };
 
                         log_notice("Shutting down.");
 
                         *ret_reexecute = false;
                         *ret_retval = m->return_value;
-                        assert_se(*ret_shutdown_verb = table[m->exit_code]);
+                        assert_se(*ret_shutdown_verb = table[m->objective]);
                         *ret_fds = NULL;
                         *ret_switch_root_dir = *ret_switch_root_init = NULL;
 
@@ -1784,7 +1784,7 @@ static int invoke_main_loop(
                 }
 
                 default:
-                        assert_not_reached("Unknown exit code.");
+                        assert_not_reached("Unknown or unexpected manager objective.");
                 }
         }
 }
diff --git a/src/core/manager.c b/src/core/manager.c
index c361a1ec07..76a0190cb8 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -723,7 +723,7 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) {
                 return -ENOMEM;
 
         m->unit_file_scope = scope;
-        m->exit_code = _MANAGER_EXIT_CODE_INVALID;
+        m->objective = _MANAGER_OBJECTIVE_INVALID;
         m->default_timer_accuracy_usec = USEC_PER_MINUTE;
         m->default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT;
         m->default_tasks_accounting = true;
@@ -1288,7 +1288,7 @@ Manager* manager_free(Manager *m) {
                         unit_vtable[c]->shutdown(m);
 
         /* If we reexecute ourselves, we keep the root cgroup around */
-        manager_shutdown_cgroup(m, m->exit_code != MANAGER_REEXECUTE);
+        manager_shutdown_cgroup(m, m->objective != MANAGER_REEXECUTE);
 
         lookup_paths_flush_generator(&m->lookup_paths);
 
@@ -2545,7 +2545,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
                         /* This is for compatibility with the original sysvinit */
                         r = verify_run_space_and_log("Refusing to reexecute");
                         if (r >= 0)
-                                m->exit_code = MANAGER_REEXECUTE;
+                                m->objective = MANAGER_REEXECUTE;
                         break;
                 }
 
@@ -2603,7 +2603,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
         case SIGHUP:
                 r = verify_run_space_and_log("Refusing to reload");
                 if (r >= 0)
-                        m->exit_code = MANAGER_RELOAD;
+                        m->objective = MANAGER_RELOAD;
                 break;
 
         default: {
@@ -2623,7 +2623,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
                 };
 
                 /* Starting SIGRTMIN+13, so that target halt and system halt are 10 apart */
-                static const ManagerExitCode code_table[] = {
+                static const ManagerObjective objective_table[] = {
                         [0] = MANAGER_HALT,
                         [1] = MANAGER_POWEROFF,
                         [2] = MANAGER_REBOOT,
@@ -2639,8 +2639,8 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
                 }
 
                 if ((int) sfsi.ssi_signo >= SIGRTMIN+13 &&
-                    (int) sfsi.ssi_signo < SIGRTMIN+13+(int) ELEMENTSOF(code_table)) {
-                        m->exit_code = code_table[sfsi.ssi_signo - SIGRTMIN - 13];
+                    (int) sfsi.ssi_signo < SIGRTMIN+13+(int) ELEMENTSOF(objective_table)) {
+                        m->objective = objective_table[sfsi.ssi_signo - SIGRTMIN - 13];
                         break;
                 }
 
@@ -2664,7 +2664,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
 
                 case 24:
                         if (MANAGER_IS_USER(m)) {
-                                m->exit_code = MANAGER_EXIT;
+                                m->objective = MANAGER_EXIT;
                                 return 0;
                         }
 
@@ -2793,7 +2793,7 @@ int manager_loop(Manager *m) {
         RATELIMIT_DEFINE(rl, 1*USEC_PER_SEC, 50000);
 
         assert(m);
-        m->exit_code = MANAGER_OK;
+        m->objective = MANAGER_OK;
 
         /* Release the path cache */
         m->unit_path_cache = set_free_free(m->unit_path_cache);
@@ -2805,7 +2805,7 @@ int manager_loop(Manager *m) {
         if (r < 0)
                 return log_error_errno(r, "Failed to enable SIGCHLD event source: %m");
 
-        while (m->exit_code == MANAGER_OK) {
+        while (m->objective == MANAGER_OK) {
                 usec_t wait_usec;
 
                 if (m->runtime_watchdog > 0 && m->runtime_watchdog != USEC_INFINITY && MANAGER_IS_SYSTEM(m))
@@ -2851,7 +2851,7 @@ int manager_loop(Manager *m) {
                         return log_error_errno(r, "Failed to run event loop: %m");
         }
 
-        return m->exit_code;
+        return m->objective;
 }
 
 int manager_load_unit_from_dbus_path(Manager *m, const char *s, sd_bus_error *e, Unit **_u) {
diff --git a/src/core/manager.h b/src/core/manager.h
index 81ff52aa4b..a26ffda3a5 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -36,7 +36,7 @@ typedef enum ManagerState {
         _MANAGER_STATE_INVALID = -1
 } ManagerState;
 
-typedef enum ManagerExitCode {
+typedef enum ManagerObjective {
         MANAGER_OK,
         MANAGER_EXIT,
         MANAGER_RELOAD,
@@ -46,9 +46,9 @@ typedef enum ManagerExitCode {
         MANAGER_HALT,
         MANAGER_KEXEC,
         MANAGER_SWITCH_ROOT,
-        _MANAGER_EXIT_CODE_MAX,
-        _MANAGER_EXIT_CODE_INVALID = -1
-} ManagerExitCode;
+        _MANAGER_OBJECTIVE_MAX,
+        _MANAGER_OBJECTIVE_INVALID = -1
+} ManagerObjective;
 
 typedef enum StatusType {
         STATUS_TYPE_EPHEMERAL,
@@ -282,9 +282,9 @@ struct Manager {
         usec_t etc_localtime_mtime;
         bool etc_localtime_accessible:1;
 
-        /* Flags */
-        ManagerExitCode exit_code:5;
+        ManagerObjective objective:5;
 
+        /* Flags */
         bool dispatching_load_queue:1;
         bool dispatching_dbus_queue:1;
 
@@ -407,8 +407,8 @@ struct Manager {
 
 #define MANAGER_IS_FINISHED(m) (dual_timestamp_is_set((m)->timestamps + MANAGER_TIMESTAMP_FINISH))
 
-/* The exit code is set to OK as soon as we enter the main loop, and set otherwise as soon as we are done with it */
-#define MANAGER_IS_RUNNING(m) ((m)->exit_code == MANAGER_OK)
+/* The objective is set to OK as soon as we enter the main loop, and set otherwise as soon as we are done with it */
+#define MANAGER_IS_RUNNING(m) ((m)->objective == MANAGER_OK)
 
 int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **m);
 Manager* manager_free(Manager *m);

From ed4ac965fac18bfb4f030ea82159384c3403ab05 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:45:05 +0200
Subject: [PATCH 04/24] manager: rework test flags set

No reason to avoid bit 0.

Also, fix some tests that pass "true" as flags value, which is just
wrong.
---
 src/core/manager.h        | 8 ++++----
 src/test/test-bpf.c       | 2 +-
 src/test/test-watch-pid.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/core/manager.h b/src/core/manager.h
index a26ffda3a5..13110b3b63 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -110,10 +110,10 @@ typedef enum ManagerTimestamp {
 
 enum {
         /* 0 = run normally */
-        MANAGER_TEST_RUN_MINIMAL        = 1 << 1,  /* create basic data structures */
-        MANAGER_TEST_RUN_BASIC          = 1 << 2,  /* interact with the environment */
-        MANAGER_TEST_RUN_ENV_GENERATORS = 1 << 3,  /* also run env generators  */
-        MANAGER_TEST_RUN_GENERATORS     = 1 << 4,  /* also run unit generators */
+        MANAGER_TEST_RUN_MINIMAL        = 1 << 0,  /* create basic data structures */
+        MANAGER_TEST_RUN_BASIC          = 1 << 1,  /* interact with the environment */
+        MANAGER_TEST_RUN_ENV_GENERATORS = 1 << 2,  /* also run env generators  */
+        MANAGER_TEST_RUN_GENERATORS     = 1 << 3,  /* also run unit generators */
         MANAGER_TEST_FULL = MANAGER_TEST_RUN_BASIC | MANAGER_TEST_RUN_ENV_GENERATORS | MANAGER_TEST_RUN_GENERATORS,
 };
 assert_cc((MANAGER_TEST_FULL & UINT8_MAX) == MANAGER_TEST_FULL);
diff --git a/src/test/test-bpf.c b/src/test/test-bpf.c
index 2fb7968dfd..ea5f0f5bc6 100644
--- a/src/test/test-bpf.c
+++ b/src/test/test-bpf.c
@@ -63,7 +63,7 @@ int main(int argc, char *argv[]) {
 
         /* The simple tests suceeded. Now let's try full unit-based use-case. */
 
-        assert_se(manager_new(UNIT_FILE_USER, true, &m) >= 0);
+        assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0);
         assert_se(manager_startup(m, NULL, NULL) >= 0);
 
         assert_se(u = unit_new(m, sizeof(Service)));
diff --git a/src/test/test-watch-pid.c b/src/test/test-watch-pid.c
index 03378ecf08..2c6ca0a1a2 100644
--- a/src/test/test-watch-pid.c
+++ b/src/test/test-watch-pid.c
@@ -24,7 +24,7 @@ int main(int argc, char *argv[]) {
         assert_se(set_unit_path(get_testdata_dir()) >= 0);
         assert_se(runtime_dir = setup_fake_runtime_dir());
 
-        assert_se(manager_new(UNIT_FILE_USER, true, &m) >= 0);
+        assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0);
         assert_se(manager_startup(m, NULL, NULL) >= 0);
 
         assert_se(a = unit_new(m, sizeof(Service)));

From 3ad228ce75a41925c399f3017355fa4b396ee447 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:54:10 +0200
Subject: [PATCH 05/24] core: use structure initialization for Manager

No changes in behaviour, just a nicer way to fill in the Manager
initially.
---
 src/core/manager.c | 62 ++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 76a0190cb8..f0261a16e2 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -718,21 +718,44 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) {
         assert(_m);
         assert(IN_SET(scope, UNIT_FILE_SYSTEM, UNIT_FILE_USER));
 
-        m = new0(Manager, 1);
+        m = new(Manager, 1);
         if (!m)
                 return -ENOMEM;
 
-        m->unit_file_scope = scope;
-        m->objective = _MANAGER_OBJECTIVE_INVALID;
-        m->default_timer_accuracy_usec = USEC_PER_MINUTE;
-        m->default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT;
-        m->default_tasks_accounting = true;
-        m->default_tasks_max = UINT64_MAX;
-        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;
-        m->original_log_target = _LOG_TARGET_INVALID;
+        *m = (Manager) {
+                .unit_file_scope = scope,
+                .objective = _MANAGER_OBJECTIVE_INVALID,
+
+                .default_timer_accuracy_usec = USEC_PER_MINUTE,
+                .default_memory_accounting = MEMORY_ACCOUNTING_DEFAULT,
+                .default_tasks_accounting = true,
+                .default_tasks_max = UINT64_MAX,
+                .default_timeout_start_usec = DEFAULT_TIMEOUT_USEC,
+                .default_timeout_stop_usec = DEFAULT_TIMEOUT_USEC,
+                .default_restart_usec = DEFAULT_RESTART_USEC,
+
+                .original_log_level = -1,
+                .original_log_target = _LOG_TARGET_INVALID,
+
+                .notify_fd = -1,
+                .cgroups_agent_fd = -1,
+                .signal_fd = -1,
+                .time_change_fd = -1,
+                .user_lookup_fds = { -1, -1 },
+                .private_listen_fd = -1,
+                .dev_autofs_fd = -1,
+                .cgroup_inotify_fd = -1,
+                .pin_cgroupfs_fd = -1,
+                .ask_password_inotify_fd = -1,
+                .idle_pipe = { -1, -1, -1, -1},
+
+                 /* start as id #1, so that we can leave #0 around as "null-like" value */
+                .current_job_id = 1,
+
+                .have_ask_password = -EINVAL, /* we don't know */
+                .first_boot = -1,
+                .test_run_flags = test_run_flags,
+        };
 
 #if ENABLE_EFI
         if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0)
@@ -756,21 +779,6 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) {
                 m->invocation_log_format_string = "USER_INVOCATION_ID=%s";
         }
 
-        m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = m->idle_pipe[3] = -1;
-
-        m->pin_cgroupfs_fd = m->notify_fd = m->cgroups_agent_fd = m->signal_fd = m->time_change_fd =
-                m->dev_autofs_fd = m->private_listen_fd = m->cgroup_inotify_fd =
-                m->ask_password_inotify_fd = -1;
-
-        m->user_lookup_fds[0] = m->user_lookup_fds[1] = -1;
-
-        m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */
-
-        m->have_ask_password = -EINVAL; /* we don't know */
-        m->first_boot = -1;
-
-        m->test_run_flags = test_run_flags;
-
         /* Reboot immediately if the user hits C-A-D more often than 7x per 2s */
         RATELIMIT_INIT(m->ctrl_alt_del_ratelimit, 2 * USEC_PER_SEC, 7);
 

From 86036b26a1da4d90ca274d37e77c56980ab689fd Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 15:56:27 +0200
Subject: [PATCH 06/24] core: tiny tweak for cgroup trimming during
 manager_free()

Instead of blacklisting when not to trim the cgroup tree, let's instead
whitelist when to do it, as an excercise of being careful when being
destructive.

This should not change behaviour with exception that during switch roots
we now won't attempt to trim the cgroup tree anymore. Which is more
correct behaviour after all we serialize/deserialize during the
transition and should be needlessly destructive.
---
 src/core/manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index f0261a16e2..0f2bff0cac 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1283,8 +1283,8 @@ static void manager_clear_jobs_and_units(Manager *m) {
 }
 
 Manager* manager_free(Manager *m) {
-        UnitType c;
         ExecDirectoryType dt;
+        UnitType c;
 
         if (!m)
                 return NULL;
@@ -1295,8 +1295,8 @@ Manager* manager_free(Manager *m) {
                 if (unit_vtable[c]->shutdown)
                         unit_vtable[c]->shutdown(m);
 
-        /* If we reexecute ourselves, we keep the root cgroup around */
-        manager_shutdown_cgroup(m, m->objective != MANAGER_REEXECUTE);
+        /* Keep the cgroup hierarchy in place except when we know we are going down for good */
+        manager_shutdown_cgroup(m, IN_SET(m->objective, MANAGER_EXIT, MANAGER_REBOOT, MANAGER_POWEROFF, MANAGER_HALT, MANAGER_KEXEC));
 
         lookup_paths_flush_generator(&m->lookup_paths);
 

From c52b19d65fa9e701d8f1625ef2d26bbd5b4fa3fe Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:02:31 +0200
Subject: [PATCH 07/24] manager: normalize /run disk space checks

Let's avoid using a variable needlessly. More importantly, special case
the error, not the regular case.
---
 src/core/manager.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 0f2bff0cac..d9e9dddea3 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2551,9 +2551,10 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
         case SIGTERM:
                 if (MANAGER_IS_SYSTEM(m)) {
                         /* This is for compatibility with the original sysvinit */
-                        r = verify_run_space_and_log("Refusing to reexecute");
-                        if (r >= 0)
-                                m->objective = MANAGER_REEXECUTE;
+                        if (verify_run_space_and_log("Refusing to reexecute") < 0)
+                                break;
+
+                        m->objective = MANAGER_REEXECUTE;
                         break;
                 }
 
@@ -2609,9 +2610,10 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
         }
 
         case SIGHUP:
-                r = verify_run_space_and_log("Refusing to reload");
-                if (r >= 0)
-                        m->objective = MANAGER_RELOAD;
+                if (verify_run_space_and_log("Refusing to reload") < 0)
+                        break;
+
+                m->objective = MANAGER_RELOAD;
                 break;
 
         default: {

From 638cece45da5a6e5fc7c410b619f35abb8df3ec7 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:15:54 +0200
Subject: [PATCH 08/24] core: clean up test run flags

Let's make them typesafe, and let's add a nice macro helper for checking
if we are in a test run, which should make testing for this much easier
to read for most cases.
---
 src/analyze/analyze-verify.c |  7 ++++---
 src/core/cgroup.c            |  6 +++---
 src/core/manager.c           | 26 +++++++++++++-------------
 src/core/manager.h           | 13 ++++++++-----
 src/core/unit.c              |  2 +-
 5 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c
index ed369532d4..d2f9e5528e 100644
--- a/src/analyze/analyze-verify.c
+++ b/src/analyze/analyze-verify.c
@@ -225,9 +225,10 @@ static int verify_unit(Unit *u, bool check_man) {
 }
 
 int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) {
-        const uint8_t flags = MANAGER_TEST_RUN_BASIC |
-                              MANAGER_TEST_RUN_ENV_GENERATORS |
-                              run_generators * MANAGER_TEST_RUN_GENERATORS;
+        const ManagerTestRunFlags flags =
+                MANAGER_TEST_RUN_BASIC |
+                MANAGER_TEST_RUN_ENV_GENERATORS |
+                run_generators * MANAGER_TEST_RUN_GENERATORS;
 
         _cleanup_(manager_freep) Manager *m = NULL;
         Unit *units[strv_length(filenames)];
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 7e07b2f13c..6a6766f6fa 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -2305,7 +2305,7 @@ int manager_setup_cgroup(Manager *m) {
 
                 (void) sd_event_source_set_description(m->cgroup_inotify_event_source, "cgroup-inotify");
 
-        } else if (MANAGER_IS_SYSTEM(m) && m->test_run_flags == 0) {
+        } else if (MANAGER_IS_SYSTEM(m) && !MANAGER_IS_TEST_RUN(m)) {
 
                 /* On the legacy hierarchy we only get notifications via cgroup agents. (Which isn't really reliable,
                  * since it does not generate events when control groups with children run empty. */
@@ -2334,11 +2334,11 @@ int manager_setup_cgroup(Manager *m) {
                 if (m->pin_cgroupfs_fd < 0)
                         return log_error_errno(errno, "Failed to open pin file: %m");
 
-        } else if (!m->test_run_flags)
+        } else if (!MANAGER_IS_TEST_RUN(m))
                 return log_error_errno(r, "Failed to create %s control group: %m", scope_path);
 
         /* 7. Always enable hierarchical support if it exists... */
-        if (!all_unified && m->test_run_flags == 0)
+        if (!all_unified && !MANAGER_IS_TEST_RUN(m))
                 (void) cg_set_attribute("memory", "/", "memory.use_hierarchy", "1");
 
         /* 8. Figure out which controllers are supported, and log about it */
diff --git a/src/core/manager.c b/src/core/manager.c
index d9e9dddea3..ba16e8259e 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -351,7 +351,7 @@ static int manager_setup_time_change(Manager *m) {
 
         assert(m);
 
-        if (m->test_run_flags)
+        if (MANAGER_IS_TEST_RUN(m))
                 return 0;
 
         m->time_change_event_source = sd_event_source_unref(m->time_change_event_source);
@@ -407,7 +407,7 @@ static int manager_setup_timezone_change(Manager *m) {
 
         assert(m);
 
-        if (m->test_run_flags != 0)
+        if (MANAGER_IS_TEST_RUN(m))
                 return 0;
 
         /* We watch /etc/localtime for three events: change of the link count (which might mean removal from /etc even
@@ -446,7 +446,7 @@ static int enable_special_signals(Manager *m) {
 
         assert(m);
 
-        if (m->test_run_flags)
+        if (MANAGER_IS_TEST_RUN(m))
                 return 0;
 
         /* Enable that we get SIGINT on control-alt-del. In containers
@@ -711,7 +711,7 @@ static int manager_setup_sigchld_event_source(Manager *m) {
         return 0;
 }
 
-int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) {
+int manager_new(UnitFileScope scope, ManagerTestRunFlags test_run_flags, Manager **_m) {
         _cleanup_(manager_freep) Manager *m = NULL;
         int r;
 
@@ -865,7 +865,7 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) {
 static int manager_setup_notify(Manager *m) {
         int r;
 
-        if (m->test_run_flags)
+        if (MANAGER_IS_TEST_RUN(m))
                 return 0;
 
         if (m->notify_fd < 0) {
@@ -944,7 +944,7 @@ static int manager_setup_cgroups_agent(Manager *m) {
          * to it. The system instance hence listens on this special socket, but the user instances listen on the system
          * bus for these messages. */
 
-        if (m->test_run_flags)
+        if (MANAGER_IS_TEST_RUN(m))
                 return 0;
 
         if (!MANAGER_IS_SYSTEM(m))
@@ -1521,7 +1521,7 @@ static bool manager_dbus_is_running(Manager *m, bool deserialized) {
          * and the service unit. If the 'deserialized' parameter is true we'll check the deserialized state of the unit
          * rather than the current one. */
 
-        if (m->test_run_flags != 0)
+        if (MANAGER_IS_TEST_RUN(m))
                 return false;
 
         u = manager_get_unit(m, SPECIAL_DBUS_SOCKET);
@@ -1569,7 +1569,7 @@ static void manager_preset_all(Manager *m) {
         if (!MANAGER_IS_SYSTEM(m))
                 return;
 
-        if (m->test_run_flags != 0)
+        if (MANAGER_IS_TEST_RUN(m))
                 return;
 
         /* If this is the first boot, and we are in the host system, then preset everything */
@@ -1589,7 +1589,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         /* If we are running in test mode, we still want to run the generators,
          * but we should not touch the real generator directories. */
         r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope,
-                              m->test_run_flags ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0,
+                              MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0,
                               NULL);
         if (r < 0)
                 return r;
@@ -3593,7 +3593,7 @@ static void manager_notify_finished(Manager *m) {
         char userspace[FORMAT_TIMESPAN_MAX], initrd[FORMAT_TIMESPAN_MAX], kernel[FORMAT_TIMESPAN_MAX], sum[FORMAT_TIMESPAN_MAX];
         usec_t firmware_usec, loader_usec, kernel_usec, initrd_usec, userspace_usec, total_usec;
 
-        if (m->test_run_flags)
+        if (MANAGER_IS_TEST_RUN(m))
                 return;
 
         if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) {
@@ -3788,7 +3788,7 @@ static int manager_run_environment_generators(Manager *m) {
         const char **paths;
         void* args[] = {&tmp, &tmp, &m->environment};
 
-        if (m->test_run_flags && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS))
+        if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS))
                 return 0;
 
         paths = MANAGER_IS_SYSTEM(m) ? system_env_generator_binary_paths : user_env_generator_binary_paths;
@@ -3806,7 +3806,7 @@ static int manager_run_generators(Manager *m) {
 
         assert(m);
 
-        if (m->test_run_flags && !(m->test_run_flags & MANAGER_TEST_RUN_GENERATORS))
+        if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_GENERATORS))
                 return 0;
 
         paths = generator_binary_paths(m->unit_file_scope);
@@ -3920,7 +3920,7 @@ static bool manager_journal_is_running(Manager *m) {
 
         assert(m);
 
-        if (m->test_run_flags != 0)
+        if (MANAGER_IS_TEST_RUN(m))
                 return false;
 
         /* If we are the user manager we can safely assume that the journal is up */
diff --git a/src/core/manager.h b/src/core/manager.h
index 13110b3b63..6e6e251a55 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -108,14 +108,15 @@ typedef enum ManagerTimestamp {
 #include "show-status.h"
 #include "unit-name.h"
 
-enum {
-        /* 0 = run normally */
+typedef enum ManagerTestRunFlags {
+        MANAGER_TEST_NORMAL             = 0,       /* run normally */
         MANAGER_TEST_RUN_MINIMAL        = 1 << 0,  /* create basic data structures */
         MANAGER_TEST_RUN_BASIC          = 1 << 1,  /* interact with the environment */
         MANAGER_TEST_RUN_ENV_GENERATORS = 1 << 2,  /* also run env generators  */
         MANAGER_TEST_RUN_GENERATORS     = 1 << 3,  /* also run unit generators */
         MANAGER_TEST_FULL = MANAGER_TEST_RUN_BASIC | MANAGER_TEST_RUN_ENV_GENERATORS | MANAGER_TEST_RUN_GENERATORS,
-};
+} ManagerTestRunFlags;
+
 assert_cc((MANAGER_TEST_FULL & UINT8_MAX) == MANAGER_TEST_FULL);
 
 struct Manager {
@@ -299,7 +300,7 @@ struct Manager {
         /* Have we ever changed the "kernel.pid_max" sysctl? */
         bool sysctl_pid_max_changed:1;
 
-        unsigned test_run_flags:8;
+        ManagerTestRunFlags test_run_flags:8;
 
         /* If non-zero, exit with the following value when the systemd
          * process terminate. Useful for containers: systemd-nspawn could get
@@ -410,7 +411,9 @@ struct Manager {
 /* The objective is set to OK as soon as we enter the main loop, and set otherwise as soon as we are done with it */
 #define MANAGER_IS_RUNNING(m) ((m)->objective == MANAGER_OK)
 
-int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **m);
+#define MANAGER_IS_TEST_RUN(m) ((m)->test_run_flags != 0)
+
+int manager_new(UnitFileScope scope, ManagerTestRunFlags test_run_flags, Manager **m);
 Manager* manager_free(Manager *m);
 DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
 
diff --git a/src/core/unit.c b/src/core/unit.c
index 853db527f7..db5dd3e884 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5283,7 +5283,7 @@ void unit_export_state_files(Unit *u) {
         if (!MANAGER_IS_SYSTEM(u->manager))
                 return;
 
-        if (u->manager->test_run_flags != 0)
+        if (MANAGER_IS_TEST_RUN(u->manager))
                 return;
 
         /* Exports a couple of unit properties to /run/systemd/units/, so that journald can quickly query this data

From fc37d875a1c6cb00379399adb67f67858ee95403 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:44:04 +0200
Subject: [PATCH 09/24] path-lookup: downgrade logging in lookup_paths_init() a
 bit

So far lookup_paths_init() did not log on its own beyond LOG_DEBUG,
fix one exception, in order to avoid duplicate logging.
---
 src/shared/path-lookup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 7d7b890b47..075f4d1e6c 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -529,7 +529,7 @@ int lookup_paths_init(
         if (flags & LOOKUP_PATHS_TEMPORARY_GENERATED) {
                 r = mkdtemp_malloc("/tmp/systemd-temporary-XXXXXX", &tempdir);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to create temporary directory: %m");
+                        return log_debug_errno(r, "Failed to create temporary directory: %m");
         }
 
         /* Note: when XDG_RUNTIME_DIR is not set this will not return -ENXIO, but simply set runtime_config to NULL */

From 4daf832afaeac471085e56445a05218217bf5107 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:45:33 +0200
Subject: [PATCH 10/24] core: allow manager_serialize() to fail correctly

If manager_serialize() fails in the middle (which it hopefully doesn't)
make sure to fix up m->n_reloading correctly again so that we don't
leave it > 0 when it really shouldn't be.
---
 src/core/manager.c | 56 +++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index ba16e8259e..c41d88f3f6 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3043,7 +3043,12 @@ int manager_open_serialization(Manager *m, FILE **_f) {
         return 0;
 }
 
-int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
+int manager_serialize(
+                Manager *m,
+                FILE *f,
+                FDSet *fds,
+                bool switching_root) {
+
         ManagerTimestamp q;
         const char *t;
         Iterator i;
@@ -3097,8 +3102,10 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
                 int copy;
 
                 copy = fdset_put_dup(fds, m->notify_fd);
-                if (copy < 0)
-                        return copy;
+                if (copy < 0) {
+                        r = copy;
+                        goto finish;
+                }
 
                 fprintf(f, "notify-fd=%i\n", copy);
                 fprintf(f, "notify-socket=%s\n", m->notify_socket);
@@ -3108,8 +3115,10 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
                 int copy;
 
                 copy = fdset_put_dup(fds, m->cgroups_agent_fd);
-                if (copy < 0)
-                        return copy;
+                if (copy < 0) {
+                        r = copy;
+                        goto finish;
+                }
 
                 fprintf(f, "cgroups-agent-fd=%i\n", copy);
         }
@@ -3118,12 +3127,16 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
                 int copy0, copy1;
 
                 copy0 = fdset_put_dup(fds, m->user_lookup_fds[0]);
-                if (copy0 < 0)
-                        return copy0;
+                if (copy0 < 0) {
+                        r = copy0;
+                        goto finish;
+                }
 
                 copy1 = fdset_put_dup(fds, m->user_lookup_fds[1]);
-                if (copy1 < 0)
-                        return copy1;
+                if (copy1 < 0) {
+                        r = copy1;
+                        goto finish;
+                }
 
                 fprintf(f, "user-lookup=%i %i\n", copy0, copy1);
         }
@@ -3132,14 +3145,14 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
 
         r = dynamic_user_serialize(m, f, fds);
         if (r < 0)
-                return r;
+                goto finish;
 
         manager_serialize_uid_refs(m, f);
         manager_serialize_gid_refs(m, f);
 
         r = exec_runtime_serialize(m, f, fds);
         if (r < 0)
-                return r;
+                goto finish;
 
         (void) fputc('\n', f);
 
@@ -3152,24 +3165,25 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
                 fputc('\n', f);
 
                 r = unit_serialize(u, f, fds, !switching_root);
-                if (r < 0) {
-                        m->n_reloading--;
-                        return r;
-                }
+                if (r < 0)
+                        goto finish;
         }
 
-        assert(m->n_reloading > 0);
-        m->n_reloading--;
-
         r = fflush_and_check(f);
         if (r < 0)
-                return r;
+                goto finish;
 
         r = bus_fdset_add_all(m, fds);
         if (r < 0)
-                return r;
+                goto finish;
 
-        return 0;
+        r = 0;
+
+finish:
+        assert(m->n_reloading > 0);
+        m->n_reloading--;
+
+        return r;
 }
 
 int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {

From 7eb4f326121b4f88c02660ca0aad35c316cd6b80 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:47:30 +0200
Subject: [PATCH 11/24] core: make sure manager_run_generators() logs about all
 errors

Since it's mostly a wrapper around execute_directories() it already logs
in most cases, but a few were missing. Fix that.
---
 src/core/manager.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index c41d88f3f6..e57d955529 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3831,8 +3831,10 @@ static int manager_run_generators(Manager *m) {
                 return 0;
 
         r = lookup_paths_mkdir_generator(&m->lookup_paths);
-        if (r < 0)
+        if (r < 0) {
+                log_error_errno(r, "Failed to create generator directories: %m");
                 goto finish;
+        }
 
         argv[0] = NULL; /* Leave this empty, execute_directory() will fill something in */
         argv[1] = m->lookup_paths.generator;
@@ -3841,8 +3843,10 @@ static int manager_run_generators(Manager *m) {
         argv[4] = NULL;
 
         RUN_WITH_UMASK(0022)
-                execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC,
-                                    NULL, NULL, (char**) argv, m->environment);
+                (void) execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC,
+                                           NULL, NULL, (char**) argv, m->environment);
+
+        r = 0;
 
 finish:
         lookup_paths_trim_generator(&m->lookup_paths);

From b2a8a3dd1079690851ef20140d43f8a8b662c8b5 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:53:35 +0200
Subject: [PATCH 12/24] core: clean up deserialization log messages a bit

Always, say that we ignore these kind of issues. We already say that for
many fields, but for a few this was missing.
---
 src/core/manager.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index e57d955529..e1568e747f 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3219,7 +3219,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         uint32_t id;
 
                         if (safe_atou32(val, &id) < 0)
-                                log_notice("Failed to parse current job id value %s", val);
+                                log_notice("Failed to parse current job id value '%s', ignoring.", val);
                         else
                                 m->current_job_id = MAX(m->current_job_id, id);
 
@@ -3227,7 +3227,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         uint32_t n;
 
                         if (safe_atou32(val, &n) < 0)
-                                log_notice("Failed to parse installed jobs counter %s", val);
+                                log_notice("Failed to parse installed jobs counter '%s', ignoring.", val);
                         else
                                 m->n_installed_jobs += n;
 
@@ -3235,7 +3235,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         uint32_t n;
 
                         if (safe_atou32(val, &n) < 0)
-                                log_notice("Failed to parse failed jobs counter %s", val);
+                                log_notice("Failed to parse failed jobs counter '%s', ignoring.", val);
                         else
                                 m->n_failed_jobs += n;
 
@@ -3244,7 +3244,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         b = parse_boolean(val);
                         if (b < 0)
-                                log_notice("Failed to parse taint /usr flag %s", val);
+                                log_notice("Failed to parse taint /usr flag '%s', ignoring.", val);
                         else
                                 m->taint_usr = m->taint_usr || b;
 
@@ -3253,7 +3253,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         b = parse_boolean(val);
                         if (b < 0)
-                                log_notice("Failed to parse ready-sent flag %s", val);
+                                log_notice("Failed to parse ready-sent flag '%s', ignoring.", val);
                         else
                                 m->ready_sent = m->ready_sent || b;
 
@@ -3262,7 +3262,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         b = parse_boolean(val);
                         if (b < 0)
-                                log_notice("Failed to parse taint-logged flag %s", val);
+                                log_notice("Failed to parse taint-logged flag '%s', ignoring.", val);
                         else
                                 m->taint_logged = m->taint_logged || b;
 
@@ -3271,7 +3271,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         b = parse_boolean(val);
                         if (b < 0)
-                                log_notice("Failed to parse service-watchdogs flag %s", val);
+                                log_notice("Failed to parse service-watchdogs flag '%s', ignoring.", val);
                         else
                                 m->service_watchdogs = b;
 
@@ -3280,7 +3280,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         s = show_status_from_string(val);
                         if (s < 0)
-                                log_notice("Failed to parse show-status flag %s", val);
+                                log_notice("Failed to parse show-status flag '%s', ignoring.", val);
                         else
                                 manager_set_show_status(m, s);
 
@@ -3307,13 +3307,13 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         if (r == -ENOMEM)
                                 goto finish;
                         if (r < 0)
-                                log_notice_errno(r, "Failed to parse environment entry: \"%s\": %m", l);
+                                log_notice_errno(r, "Failed to parse environment entry: \"%s\", ignoring: %m", l);
 
                 } else if ((val = startswith(l, "notify-fd="))) {
                         int fd;
 
                         if (safe_atoi(val, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
-                                log_notice("Failed to parse notify fd: \"%s\"", val);
+                                log_notice("Failed to parse notify fd, ignoring: \"%s\"", val);
                         else {
                                 m->notify_event_source = sd_event_source_unref(m->notify_event_source);
                                 safe_close(m->notify_fd);
@@ -3336,7 +3336,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         int fd;
 
                         if (safe_atoi(val, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
-                                log_notice("Failed to parse cgroups agent fd: %s", val);
+                                log_notice("Failed to parse cgroups agent fd, ignoring.: %s", val);
                         else {
                                 m->cgroups_agent_event_source = sd_event_source_unref(m->cgroups_agent_event_source);
                                 safe_close(m->cgroups_agent_fd);
@@ -3347,7 +3347,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         int fd0, fd1;
 
                         if (sscanf(val, "%i %i", &fd0, &fd1) != 2 || fd0 < 0 || fd1 < 0 || fd0 == fd1 || !fdset_contains(fds, fd0) || !fdset_contains(fds, fd1))
-                                log_notice("Failed to parse user lookup fd: %s", val);
+                                log_notice("Failed to parse user lookup fd, ignoring: %s", val);
                         else {
                                 m->user_lookup_event_source = sd_event_source_unref(m->user_lookup_event_source);
                                 safe_close_pair(m->user_lookup_fds);
@@ -3383,7 +3383,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         if (q < _MANAGER_TIMESTAMP_MAX) /* found it */
                                 dual_timestamp_deserialize(val, m->timestamps + q);
                         else if (!startswith(l, "kdbus-fd=")) /* ignore kdbus */
-                                log_notice("Unknown serialization item '%s'", l);
+                                log_notice("Unknown serialization item '%s', ignoring.", l);
                 }
         }
 

From 18869883f2ea082ebf816ac37a01e6d6e27969cb Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:54:40 +0200
Subject: [PATCH 13/24] core: handle OOM during deserialization always the same
 way

OOM failures we consider fatal, while other failures we generally skip
over.
---
 src/core/manager.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index e1568e747f..48fca62252 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3365,8 +3365,11 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         exec_runtime_deserialize_one(m, val, fds);
                 else if ((val = startswith(l, "subscribed="))) {
 
-                        if (strv_extend(&m->deserialized_subscribed, val) < 0)
-                                log_oom();
+                        if (strv_extend(&m->deserialized_subscribed, val) < 0) {
+                                r = -ENOMEM;
+                                goto finish;
+                        }
+
                 } else {
                         ManagerTimestamp q;
 
@@ -3407,18 +3410,20 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                 r = manager_load_unit(m, unit_name, NULL, NULL, &u);
                 if (r < 0) {
-                        log_notice_errno(r, "Failed to load unit \"%s\", skipping deserialization: %m", unit_name);
                         if (r == -ENOMEM)
                                 goto finish;
+
+                        log_notice_errno(r, "Failed to load unit \"%s\", skipping deserialization: %m", unit_name);
                         unit_deserialize_skip(f);
                         continue;
                 }
 
                 r = unit_deserialize(u, f, fds);
                 if (r < 0) {
-                        log_notice_errno(r, "Failed to deserialize unit \"%s\": %m", unit_name);
                         if (r == -ENOMEM)
                                 goto finish;
+
+                        log_notice_errno(r, "Failed to deserialize unit \"%s\": %m", unit_name);
                 }
         }
 

From eb10d0bf8a40e154614eb3b046dc619dcfd55e2b Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 16:59:16 +0200
Subject: [PATCH 14/24] core: add comments about n_reloading to
 manager_deserialize()

---
 src/core/manager.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/core/manager.c b/src/core/manager.c
index 48fca62252..56d4b11bae 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3194,6 +3194,9 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
         log_debug("Deserializing state...");
 
+        /* If we are not in reload mode yet, enter it now. Not that this is recursive, a caller might already have
+         * increased it to non-zero, which is why we just increase it by one here and down again at the end of this
+         * call. */
         m->n_reloading++;
 
         for (;;) {
@@ -3431,6 +3434,7 @@ finish:
         if (ferror(f))
                 r = -EIO;
 
+        /* We are done with reloading, decrease counter again */
         assert(m->n_reloading > 0);
         m->n_reloading--;
 

From 6a33af40da2b31f3f253c533b11d328c26a8d3b2 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:02:14 +0200
Subject: [PATCH 15/24] manager: rework error handling and logging in
 manager_reload()

let's clean up error handling and logging in manager_reload() a bit.
Specifically: make sure we log about every error we might encounter at
least and at most once.

When we encounter an error before the "point of no return" then log at
LOG_ERR about it and propagate it. Otherwise, eat it up, but warn about
it and proceed, it's the best we can do.
---
 src/core/main.c    |  5 +--
 src/core/manager.c | 91 +++++++++++++++++++++++-----------------------
 2 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 2a2fe89c28..672422e155 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1698,10 +1698,7 @@ static int invoke_main_loop(
                         if (saved_log_target >= 0)
                                 manager_override_log_target(m, saved_log_target);
 
-                        r = manager_reload(m);
-                        if (r < 0)
-                                log_warning_errno(r, "Failed to reload, ignoring: %m");
-
+                        (void) manager_reload(m);
                         break;
                 }
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 56d4b11bae..947c982657 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3453,37 +3453,42 @@ static void manager_flush_finished_jobs(Manager *m) {
 }
 
 int manager_reload(Manager *m) {
-        int r, q;
-        _cleanup_fclose_ FILE *f = NULL;
         _cleanup_fdset_free_ FDSet *fds = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        int r;
 
         assert(m);
 
         r = manager_open_serialization(m, &f);
         if (r < 0)
-                return r;
-
-        m->n_reloading++;
-        bus_manager_send_reloading(m, true);
+                return log_error_errno(r, "Failed to create serialization file: %m");
 
         fds = fdset_new();
-        if (!fds) {
-                m->n_reloading--;
-                return -ENOMEM;
-        }
+        if (!fds)
+                return log_oom();
+
+        /* We are officially in reload mode from here on */
+        m->n_reloading++;
 
         r = manager_serialize(m, f, fds, false);
         if (r < 0) {
-                m->n_reloading--;
-                return r;
+                log_error_errno(r, "Failed to serialize manager: %m");
+                goto fail;
         }
 
         if (fseeko(f, 0, SEEK_SET) < 0) {
-                m->n_reloading--;
-                return -errno;
+                r = log_error_errno(errno, "Failed to seek to beginning of serialization: %m");
+                goto fail;
         }
 
-        /* From here on there is no way back. */
+        /* 💀 This is the point of no return, from here on there is no way back. 💀 */
+
+        bus_manager_send_reloading(m, true);
+
+        /* Start by flushing out all jobs and units, all generated units, all runtime environments, all dynamic users
+         * and everything else that is worth flushing out. We'll get it all back from the serialization — if we need
+         * it.*/
+
         manager_clear_jobs_and_units(m);
         lookup_paths_flush_generator(&m->lookup_paths);
         lookup_paths_free(&m->lookup_paths);
@@ -3493,45 +3498,33 @@ int manager_reload(Manager *m) {
         m->gid_refs = hashmap_free(m->gid_refs);
 
         r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, 0, NULL);
+        if (r < 0)
+                log_warning_errno(r, "Failed to initialize path lookup table, ignoring: %m");
 
-        q = manager_run_environment_generators(m);
-        if (q < 0 && r >= 0)
-                r = q;
+        (void) manager_run_environment_generators(m);
+        (void) manager_run_generators(m);
 
-        /* Find new unit paths */
-        q = manager_run_generators(m);
-        if (q < 0 && r >= 0)
-                r = q;
+        r = lookup_paths_reduce(&m->lookup_paths);
+        if (r < 0)
+                log_warning_errno(r, "Failed ot reduce unit file paths, ignoring: %m");
 
-        lookup_paths_reduce(&m->lookup_paths);
         manager_build_unit_path_cache(m);
 
-        /* First, enumerate what we can from all config files */
+        /* First, enumerate what we can from kernel and suchlike */
         manager_enumerate(m);
 
         /* Second, deserialize our stored data */
-        q = manager_deserialize(m, f, fds);
-        if (q < 0) {
-                log_error_errno(q, "Deserialization failed: %m");
-
-                if (r >= 0)
-                        r = q;
-        }
+        r = manager_deserialize(m, f, fds);
+        if (r < 0)
+                log_warning_errno(r, "Deserialization failed, proceeding anyway: %m");
 
+        /* We don't need the serialization anymore */
         f = safe_fclose(f);
 
-        /* Re-register notify_fd as event source */
-        q = manager_setup_notify(m);
-        if (q < 0 && r >= 0)
-                r = q;
-
-        q = manager_setup_cgroups_agent(m);
-        if (q < 0 && r >= 0)
-                r = q;
-
-        q = manager_setup_user_lookup_fd(m);
-        if (q < 0 && r >= 0)
-                r = q;
+        /* Re-register notify_fd as event source, and set up other sockets/communication channels we might need */
+        (void) manager_setup_notify(m);
+        (void) manager_setup_cgroups_agent(m);
+        (void) manager_setup_user_lookup_fd(m);
 
         /* Third, fire things up! */
         manager_coldplug(m);
@@ -3545,6 +3538,7 @@ int manager_reload(Manager *m) {
 
         exec_runtime_vacuum(m);
 
+        /* Consider the reload process complete now. */
         assert(m->n_reloading > 0);
         m->n_reloading--;
 
@@ -3556,14 +3550,19 @@ int manager_reload(Manager *m) {
         manager_catchup(m);
 
         /* Sync current state of bus names with our set of listening units */
-        q = manager_enqueue_sync_bus_names(m);
-        if (q < 0 && r >= 0)
-                r = q;
+        (void) manager_enqueue_sync_bus_names(m);
 
         if (!MANAGER_IS_RELOADING(m))
                 manager_flush_finished_jobs(m);
 
         m->send_reloading_done = true;
+        return 0;
+
+fail:
+        /* Fail the call. Note that we hit this only if we fail before the point of no return, i.e. when the error is
+         * still something that can be handled. */
+        assert(m->n_reloading > 0);
+        m->n_reloading--;
 
         return r;
 }

From 572986ca149db01f913b9605e20293986fe0756f Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:16:08 +0200
Subject: [PATCH 16/24] core: log in all cases in manager_startup()

We missed some cases where we'd fail without any logging at all. Let's
fix that.
---
 src/analyze/analyze-verify.c |  2 +-
 src/core/main.c              |  1 -
 src/core/manager.c           | 12 +++++++++---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c
index d2f9e5528e..402203a3c7 100644
--- a/src/analyze/analyze-verify.c
+++ b/src/analyze/analyze-verify.c
@@ -254,7 +254,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run
 
         r = manager_startup(m, NULL, NULL);
         if (r < 0)
-                return log_error_errno(r, "Failed to start manager: %m");
+                return r;
 
         manager_clear_jobs(m);
 
diff --git a/src/core/main.c b/src/core/main.c
index 672422e155..2810e88109 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -2427,7 +2427,6 @@ int main(int argc, char *argv[]) {
 
         r = manager_startup(m, arg_serialization, fds);
         if (r < 0) {
-                log_error_errno(r, "Failed to fully start up daemon: %m");
                 error_message = "Failed to start up manager";
                 goto finish;
         }
diff --git a/src/core/manager.c b/src/core/manager.c
index 947c982657..cbb9ec6924 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1592,7 +1592,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                               MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0,
                               NULL);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to initialize path lookup table: %m");
 
         r = manager_run_environment_generators(m);
         if (r < 0)
@@ -1605,7 +1605,11 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                 return r;
 
         manager_preset_all(m);
-        lookup_paths_reduce(&m->lookup_paths);
+
+        r = lookup_paths_reduce(&m->lookup_paths);
+        if (r < 0)
+                log_warning_errno(r, "Failed ot reduce unit file paths, ignoring: %m");
+
         manager_build_unit_path_cache(m);
 
         /* If we will deserialize make sure that during enumeration
@@ -1654,7 +1658,9 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         manager_setup_bus(m);
 
         /* Now that we are connected to all possible busses, let's deserialize who is tracking us. */
-        (void) bus_track_coldplug(m, &m->subscribed, false, m->deserialized_subscribed);
+        r = bus_track_coldplug(m, &m->subscribed, false, m->deserialized_subscribed);
+        if (r < 0)
+                log_warning_errno(r, "Failed to deserialized tracked clients, ignoring: %m");
         m->deserialized_subscribed = strv_free(m->deserialized_subscribed);
 
         /* Third, fire things up! */

From 6eb3af7a6e52bcf1ce80fed9deaff37e329629da Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:16:55 +0200
Subject: [PATCH 17/24] core: break lines in comments

---
 src/core/manager.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index cbb9ec6924..97e693a58c 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1612,9 +1612,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
 
         manager_build_unit_path_cache(m);
 
-        /* If we will deserialize make sure that during enumeration
-         * this is already known, so we increase the counter here
-         * already */
+        /* If we will deserialize make sure that during enumeration this is already known, so we increase the counter
+         * here already */
         if (serialization)
                 m->n_reloading++;
 
@@ -1631,14 +1630,11 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                         return log_error_errno(r, "Deserialization failed: %m");
         }
 
-        /* Any fds left? Find some unit which wants them. This is
-         * useful to allow container managers to pass some file
-         * descriptors to us pre-initialized. This enables
-         * socket-based activation of entire containers. */
+        /* Any fds left? Find some unit which wants them. This is useful to allow container managers to pass some file
+         * descriptors to us pre-initialized. This enables socket-based activation of entire containers. */
         manager_distribute_fds(m, fds);
 
-        /* We might have deserialized the notify fd, but if we didn't
-         * then let's create the bus now */
+        /* We might have deserialized the notify fd, but if we didn't then let's create the bus now */
         r = manager_setup_notify(m);
         if (r < 0)
                 /* No sense to continue without notifications, our children would fail anyway. */
@@ -1679,8 +1675,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                 assert(m->n_reloading > 0);
                 m->n_reloading--;
 
-                /* Let's wait for the UnitNew/JobNew messages being
-                 * sent, before we notify that the reload is
+                /* Let's wait for the UnitNew/JobNew messages being sent, before we notify that the reload is
                  * finished */
                 m->send_reloading_done = true;
         }

From 3ad2afb6a204513c7834c64ab864e40169874390 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:37:57 +0200
Subject: [PATCH 18/24] core: bring manager_startup() and manager_reload() more
 inline

Both functions do partly the same, let's make sure they do it in the
same order, and that we don't miss some calls.

This makes a number of changes:

1. Moves exec_runtime_vacuum() two calls down in manager_startup(). This
   should not have any effect but makes manager_startup() more like
   manager_reload().

2. Calls manager_recheck_journal(), manager_recheck_dbus(),
   manager_enqueue_sync_bus_names() in manager_startup() too. This is a
   good idea since during reeexec we pass through manager_startup() and
   hence can't assume dbus and journald weren't up yet, hence let's
   check if they are ready to be connected to.

3. Include manager_enumerate_perpetual() in manager_reload(), too. This
   is not strictly necessary, since these units are included in the
   serialization anyway, but it's still a nice thing, in particular as
   theoretically the deserialization could fail.
---
 src/core/manager.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 97e693a58c..5a5212f6c2 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1665,12 +1665,12 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         /* Release any dynamic users no longer referenced */
         dynamic_user_vacuum(m, true);
 
-        exec_runtime_vacuum(m);
-
         /* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */
         manager_vacuum_uid_refs(m);
         manager_vacuum_gid_refs(m);
 
+        exec_runtime_vacuum(m);
+
         if (serialization) {
                 assert(m->n_reloading > 0);
                 m->n_reloading--;
@@ -1680,6 +1680,13 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                 m->send_reloading_done = true;
         }
 
+        /* It might be safe to log to the journal now and connect to dbus */
+        manager_recheck_journal(m);
+        manager_recheck_dbus(m);
+
+        /* Sync current state of bus names with our set of listening units */
+        (void) manager_enqueue_sync_bus_names(m);
+
         /* Let's finally catch up with any changes that took place while we were reloading/reexecing */
         manager_catchup(m);
 
@@ -3512,6 +3519,7 @@ int manager_reload(Manager *m) {
         manager_build_unit_path_cache(m);
 
         /* First, enumerate what we can from kernel and suchlike */
+        manager_enumerate_perpetual(m);
         manager_enumerate(m);
 
         /* Second, deserialize our stored data */

From 7a35fa24ffdd3af167cb6d6b88bd73869c6d35b7 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:56:54 +0200
Subject: [PATCH 19/24] core: try to recover from failed reloads

Let's simply continue with everything we loaded, in the hope it's
somewhat useful.
---
 src/core/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/core/main.c b/src/core/main.c
index 2810e88109..9bc5b3c6dc 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1698,7 +1698,11 @@ static int invoke_main_loop(
                         if (saved_log_target >= 0)
                                 manager_override_log_target(m, saved_log_target);
 
-                        (void) manager_reload(m);
+                        r = manager_reload(m);
+                        if (r < 0)
+                                /* Reloading failed before the point of no return. Let's continue running as if nothing happened. */
+                                m->objective = MANAGER_OK;
+
                         break;
                 }
 

From 3ca4d0b3eb5982b673acb9607c88d0961dd418fe Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:57:41 +0200
Subject: [PATCH 20/24] core: make use of manager_loop()'s return value

The objective is returned in the return value, let's make use of that,
instead of reaching into the object.
---
 src/core/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/main.c b/src/core/main.c
index 9bc5b3c6dc..44e8a38f61 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1673,7 +1673,7 @@ static int invoke_main_loop(
                         return log_emergency_errno(r, "Failed to run main loop: %m");
                 }
 
-                switch (m->objective) {
+                switch ((ManagerObjective) r) {
 
                 case MANAGER_RELOAD: {
                         LogTarget saved_log_target;

From 1fb70e66488d6c124d1d1e8d7cc2a98d2da5eb23 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:58:08 +0200
Subject: [PATCH 21/24] core: rework how we set the objective to MANAGER_OK

Let's do so already when we are about to complete startup/reload, so
that manager_catchup() is run in a context where MANAGER_IS_RUNNING()
returns true, as the intention is.

Fixes: #9518
---
 src/core/manager.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 5a5212f6c2..0282ae3253 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1680,6 +1680,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                 m->send_reloading_done = true;
         }
 
+        m->objective = MANAGER_OK;
+
         /* It might be safe to log to the journal now and connect to dbus */
         manager_recheck_journal(m);
         manager_recheck_dbus(m);
@@ -2811,7 +2813,7 @@ int manager_loop(Manager *m) {
         RATELIMIT_DEFINE(rl, 1*USEC_PER_SEC, 50000);
 
         assert(m);
-        m->objective = MANAGER_OK;
+        assert(m->objective == MANAGER_OK); /* Ensure manager_startup() has been called */
 
         /* Release the path cache */
         m->unit_path_cache = set_free_free(m->unit_path_cache);
@@ -3550,6 +3552,7 @@ int manager_reload(Manager *m) {
         /* Consider the reload process complete now. */
         assert(m->n_reloading > 0);
         m->n_reloading--;
+        m->objective = MANAGER_OK;
 
         /* It might be safe to log to the journal now and connect to dbus */
         manager_recheck_journal(m);

From 5197be06e03a60dd1ebab7cdb9d2ef8f585a46c6 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 19:34:23 +0200
Subject: [PATCH 22/24] core: turn our four vacuum calls into a new helper
 function

Just share some code. No functional changes.
---
 src/core/manager.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 0282ae3253..d12d07707a 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1581,6 +1581,20 @@ static void manager_preset_all(Manager *m) {
                 log_info("Populated /etc with preset unit settings.");
 }
 
+static void manager_vacuum(Manager *m) {
+        assert(m);
+
+        /* Release any dynamic users no longer referenced */
+        dynamic_user_vacuum(m, true);
+
+        /* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */
+        manager_vacuum_uid_refs(m);
+        manager_vacuum_gid_refs(m);
+
+        /* Release any runtimes no longer referenced */
+        exec_runtime_vacuum(m);
+}
+
 int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         int r;
 
@@ -1662,14 +1676,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         /* Third, fire things up! */
         manager_coldplug(m);
 
-        /* Release any dynamic users no longer referenced */
-        dynamic_user_vacuum(m, true);
-
-        /* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */
-        manager_vacuum_uid_refs(m);
-        manager_vacuum_gid_refs(m);
-
-        exec_runtime_vacuum(m);
+        /* Clean up runtime objects */
+        manager_vacuum(m);
 
         if (serialization) {
                 assert(m->n_reloading > 0);
@@ -3540,14 +3548,8 @@ int manager_reload(Manager *m) {
         /* Third, fire things up! */
         manager_coldplug(m);
 
-        /* Release any dynamic users no longer referenced */
-        dynamic_user_vacuum(m, true);
-
-        /* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */
-        manager_vacuum_uid_refs(m);
-        manager_vacuum_gid_refs(m);
-
-        exec_runtime_vacuum(m);
+        /* Clean up runtime objects no longer referenced */
+        manager_vacuum(m);
 
         /* Consider the reload process complete now. */
         assert(m->n_reloading > 0);

From 5ce5e1ad08f5a9a9f6bd88f19d7abdb94e554dd8 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 19:41:24 +0200
Subject: [PATCH 23/24] core: add a common helper call manager_ready() sharing
 some common code between manager_reload() and manager_startup()

Just sharing some common code. No functional changes
---
 src/core/manager.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index d12d07707a..653fce8368 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1595,6 +1595,24 @@ static void manager_vacuum(Manager *m) {
         exec_runtime_vacuum(m);
 }
 
+static void manager_ready(Manager *m) {
+        assert(m);
+
+        /* After having loaded everything, do the final round of catching up with what might have changed */
+
+        m->objective = MANAGER_OK; /* Tell everyone we are up now */
+
+        /* It might be safe to log to the journal now and connect to dbus */
+        manager_recheck_journal(m);
+        manager_recheck_dbus(m);
+
+        /* Sync current state of bus names with our set of listening units */
+        (void) manager_enqueue_sync_bus_names(m);
+
+        /* Let's finally catch up with any changes that took place while we were reloading/reexecing */
+        manager_catchup(m);
+}
+
 int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         int r;
 
@@ -1688,17 +1706,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
                 m->send_reloading_done = true;
         }
 
-        m->objective = MANAGER_OK;
-
-        /* It might be safe to log to the journal now and connect to dbus */
-        manager_recheck_journal(m);
-        manager_recheck_dbus(m);
-
-        /* Sync current state of bus names with our set of listening units */
-        (void) manager_enqueue_sync_bus_names(m);
-
-        /* Let's finally catch up with any changes that took place while we were reloading/reexecing */
-        manager_catchup(m);
+        manager_ready(m);
 
         return 0;
 }
@@ -3554,17 +3562,8 @@ int manager_reload(Manager *m) {
         /* Consider the reload process complete now. */
         assert(m->n_reloading > 0);
         m->n_reloading--;
-        m->objective = MANAGER_OK;
 
-        /* It might be safe to log to the journal now and connect to dbus */
-        manager_recheck_journal(m);
-        manager_recheck_dbus(m);
-
-        /* Let's finally catch up with any changes that took place while we were reloading/reexecing */
-        manager_catchup(m);
-
-        /* Sync current state of bus names with our set of listening units */
-        (void) manager_enqueue_sync_bus_names(m);
+        manager_ready(m);
 
         if (!MANAGER_IS_RELOADING(m))
                 manager_flush_finished_jobs(m);

From eb523bfb5155f07bbc2a4b4a37c81270c3c6153b Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 19:42:28 +0200
Subject: [PATCH 24/24] core: include environment generator runtime in
 generator timestamps

Currently they aren't covered and it probably isn't worth adding another
kind of timestamp just for this, hence simply include it in the regular
generator timestamps.
---
 src/core/manager.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 653fce8368..2dea89dc01 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1626,12 +1626,10 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         if (r < 0)
                 return log_error_errno(r, "Failed to initialize path lookup table: %m");
 
-        r = manager_run_environment_generators(m);
-        if (r < 0)
-                return r;
-
         dual_timestamp_get(m->timestamps + manager_timestamp_initrd_mangle(MANAGER_TIMESTAMP_GENERATORS_START));
-        r = manager_run_generators(m);
+        r = manager_run_environment_generators(m);
+        if (r >= 0)
+                r = manager_run_generators(m);
         dual_timestamp_get(m->timestamps + manager_timestamp_initrd_mangle(MANAGER_TIMESTAMP_GENERATORS_FINISH));
         if (r < 0)
                 return r;