From 9bb656bd0cd9ca8d7c593c48d51c19d66557df53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Apr 2019 13:21:54 +0200 Subject: [PATCH 1/2] calendarspec: make return time value of calendar_spec_next_usec() optional If noone is interested, there's no reason to return it. (Also document the ENOENT error code in a comment) --- src/shared/calendarspec.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 6e318fa265..4365dbaca8 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -1193,6 +1193,8 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) { int tm_usec; int r; + /* Returns -ENOENT if the expression is not going to elapse anymore */ + assert(spec); assert(tm); @@ -1285,14 +1287,13 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) { } } -static int calendar_spec_next_usec_impl(const CalendarSpec *spec, usec_t usec, usec_t *next) { +static int calendar_spec_next_usec_impl(const CalendarSpec *spec, usec_t usec, usec_t *ret_next) { struct tm tm; time_t t; int r; usec_t tm_usec; assert(spec); - assert(next); if (usec > USEC_TIMESTAMP_FORMATTABLE_MAX) return -EINVAL; @@ -1310,7 +1311,9 @@ static int calendar_spec_next_usec_impl(const CalendarSpec *spec, usec_t usec, u if (t < 0) return -EINVAL; - *next = (usec_t) t * USEC_PER_SEC + tm_usec; + if (ret_next) + *ret_next = (usec_t) t * USEC_PER_SEC + tm_usec; + return 0; } @@ -1319,12 +1322,14 @@ typedef struct SpecNextResult { int return_value; } SpecNextResult; -int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) { +int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *ret_next) { SpecNextResult *shared, tmp; int r; + assert(spec); + if (isempty(spec->timezone)) - return calendar_spec_next_usec_impl(spec, usec, next); + return calendar_spec_next_usec_impl(spec, usec, ret_next); shared = mmap(NULL, sizeof *shared, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1, 0); if (shared == MAP_FAILED) @@ -1352,8 +1357,8 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) if (munmap(shared, sizeof *shared) < 0) return negative_errno(); - if (tmp.return_value == 0) - *next = tmp.next; + if (tmp.return_value == 0 && ret_next) + *ret_next = tmp.next; return tmp.return_value; } From 04220fda5c42427a3513078ab0fd4e3431460509 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Apr 2019 13:22:55 +0200 Subject: [PATCH 2/2] run: when we determine a timer cannot elapse anymore, really just warn, nothing else When we determine that a calendar expression cannot elapse anymore, print a warning but proceed regardless like we normally would. Quite possibly a remote system has a different understanding of time (timezone, system clock) than we have, hence we really shouldn't change behaviour here client side, but log at best, and then leave the decision what to do to the server side. Follow-up for #12299 --- src/run/run.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 6a0b0d78b9..babd9027e4 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -382,21 +382,22 @@ static int parse_argv(int argc, char *argv[]) { case ARG_ON_CALENDAR: { _cleanup_(calendar_spec_freep) CalendarSpec *cs = NULL; - usec_t next, curr; /* Let's make sure the given calendar event is not in the past */ - curr = now(CLOCK_REALTIME); r = calendar_spec_from_string(optarg, &cs); if (r < 0) - return log_error_errno(r, "Failed to parse calendar event specification"); - r = calendar_spec_next_usec(cs, curr, &next); - if (r < 0) { - /* The calendar event is in the past - in such case - * don't add an OnCalendar property and execute - * the command immediately instead */ - log_warning("Specified calendar event is in the past, executing immediately"); - break; - } + return log_error_errno(r, "Failed to parse calendar event specification: %m"); + + r = calendar_spec_next_usec(cs, now(CLOCK_REALTIME), NULL); + if (r == -ENOENT) + /* The calendar event is in the past - let's warn about this, but install it + * anyway as it is. The service manager will trigger the service right-away + * then, but everything is discoverable as usual. Moreover, the server side + * might have a different clock or timezone than we do, hence it should + * decide when or whether to run something. */ + log_warning("Specified calendar expression is in the past, proceeding anyway."); + else if (r < 0) + return log_error_errno(r, "Failed to calculate next time calendar expression elapses: %m"); r = add_timer_property("OnCalendar", optarg); if (r < 0)