Merge pull request #1936 from poettering/transient-timers

Fix transient timers and other fixes

8 files changed, 160 insertions, 89 deletions

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 67e4e8b218..4d730290b2 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -644,6 +644,7 @@ static int transient_unit_from_message(
                 Unit **unit,
                 sd_bus_error *error) {
 
+        UnitType t;
         Unit *u;
         int r;
 
@@ -651,23 +652,18 @@ static int transient_unit_from_message(
         assert(message);
         assert(name);
 
+        t = unit_name_to_type(name);
+        if (t < 0)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name or type.");
+
+        if (!unit_vtable[t]->can_transient)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit type %s does not support transient units.", unit_type_to_string(t));
+
         r = manager_load_unit(m, name, NULL, error, &u);
         if (r < 0)
                 return r;
 
-        /* Check if the unit already exists or is already referenced,
-         * in a number of different ways. Note that to cater for unit
-         * types such as slice, we are generally fine with units that
-         * are marked UNIT_LOADED even even though nothing was
-         * actually loaded, as those unit types don't require a file
-         * on disk to validly load. */
-
-        if (!IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_LOADED) ||
-            u->fragment_path ||
-            u->source_path ||
-            !strv_isempty(u->dropin_paths) ||
-            u->refs ||
-            set_size(u->dependencies[UNIT_REFERENCED_BY]) > 0)
+        if (!unit_is_pristine(u))
                 return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit %s already exists.", name);
 
         /* OK, the unit failed to load and is unreferenced, now let's
@@ -681,6 +677,9 @@ static int transient_unit_from_message(
         if (r < 0)
                 return r;
 
+        /* Now load the missing bits of the unit we just created */
+        manager_dispatch_load_queue(m);
+
         *unit = u;
 
         return 0;
@@ -691,8 +690,6 @@ static int transient_aux_units_from_message(
                 sd_bus_message *message,
                 sd_bus_error *error) {
 
-        Unit *u;
-        char *name = NULL;
         int r;
 
         assert(m);
@@ -703,20 +700,17 @@ static int transient_aux_units_from_message(
                 return r;
 
         while ((r = sd_bus_message_enter_container(message, 'r', "sa(sv)")) > 0) {
+                const char *name = NULL;
+                Unit *u;
+
                 r = sd_bus_message_read(message, "s", &name);
                 if (r < 0)
                         return r;
 
                 r = transient_unit_from_message(m, message, name, &u, error);
-                if (r < 0 && r != -EEXIST)
+                if (r < 0)
                         return r;
 
-                if (r != -EEXIST) {
-                        r = unit_load(u);
-                        if (r < 0)
-                                return r;
-                }
-
                 r = sd_bus_message_exit_container(message);
                 if (r < 0)
                         return r;
@@ -735,7 +729,6 @@ static int method_start_transient_unit(sd_bus_message *message, void *userdata,
         const char *name, *smode;
         Manager *m = userdata;
         JobMode mode;
-        UnitType t;
         Unit *u;
         int r;
 
@@ -750,13 +743,6 @@ static int method_start_transient_unit(sd_bus_message *message, void *userdata,
         if (r < 0)
                 return r;
 
-        t = unit_name_to_type(name);
-        if (t < 0)
-                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit type.");
-
-        if (!unit_vtable[t]->can_transient)
-                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit type %s does not support transient units.", unit_type_to_string(t));
-
         mode = job_mode_from_string(smode);
         if (mode < 0)
                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Job mode %s is invalid.", smode);
@@ -775,13 +761,6 @@ static int method_start_transient_unit(sd_bus_message *message, void *userdata,
         if (r < 0)
                 return r;
 
-        /* And load this stub fully */
-        r = unit_load(u);
-        if (r < 0)
-                return r;
-
-        manager_dispatch_load_queue(m);
-
         /* Finally, start it */
         return bus_unit_queue_job(message, u, JOB_START, mode, false, error);
 }
diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c
index a8a280d961..2e6c479c3a 100644
--- a/src/core/dbus-timer.c
+++ b/src/core/dbus-timer.c
@@ -182,6 +182,7 @@ const sd_bus_vtable bus_timer_vtable[] = {
         SD_BUS_PROPERTY("AccuracyUSec", "t", bus_property_get_usec, offsetof(Timer, accuracy_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Persistent", "b", bus_property_get_bool, offsetof(Timer, persistent), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("WakeSystem", "b", bus_property_get_bool, offsetof(Timer, wake_system), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("RemainAfterElapse", "b", bus_property_get_bool, offsetof(Timer, remain_after_elapse), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_VTABLE_END
 };
 
@@ -283,7 +284,6 @@ static int bus_timer_set_transient_property(
                 return 1;
 
         } else if (streq(name, "WakeSystem")) {
-
                 int b;
 
                 r = sd_bus_message_read(message, "b", &b);
@@ -292,11 +292,24 @@ static int bus_timer_set_transient_property(
 
                 if (mode != UNIT_CHECK) {
                         t->wake_system = b;
-                        unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s\n", name, yes_no(t->wake_system));
+                        unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s\n", name, yes_no(b));
                 }
 
                 return 1;
 
+        } else if (streq(name, "RemainAfterElapse")) {
+                int b;
+
+                r = sd_bus_message_read(message, "b", &b);
+                if (r < 0)
+                        return r;
+
+                if (mode != UNIT_CHECK) {
+                        t->remain_after_elapse = b;
+                        unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s\n", name, yes_no(b));
+                }
+
+                return 1;
         }
 
         return 0;
diff --git a/src/core/job.c b/src/core/job.c
index 53e0947215..9654590635 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -500,17 +500,26 @@ static void job_change_type(Job *j, JobType newtype) {
 }
 
 static int job_perform_on_unit(Job **j) {
-        /* While we execute this operation the job might go away (for
-         * example: because it finishes immediately or is replaced by a new,
-         * conflicting job.) To make sure we don't access a freed job later on
-         * we store the id here, so that we can verify the job is still
-         * valid. */
-        Manager *m  = (*j)->manager;
-        Unit *u     = (*j)->unit;
-        JobType t   = (*j)->type;
-        uint32_t id = (*j)->id;
+        uint32_t id;
+        Manager *m;
+        JobType t;
+        Unit *u;
         int r;
 
+        /* While we execute this operation the job might go away (for
+         * example: because it finishes immediately or is replaced by
+         * a new, conflicting job.) To make sure we don't access a
+         * freed job later on we store the id here, so that we can
+         * verify the job is still valid. */
+
+        assert(j);
+        assert(*j);
+
+        m = (*j)->manager;
+        u = (*j)->unit;
+        t = (*j)->type;
+        id = (*j)->id;
+
         switch (t) {
                 case JOB_START:
                         r = unit_start(u);
@@ -518,6 +527,7 @@ static int job_perform_on_unit(Job **j) {
 
                 case JOB_RESTART:
                         t = JOB_STOP;
+                        /* fall through */
                 case JOB_STOP:
                         r = unit_stop(u);
                         break;
@@ -617,8 +627,7 @@ int job_run_and_invalidate(Job *j) {
 }
 
 _pure_ static const char *job_get_status_message_format(Unit *u, JobType t, JobResult result) {
-        const char *format;
-        const UnitStatusMessageFormats *format_table;
+
         static const char *const generic_finished_start_job[_JOB_RESULT_MAX] = {
                 [JOB_DONE]        = "Started %s.",
                 [JOB_TIMEOUT]     = "Timed out starting %s.",
@@ -644,11 +653,14 @@ _pure_ static const char *job_get_status_message_format(Unit *u, JobType t, JobR
                 [JOB_SKIPPED]     = "%s is not active.",
         };
 
+        const UnitStatusMessageFormats *format_table;
+        const char *format;
+
         assert(u);
         assert(t >= 0);
         assert(t < _JOB_TYPE_MAX);
 
-        if (t == JOB_START || t == JOB_STOP || t == JOB_RESTART) {
+        if (IN_SET(t, JOB_START, JOB_STOP, JOB_RESTART)) {
                 format_table = &UNIT_VTABLE(u)->status_message_formats;
                 if (format_table) {
                         format = t == JOB_START ? format_table->finished_start_job[result] :
@@ -672,7 +684,6 @@ _pure_ static const char *job_get_status_message_format(Unit *u, JobType t, JobR
 }
 
 static void job_print_status_message(Unit *u, JobType t, JobResult result) {
-        const char *format;
         static const char* const job_result_status_table[_JOB_RESULT_MAX] = {
                 [JOB_DONE]        = ANSI_GREEN            "  OK  " ANSI_NORMAL,
                 [JOB_TIMEOUT]     = ANSI_HIGHLIGHT_RED    " TIME " ANSI_NORMAL,
@@ -683,10 +694,16 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
                 [JOB_UNSUPPORTED] = ANSI_HIGHLIGHT_YELLOW "UNSUPP" ANSI_NORMAL,
         };
 
+        const char *format;
+
         assert(u);
         assert(t >= 0);
         assert(t < _JOB_TYPE_MAX);
 
+        /* Reload status messages have traditionally not been printed to console. */
+        if (t == JOB_RELOAD)
+                return;
+
         format = job_get_status_message_format(u, t, result);
         if (!format)
                 return;
@@ -699,10 +716,10 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
         REENABLE_WARNING;
 
         if (t == JOB_START && result == JOB_FAILED) {
-                _cleanup_free_ char *quoted = shell_maybe_quote(u->id);
+                _cleanup_free_ char *quoted;
 
-                manager_status_printf(u->manager, STATUS_TYPE_NORMAL, NULL,
-                                      "See 'systemctl status %s' for details.", strna(quoted));
+                quoted = shell_maybe_quote(u->id);
+                manager_status_printf(u->manager, STATUS_TYPE_NORMAL, NULL, "See 'systemctl status %s' for details.", strna(quoted));
         }
 }
 
@@ -740,13 +757,22 @@ static void job_log_status_message(Unit *u, JobType t, JobResult result) {
         snprintf(buf, sizeof(buf), format, unit_description(u));
         REENABLE_WARNING;
 
-        if (t == JOB_START)
+        switch (t) {
+
+        case JOB_START:
                 mid = result == JOB_DONE ? SD_MESSAGE_UNIT_STARTED : SD_MESSAGE_UNIT_FAILED;
-        else if (t == JOB_STOP || t == JOB_RESTART)
-                mid = SD_MESSAGE_UNIT_STOPPED;
-        else if (t == JOB_RELOAD)
+                break;
+
+        case JOB_RELOAD:
                 mid = SD_MESSAGE_UNIT_RELOADED;
-        else {
+                break;
+
+        case JOB_STOP:
+        case JOB_RESTART:
+                mid = SD_MESSAGE_UNIT_STOPPED;
+                break;
+
+        default:
                 log_struct(job_result_log_level[result],
                            LOG_UNIT_ID(u),
                            LOG_MESSAGE("%s", buf),
@@ -770,10 +796,7 @@ static void job_emit_status_message(Unit *u, JobType t, JobResult result) {
                 return;
 
         job_log_status_message(u, t, result);
-
-        /* Reload status messages have traditionally not been printed to console. */
-        if (t != JOB_RELOAD)
-                job_print_status_message(u, t, result);
+        job_print_status_message(u, t, result);
 }
 
 static void job_fail_dependencies(Unit *u, UnitDependency d) {
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 799418033d..74ef72de88 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -344,6 +344,7 @@ Timer.OnUnitActiveSec,           config_parse_timer,                 0,
 Timer.OnUnitInactiveSec,         config_parse_timer,                 0,                             0
 Timer.Persistent,                config_parse_bool,                  0,                             offsetof(Timer, persistent)
 Timer.WakeSystem,                config_parse_bool,                  0,                             offsetof(Timer, wake_system)
+Timer.RemainAfterElapse,         config_parse_bool,                  0,                             offsetof(Timer, remain_after_elapse)
 Timer.AccuracySec,               config_parse_sec,                   0,                             offsetof(Timer, accuracy_usec)
 Timer.Unit,                      config_parse_trigger_unit,          0,                             0
 m4_dnl
diff --git a/src/core/timer.c b/src/core/timer.c
index 51b1d875be..0587452cfb 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -55,6 +55,7 @@ static void timer_init(Unit *u) {
         t->next_elapse_monotonic_or_boottime = USEC_INFINITY;
         t->next_elapse_realtime = USEC_INFINITY;
         t->accuracy_usec = u->manager->default_timer_accuracy_usec;
+        t->remain_after_elapse = true;
 }
 
 void timer_free_values(Timer *t) {
@@ -217,13 +218,15 @@ static void timer_dump(Unit *u, FILE *f, const char *prefix) {
                 "%sUnit: %s\n"
                 "%sPersistent: %s\n"
                 "%sWakeSystem: %s\n"
-                "%sAccuracy: %s\n",
+                "%sAccuracy: %s\n"
+                "%sRemainAfterElapse: %s\n",
                 prefix, timer_state_to_string(t->state),
                 prefix, timer_result_to_string(t->result),
                 prefix, trigger ? trigger->id : "n/a",
                 prefix, yes_no(t->persistent),
                 prefix, yes_no(t->wake_system),
-                prefix, format_timespan(buf, sizeof(buf), t->accuracy_usec, 1));
+                prefix, format_timespan(buf, sizeof(buf), t->accuracy_usec, 1),
+                prefix, yes_no(t->remain_after_elapse));
 
         LIST_FOREACH(value, v, t->values) {
 
@@ -275,13 +278,13 @@ static int timer_coldplug(Unit *u) {
         assert(t);
         assert(t->state == TIMER_DEAD);
 
-        if (t->deserialized_state != t->state) {
+        if (t->deserialized_state == t->state)
+                return 0;
 
-                if (t->deserialized_state == TIMER_WAITING)
-                        timer_enter_waiting(t, false);
-                else
-                        timer_set_state(t, t->deserialized_state);
-        }
+        if (t->deserialized_state == TIMER_WAITING)
+                timer_enter_waiting(t, false);
+        else
+                timer_set_state(t, t->deserialized_state);
 
         return 0;
 }
@@ -295,6 +298,23 @@ static void timer_enter_dead(Timer *t, TimerResult f) {
         timer_set_state(t, t->result != TIMER_SUCCESS ? TIMER_FAILED : TIMER_DEAD);
 }
 
+static void timer_enter_elapsed(Timer *t, bool leave_around) {
+        assert(t);
+
+        /* If a unit is marked with RemainAfterElapse=yes we leave it
+         * around even after it elapsed once, so that starting it
+         * later again does not necessarily mean immediate
+         * retriggering. We unconditionally leave units with
+         * TIMER_UNIT_ACTIVE or TIMER_UNIT_INACTIVE triggers around,
+         * since they might be restarted automatically at any time
+         * later on. */
+
+        if (t->remain_after_elapse || leave_around)
+                timer_set_state(t, TIMER_ELAPSED);
+        else
+                timer_enter_dead(t, TIMER_SUCCESS);
+}
+
 static usec_t monotonic_to_boottime(usec_t t) {
         usec_t a, b;
 
@@ -314,6 +334,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
         bool found_monotonic = false, found_realtime = false;
         usec_t ts_realtime, ts_monotonic;
         usec_t base = 0;
+        bool leave_around = false;
         TimerValue *v;
         int r;
 
@@ -374,7 +395,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                                 break;
 
                         case TIMER_UNIT_ACTIVE:
-
+                                leave_around = true;
                                 base = UNIT_TRIGGER(UNIT(t))->inactive_exit_timestamp.monotonic;
 
                                 if (base <= 0)
@@ -386,7 +407,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                                 break;
 
                         case TIMER_UNIT_INACTIVE:
-
+                                leave_around = true;
                                 base = UNIT_TRIGGER(UNIT(t))->inactive_enter_timestamp.monotonic;
 
                                 if (base <= 0)
@@ -423,14 +444,16 @@ static void timer_enter_waiting(Timer *t, bool initial) {
 
         if (!found_monotonic && !found_realtime) {
                 log_unit_debug(UNIT(t), "Timer is elapsed.");
-                timer_set_state(t, TIMER_ELAPSED);
+                timer_enter_elapsed(t, leave_around);
                 return;
         }
 
         if (found_monotonic) {
                 char buf[FORMAT_TIMESPAN_MAX];
+                usec_t left;
 
-                log_unit_debug(UNIT(t), "Monotonic timer elapses in %s.", format_timespan(buf, sizeof(buf), t->next_elapse_monotonic_or_boottime > ts_monotonic ? t->next_elapse_monotonic_or_boottime - ts_monotonic : 0, 0));
+                left = t->next_elapse_monotonic_or_boottime > ts_monotonic ? t->next_elapse_monotonic_or_boottime - ts_monotonic : 0;
+                log_unit_debug(UNIT(t), "Monotonic timer elapses in %s.", format_timespan(buf, sizeof(buf), left, 0));
 
                 if (t->monotonic_event_source) {
                         r = sd_event_source_set_time(t->monotonic_event_source, t->next_elapse_monotonic_or_boottime);
diff --git a/src/core/timer.h b/src/core/timer.h
index ac5af6a93c..6bc9fbed3d 100644
--- a/src/core/timer.h
+++ b/src/core/timer.h
@@ -73,6 +73,7 @@ struct Timer {
 
         bool persistent;
         bool wake_system;
+        bool remain_after_elapse;
 
         char *stamp_path;
 };
diff --git a/src/core/unit.c b/src/core/unit.c
index d199d87bf8..0a02e38aa8 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1347,12 +1347,18 @@ static bool unit_assert_test(Unit *u) {
         return u->assert_result;
 }
 
+void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg_format) {
+        DISABLE_WARNING_FORMAT_NONLITERAL;
+        manager_status_printf(u->manager, STATUS_TYPE_NORMAL, status, unit_status_msg_format, unit_description(u));
+        REENABLE_WARNING;
+}
+
 _pure_ static const char* unit_get_status_message_format(Unit *u, JobType t) {
         const char *format;
         const UnitStatusMessageFormats *format_table;
 
         assert(u);
-        assert(t == JOB_START || t == JOB_STOP || t == JOB_RELOAD);
+        assert(IN_SET(t, JOB_START, JOB_STOP, JOB_RELOAD));
 
         if (t != JOB_RELOAD) {
                 format_table = &UNIT_VTABLE(u)->status_message_formats;
@@ -1377,6 +1383,10 @@ static void unit_status_print_starting_stopping(Unit *u, JobType t) {
 
         assert(u);
 
+        /* Reload status messages have traditionally not been printed to console. */
+        if (!IN_SET(t, JOB_START, JOB_STOP))
+                return;
+
         format = unit_get_status_message_format(u, t);
 
         DISABLE_WARNING_FORMAT_NONLITERAL;
@@ -1391,7 +1401,7 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) {
 
         assert(u);
 
-        if (t != JOB_START && t != JOB_STOP && t != JOB_RELOAD)
+        if (!IN_SET(t, JOB_START, JOB_STOP, JOB_RELOAD))
                 return;
 
         if (log_on_console())
@@ -1423,12 +1433,12 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) {
 }
 
 void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) {
+        assert(u);
+        assert(t >= 0);
+        assert(t < _JOB_TYPE_MAX);
 
         unit_status_log_starting_stopping_reloading(u, t);
-
-        /* Reload status messages have traditionally not been printed to console. */
-        if (t != JOB_RELOAD)
-                unit_status_print_starting_stopping(u, t);
+        unit_status_print_starting_stopping(u, t);
 }
 
 /* Errors:
@@ -2896,13 +2906,6 @@ int unit_coldplug(Unit *u) {
         return 0;
 }
 
-void unit_status_printf(Unit *u, const char *status, const char *unit_status_msg_format) {
-        DISABLE_WARNING_FORMAT_NONLITERAL;
-        manager_status_printf(u->manager, STATUS_TYPE_NORMAL,
-                              status, unit_status_msg_format, unit_description(u));
-        REENABLE_WARNING;
-}
-
 bool unit_need_daemon_reload(Unit *u) {
         _cleanup_strv_free_ char **t = NULL;
         char **path;
@@ -3428,7 +3431,15 @@ int unit_make_transient(Unit *u) {
         u->load_state = UNIT_STUB;
         u->load_error = 0;
         u->transient = true;
+
         u->fragment_path = mfree(u->fragment_path);
+        u->source_path = mfree(u->source_path);
+        u->dropin_paths = strv_free(u->dropin_paths);
+        u->fragment_mtime = u->source_mtime = u->dropin_mtime = 0;
+
+        unit_add_to_dbus_queue(u);
+        unit_add_to_gc_queue(u);
+        unit_add_to_load_queue(u);
 
         return 0;
 }
@@ -3704,3 +3715,21 @@ int unit_fail_if_symlink(Unit *u, const char* where) {
 
         return -ELOOP;
 }
+
+bool unit_is_pristine(Unit *u) {
+        assert(u);
+
+        /* Check if the unit already exists or is already around,
+         * in a number of different ways. Note that to cater for unit
+         * types such as slice, we are generally fine with units that
+         * are marked UNIT_LOADED even even though nothing was
+         * actually loaded, as those unit types don't require a file
+         * on disk to validly load. */
+
+        return !(!IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_LOADED) ||
+                 u->fragment_path ||
+                 u->source_path ||
+                 !strv_isempty(u->dropin_paths) ||
+                 u->job ||
+                 u->merged_into);
+}
diff --git a/src/core/unit.h b/src/core/unit.h
index bcf41d2348..a69d624fad 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -586,6 +586,8 @@ int unit_require_mounts_for(Unit *u, const char *path);
 
 bool unit_type_supported(UnitType t);
 
+bool unit_is_pristine(Unit *u);
+
 static inline bool unit_supported(Unit *u) {
         return unit_type_supported(u->type);
 }