Merge pull request #17031 from poettering/path-start-limit

core: propagate start limit hit from triggered unit to path unit

src/basic/unit-def.h

@@ -9,7 +9,7 @@
  * when other criteria (cpu weight, nice level) are identical.
  * In this case service units have the highest priority. */
 typedef enum UnitType {
-        UNIT_SERVICE = 0,
+        UNIT_SERVICE,
         UNIT_MOUNT,
         UNIT_SWAP,
         UNIT_SOCKET,
@@ -25,7 +25,7 @@ typedef enum UnitType {
 } UnitType;
 
 typedef enum UnitLoadState {
-        UNIT_STUB = 0,
+        UNIT_STUB,
         UNIT_LOADED,
         UNIT_NOT_FOUND,    /* error condition #1: unit file not found */
         UNIT_BAD_SETTING,  /* error condition #2: we couldn't parse some essential unit file setting */

src/core/automount.c

@@ -507,8 +507,8 @@ static void automount_trigger_notify(Unit *u, Unit *other) {
         assert(other);
 
         /* Filter out invocations with bogus state */
-        if (other->load_state != UNIT_LOADED || other->type != UNIT_MOUNT)
-                return;
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
+        assert(other->type == UNIT_MOUNT);
 
         /* Don't propagate state changes from the mount if we are already down */
         if (!IN_SET(a->state, AUTOMOUNT_WAITING, AUTOMOUNT_RUNNING))

src/core/path.c

@@ -748,10 +748,23 @@ static void path_trigger_notify(Unit *u, Unit *other) {
         assert(u);
         assert(other);
 
-        /* Invoked whenever the unit we trigger changes state or gains
-         * or loses a job */
+        /* Invoked whenever the unit we trigger changes state or gains or loses a job */
 
-        if (other->load_state != UNIT_LOADED)
+        /* Filter out invocations with bogus state */
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
+
+        /* Don't propagate state changes from the triggered unit if we are already down */
+        if (!IN_SET(p->state, PATH_WAITING, PATH_RUNNING))
+                return;
+
+        /* Propagate start limit hit state */
+        if (other->start_limit_hit) {
+                path_enter_dead(p, PATH_FAILURE_UNIT_START_LIMIT_HIT);
+                return;
+        }
+
+        /* Don't propagate anything if there's still a job queued */
+        if (other->job)
                 return;
 
         if (p->state == PATH_RUNNING &&
@@ -790,6 +803,7 @@ static const char* const path_result_table[_PATH_RESULT_MAX] = {
         [PATH_SUCCESS] = "success",
         [PATH_FAILURE_RESOURCES] = "resources",
         [PATH_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
+        [PATH_FAILURE_UNIT_START_LIMIT_HIT] = "unit-start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);

src/core/path.h

@@ -45,6 +45,7 @@ typedef enum PathResult {
         PATH_SUCCESS,
         PATH_FAILURE_RESOURCES,
         PATH_FAILURE_START_LIMIT_HIT,
+        PATH_FAILURE_UNIT_START_LIMIT_HIT,
         _PATH_RESULT_MAX,
         _PATH_RESULT_INVALID = -1
 } PathResult;

src/core/socket.c

@@ -3265,13 +3265,8 @@ static void socket_trigger_notify(Unit *u, Unit *other) {
         assert(other);
 
         /* Filter out invocations with bogus state */
-        if (!IN_SET(other->load_state,
-                    UNIT_LOADED,
-                    UNIT_NOT_FOUND,
-                    UNIT_BAD_SETTING,
-                    UNIT_ERROR,
-                    UNIT_MASKED) || other->type != UNIT_SERVICE)
-                return;
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
+        assert(other->type == UNIT_SERVICE);
 
         /* Don't propagate state changes from the service if we are already down */
         if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING))

src/core/timer.c

@@ -746,8 +746,8 @@ static void timer_trigger_notify(Unit *u, Unit *other) {
         assert(u);
         assert(other);
 
-        if (other->load_state != UNIT_LOADED)
-                return;
+        /* Filter out invocations with bogus state */
+        assert(UNIT_IS_LOAD_COMPLETE(other->load_state));
 
         /* Reenable all timers that depend on unit state */
         LIST_FOREACH(value, v, t->values)

src/core/transaction.c

@@ -938,7 +938,7 @@ int transaction_add_job_and_dependencies(
 
         /* Safety check that the unit is a valid state, i.e. not in UNIT_STUB or UNIT_MERGED which should only be set
          * temporarily. */
-        if (!IN_SET(unit->load_state, UNIT_LOADED, UNIT_ERROR, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_MASKED))
+        if (!UNIT_IS_LOAD_COMPLETE(unit->load_state))
                 return sd_bus_error_setf(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id);
 
         if (type != JOB_STOP) {

src/core/unit.h

@@ -49,6 +49,10 @@ static inline bool UNIT_IS_INACTIVE_OR_FAILED(UnitActiveState t) {
         return IN_SET(t, UNIT_INACTIVE, UNIT_FAILED);
 }
 
+static inline bool UNIT_IS_LOAD_COMPLETE(UnitLoadState t) {
+        return t >= 0 && t < _UNIT_LOAD_STATE_MAX && t != UNIT_STUB && t != UNIT_MERGED;
+}
+
 /* Stores the 'reason' a dependency was created as a bit mask, i.e. due to which configuration source it came to be. We
  * use this so that we can selectively flush out parts of dependencies again. Note that the same dependency might be
  * created as a result of multiple "reasons", hence the bitmask. */