[systemd-devel] [PATCH v3] timer: order OnCalendar units after timer-sync.target if DefaultDependencies=no

[Tobias Geerinckx-Rice]

Avoids triggering timers prematurely on systems with significantly inaccurate
clocks, or some embedded platforms that lack one entirely.
---
v2:
  - Change systemd.timer.xml to clarify that only OnCalendar= timers are
    affected. Lennart, I didn't use your wording because a) I had already
    spotted the mistake before reading your e-mail and b) IMO it made the
    sentence overly long and unclear;
  - use LIST_FOREACH to loop over all entries until an OnCalendar= directive
    is found.
v3:
  - Fix a stupid tabbo in man/systemd.timer.xml

Regards,

T G-R

 TODO                  |  2 --
 man/systemd.timer.xml | 17 +++++++++++------
 src/core/timer.c      | 10 ++++++++++
 src/shared/special.h  |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/TODO b/TODO
index 1dbb9ff..7dad2ce 100644
--- a/TODO
+++ b/TODO
@@ -60,8 +60,6 @@ Features:
 
 * Add a new verb "systemctl top"
 
-* order OnCalendar timer units after timer-sync.target if DefaultDependencies=no so that we don't trigger them prematurely
-
 * refuse mounting on symlinks
 
 * logind: allow users to kill or lock their own sessions
diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml
index d82b9bd..a7aeb75 100644
--- a/man/systemd.timer.xml
+++ b/man/systemd.timer.xml
@@ -78,15 +78,20 @@
                 <varname>Unit=</varname> (see below).</para>
 
                 <para>Unless <varname>DefaultDependencies=</varname>
-                is set to <option>false</option>, timer units will
+                is set to <option>false</option>, all timer units will
                 implicitly have dependencies of type
                 <varname>Conflicts=</varname> and
                 <varname>Before=</varname> on
-                <filename>shutdown.target</filename>. These ensure
-                that timer units are stopped cleanly prior to system
-                shutdown. Only timer units involved with early boot or
-                late system shutdown should disable this
-                option.</para>
+                <filename>shutdown.target</filename> to ensure that
+                they are stopped cleanly prior to system shutdown.
+                Timer units with at least one
+                <varname>OnCalendar=</varname> directive will have an
+                additional <varname>After=</varname> dependency on
+                <filename>timer-sync.target</filename> to avoid
+                being started before the system clock has been
+                correctly set. Only timer units involved with early
+                boot or late system shutdown should disable the
+                <varname>DefaultDependencies=</varname> option.</para>
         </refsect1>
 
         <refsect1>
diff --git a/src/core/timer.c b/src/core/timer.c
index a5a33a6..dc0f289 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -95,6 +95,7 @@ static int timer_verify(Timer *t) {
 
 static int timer_add_default_dependencies(Timer *t) {
         int r;
+        TimerValue *v;
 
         assert(t);
 
@@ -106,6 +107,15 @@ static int timer_add_default_dependencies(Timer *t) {
                 r = unit_add_two_dependencies_by_name(UNIT(t), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true);
                 if (r < 0)
                         return r;
+
+                LIST_FOREACH(value, v, t->values) {
+                        if (v->base == TIMER_CALENDAR) {
+                                r = unit_add_dependency_by_name(UNIT(t), UNIT_AFTER, SPECIAL_TIME_SYNC_TARGET, NULL, true);
+                                if (r < 0)
+                                        return r;
+                                break;
+                        }
+                }
         }
 
         return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
diff --git a/src/shared/special.h b/src/shared/special.h
index 2fe5db5..b045047 100644
--- a/src/shared/special.h
+++ b/src/shared/special.h
@@ -57,13 +57,13 @@
 #define SPECIAL_REMOTE_FS_PRE_TARGET "remote-fs-pre.target"
 #define SPECIAL_SWAP_TARGET "swap.target"
 #define SPECIAL_NETWORK_ONLINE_TARGET "network-online.target"
+#define SPECIAL_TIME_SYNC_TARGET "time-sync.target"       /* LSB's $time */
 #define SPECIAL_BASIC_TARGET "basic.target"
 
 /* LSB compatibility */
 #define SPECIAL_NETWORK_TARGET "network.target"           /* LSB's $network */
 #define SPECIAL_NSS_LOOKUP_TARGET "nss-lookup.target"     /* LSB's $named */
 #define SPECIAL_RPCBIND_TARGET "rpcbind.target"           /* LSB's $portmap */
-#define SPECIAL_TIME_SYNC_TARGET "time-sync.target"       /* LSB's $time */
 
 /*
  * Rules regarding adding further high level targets like the above:
-- 
2.0.4