From 072993504e3e4206ae1019f5461a0372f7d82ddf Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 2 May 2016 13:01:26 +0200
Subject: [PATCH] core: move enforcement of the start limit into per-unit-type
 code again

Let's move the enforcement of the per-unit start limit from unit.c into the
type-specific files again. For unit types that know a concept of "result" codes
this allows us to hook up the start limit condition to it with an explicit
result code. Also, this makes sure that the state checks in clal like
service_start() may be done before the start limit is checked, as the start
limit really should be checked last, right before everything has been verified
to be in order.

The generic start limit logic is left in unit.c, but the invocation of it is
moved into the per-type files, in the various xyz_start() functions, so that
they may place the check at the right location.

Note that this change drops the enforcement entirely from device, slice, target
and scope units, since these unit types generally may not fail activation, or
may only be activated a single time. This is also documented now.

Note that restores the "start-limit-hit" result code that existed before
6bf0f408e4833152197fb38fb10a9989c89f3a59 already in the service code. However,
it's not introduced for all units that have a result code concept.

Fixes #3166.
---
 man/systemd.unit.xml |  4 +++-
 src/core/automount.c | 10 +++++++++-
 src/core/automount.h |  1 +
 src/core/busname.c   |  8 ++++++++
 src/core/busname.h   |  1 +
 src/core/mount.c     | 10 +++++++++-
 src/core/mount.h     |  1 +
 src/core/path.c      |  8 ++++++++
 src/core/path.h      |  1 +
 src/core/service.c   |  9 +++++++++
 src/core/service.h   |  1 +
 src/core/socket.c    |  8 ++++++++
 src/core/socket.h    |  1 +
 src/core/swap.c      | 10 +++++++++-
 src/core/swap.h      |  1 +
 src/core/timer.c     | 10 +++++++++-
 src/core/timer.h     |  1 +
 src/core/unit.c      |  8 +-------
 src/core/unit.h      |  2 ++
 19 files changed, 83 insertions(+), 12 deletions(-)

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index abd47bd2372..90a1ec6b9c9 100644
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -770,7 +770,9 @@
         <command>systemctl reset-failed</command> will cause the restart rate counter for a service to be flushed,
         which is useful if the administrator wants to manually start a unit and the start limit interferes with
         that. Note that this rate-limiting is enforced after any unit condition checks are executed, and hence unit
-        activations with failing conditions are not counted by this rate limiting.</para></listitem>
+        activations with failing conditions are not counted by this rate limiting. Slice, target, device and scope
+        units do not enforce this setting, as they are unit types whose activation may either never fail, or may
+        succeed only a single time.</para></listitem>
       </varlistentry>
 
       <varlistentry>
diff --git a/src/core/automount.c b/src/core/automount.c
index 7c55d7bc49d..c871b02a94c 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -751,6 +751,7 @@ fail:
 static int automount_start(Unit *u) {
         Automount *a = AUTOMOUNT(u);
         Unit *trigger;
+        int r;
 
         assert(a);
         assert(a->state == AUTOMOUNT_DEAD || a->state == AUTOMOUNT_FAILED);
@@ -766,6 +767,12 @@ static int automount_start(Unit *u) {
                 return -ENOENT;
         }
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                automount_enter_dead(a, AUTOMOUNT_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         a->result = AUTOMOUNT_SUCCESS;
         automount_enter_waiting(a);
         return 1;
@@ -1037,7 +1044,8 @@ static bool automount_supported(void) {
 
 static const char* const automount_result_table[_AUTOMOUNT_RESULT_MAX] = {
         [AUTOMOUNT_SUCCESS] = "success",
-        [AUTOMOUNT_FAILURE_RESOURCES] = "resources"
+        [AUTOMOUNT_FAILURE_RESOURCES] = "resources",
+        [AUTOMOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(automount_result, AutomountResult);
diff --git a/src/core/automount.h b/src/core/automount.h
index cf5b1cf9946..414717e5b82 100644
--- a/src/core/automount.h
+++ b/src/core/automount.h
@@ -26,6 +26,7 @@ typedef struct Automount Automount;
 typedef enum AutomountResult {
         AUTOMOUNT_SUCCESS,
         AUTOMOUNT_FAILURE_RESOURCES,
+        AUTOMOUNT_FAILURE_START_LIMIT_HIT,
         _AUTOMOUNT_RESULT_MAX,
         _AUTOMOUNT_RESULT_INVALID = -1
 } AutomountResult;
diff --git a/src/core/busname.c b/src/core/busname.c
index f4f433340c7..5600d1ac90f 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -607,6 +607,7 @@ fail:
 
 static int busname_start(Unit *u) {
         BusName *n = BUSNAME(u);
+        int r;
 
         assert(n);
 
@@ -632,6 +633,12 @@ static int busname_start(Unit *u) {
 
         assert(IN_SET(n->state, BUSNAME_DEAD, BUSNAME_FAILED));
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                busname_enter_dead(n, BUSNAME_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         n->result = BUSNAME_SUCCESS;
         busname_enter_making(n);
 
@@ -1014,6 +1021,7 @@ static const char* const busname_result_table[_BUSNAME_RESULT_MAX] = {
         [BUSNAME_FAILURE_EXIT_CODE] = "exit-code",
         [BUSNAME_FAILURE_SIGNAL] = "signal",
         [BUSNAME_FAILURE_CORE_DUMP] = "core-dump",
+        [BUSNAME_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
         [BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit",
 };
 
diff --git a/src/core/busname.h b/src/core/busname.h
index 52c4055dbb7..a8562db4586 100644
--- a/src/core/busname.h
+++ b/src/core/busname.h
@@ -32,6 +32,7 @@ typedef enum BusNameResult {
         BUSNAME_FAILURE_EXIT_CODE,
         BUSNAME_FAILURE_SIGNAL,
         BUSNAME_FAILURE_CORE_DUMP,
+        BUSNAME_FAILURE_START_LIMIT_HIT,
         BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT,
         _BUSNAME_RESULT_MAX,
         _BUSNAME_RESULT_INVALID = -1
diff --git a/src/core/mount.c b/src/core/mount.c
index cc07873b24b..aa48941f0d1 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -984,6 +984,7 @@ fail:
 
 static int mount_start(Unit *u) {
         Mount *m = MOUNT(u);
+        int r;
 
         assert(m);
 
@@ -1002,6 +1003,12 @@ static int mount_start(Unit *u) {
 
         assert(m->state == MOUNT_DEAD || m->state == MOUNT_FAILED);
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         m->result = MOUNT_SUCCESS;
         m->reload_result = MOUNT_SUCCESS;
         m->reset_cpu_usage = true;
@@ -1821,7 +1828,8 @@ static const char* const mount_result_table[_MOUNT_RESULT_MAX] = {
         [MOUNT_FAILURE_TIMEOUT] = "timeout",
         [MOUNT_FAILURE_EXIT_CODE] = "exit-code",
         [MOUNT_FAILURE_SIGNAL] = "signal",
-        [MOUNT_FAILURE_CORE_DUMP] = "core-dump"
+        [MOUNT_FAILURE_CORE_DUMP] = "core-dump",
+        [MOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(mount_result, MountResult);
diff --git a/src/core/mount.h b/src/core/mount.h
index 3b343c6b1f3..da529c44f43 100644
--- a/src/core/mount.h
+++ b/src/core/mount.h
@@ -39,6 +39,7 @@ typedef enum MountResult {
         MOUNT_FAILURE_EXIT_CODE,
         MOUNT_FAILURE_SIGNAL,
         MOUNT_FAILURE_CORE_DUMP,
+        MOUNT_FAILURE_START_LIMIT_HIT,
         _MOUNT_RESULT_MAX,
         _MOUNT_RESULT_INVALID = -1
 } MountResult;
diff --git a/src/core/path.c b/src/core/path.c
index 5e7b3eb2342..0dd0d375d8a 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -560,6 +560,7 @@ static void path_mkdir(Path *p) {
 static int path_start(Unit *u) {
         Path *p = PATH(u);
         Unit *trigger;
+        int r;
 
         assert(p);
         assert(p->state == PATH_DEAD || p->state == PATH_FAILED);
@@ -570,6 +571,12 @@ static int path_start(Unit *u) {
                 return -ENOENT;
         }
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                path_enter_dead(p, PATH_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         path_mkdir(p);
 
         p->result = PATH_SUCCESS;
@@ -739,6 +746,7 @@ DEFINE_STRING_TABLE_LOOKUP(path_type, PathType);
 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",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);
diff --git a/src/core/path.h b/src/core/path.h
index bbbcebd78e4..4230c8fb99c 100644
--- a/src/core/path.h
+++ b/src/core/path.h
@@ -62,6 +62,7 @@ static inline bool path_spec_owns_inotify_fd(PathSpec *s, int fd) {
 typedef enum PathResult {
         PATH_SUCCESS,
         PATH_FAILURE_RESOURCES,
+        PATH_FAILURE_START_LIMIT_HIT,
         _PATH_RESULT_MAX,
         _PATH_RESULT_INVALID = -1
 } PathResult;
diff --git a/src/core/service.c b/src/core/service.c
index f7a3fcf2b99..7ebabca5d6e 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1957,6 +1957,7 @@ fail:
 
 static int service_start(Unit *u) {
         Service *s = SERVICE(u);
+        int r;
 
         assert(s);
 
@@ -1983,6 +1984,13 @@ static int service_start(Unit *u) {
 
         assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED));
 
+        /* Make sure we don't enter a busy loop of some kind. */
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                service_enter_dead(s, SERVICE_FAILURE_START_LIMIT_HIT, false);
+                return r;
+        }
+
         s->result = SERVICE_SUCCESS;
         s->reload_result = SERVICE_SUCCESS;
         s->main_pid_known = false;
@@ -3266,6 +3274,7 @@ static const char* const service_result_table[_SERVICE_RESULT_MAX] = {
         [SERVICE_FAILURE_SIGNAL] = "signal",
         [SERVICE_FAILURE_CORE_DUMP] = "core-dump",
         [SERVICE_FAILURE_WATCHDOG] = "watchdog",
+        [SERVICE_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(service_result, ServiceResult);
diff --git a/src/core/service.h b/src/core/service.h
index c7f1e81bdb7..4af3d404396 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -86,6 +86,7 @@ typedef enum ServiceResult {
         SERVICE_FAILURE_SIGNAL,
         SERVICE_FAILURE_CORE_DUMP,
         SERVICE_FAILURE_WATCHDOG,
+        SERVICE_FAILURE_START_LIMIT_HIT,
         _SERVICE_RESULT_MAX,
         _SERVICE_RESULT_INVALID = -1
 } ServiceResult;
diff --git a/src/core/socket.c b/src/core/socket.c
index 7eeed068bd8..c500d122d80 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2057,6 +2057,7 @@ fail:
 
 static int socket_start(Unit *u) {
         Socket *s = SOCKET(u);
+        int r;
 
         assert(s);
 
@@ -2101,6 +2102,12 @@ static int socket_start(Unit *u) {
 
         assert(s->state == SOCKET_DEAD || s->state == SOCKET_FAILED);
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                socket_enter_dead(s, SOCKET_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         s->result = SOCKET_SUCCESS;
         s->reset_cpu_usage = true;
 
@@ -2818,6 +2825,7 @@ static const char* const socket_result_table[_SOCKET_RESULT_MAX] = {
         [SOCKET_FAILURE_EXIT_CODE] = "exit-code",
         [SOCKET_FAILURE_SIGNAL] = "signal",
         [SOCKET_FAILURE_CORE_DUMP] = "core-dump",
+        [SOCKET_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
         [SOCKET_FAILURE_TRIGGER_LIMIT_HIT] = "trigger-limit-hit",
         [SOCKET_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit"
 };
diff --git a/src/core/socket.h b/src/core/socket.h
index 2a4b1bb6741..0f1ac69c6f5 100644
--- a/src/core/socket.h
+++ b/src/core/socket.h
@@ -52,6 +52,7 @@ typedef enum SocketResult {
         SOCKET_FAILURE_EXIT_CODE,
         SOCKET_FAILURE_SIGNAL,
         SOCKET_FAILURE_CORE_DUMP,
+        SOCKET_FAILURE_START_LIMIT_HIT,
         SOCKET_FAILURE_TRIGGER_LIMIT_HIT,
         SOCKET_FAILURE_SERVICE_START_LIMIT_HIT,
         _SOCKET_RESULT_MAX,
diff --git a/src/core/swap.c b/src/core/swap.c
index d8802470d26..300911866f2 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -814,6 +814,7 @@ fail:
 
 static int swap_start(Unit *u) {
         Swap *s = SWAP(u), *other;
+        int r;
 
         assert(s);
 
@@ -842,6 +843,12 @@ static int swap_start(Unit *u) {
                 if (UNIT(other)->job && UNIT(other)->job->state == JOB_RUNNING)
                         return -EAGAIN;
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                swap_enter_dead(s, SWAP_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         s->result = SWAP_SUCCESS;
         s->reset_cpu_usage = true;
 
@@ -1447,7 +1454,8 @@ static const char* const swap_result_table[_SWAP_RESULT_MAX] = {
         [SWAP_FAILURE_TIMEOUT] = "timeout",
         [SWAP_FAILURE_EXIT_CODE] = "exit-code",
         [SWAP_FAILURE_SIGNAL] = "signal",
-        [SWAP_FAILURE_CORE_DUMP] = "core-dump"
+        [SWAP_FAILURE_CORE_DUMP] = "core-dump",
+        [SWAP_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(swap_result, SwapResult);
diff --git a/src/core/swap.h b/src/core/swap.h
index ac7a63d81b5..fbf66debdc7 100644
--- a/src/core/swap.h
+++ b/src/core/swap.h
@@ -38,6 +38,7 @@ typedef enum SwapResult {
         SWAP_FAILURE_EXIT_CODE,
         SWAP_FAILURE_SIGNAL,
         SWAP_FAILURE_CORE_DUMP,
+        SWAP_FAILURE_START_LIMIT_HIT,
         _SWAP_RESULT_MAX,
         _SWAP_RESULT_INVALID = -1
 } SwapResult;
diff --git a/src/core/timer.c b/src/core/timer.c
index f8f5f4b2e48..3206296f09d 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -599,6 +599,7 @@ static int timer_start(Unit *u) {
         Timer *t = TIMER(u);
         TimerValue *v;
         Unit *trigger;
+        int r;
 
         assert(t);
         assert(t->state == TIMER_DEAD || t->state == TIMER_FAILED);
@@ -609,6 +610,12 @@ static int timer_start(Unit *u) {
                 return -ENOENT;
         }
 
+        r = unit_start_limit_test(u);
+        if (r < 0) {
+                timer_enter_dead(t, TIMER_FAILURE_START_LIMIT_HIT);
+                return r;
+        }
+
         t->last_trigger = DUAL_TIMESTAMP_NULL;
 
         /* Reenable all timers that depend on unit activation time */
@@ -808,7 +815,8 @@ DEFINE_STRING_TABLE_LOOKUP(timer_base, TimerBase);
 
 static const char* const timer_result_table[_TIMER_RESULT_MAX] = {
         [TIMER_SUCCESS] = "success",
-        [TIMER_FAILURE_RESOURCES] = "resources"
+        [TIMER_FAILURE_RESOURCES] = "resources",
+        [TIMER_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(timer_result, TimerResult);
diff --git a/src/core/timer.h b/src/core/timer.h
index 698e6da2f5e..9c4b64f8981 100644
--- a/src/core/timer.h
+++ b/src/core/timer.h
@@ -48,6 +48,7 @@ typedef struct TimerValue {
 typedef enum TimerResult {
         TIMER_SUCCESS,
         TIMER_FAILURE_RESOURCES,
+        TIMER_FAILURE_START_LIMIT_HIT,
         _TIMER_RESULT_MAX,
         _TIMER_RESULT_INVALID = -1
 } TimerResult;
diff --git a/src/core/unit.c b/src/core/unit.c
index 64466e4fb45..fd9ecc36cef 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1462,7 +1462,7 @@ void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) {
         unit_status_print_starting_stopping(u, t);
 }
 
-static int unit_start_limit_test(Unit *u) {
+int unit_start_limit_test(Unit *u) {
         assert(u);
 
         if (ratelimit_test(&u->start_limit)) {
@@ -1488,7 +1488,6 @@ static int unit_start_limit_test(Unit *u) {
 int unit_start(Unit *u) {
         UnitActiveState state;
         Unit *following;
-        int r;
 
         assert(u);
 
@@ -1541,11 +1540,6 @@ int unit_start(Unit *u) {
         if (!UNIT_VTABLE(u)->start)
                 return -EBADR;
 
-        /* Make sure we don't enter a busy loop of some kind. */
-        r = unit_start_limit_test(u);
-        if (r < 0)
-                return r;
-
         /* We don't suppress calls to ->start() here when we are
          * already starting, to allow this request to be used as a
          * "hurry up" call, for example when the unit is in some "auto
diff --git a/src/core/unit.h b/src/core/unit.h
index 5909652976a..6ae1a8984a3 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -617,6 +617,8 @@ static inline bool unit_supported(Unit *u) {
 void unit_warn_if_dir_nonempty(Unit *u, const char* where);
 int unit_fail_if_symlink(Unit *u, const char* where);
 
+int unit_start_limit_test(Unit *u);
+
 /* Macros which append UNIT= or USER_UNIT= to the message */
 
 #define log_unit_full(unit, level, error, ...)                          \
-- 
2.39.2