daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
803fb7
From f997080b4d17a40b59b398e4354b6368d9c85f69 Mon Sep 17 00:00:00 2001
803fb7
From: Ivan Shapovalov <intelfx100@gmail.com>
803fb7
Date: Sat, 7 Mar 2015 08:44:52 -0500
803fb7
Subject: [PATCH] core: do not spawn jobs or touch other units during
803fb7
 coldplugging
803fb7
803fb7
Because the order of coldplugging is not defined, we can reference a
803fb7
not-yet-coldplugged unit and read its state while it has not yet been
803fb7
set to a meaningful value.
803fb7
803fb7
This way, already active units may get started again.
803fb7
803fb7
We fix this by deferring such actions until all units have been at
803fb7
least somehow coldplugged.
803fb7
803fb7
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=88401
803fb7
803fb7
(cherry picked from commit 6e392c9c45643d106673c6643ac8bf4e65da13c1)
803fb7
---
803fb7
 src/core/automount.c |  2 +-
803fb7
 src/core/busname.c   |  2 +-
803fb7
 src/core/device.c    |  2 +-
803fb7
 src/core/manager.c   | 35 +++++++++++++++++++++++++++++++++--
803fb7
 src/core/mount.c     |  2 +-
803fb7
 src/core/path.c      | 14 ++++++++++----
803fb7
 src/core/scope.c     |  2 +-
803fb7
 src/core/service.c   |  2 +-
803fb7
 src/core/slice.c     |  2 +-
803fb7
 src/core/snapshot.c  |  2 +-
803fb7
 src/core/socket.c    |  2 +-
803fb7
 src/core/swap.c      |  2 +-
803fb7
 src/core/target.c    |  2 +-
803fb7
 src/core/timer.c     | 14 ++++++++++----
803fb7
 src/core/unit.c      | 25 ++++++++++++++++---------
803fb7
 src/core/unit.h      | 12 +++++++++---
803fb7
 16 files changed, 89 insertions(+), 33 deletions(-)
803fb7
803fb7
diff --git a/src/core/automount.c b/src/core/automount.c
803fb7
index 9f6bd84b2..e4c79415d 100644
803fb7
--- a/src/core/automount.c
803fb7
+++ b/src/core/automount.c
803fb7
@@ -235,7 +235,7 @@ static void automount_set_state(Automount *a, AutomountState state) {
803fb7
         unit_notify(UNIT(a), state_translation_table[old_state], state_translation_table[state], true);
803fb7
 }
803fb7
 
803fb7
-static int automount_coldplug(Unit *u) {
803fb7
+static int automount_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Automount *a = AUTOMOUNT(u);
803fb7
         int r;
803fb7
 
803fb7
diff --git a/src/core/busname.c b/src/core/busname.c
803fb7
index 1d77292f9..43d7607a3 100644
803fb7
--- a/src/core/busname.c
803fb7
+++ b/src/core/busname.c
803fb7
@@ -335,7 +335,7 @@ static void busname_set_state(BusName *n, BusNameState state) {
803fb7
         unit_notify(UNIT(n), state_translation_table[old_state], state_translation_table[state], true);
803fb7
 }
803fb7
 
803fb7
-static int busname_coldplug(Unit *u) {
803fb7
+static int busname_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         BusName *n = BUSNAME(u);
803fb7
         int r;
803fb7
 
803fb7
diff --git a/src/core/device.c b/src/core/device.c
803fb7
index 1cc103c29..4ff882721 100644
803fb7
--- a/src/core/device.c
803fb7
+++ b/src/core/device.c
803fb7
@@ -142,7 +142,7 @@ static void device_set_state(Device *d, DeviceState state) {
803fb7
         unit_notify(UNIT(d), state_translation_table[old_state], state_translation_table[state], true);
803fb7
 }
803fb7
 
803fb7
-static int device_coldplug(Unit *u) {
803fb7
+static int device_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Device *d = DEVICE(u);
803fb7
 
803fb7
         assert(d);
803fb7
diff --git a/src/core/manager.c b/src/core/manager.c
803fb7
index bc9b7ec62..203a6a0a1 100644
803fb7
--- a/src/core/manager.c
803fb7
+++ b/src/core/manager.c
803fb7
@@ -983,7 +983,28 @@ static int manager_coldplug(Manager *m) {
803fb7
         Unit *u;
803fb7
         char *k;
803fb7
 
803fb7
-        assert(m);
803fb7
+        /*
803fb7
+         * Some unit types tend to spawn jobs or check other units' state
803fb7
+         * during coldplug. This is wrong because it is undefined whether the
803fb7
+         * units in question have been already coldplugged (i. e. their state
803fb7
+         * restored). This way, we can easily re-start an already started unit
803fb7
+         * or otherwise make a wrong decision based on the unit's state.
803fb7
+         *
803fb7
+         * Solve this by providing a way for coldplug functions to defer
803fb7
+         * such actions until after all units have been coldplugged.
803fb7
+         *
803fb7
+         * We store Unit* -> int(*)(Unit*).
803fb7
+         *
803fb7
+         * https://bugs.freedesktop.org/show_bug.cgi?id=88401
803fb7
+         */
803fb7
+        _cleanup_hashmap_free_ Hashmap *deferred_work = NULL;
803fb7
+        int(*proc)(Unit*);
803fb7
+
803fb7
+        assert(m);
803fb7
+
803fb7
+        deferred_work = hashmap_new(&trivial_hash_ops);
803fb7
+        if (!deferred_work)
803fb7
+                return -ENOMEM;
803fb7
 
803fb7
         /* Then, let's set up their initial state. */
803fb7
         HASHMAP_FOREACH_KEY(u, k, m->units, i) {
803fb7
@@ -993,7 +1014,17 @@ static int manager_coldplug(Manager *m) {
803fb7
                 if (u->id != k)
803fb7
                         continue;
803fb7
 
803fb7
-                q = unit_coldplug(u);
803fb7
+                q = unit_coldplug(u, deferred_work);
803fb7
+                if (q < 0)
803fb7
+                        r = q;
803fb7
+        }
803fb7
+
803fb7
+        /* After coldplugging and setting up initial state of the units,
803fb7
+         * let's perform operations which spawn jobs or query units' state. */
803fb7
+        HASHMAP_FOREACH_KEY(proc, u, deferred_work, i) {
803fb7
+                int q;
803fb7
+
803fb7
+                q = proc(u);
803fb7
                 if (q < 0)
803fb7
                         r = q;
803fb7
         }
803fb7
diff --git a/src/core/mount.c b/src/core/mount.c
803fb7
index c971330af..3ae0eb462 100644
803fb7
--- a/src/core/mount.c
803fb7
+++ b/src/core/mount.c
803fb7
@@ -617,7 +617,7 @@ static void mount_set_state(Mount *m, MountState state) {
803fb7
         m->reload_result = MOUNT_SUCCESS;
803fb7
 }
803fb7
 
803fb7
-static int mount_coldplug(Unit *u) {
803fb7
+static int mount_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Mount *m = MOUNT(u);
803fb7
         MountState new_state = MOUNT_DEAD;
803fb7
         int r;
803fb7
diff --git a/src/core/path.c b/src/core/path.c
803fb7
index e5ea79fec..51e36fa8b 100644
803fb7
--- a/src/core/path.c
803fb7
+++ b/src/core/path.c
803fb7
@@ -440,7 +440,12 @@ static void path_set_state(Path *p, PathState state) {
803fb7
 
803fb7
 static void path_enter_waiting(Path *p, bool initial, bool recheck);
803fb7
 
803fb7
-static int path_coldplug(Unit *u) {
803fb7
+static int path_enter_waiting_coldplug(Unit *u) {
803fb7
+        path_enter_waiting(PATH(u), true, true);
803fb7
+        return 0;
803fb7
+}
803fb7
+
803fb7
+static int path_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Path *p = PATH(u);
803fb7
 
803fb7
         assert(p);
803fb7
@@ -449,9 +454,10 @@ static int path_coldplug(Unit *u) {
803fb7
         if (p->deserialized_state != p->state) {
803fb7
 
803fb7
                 if (p->deserialized_state == PATH_WAITING ||
803fb7
-                    p->deserialized_state == PATH_RUNNING)
803fb7
-                        path_enter_waiting(p, true, true);
803fb7
-                else
803fb7
+                    p->deserialized_state == PATH_RUNNING) {
803fb7
+                        hashmap_put(deferred_work, u, &path_enter_waiting_coldplug);
803fb7
+                        path_set_state(p, PATH_WAITING);
803fb7
+                } else
803fb7
                         path_set_state(p, p->deserialized_state);
803fb7
         }
803fb7
 
803fb7
diff --git a/src/core/scope.c b/src/core/scope.c
803fb7
index b41db7872..ae6614fbf 100644
803fb7
--- a/src/core/scope.c
803fb7
+++ b/src/core/scope.c
803fb7
@@ -173,7 +173,7 @@ static int scope_load(Unit *u) {
803fb7
         return scope_verify(s);
803fb7
 }
803fb7
 
803fb7
-static int scope_coldplug(Unit *u) {
803fb7
+static int scope_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Scope *s = SCOPE(u);
803fb7
         int r;
803fb7
 
803fb7
diff --git a/src/core/service.c b/src/core/service.c
803fb7
index 15e29be14..7781b4e62 100644
803fb7
--- a/src/core/service.c
803fb7
+++ b/src/core/service.c
803fb7
@@ -879,7 +879,7 @@ static void service_set_state(Service *s, ServiceState state) {
803fb7
         s->reload_result = SERVICE_SUCCESS;
803fb7
 }
803fb7
 
803fb7
-static int service_coldplug(Unit *u) {
803fb7
+static int service_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Service *s = SERVICE(u);
803fb7
         int r;
803fb7
 
803fb7
diff --git a/src/core/slice.c b/src/core/slice.c
803fb7
index ae9819d01..61ff9d331 100644
803fb7
--- a/src/core/slice.c
803fb7
+++ b/src/core/slice.c
803fb7
@@ -153,7 +153,7 @@ static int slice_load(Unit *u) {
803fb7
         return slice_verify(s);
803fb7
 }
803fb7
 
803fb7
-static int slice_coldplug(Unit *u) {
803fb7
+static int slice_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Slice *t = SLICE(u);
803fb7
 
803fb7
         assert(t);
803fb7
diff --git a/src/core/snapshot.c b/src/core/snapshot.c
803fb7
index b70c3beb6..b1d844877 100644
803fb7
--- a/src/core/snapshot.c
803fb7
+++ b/src/core/snapshot.c
803fb7
@@ -75,7 +75,7 @@ static int snapshot_load(Unit *u) {
803fb7
         return 0;
803fb7
 }
803fb7
 
803fb7
-static int snapshot_coldplug(Unit *u) {
803fb7
+static int snapshot_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Snapshot *s = SNAPSHOT(u);
803fb7
 
803fb7
         assert(s);
803fb7
diff --git a/src/core/socket.c b/src/core/socket.c
803fb7
index 88aae4815..760de0203 100644
803fb7
--- a/src/core/socket.c
803fb7
+++ b/src/core/socket.c
803fb7
@@ -1326,7 +1326,7 @@ static void socket_set_state(Socket *s, SocketState state) {
803fb7
         unit_notify(UNIT(s), state_translation_table[old_state], state_translation_table[state], true);
803fb7
 }
803fb7
 
803fb7
-static int socket_coldplug(Unit *u) {
803fb7
+static int socket_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Socket *s = SOCKET(u);
803fb7
         int r;
803fb7
 
803fb7
diff --git a/src/core/swap.c b/src/core/swap.c
803fb7
index 5c19af5d9..369abf0f5 100644
803fb7
--- a/src/core/swap.c
803fb7
+++ b/src/core/swap.c
803fb7
@@ -513,7 +513,7 @@ static void swap_set_state(Swap *s, SwapState state) {
803fb7
                         job_add_to_run_queue(UNIT(other)->job);
803fb7
 }
803fb7
 
803fb7
-static int swap_coldplug(Unit *u) {
803fb7
+static int swap_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Swap *s = SWAP(u);
803fb7
         SwapState new_state = SWAP_DEAD;
803fb7
         int r;
803fb7
diff --git a/src/core/target.c b/src/core/target.c
803fb7
index 33fb66bc3..2411a8e75 100644
803fb7
--- a/src/core/target.c
803fb7
+++ b/src/core/target.c
803fb7
@@ -107,7 +107,7 @@ static int target_load(Unit *u) {
803fb7
         return 0;
803fb7
 }
803fb7
 
803fb7
-static int target_coldplug(Unit *u) {
803fb7
+static int target_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Target *t = TARGET(u);
803fb7
 
803fb7
         assert(t);
803fb7
diff --git a/src/core/timer.c b/src/core/timer.c
803fb7
index 45744c7de..48cf9c16a 100644
803fb7
--- a/src/core/timer.c
803fb7
+++ b/src/core/timer.c
803fb7
@@ -268,7 +268,12 @@ static void timer_set_state(Timer *t, TimerState state) {
803fb7
 
803fb7
 static void timer_enter_waiting(Timer *t, bool initial);
803fb7
 
803fb7
-static int timer_coldplug(Unit *u) {
803fb7
+static int timer_enter_waiting_coldplug(Unit *u) {
803fb7
+        timer_enter_waiting(TIMER(u), false);
803fb7
+        return 0;
803fb7
+}
803fb7
+
803fb7
+static int timer_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         Timer *t = TIMER(u);
803fb7
 
803fb7
         assert(t);
803fb7
@@ -276,9 +281,10 @@ static int timer_coldplug(Unit *u) {
803fb7
 
803fb7
         if (t->deserialized_state != t->state) {
803fb7
 
803fb7
-                if (t->deserialized_state == TIMER_WAITING)
803fb7
-                        timer_enter_waiting(t, false);
803fb7
-                else
803fb7
+                if (t->deserialized_state == TIMER_WAITING) {
803fb7
+                        hashmap_put(deferred_work, u, &timer_enter_waiting_coldplug);
803fb7
+                        timer_set_state(t, TIMER_WAITING);
803fb7
+                } else
803fb7
                         timer_set_state(t, t->deserialized_state);
803fb7
         }
803fb7
 
803fb7
diff --git a/src/core/unit.c b/src/core/unit.c
803fb7
index a6558ee23..565455bd6 100644
803fb7
--- a/src/core/unit.c
803fb7
+++ b/src/core/unit.c
803fb7
@@ -2859,27 +2859,34 @@ int unit_add_node_link(Unit *u, const char *what, bool wants) {
803fb7
         return 0;
803fb7
 }
803fb7
 
803fb7
-int unit_coldplug(Unit *u) {
803fb7
+static int unit_add_deserialized_job_coldplug(Unit *u) {
803fb7
+        int r;
803fb7
+
803fb7
+        r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL);
803fb7
+        if (r < 0)
803fb7
+                return r;
803fb7
+
803fb7
+        u->deserialized_job = _JOB_TYPE_INVALID;
803fb7
+
803fb7
+        return 0;
803fb7
+}
803fb7
+
803fb7
+int unit_coldplug(Unit *u, Hashmap *deferred_work) {
803fb7
         int r;
803fb7
 
803fb7
         assert(u);
803fb7
 
803fb7
         if (UNIT_VTABLE(u)->coldplug)
803fb7
-                if ((r = UNIT_VTABLE(u)->coldplug(u)) < 0)
803fb7
+                if ((r = UNIT_VTABLE(u)->coldplug(u, deferred_work)) < 0)
803fb7
                         return r;
803fb7
 
803fb7
         if (u->job) {
803fb7
                 r = job_coldplug(u->job);
803fb7
                 if (r < 0)
803fb7
                         return r;
803fb7
-        } else if (u->deserialized_job >= 0) {
803fb7
+        } else if (u->deserialized_job >= 0)
803fb7
                 /* legacy */
803fb7
-                r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL);
803fb7
-                if (r < 0)
803fb7
-                        return r;
803fb7
-
803fb7
-                u->deserialized_job = _JOB_TYPE_INVALID;
803fb7
-        }
803fb7
+                hashmap_put(deferred_work, u, &unit_add_deserialized_job_coldplug);
803fb7
 
803fb7
         return 0;
803fb7
 }
803fb7
diff --git a/src/core/unit.h b/src/core/unit.h
803fb7
index 291bc77a7..7ebc489c8 100644
803fb7
--- a/src/core/unit.h
803fb7
+++ b/src/core/unit.h
803fb7
@@ -307,8 +307,14 @@ struct UnitVTable {
803fb7
         int (*load)(Unit *u);
803fb7
 
803fb7
         /* If a lot of units got created via enumerate(), this is
803fb7
-         * where to actually set the state and call unit_notify(). */
803fb7
-        int (*coldplug)(Unit *u);
803fb7
+         * where to actually set the state and call unit_notify().
803fb7
+         *
803fb7
+         * This must not reference other units (maybe implicitly through spawning
803fb7
+         * jobs), because it is possible that they are not yet coldplugged.
803fb7
+         * Such actions must be deferred until the end of coldplug bу adding
803fb7
+         * a "Unit* -> int(*)(Unit*)" entry into the hashmap.
803fb7
+         */
803fb7
+        int (*coldplug)(Unit *u, Hashmap *deferred_work);
803fb7
 
803fb7
         void (*dump)(Unit *u, FILE *f, const char *prefix);
803fb7
 
803fb7
@@ -544,7 +550,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds);
803fb7
 
803fb7
 int unit_add_node_link(Unit *u, const char *what, bool wants);
803fb7
 
803fb7
-int unit_coldplug(Unit *u);
803fb7
+int unit_coldplug(Unit *u, Hashmap *deferred_work);
803fb7
 
803fb7
 void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg_format) _printf_(3, 0);
803fb7