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.

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index abd47bd2372cb2793bd7d6005830ee2bf49b58c8..90a1ec6b9c978414a8cd9a2e130e4dc315d93271 100644 (file)
--- 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 7c55d7bc49d9064eda035e546dd93466b0b6ddd5..c871b02a94cbc474483f7e74aaadd35a38b6be94 100644 (file)
--- 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 cf5b1cf99461e5e4dd24cb86f489620ff5024a8a..414717e5b82917e00ed894016c085536e7f93e17 100644 (file)
--- 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 f4f433340c7b9f40cee50ee0d67be10df99f8921..5600d1ac90f9b8eb2f8c19f145f25c45320b1f13 100644 (file)
--- 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 52c4055dbb7ae6ecf7fbd97242134efee3fd914a..a8562db45861fe642bff12e411f13a9af1f5acba 100644 (file)
--- 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 cc07873b24bffc000ece43db61a55b3dcdee37f3..aa48941f0d1fb99e4419c91ed6421b44976ebde3 100644 (file)
--- 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 3b343c6b1f361612454264dc061898857b5198fe..da529c44f4399547fd735e7128790ecd38e5865d 100644 (file)
--- 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 5e7b3eb2342f1990980a7bd1352ed9266197decc..0dd0d375d8a6294b464eeb5d79ddd96a54781758 100644 (file)
--- 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 bbbcebd78e4090750f549be2d8bf6f392174cc1b..4230c8fb99c70c21f8faf1f992e2ddc31f72c803 100644 (file)
--- 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 f7a3fcf2b99c646ecac343ea743d4b4eb975bb32..7ebabca5d6ed0ac3d37508d74029a5cb1b21059c 100644 (file)
--- 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 c7f1e81bdb743e7c61c37d1e4215a8fc8446e32a..4af3d4043964c1528eb08423b5b889540f01416b 100644 (file)
--- 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 7eeed068bd87064669358e265f1366e8f27016db..c500d122d8004c3bd3f1f0ea13a2717e46088e5a 100644 (file)
--- 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 2a4b1bb6741a6117a3b0dda7db4c88cb41bc253c..0f1ac69c6f5e0078f29eac74a1ac7874c3b351f8 100644 (file)
--- 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 d8802470d262cd8f465671f0d3d435ce909d07e1..300911866f28f6adab07aa5b3493cd258ec41287 100644 (file)
--- 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 ac7a63d81b55aec0a57bd464c5f06ea0d0bd6dfc..fbf66debdc7b221a914fa09911d8136f48b4719a 100644 (file)
--- 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 f8f5f4b2e48d00192c15398684df5a2b85e399cc..3206296f09d3aa11faebaf73cd8940d85d84e2aa 100644 (file)
--- 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 698e6da2f5e0b454604d2b2b0b16d560fefdcc24..9c4b64f8981ba2b2fc53485a877091113966d0b2 100644 (file)
--- 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 64466e4fb451cdc6c808ea6de3e891801c6f9fe9..fd9ecc36cef2168135d24d7389bbd823e2783414 100644 (file)
--- 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 5909652976a13d114b3a22556c09ad3a10405928..6ae1a8984a381f607bde99fe8c8c1f8f4fe66a37 100644 (file)
--- 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, ...)                          \