Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
On Sat, Apr 12, 2014 at 01:02:53PM +0200, Tom Gundersen wrote: > On Sat, Apr 12, 2014 at 12:57 PM, Thomas Bächler wrote: > > Am 05.04.2014 17:32, schrieb Thomas Bächler: > >> Am 05.04.2014 11:35, schrieb Tom Gundersen: > >>> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler > >>> wrote: > If a persistent timer has no stamp file yet, it behaves just like a > normal > timer until it runs for the first time. If the system is always shut down > while the timer is supposed to run, a stamp file is never created and > Peristent=true has no effect. > > This patch fixes this by creating a stamp file with the current time > when the timer is first started. > >>> > >>> If timers are started at early boot (which sounds like a common > >>> scenario), I guess /var will not yet be writable so this will be a > >>> noop, no? Maybe it would be better to write out these files at > >>> shutdown instead (before unmounting anything)? > >> > >> I failed to hit "reply all" last time, so apologies for sending you this > >> mail twice, Tom. > >> > >> Persistent=true timers have an implicit dependency on > >> RequiresMountsFor=/var/lib/systemd/timers. > >> > >> $ systemctl show -p RequiresMountsFor updatedb.timer > >> RequiresMountsFor=/var/lib/systemd/timers > >> > >> $ systemctl cat updatedb.timer > >> # /usr/lib/systemd/system/updatedb.timer > >> [Unit] > >> Description=Daily locate database update > >> > >> [Timer] > >> OnCalendar=daily > >> AccuracySec=12h > >> Persistent=true > > > > I don't want to be annoying, but I'd really like an ACK or NAK on that > > patch. Applied. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
On Sat, Apr 12, 2014 at 12:57 PM, Thomas Bächler wrote: > Am 05.04.2014 17:32, schrieb Thomas Bächler: >> Am 05.04.2014 11:35, schrieb Tom Gundersen: >>> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler wrote: If a persistent timer has no stamp file yet, it behaves just like a normal timer until it runs for the first time. If the system is always shut down while the timer is supposed to run, a stamp file is never created and Peristent=true has no effect. This patch fixes this by creating a stamp file with the current time when the timer is first started. >>> >>> If timers are started at early boot (which sounds like a common >>> scenario), I guess /var will not yet be writable so this will be a >>> noop, no? Maybe it would be better to write out these files at >>> shutdown instead (before unmounting anything)? >> >> I failed to hit "reply all" last time, so apologies for sending you this >> mail twice, Tom. >> >> Persistent=true timers have an implicit dependency on >> RequiresMountsFor=/var/lib/systemd/timers. >> >> $ systemctl show -p RequiresMountsFor updatedb.timer >> RequiresMountsFor=/var/lib/systemd/timers >> >> $ systemctl cat updatedb.timer >> # /usr/lib/systemd/system/updatedb.timer >> [Unit] >> Description=Daily locate database update >> >> [Timer] >> OnCalendar=daily >> AccuracySec=12h >> Persistent=true > > I don't want to be annoying, but I'd really like an ACK or NAK on that > patch. To me it looks good, but I don't know this area too well, so would prefer Lennart (or someone else) to look at it before applying. Lennart is currently travelling, so may take a bit more time. Sorry about that. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
Am 05.04.2014 17:32, schrieb Thomas Bächler: > Am 05.04.2014 11:35, schrieb Tom Gundersen: >> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler wrote: >>> If a persistent timer has no stamp file yet, it behaves just like a normal >>> timer until it runs for the first time. If the system is always shut down >>> while the timer is supposed to run, a stamp file is never created and >>> Peristent=true has no effect. >>> >>> This patch fixes this by creating a stamp file with the current time >>> when the timer is first started. >> >> If timers are started at early boot (which sounds like a common >> scenario), I guess /var will not yet be writable so this will be a >> noop, no? Maybe it would be better to write out these files at >> shutdown instead (before unmounting anything)? > > I failed to hit "reply all" last time, so apologies for sending you this > mail twice, Tom. > > Persistent=true timers have an implicit dependency on > RequiresMountsFor=/var/lib/systemd/timers. > > $ systemctl show -p RequiresMountsFor updatedb.timer > RequiresMountsFor=/var/lib/systemd/timers > > $ systemctl cat updatedb.timer > # /usr/lib/systemd/system/updatedb.timer > [Unit] > Description=Daily locate database update > > [Timer] > OnCalendar=daily > AccuracySec=12h > Persistent=true I don't want to be annoying, but I'd really like an ACK or NAK on that patch. signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
Am 05.04.2014 11:35, schrieb Tom Gundersen: > On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler wrote: >> If a persistent timer has no stamp file yet, it behaves just like a normal >> timer until it runs for the first time. If the system is always shut down >> while the timer is supposed to run, a stamp file is never created and >> Peristent=true has no effect. >> >> This patch fixes this by creating a stamp file with the current time >> when the timer is first started. > > If timers are started at early boot (which sounds like a common > scenario), I guess /var will not yet be writable so this will be a > noop, no? Maybe it would be better to write out these files at > shutdown instead (before unmounting anything)? I failed to hit "reply all" last time, so apologies for sending you this mail twice, Tom. Persistent=true timers have an implicit dependency on RequiresMountsFor=/var/lib/systemd/timers. $ systemctl show -p RequiresMountsFor updatedb.timer RequiresMountsFor=/var/lib/systemd/timers $ systemctl cat updatedb.timer # /usr/lib/systemd/system/updatedb.timer [Unit] Description=Daily locate database update [Timer] OnCalendar=daily AccuracySec=12h Persistent=true signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
On Sat, Apr 05, 2014 at 11:35:24AM +0200, Tom Gundersen wrote: > On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler wrote: > > If a persistent timer has no stamp file yet, it behaves just like a normal > > timer until it runs for the first time. If the system is always shut down > > while the timer is supposed to run, a stamp file is never created and > > Peristent=true has no effect. > > > > This patch fixes this by creating a stamp file with the current time > > when the timer is first started. > > If timers are started at early boot (which sounds like a common > scenario), I guess /var will not yet be writable so this will be a > noop, no? Maybe it would be better to write out these files at > shutdown instead (before unmounting anything)? Isn't timers.target starter after sysinit.target, i.e. after /var has been mounted? Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler wrote: > If a persistent timer has no stamp file yet, it behaves just like a normal > timer until it runs for the first time. If the system is always shut down > while the timer is supposed to run, a stamp file is never created and > Peristent=true has no effect. > > This patch fixes this by creating a stamp file with the current time > when the timer is first started. If timers are started at early boot (which sounds like a common scenario), I guess /var will not yet be writable so this will be a noop, no? Maybe it would be better to write out these files at shutdown instead (before unmounting anything)? Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers
If a persistent timer has no stamp file yet, it behaves just like a normal
timer until it runs for the first time. If the system is always shut down
while the timer is supposed to run, a stamp file is never created and
Peristent=true has no effect.
This patch fixes this by creating a stamp file with the current time
when the timer is first started.
---
src/core/timer.c | 40
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/src/core/timer.c b/src/core/timer.c
index 6c85304..b0a9023 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -111,6 +111,23 @@ static int timer_add_default_dependencies(Timer *t) {
return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE,
UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true);
}
+static void update_stampfile(Timer *t, usec_t timestamp) {
+_cleanup_close_ int fd = -1;
+
+mkdir_parents_label(t->stamp_path, 0755);
+
+/* Update the file atime + mtime, if we can */
+fd = open(t->stamp_path, O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0644);
+if (fd >= 0) {
+struct timespec ts[2];
+
+timespec_store(&ts[0], timestamp);
+ts[1] = ts[0];
+
+futimens(fd, ts);
+}
+}
+
static int timer_setup_persistent(Timer *t) {
int r;
@@ -496,22 +513,8 @@ static void timer_enter_running(Timer *t) {
dual_timestamp_get(&t->last_trigger);
-if (t->stamp_path) {
-_cleanup_close_ int fd = -1;
-
-mkdir_parents_label(t->stamp_path, 0755);
-
-/* Update the file atime + mtime, if we can */
-fd = open(t->stamp_path, O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC,
0644);
-if (fd >= 0) {
-struct timespec ts[2];
-
-timespec_store(&ts[0], t->last_trigger.realtime);
-ts[1] = ts[0];
-
-futimens(fd, ts);
-}
-}
+if (t->stamp_path)
+update_stampfile(t, t->last_trigger.realtime);
timer_set_state(t, TIMER_RUNNING);
return;
@@ -539,6 +542,11 @@ static int timer_start(Unit *u) {
if (stat(t->stamp_path, &st) >= 0)
t->last_trigger.realtime = timespec_load(&st.st_atim);
+else if (errno == ENOENT)
+/* The timer has never run before,
+ * make sure a stamp file exists.
+ */
+update_stampfile(t, now(CLOCK_REALTIME));
}
t->result = TIMER_SUCCESS;
--
1.9.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
