Revert "core: do not spawn jobs or touch other units during coldplugging"

This reverts commit 6e392c9c45643d106673c6643ac8bf4e65da13c1.

We really shouldn't invent external state keeping hashmaps, if we can
keep this state in the units themselves.

diff --git a/src/core/automount.c b/src/core/automount.c
index b3e96fc2e1848f8cc098ea70b04ebdedf1ed153d..6fc214b70494769d10685dd7d0bd5310e57dede5 100644 (file)
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -259,7 +259,7 @@ static void automount_set_state(Automount *a, AutomountState state) {
         unit_notify(UNIT(a), state_translation_table[old_state], state_translation_table[state], true);
 }
 
-static int automount_coldplug(Unit *u, Hashmap *deferred_work) {
+static int automount_coldplug(Unit *u) {
         Automount *a = AUTOMOUNT(u);
         int r;
 

diff --git a/src/core/busname.c b/src/core/busname.c
index b1b953aadf5b793c079ea9d7391bcbbad22bd7c1..aba2a96c1a3e9dbbb6f474ed9c41ba74dc745073 100644 (file)
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -336,7 +336,7 @@ static void busname_set_state(BusName *n, BusNameState state) {
         unit_notify(UNIT(n), state_translation_table[old_state], state_translation_table[state], true);
 }
 
-static int busname_coldplug(Unit *u, Hashmap *deferred_work) {
+static int busname_coldplug(Unit *u) {
         BusName *n = BUSNAME(u);
         int r;
 

diff --git a/src/core/device.c b/src/core/device.c
index a7572306d702983a6bb5a27de0611f9782ae9fcd..63bae978a6223bb4c06072b0d758b653569ff02f 100644 (file)
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -140,7 +140,7 @@ static void device_set_state(Device *d, DeviceState state) {
         unit_notify(UNIT(d), state_translation_table[old_state], state_translation_table[state], true);
 }
 
-static int device_coldplug(Unit *u, Hashmap *deferred_work) {
+static int device_coldplug(Unit *u) {
         Device *d = DEVICE(u);
 
         assert(d);

diff --git a/src/core/manager.c b/src/core/manager.c
index 2b04644dc1b944c362021beee600e135f544fc64..39a868fed63eeaa956a64e55f37c14a5f144694c 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -981,28 +981,7 @@ static int manager_coldplug(Manager *m) {
         Unit *u;
         char *k;
 
-        /*
-         * Some unit types tend to spawn jobs or check other units' state
-         * during coldplug. This is wrong because it is undefined whether the
-         * units in question have been already coldplugged (i. e. their state
-         * restored). This way, we can easily re-start an already started unit
-         * or otherwise make a wrong decision based on the unit's state.
-         *
-         * Solve this by providing a way for coldplug functions to defer
-         * such actions until after all units have been coldplugged.
-         *
-         * We store Unit* -> int(*)(Unit*).
-         *
-         * https://bugs.freedesktop.org/show_bug.cgi?id=88401
-         */
-        _cleanup_hashmap_free_ Hashmap *deferred_work = NULL;
-        int(*proc)(Unit*);
-
-        assert(m);
-
-        deferred_work = hashmap_new(&trivial_hash_ops);
-        if (!deferred_work)
-                return -ENOMEM;
+        assert(m);
 
         /* Then, let's set up their initial state. */
         HASHMAP_FOREACH_KEY(u, k, m->units, i) {
@@ -1012,17 +991,7 @@ static int manager_coldplug(Manager *m) {
                 if (u->id != k)
                         continue;
 
-                q = unit_coldplug(u, deferred_work);
-                if (q < 0)
-                        r = q;
-        }
-
-        /* After coldplugging and setting up initial state of the units,
-         * let's perform operations which spawn jobs or query units' state. */
-        HASHMAP_FOREACH_KEY(proc, u, deferred_work, i) {
-                int q;
-
-                q = proc(u);
+                q = unit_coldplug(u);
                 if (q < 0)
                         r = q;
         }

diff --git a/src/core/mount.c b/src/core/mount.c
index d3a4098f1a49f95cb13c86bf8a8519bf2cd38a9d..eb25bcbdbe9540016cba62a940efbccec43c13d1 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -601,7 +601,7 @@ static void mount_set_state(Mount *m, MountState state) {
         m->reload_result = MOUNT_SUCCESS;
 }
 
-static int mount_coldplug(Unit *u, Hashmap *deferred_work) {
+static int mount_coldplug(Unit *u) {
         Mount *m = MOUNT(u);
         MountState new_state = MOUNT_DEAD;
         int r;

diff --git a/src/core/path.c b/src/core/path.c
index 6be9ac84bed841393fd364d8ee14381da8ce3ce8..fbb695d87ff8283897a3f24c71b9f5e060facc71 100644 (file)
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -438,12 +438,7 @@ static void path_set_state(Path *p, PathState state) {
 
 static void path_enter_waiting(Path *p, bool initial, bool recheck);
 
-static int path_enter_waiting_coldplug(Unit *u) {
-        path_enter_waiting(PATH(u), true, true);
-        return 0;
-}
-
-static int path_coldplug(Unit *u, Hashmap *deferred_work) {
+static int path_coldplug(Unit *u) {
         Path *p = PATH(u);
 
         assert(p);
@@ -452,10 +447,9 @@ static int path_coldplug(Unit *u, Hashmap *deferred_work) {
         if (p->deserialized_state != p->state) {
 
                 if (p->deserialized_state == PATH_WAITING ||
-                    p->deserialized_state == PATH_RUNNING) {
-                        hashmap_put(deferred_work, u, &path_enter_waiting_coldplug);
-                        path_set_state(p, PATH_WAITING);
-                } else
+                    p->deserialized_state == PATH_RUNNING)
+                        path_enter_waiting(p, true, true);
+                else
                         path_set_state(p, p->deserialized_state);
         }
 

diff --git a/src/core/scope.c b/src/core/scope.c
index 8b2bb29ed8a9f4807d2f518cff0894565387fa8d..1c3c6bb5409a381b2e5c07f1d5c0d91d0db0dcda 100644 (file)
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -171,7 +171,7 @@ static int scope_load(Unit *u) {
         return scope_verify(s);
 }
 
-static int scope_coldplug(Unit *u, Hashmap *deferred_work) {
+static int scope_coldplug(Unit *u) {
         Scope *s = SCOPE(u);
         int r;
 

diff --git a/src/core/service.c b/src/core/service.c
index 0e2f114135c0fedb3b4dbd1b138c1a8fb9fd8f15..9104347f27adec09b7040b9ce694a6e6b6f13b9e 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -879,7 +879,7 @@ static void service_set_state(Service *s, ServiceState state) {
         s->reload_result = SERVICE_SUCCESS;
 }
 
-static int service_coldplug(Unit *u, Hashmap *deferred_work) {
+static int service_coldplug(Unit *u) {
         Service *s = SERVICE(u);
         int r;
 

diff --git a/src/core/slice.c b/src/core/slice.c
index 0285c49aebfde7096463447ee6b7aa68167b09b8..0bebdbcbc649591637ed2595f344eae088ee900d 100644 (file)
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -150,7 +150,7 @@ static int slice_load(Unit *u) {
         return slice_verify(s);
 }
 
-static int slice_coldplug(Unit *u, Hashmap *deferred_work) {
+static int slice_coldplug(Unit *u) {
         Slice *t = SLICE(u);
 
         assert(t);

diff --git a/src/core/snapshot.c b/src/core/snapshot.c
index b1d8448771d9fe6ef290448bc44ef4fb3456f28d..b70c3beb6023c1b75455c636bbaf8d8d9a0c0dc1 100644 (file)
--- a/src/core/snapshot.c
+++ b/src/core/snapshot.c
@@ -75,7 +75,7 @@ static int snapshot_load(Unit *u) {
         return 0;
 }
 
-static int snapshot_coldplug(Unit *u, Hashmap *deferred_work) {
+static int snapshot_coldplug(Unit *u) {
         Snapshot *s = SNAPSHOT(u);
 
         assert(s);

diff --git a/src/core/socket.c b/src/core/socket.c
index dd805d51df31c3f62ccd35f495d4270c7914c578..1f931eb986adbefe007916ceef8d6c8d0f2f9850 100644 (file)
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1323,7 +1323,7 @@ static void socket_set_state(Socket *s, SocketState state) {
         unit_notify(UNIT(s), state_translation_table[old_state], state_translation_table[state], true);
 }
 
-static int socket_coldplug(Unit *u, Hashmap *deferred_work) {
+static int socket_coldplug(Unit *u) {
         Socket *s = SOCKET(u);
         int r;
 

diff --git a/src/core/swap.c b/src/core/swap.c
index 76660cc963003ef74f138b372af1fddfc30d4746..74f26b7a76967d226260c72066c8f43f5558f350 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -507,7 +507,7 @@ static void swap_set_state(Swap *s, SwapState state) {
                         job_add_to_run_queue(UNIT(other)->job);
 }
 
-static int swap_coldplug(Unit *u, Hashmap *deferred_work) {
+static int swap_coldplug(Unit *u) {
         Swap *s = SWAP(u);
         SwapState new_state = SWAP_DEAD;
         int r;

diff --git a/src/core/target.c b/src/core/target.c
index 5f64402475976643b1532ba9e9dd0c24cecd997c..8817ef21c4c61a3daba8d24b717e9a3c19f83406 100644 (file)
--- a/src/core/target.c
+++ b/src/core/target.c
@@ -103,7 +103,7 @@ static int target_load(Unit *u) {
         return 0;
 }
 
-static int target_coldplug(Unit *u, Hashmap *deferred_work) {
+static int target_coldplug(Unit *u) {
         Target *t = TARGET(u);
 
         assert(t);

diff --git a/src/core/timer.c b/src/core/timer.c
index 79a7540553ab0ed0f5b03cdf73482d41d562a566..940550194b15e238b2045d3519e2d65ecd2a873a 100644 (file)
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -267,12 +267,7 @@ static void timer_set_state(Timer *t, TimerState state) {
 
 static void timer_enter_waiting(Timer *t, bool initial);
 
-static int timer_enter_waiting_coldplug(Unit *u) {
-        timer_enter_waiting(TIMER(u), false);
-        return 0;
-}
-
-static int timer_coldplug(Unit *u, Hashmap *deferred_work) {
+static int timer_coldplug(Unit *u) {
         Timer *t = TIMER(u);
 
         assert(t);
@@ -280,10 +275,9 @@ static int timer_coldplug(Unit *u, Hashmap *deferred_work) {
 
         if (t->deserialized_state != t->state) {
 
-                if (t->deserialized_state == TIMER_WAITING) {
-                        hashmap_put(deferred_work, u, &timer_enter_waiting_coldplug);
-                        timer_set_state(t, TIMER_WAITING);
-                } else
+                if (t->deserialized_state == TIMER_WAITING)
+                        timer_enter_waiting(t, false);
+                else
                         timer_set_state(t, t->deserialized_state);
         }
 

diff --git a/src/core/unit.c b/src/core/unit.c
index e921b48fc491197c763c3eeeac6719412e439ae3..70a2b57fa3dd8aefe3a3c9096c3d5d0a0c802776 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2875,34 +2875,27 @@ int unit_add_node_link(Unit *u, const char *what, bool wants) {
         return 0;
 }
 
-static int unit_add_deserialized_job_coldplug(Unit *u) {
-        int r;
-
-        r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL);
-        if (r < 0)
-                return r;
-
-        u->deserialized_job = _JOB_TYPE_INVALID;
-
-        return 0;
-}
-
-int unit_coldplug(Unit *u, Hashmap *deferred_work) {
+int unit_coldplug(Unit *u) {
         int r;
 
         assert(u);
 
         if (UNIT_VTABLE(u)->coldplug)
-                if ((r = UNIT_VTABLE(u)->coldplug(u, deferred_work)) < 0)
+                if ((r = UNIT_VTABLE(u)->coldplug(u)) < 0)
                         return r;
 
         if (u->job) {
                 r = job_coldplug(u->job);
                 if (r < 0)
                         return r;
-        } else if (u->deserialized_job >= 0)
+        } else if (u->deserialized_job >= 0) {
                 /* legacy */
-                hashmap_put(deferred_work, u, &unit_add_deserialized_job_coldplug);
+                r = manager_add_job(u->manager, u->deserialized_job, u, JOB_IGNORE_REQUIREMENTS, false, NULL, NULL);
+                if (r < 0)
+                        return r;
+
+                u->deserialized_job = _JOB_TYPE_INVALID;
+        }
 
         return 0;
 }

diff --git a/src/core/unit.h b/src/core/unit.h
index 260dc451c7b297487f9664071c9823c13a70a0ba..be306a004b809b0bfc8952e69a72daf72d97440b 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -303,14 +303,8 @@ struct UnitVTable {
         int (*load)(Unit *u);
 
         /* If a lot of units got created via enumerate(), this is
-         * where to actually set the state and call unit_notify().
-         *
-         * This must not reference other units (maybe implicitly through spawning
-         * jobs), because it is possible that they are not yet coldplugged.
-         * Such actions must be deferred until the end of coldplug bу adding
-         * a "Unit* -> int(*)(Unit*)" entry into the hashmap.
-         */
-        int (*coldplug)(Unit *u, Hashmap *deferred_work);
+         * where to actually set the state and call unit_notify(). */
+        int (*coldplug)(Unit *u);
 
         void (*dump)(Unit *u, FILE *f, const char *prefix);
 
@@ -546,7 +540,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds);
 
 int unit_add_node_link(Unit *u, const char *what, bool wants);
 
-int unit_coldplug(Unit *u, Hashmap *deferred_work);
+int unit_coldplug(Unit *u);
 
 void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg_format) _printf_(3, 0);