anitazha / rpms / systemd

Forked from rpms/systemd 3 years ago
Clone
d0811f
From 7a481a17ad01c7be526829a835f7da3d6b71577f Mon Sep 17 00:00:00 2001
d0811f
From: Lennart Poettering <lennart@poettering.net>
d0811f
Date: Fri, 11 Sep 2020 19:49:33 +0200
d0811f
Subject: [PATCH 1/3] core: propagate triggered unit in more load states
d0811f
d0811f
In 4c2ef3276735ad9f7fccf33f5bdcbe7d8751e7ec we enabled propagating
d0811f
triggered unit state to the triggering unit for service units in more
d0811f
load states, so that we don't accidentally stop tracking state
d0811f
correctly.
d0811f
d0811f
Do the same for our other triggering unit states: automounts, paths, and
d0811f
timers.
d0811f
d0811f
Also, make this an assertion rather than a simple test. After all it
d0811f
should never happen that we get called for half-loaded units or units of
d0811f
the wrong type. The load routines should already have made this
d0811f
impossible.
d0811f
---
d0811f
 src/core/automount.c   | 4 ++--
d0811f
 src/core/path.c        | 7 +++----
d0811f
 src/core/socket.c      | 9 ++-------
d0811f
 src/core/timer.c       | 4 ++--
d0811f
 src/core/transaction.c | 2 +-
d0811f
 src/core/unit.h        | 4 ++++
d0811f
 6 files changed, 14 insertions(+), 16 deletions(-)
d0811f
d0811f
diff --git a/src/core/automount.c b/src/core/automount.c
d0811f
index 1f05198766..73f0fb8c71 100644
d0811f
--- a/src/core/automount.c
d0811f
+++ b/src/core/automount.c
d0811f
@@ -507,8 +507,8 @@ static void automount_trigger_notify(Unit *u, Unit *other) {
d0811f
         assert(other);
d0811f
 
d0811f
         /* Filter out invocations with bogus state */
d0811f
-        if (other->load_state != UNIT_LOADED || other->type != UNIT_MOUNT)
d0811f
-                return;
d0811f
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
d0811f
+        assert(other->type == UNIT_MOUNT);
d0811f
 
d0811f
         /* Don't propagate state changes from the mount if we are already down */
d0811f
         if (!IN_SET(a->state, AUTOMOUNT_WAITING, AUTOMOUNT_RUNNING))
d0811f
diff --git a/src/core/path.c b/src/core/path.c
d0811f
index 1c3c28e341..8ffec72ede 100644
d0811f
--- a/src/core/path.c
d0811f
+++ b/src/core/path.c
d0811f
@@ -748,11 +748,10 @@ static void path_trigger_notify(Unit *u, Unit *other) {
d0811f
         assert(u);
d0811f
         assert(other);
d0811f
 
d0811f
-        /* Invoked whenever the unit we trigger changes state or gains
d0811f
-         * or loses a job */
d0811f
+        /* Invoked whenever the unit we trigger changes state or gains or loses a job */
d0811f
 
d0811f
-        if (other->load_state != UNIT_LOADED)
d0811f
-                return;
d0811f
+        /* Filter out invocations with bogus state */
d0811f
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
d0811f
 
d0811f
         if (p->state == PATH_RUNNING &&
d0811f
             UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
d0811f
diff --git a/src/core/socket.c b/src/core/socket.c
d0811f
index 127195c9fe..ebf5ce3b16 100644
d0811f
--- a/src/core/socket.c
d0811f
+++ b/src/core/socket.c
d0811f
@@ -3274,13 +3274,8 @@ static void socket_trigger_notify(Unit *u, Unit *other) {
d0811f
         assert(other);
d0811f
 
d0811f
         /* Filter out invocations with bogus state */
d0811f
-        if (!IN_SET(other->load_state,
d0811f
-                    UNIT_LOADED,
d0811f
-                    UNIT_NOT_FOUND,
d0811f
-                    UNIT_BAD_SETTING,
d0811f
-                    UNIT_ERROR,
d0811f
-                    UNIT_MASKED) || other->type != UNIT_SERVICE)
d0811f
-                return;
d0811f
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
d0811f
+        assert(other->type == UNIT_SERVICE);
d0811f
 
d0811f
         /* Don't propagate state changes from the service if we are already down */
d0811f
         if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING))
d0811f
diff --git a/src/core/timer.c b/src/core/timer.c
d0811f
index 03a9c14f76..94388f0727 100644
d0811f
--- a/src/core/timer.c
d0811f
+++ b/src/core/timer.c
d0811f
@@ -746,8 +746,8 @@ static void timer_trigger_notify(Unit *u, Unit *other) {
d0811f
         assert(u);
d0811f
         assert(other);
d0811f
 
d0811f
-        if (other->load_state != UNIT_LOADED)
d0811f
-                return;
d0811f
+        /* Filter out invocations with bogus state */
d0811f
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
d0811f
 
d0811f
         /* Reenable all timers that depend on unit state */
d0811f
         LIST_FOREACH(value, v, t->values)
d0811f
diff --git a/src/core/transaction.c b/src/core/transaction.c
d0811f
index 0fa419787e..befac19788 100644
d0811f
--- a/src/core/transaction.c
d0811f
+++ b/src/core/transaction.c
d0811f
@@ -949,7 +949,7 @@ int transaction_add_job_and_dependencies(
d0811f
 
d0811f
         /* Safety check that the unit is a valid state, i.e. not in UNIT_STUB or UNIT_MERGED which should only be set
d0811f
          * temporarily. */
d0811f
-        if (!IN_SET(unit->load_state, UNIT_LOADED, UNIT_ERROR, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_MASKED))
d0811f
+        if (!UNIT_IS_LOAD_COMPLETE(unit->load_state))
d0811f
                 return sd_bus_error_setf(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id);
d0811f
 
d0811f
         if (type != JOB_STOP) {
d0811f
diff --git a/src/core/unit.h b/src/core/unit.h
d0811f
index 4130cd50a9..ae2ce74243 100644
d0811f
--- a/src/core/unit.h
d0811f
+++ b/src/core/unit.h
d0811f
@@ -49,6 +49,10 @@ static inline bool UNIT_IS_INACTIVE_OR_FAILED(UnitActiveState t) {
d0811f
         return IN_SET(t, UNIT_INACTIVE, UNIT_FAILED);
d0811f
 }
d0811f
 
d0811f
+static inline bool UNIT_IS_LOAD_COMPLETE(UnitLoadState t) {
d0811f
+        return t >= 0 && t < _UNIT_LOAD_STATE_MAX && t != UNIT_STUB && t != UNIT_MERGED;
d0811f
+}
d0811f
+
d0811f
 /* Stores the 'reason' a dependency was created as a bit mask, i.e. due to which configuration source it came to be. We
d0811f
  * use this so that we can selectively flush out parts of dependencies again. Note that the same dependency might be
d0811f
  * created as a result of multiple "reasons", hence the bitmask. */
d0811f
-- 
d0811f
2.26.2
d0811f
d0811f
d0811f
From 6b083e21c2bfdba79d43d5d56f02dc795dae9368 Mon Sep 17 00:00:00 2001
d0811f
From: Lennart Poettering <lennart@poettering.net>
d0811f
Date: Fri, 11 Sep 2020 19:57:09 +0200
d0811f
Subject: [PATCH 2/3] core: propagate unit start limit hit state to triggering
d0811f
 path unit
d0811f
d0811f
We already do this for socket and automount units, do it for path units
d0811f
too: if the triggered service keeps hitting the start limit, then fail
d0811f
the triggering unit too, so that we don#t busy loop forever.
d0811f
d0811f
(Note that this leaves only timer units out in the cold for this kind of
d0811f
protection, but it shouldn't matter there, as they are naturally
d0811f
protected against busy loops: they are scheduled by time anyway).
d0811f
d0811f
Fixes: #16669
d0811f
---
d0811f
 src/core/path.c | 15 +++++++++++++++
d0811f
 src/core/path.h |  1 +
d0811f
 2 files changed, 16 insertions(+)
d0811f
d0811f
diff --git a/src/core/path.c b/src/core/path.c
d0811f
index 8ffec72ede..4f4e7100cf 100644
d0811f
--- a/src/core/path.c
d0811f
+++ b/src/core/path.c
d0811f
@@ -753,6 +753,20 @@ static void path_trigger_notify(Unit *u, Unit *other) {
d0811f
         /* Filter out invocations with bogus state */
d0811f
         assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
d0811f
 
d0811f
+        /* Don't propagate state changes from the triggered unit if we are already down */
d0811f
+        if (!IN_SET(p->state, PATH_WAITING, PATH_RUNNING))
d0811f
+                return;
d0811f
+
d0811f
+        /* Propagate start limit hit state */
d0811f
+        if (other->start_limit_hit) {
d0811f
+                path_enter_dead(p, PATH_FAILURE_UNIT_START_LIMIT_HIT);
d0811f
+                return;
d0811f
+        }
d0811f
+
d0811f
+        /* Don't propagate anything if there's still a job queued */
d0811f
+        if (other->job)
d0811f
+                return;
d0811f
+
d0811f
         if (p->state == PATH_RUNNING &&
d0811f
             UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
d0811f
                 log_unit_debug(UNIT(p), "Got notified about unit deactivation.");
d0811f
@@ -789,6 +803,7 @@ static const char* const path_result_table[_PATH_RESULT_MAX] = {
d0811f
         [PATH_SUCCESS] = "success",
d0811f
         [PATH_FAILURE_RESOURCES] = "resources",
d0811f
         [PATH_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
d0811f
+        [PATH_FAILURE_UNIT_START_LIMIT_HIT] = "unit-start-limit-hit",
d0811f
 };
d0811f
 
d0811f
 DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);
d0811f
diff --git a/src/core/path.h b/src/core/path.h
d0811f
index 9e2836535a..4043650fe0 100644
d0811f
--- a/src/core/path.h
d0811f
+++ b/src/core/path.h
d0811f
@@ -45,6 +45,7 @@ typedef enum PathResult {
d0811f
         PATH_SUCCESS,
d0811f
         PATH_FAILURE_RESOURCES,
d0811f
         PATH_FAILURE_START_LIMIT_HIT,
d0811f
+        PATH_FAILURE_UNIT_START_LIMIT_HIT,
d0811f
         _PATH_RESULT_MAX,
d0811f
         _PATH_RESULT_INVALID = -1
d0811f
 } PathResult;
d0811f
-- 
d0811f
2.26.2
d0811f
d0811f
d0811f
From 32c556c612ff38b09fe7d14d1840aceb2d76360d Mon Sep 17 00:00:00 2001
d0811f
From: Lennart Poettering <lennart@poettering.net>
d0811f
Date: Mon, 14 Sep 2020 12:59:38 +0200
d0811f
Subject: [PATCH 3/3] unit-def: drop pointless 0 initialization of first enum
d0811f
 value
d0811f
d0811f
This is implied in C and we generally don't bother with this, so don't
d0811f
bother with this here either.
d0811f
---
d0811f
 src/basic/unit-def.h | 4 ++--
d0811f
 1 file changed, 2 insertions(+), 2 deletions(-)
d0811f
d0811f
diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h
d0811f
index 53419ecd8a..1fab6c78ab 100644
d0811f
--- a/src/basic/unit-def.h
d0811f
+++ b/src/basic/unit-def.h
d0811f
@@ -9,7 +9,7 @@
d0811f
  * when other criteria (cpu weight, nice level) are identical.
d0811f
  * In this case service units have the highest priority. */
d0811f
 typedef enum UnitType {
d0811f
-        UNIT_SERVICE = 0,
d0811f
+        UNIT_SERVICE,
d0811f
         UNIT_MOUNT,
d0811f
         UNIT_SWAP,
d0811f
         UNIT_SOCKET,
d0811f
@@ -25,7 +25,7 @@ typedef enum UnitType {
d0811f
 } UnitType;
d0811f
 
d0811f
 typedef enum UnitLoadState {
d0811f
-        UNIT_STUB = 0,
d0811f
+        UNIT_STUB,
d0811f
         UNIT_LOADED,
d0811f
         UNIT_NOT_FOUND,    /* error condition #1: unit file not found */
d0811f
         UNIT_BAD_SETTING,  /* error condition #2: we couldn't parse some essential unit file setting */
d0811f
-- 
d0811f
2.26.2
d0811f