core: move start ratelimiting check after condition checks

With #2564 unit start rate limiting was moved from after the condition checks
are to before they are made, in an attempt to fix #2467. This however resulted
in #2684. However, with a previous commit a concept of per socket unit trigger
rate limiting has been added, to fix #2467 more comprehensively, hence the
start limit can be moved after the condition checks again, thus fixing #2684.

Fixes: #2684

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index 7993301167946604f00ffffcfab28fb9730575c5..9c869b980502c5cfcebe8dcaddffd52e3e99d333 100644 (file)
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -769,7 +769,8 @@
         manually at a later point, from which point on, the restart logic is again activated. Note that
         <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.</para></listitem>
+        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>
       </varlistentry>
 
       <varlistentry>

diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 32bb62fa63c498d5143936b6aaa4c04a0504fb1c..5f4d66d64d1b3ccebe80ab8a1b281a5c870f3b8f 100644 (file)
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -220,6 +220,7 @@ Service.TimeoutStartSec,         config_parse_service_timeout,       0,
 Service.TimeoutStopSec,          config_parse_service_timeout,       0,                             0
 Service.RuntimeMaxSec,           config_parse_sec,                   0,                             offsetof(Service, runtime_max_usec)
 Service.WatchdogSec,             config_parse_sec,                   0,                             offsetof(Service, watchdog_usec)
+m4_dnl The following three only exist for compatibility, they moved into Unit, see above
 Service.StartLimitInterval,      config_parse_sec,                   0,                             offsetof(Unit, start_limit.interval)
 Service.StartLimitBurst,         config_parse_unsigned,              0,                             offsetof(Unit, start_limit.burst)
 Service.StartLimitAction,        config_parse_failure_action,        0,                             offsetof(Unit, start_limit_action)

diff --git a/src/core/unit.c b/src/core/unit.c
index cb79c7c6b1d7cde10c541d822465ccedbd0a559a..6c0684225a3fe229541014d7c7e4cbe07af9e245 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1497,11 +1497,6 @@ int unit_start(Unit *u) {
         if (UNIT_IS_ACTIVE_OR_RELOADING(state))
                 return -EALREADY;
 
-        /* Make sure we don't enter a busy loop of some kind. */
-        r = unit_start_limit_test(u);
-        if (r < 0)
-                return r;
-
         /* Units that aren't loaded cannot be started */
         if (u->load_state != UNIT_LOADED)
                 return -EINVAL;
@@ -1543,6 +1538,11 @@ 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