anitazha / rpms / systemd

Forked from rpms/systemd 3 years ago
Clone

Blame SOURCES/0648-core-delay-adding-target-dependencies-until-all-unit.patch

a9339c
From 36226a9afe96bdffce9d0697be020f2ca9d7fe6f Mon Sep 17 00:00:00 2001
a9339c
From: Michal Sekletar <msekletar@users.noreply.github.com>
a9339c
Date: Fri, 23 Mar 2018 15:28:06 +0100
a9339c
Subject: [PATCH] core: delay adding target dependencies until all units are
a9339c
 loaded and aliases resolved (#8381)
a9339c
a9339c
Currently we add target dependencies while we are loading units. This
a9339c
can create ordering loops even if configuration doesn't contain any
a9339c
loop. Take for example following configuration,
a9339c
a9339c
$ systemctl get-default
a9339c
multi-user.target
a9339c
a9339c
$ cat /etc/systemd/system/test.service
a9339c
[Unit]
a9339c
After=default.target
a9339c
a9339c
[Service]
a9339c
ExecStart=/bin/true
a9339c
a9339c
[Install]
a9339c
WantedBy=multi-user.target
a9339c
a9339c
If we encounter such unit file early during manager start-up (e.g. load
a9339c
queue is dispatched while enumerating devices due to SYSTEMD_WANTS in
a9339c
udev rules) we would add stub unit default.target and we order it Before
a9339c
test.service. At the same time we add implicit Before to
a9339c
multi-user.target. Later we merge two units and we create ordering cycle
a9339c
in the process.
a9339c
a9339c
To fix the issue we will now never add any target dependencies until we
a9339c
loaded all the unit files and resolved all the aliases.
a9339c
a9339c
(cherry picked from commit 19496554e23ea4861ce780430052dcf86a2ffcba)
a9339c
a9339c
Resolves: #1368856
a9339c
---
a9339c
 src/core/manager.c | 40 ++++++++++++++++++++++++++++++++++++++++
a9339c
 src/core/manager.h |  3 +++
a9339c
 src/core/unit.c    | 46 ++++++++++++++++------------------------------
a9339c
 src/core/unit.h    |  5 +++++
a9339c
 4 files changed, 64 insertions(+), 30 deletions(-)
a9339c
a9339c
diff --git a/src/core/manager.c b/src/core/manager.c
a9339c
index 9c406bb5b..0466e4bb8 100644
a9339c
--- a/src/core/manager.c
a9339c
+++ b/src/core/manager.c
a9339c
@@ -1373,6 +1373,41 @@ Unit *manager_get_unit(Manager *m, const char *name) {
a9339c
         return hashmap_get(m->units, name);
a9339c
 }
a9339c
 
a9339c
+static int manager_dispatch_target_deps_queue(Manager *m) {
a9339c
+        Unit *u;
a9339c
+        unsigned k;
a9339c
+        int r = 0;
a9339c
+
a9339c
+        static const UnitDependency deps[] = {
a9339c
+                UNIT_REQUIRED_BY,
a9339c
+                UNIT_REQUIRED_BY_OVERRIDABLE,
a9339c
+                UNIT_WANTED_BY,
a9339c
+                UNIT_BOUND_BY
a9339c
+        };
a9339c
+
a9339c
+        assert(m);
a9339c
+
a9339c
+        while ((u = m->target_deps_queue)) {
a9339c
+                assert(u->in_target_deps_queue);
a9339c
+
a9339c
+                LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
a9339c
+                u->in_target_deps_queue = false;
a9339c
+
a9339c
+                for (k = 0; k < ELEMENTSOF(deps); k++) {
a9339c
+                        Unit *target;
a9339c
+                        Iterator i;
a9339c
+
a9339c
+                        SET_FOREACH(target, u->dependencies[deps[k]], i) {
a9339c
+                                r = unit_add_default_target_dependency(u, target);
a9339c
+                                if (r < 0)
a9339c
+                                        return r;
a9339c
+                        }
a9339c
+                }
a9339c
+        }
a9339c
+
a9339c
+        return r;
a9339c
+}
a9339c
+
a9339c
 unsigned manager_dispatch_load_queue(Manager *m) {
a9339c
         Unit *u;
a9339c
         unsigned n = 0;
a9339c
@@ -1396,6 +1431,11 @@ unsigned manager_dispatch_load_queue(Manager *m) {
a9339c
         }
a9339c
 
a9339c
         m->dispatching_load_queue = false;
a9339c
+
a9339c
+        /* Dispatch the units waiting for their target dependencies to be added now, as all targets that we know about
a9339c
+         * should be loaded and have aliases resolved */
a9339c
+        (void) manager_dispatch_target_deps_queue(m);
a9339c
+
a9339c
         return n;
a9339c
 }
a9339c
 
a9339c
diff --git a/src/core/manager.h b/src/core/manager.h
a9339c
index 90d2d982e..b0e4cad1f 100644
a9339c
--- a/src/core/manager.h
a9339c
+++ b/src/core/manager.h
a9339c
@@ -114,6 +114,9 @@ struct Manager {
a9339c
         /* Units that should be realized */
a9339c
         LIST_HEAD(Unit, cgroup_queue);
a9339c
 
a9339c
+        /* Target units whose default target dependencies haven't been set yet */
a9339c
+        LIST_HEAD(Unit, target_deps_queue);
a9339c
+
a9339c
         sd_event *event;
a9339c
 
a9339c
         /* We use two hash tables here, since the same PID might be
a9339c
diff --git a/src/core/unit.c b/src/core/unit.c
a9339c
index 22d9beed7..cfddce34d 100644
a9339c
--- a/src/core/unit.c
a9339c
+++ b/src/core/unit.c
a9339c
@@ -519,6 +519,9 @@ void unit_free(Unit *u) {
a9339c
                 u->manager->n_in_gc_queue--;
a9339c
         }
a9339c
 
a9339c
+        if (u->in_target_deps_queue)
a9339c
+                LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
a9339c
+
a9339c
         if (u->in_cgroup_queue)
a9339c
                 LIST_REMOVE(cgroup_queue, u->manager->cgroup_queue, u);
a9339c
 
a9339c
@@ -1065,6 +1068,18 @@ int unit_load_fragment_and_dropin_optional(Unit *u) {
a9339c
         return 0;
a9339c
 }
a9339c
 
a9339c
+void unit_add_to_target_deps_queue(Unit *u) {
a9339c
+        Manager *m = u->manager;
a9339c
+
a9339c
+        assert(u);
a9339c
+
a9339c
+        if (u->in_target_deps_queue)
a9339c
+                return;
a9339c
+
a9339c
+        LIST_PREPEND(target_deps_queue, m->target_deps_queue, u);
a9339c
+        u->in_target_deps_queue = true;
a9339c
+}
a9339c
+
a9339c
 int unit_add_default_target_dependency(Unit *u, Unit *target) {
a9339c
         assert(u);
a9339c
         assert(target);
a9339c
@@ -1091,32 +1106,6 @@ int unit_add_default_target_dependency(Unit *u, Unit *target) {
a9339c
         return unit_add_dependency(target, UNIT_AFTER, u, true);
a9339c
 }
a9339c
 
a9339c
-static int unit_add_target_dependencies(Unit *u) {
a9339c
-
a9339c
-        static const UnitDependency deps[] = {
a9339c
-                UNIT_REQUIRED_BY,
a9339c
-                UNIT_REQUIRED_BY_OVERRIDABLE,
a9339c
-                UNIT_WANTED_BY,
a9339c
-                UNIT_BOUND_BY
a9339c
-        };
a9339c
-
a9339c
-        Unit *target;
a9339c
-        Iterator i;
a9339c
-        unsigned k;
a9339c
-        int r = 0;
a9339c
-
a9339c
-        assert(u);
a9339c
-
a9339c
-        for (k = 0; k < ELEMENTSOF(deps); k++)
a9339c
-                SET_FOREACH(target, u->dependencies[deps[k]], i) {
a9339c
-                        r = unit_add_default_target_dependency(u, target);
a9339c
-                        if (r < 0)
a9339c
-                                return r;
a9339c
-                }
a9339c
-
a9339c
-        return r;
a9339c
-}
a9339c
-
a9339c
 static int unit_add_slice_dependencies(Unit *u) {
a9339c
         assert(u);
a9339c
 
a9339c
@@ -1217,10 +1206,7 @@ int unit_load(Unit *u) {
a9339c
         }
a9339c
 
a9339c
         if (u->load_state == UNIT_LOADED) {
a9339c
-
a9339c
-                r = unit_add_target_dependencies(u);
a9339c
-                if (r < 0)
a9339c
-                        goto fail;
a9339c
+                unit_add_to_target_deps_queue(u);
a9339c
 
a9339c
                 r = unit_add_slice_dependencies(u);
a9339c
                 if (r < 0)
a9339c
diff --git a/src/core/unit.h b/src/core/unit.h
a9339c
index 480e2e95f..dfec9cea0 100644
a9339c
--- a/src/core/unit.h
a9339c
+++ b/src/core/unit.h
a9339c
@@ -162,6 +162,9 @@ struct Unit {
a9339c
         /* CGroup realize members queue */
a9339c
         LIST_FIELDS(Unit, cgroup_queue);
a9339c
 
a9339c
+        /* Target dependencies queue */
a9339c
+        LIST_FIELDS(Unit, target_deps_queue);
a9339c
+
a9339c
         /* PIDs we keep an eye on. Note that a unit might have many
a9339c
          * more, but these are the ones we care enough about to
a9339c
          * process SIGCHLD for */
a9339c
@@ -228,6 +231,7 @@ struct Unit {
a9339c
         bool in_cleanup_queue:1;
a9339c
         bool in_gc_queue:1;
a9339c
         bool in_cgroup_queue:1;
a9339c
+        bool in_target_deps_queue:1;
a9339c
 
a9339c
         bool sent_dbus_new_signal:1;
a9339c
 
a9339c
@@ -498,6 +502,7 @@ void unit_add_to_load_queue(Unit *u);
a9339c
 void unit_add_to_dbus_queue(Unit *u);
a9339c
 void unit_add_to_cleanup_queue(Unit *u);
a9339c
 void unit_add_to_gc_queue(Unit *u);
a9339c
+void unit_add_to_target_deps_queue(Unit *u);
a9339c
 
a9339c
 int unit_merge(Unit *u, Unit *other);
a9339c
 int unit_merge_by_name(Unit *u, const char *other);