From 294446dcb98eaa1ced5839da674c2065b1367a3a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 14 Apr 2021 14:36:15 +0200 Subject: [PATCH] core: add new OnSuccess= dependency type This is similar to OnFailure= but is activated whenever a unit returns into inactive state successfully. I was always afraid of adding this, since it effectively allows building loops and makes our engine Turing complete, but it pretty much already was it was just hidden. Given that we have per-unit ratelimits as well as an event loop global ratelimit I feel safe to add this finally, given it actually is useful. Fixes: #13386 --- man/org.freedesktop.systemd1.xml | 18 +++++ man/systemd.unit.xml | 15 ++-- src/basic/unit-def.c | 2 + src/basic/unit-def.h | 4 +- src/core/dbus-unit.c | 7 ++ src/core/job.c | 2 +- src/core/load-fragment-gperf.gperf.in | 2 + src/core/unit-dependency-atom.c | 5 ++ src/core/unit-dependency-atom.h | 1 + src/core/unit-serialize.c | 2 + src/core/unit.c | 77 ++++++++++++++++----- src/core/unit.h | 5 +- test/fuzz/fuzz-unit-file/directives.service | 2 + 13 files changed, 115 insertions(+), 27 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 13322746fa5..d55f3c8f9bd 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -1632,6 +1632,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly as OnFailureOf = ['...', ...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly as OnSuccess = ['...', ...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly as OnSuccessOf = ['...', ...]; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly as Triggers = ['...', ...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly as TriggeredBy = ['...', ...]; @@ -1702,6 +1706,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b DefaultDependencies = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") + readonly s OnSuccesJobMode = '...'; + @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly s OnFailureJobMode = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b IgnoreOnIsolate = ...; @@ -1781,6 +1787,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + + @@ -1805,6 +1815,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -1919,6 +1931,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + + @@ -2003,6 +2019,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 07ea21a6e7f..d1d7a9f6e0f 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -776,11 +776,16 @@ OnFailure= - A space-separated list of one or more units - that are activated when this unit enters the - failed state. A service unit using - Restart= enters the failed state only after - the start limits are reached. + A space-separated list of one or more units that are activated when this unit enters + the failed state. A service unit using Restart= enters the + failed state only after the start limits are reached. + + + + OnSuccess= + + A space-separated list of one or more units that are activated when this unit enters + the inactive state. diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index 32be5219fce..f83e398a4b1 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -274,6 +274,8 @@ static const char* const unit_dependency_table[_UNIT_DEPENDENCY_MAX] = { [UNIT_CONFLICTED_BY] = "ConflictedBy", [UNIT_BEFORE] = "Before", [UNIT_AFTER] = "After", + [UNIT_ON_SUCCESS] = "OnSuccess", + [UNIT_ON_SUCCESS_OF] = "OnSuccessOf", [UNIT_ON_FAILURE] = "OnFailure", [UNIT_ON_FAILURE_OF] = "OnFailureOf", [UNIT_TRIGGERS] = "Triggers", diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h index dc5aa5555d7..92b40e8a3de 100644 --- a/src/basic/unit-def.h +++ b/src/basic/unit-def.h @@ -227,7 +227,9 @@ typedef enum UnitDependency { UNIT_BEFORE, /* inverse of 'before' is 'after' and vice versa */ UNIT_AFTER, - /* On Failure */ + /* OnSuccess= + OnFailure= */ + UNIT_ON_SUCCESS, + UNIT_ON_SUCCESS_OF, UNIT_ON_FAILURE, UNIT_ON_FAILURE_OF, diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 5e5a0a33a11..60f406a160a 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -866,6 +866,8 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("After", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("OnFailure", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("OnFailureOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("OnSuccess", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("OnSuccessOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Triggers", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TriggeredBy", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("PropagatesReloadTo", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST), @@ -903,6 +905,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("RefuseManualStop", "b", bus_property_get_bool, offsetof(Unit, refuse_manual_stop), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("AllowIsolate", "b", bus_property_get_bool, offsetof(Unit, allow_isolate), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("DefaultDependencies", "b", bus_property_get_bool, offsetof(Unit, default_dependencies), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("OnSuccesJobMode", "s", property_get_job_mode, offsetof(Unit, on_success_job_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("OnFailureJobMode", "s", property_get_job_mode, offsetof(Unit, on_failure_job_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("IgnoreOnIsolate", "b", bus_property_get_bool, offsetof(Unit, ignore_on_isolate), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NeedDaemonReload", "b", property_get_need_daemon_reload, 0, SD_BUS_VTABLE_PROPERTY_CONST), @@ -2126,6 +2129,9 @@ static int bus_unit_set_transient_property( if (streq(name, "DefaultDependencies")) return bus_set_transient_bool(u, name, &u->default_dependencies, message, flags, error); + if (streq(name, "OnSuccessJobMode")) + return bus_set_transient_job_mode(u, name, &u->on_success_job_mode, message, flags, error); + if (streq(name, "OnFailureJobMode")) return bus_set_transient_job_mode(u, name, &u->on_failure_job_mode, message, flags, error); @@ -2301,6 +2307,7 @@ static int bus_unit_set_transient_property( UNIT_CONFLICTS, UNIT_BEFORE, UNIT_AFTER, + UNIT_ON_SUCCESS, UNIT_ON_FAILURE, UNIT_PROPAGATES_RELOAD_TO, UNIT_RELOAD_PROPAGATED_FROM, diff --git a/src/core/job.c b/src/core/job.c index 45bb6bfd92f..a273e4220e2 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1047,7 +1047,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr job_type_to_string(t), job_result_to_string(result))); - unit_start_on_failure(u); + unit_start_on_failure(u, "OnFailure=", UNIT_ATOM_ON_FAILURE, u->on_failure_job_mode); } unit_trigger_notify(u); diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index bb3c4453df2..25cd55e2f9e 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -264,6 +264,7 @@ Unit.BindTo, config_parse_unit_deps, Unit.Conflicts, config_parse_unit_deps, UNIT_CONFLICTS, 0 Unit.Before, config_parse_unit_deps, UNIT_BEFORE, 0 Unit.After, config_parse_unit_deps, UNIT_AFTER, 0 +Unit.OnSuccess, config_parse_unit_deps, UNIT_ON_SUCCESS, 0 Unit.OnFailure, config_parse_unit_deps, UNIT_ON_FAILURE, 0 Unit.PropagatesReloadTo, config_parse_unit_deps, UNIT_PROPAGATES_RELOAD_TO, 0 Unit.PropagateReloadTo, config_parse_unit_deps, UNIT_PROPAGATES_RELOAD_TO, 0 @@ -281,6 +282,7 @@ Unit.RefuseManualStart, config_parse_bool, Unit.RefuseManualStop, config_parse_bool, 0, offsetof(Unit, refuse_manual_stop) Unit.AllowIsolate, config_parse_bool, 0, offsetof(Unit, allow_isolate) Unit.DefaultDependencies, config_parse_bool, 0, offsetof(Unit, default_dependencies) +Unit.OnSuccessJobMode, config_parse_job_mode, 0, offsetof(Unit, on_success_job_mode) Unit.OnFailureJobMode, config_parse_job_mode, 0, offsetof(Unit, on_failure_job_mode) {# The following is a legacy alias name for compatibility #} Unit.OnFailureIsolate, config_parse_job_mode_isolate, 0, offsetof(Unit, on_failure_job_mode) diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c index eaeb1d389e4..6bd961e92b0 100644 --- a/src/core/unit-dependency-atom.c +++ b/src/core/unit-dependency-atom.c @@ -74,6 +74,7 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { /* These are simple dependency types: they consist of a single atom only */ [UNIT_BEFORE] = UNIT_ATOM_BEFORE, [UNIT_AFTER] = UNIT_ATOM_AFTER, + [UNIT_ON_SUCCESS] = UNIT_ATOM_ON_SUCCESS, [UNIT_ON_FAILURE] = UNIT_ATOM_ON_FAILURE, [UNIT_TRIGGERS] = UNIT_ATOM_TRIGGERS, [UNIT_TRIGGERED_BY] = UNIT_ATOM_TRIGGERED_BY, @@ -88,6 +89,7 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { * things discoverable/debuggable as they are the inverse dependencies to some of the above. As they * have no effect of their own, they all map to no atoms at all, i.e. the value 0. */ [UNIT_RELOAD_PROPAGATED_FROM] = 0, + [UNIT_ON_SUCCESS_OF] = 0, [UNIT_ON_FAILURE_OF] = 0, [UNIT_STOP_PROPAGATED_FROM] = 0, }; @@ -175,6 +177,9 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { case UNIT_ATOM_AFTER: return UNIT_AFTER; + case UNIT_ATOM_ON_SUCCESS: + return UNIT_ON_SUCCESS; + case UNIT_ATOM_ON_FAILURE: return UNIT_ON_FAILURE; diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h index 01482215118..daa296f6012 100644 --- a/src/core/unit-dependency-atom.h +++ b/src/core/unit-dependency-atom.h @@ -62,6 +62,7 @@ typedef enum UnitDependencyAtom { /* The remaining atoms map 1:1 to the equally named high-level deps */ UNIT_ATOM_BEFORE = UINT64_C(1) << 22, UNIT_ATOM_AFTER = UINT64_C(1) << 23, + UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 24, UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 25, UNIT_ATOM_TRIGGERS = UINT64_C(1) << 26, UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 27, diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index caee3a2823b..7da0d8a3dbc 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -774,12 +774,14 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { "%s\tRefuseManualStart: %s\n" "%s\tRefuseManualStop: %s\n" "%s\tDefaultDependencies: %s\n" + "%s\tOnSuccessJobMode: %s\n" "%s\tOnFailureJobMode: %s\n" "%s\tIgnoreOnIsolate: %s\n", prefix, yes_no(u->stop_when_unneeded), prefix, yes_no(u->refuse_manual_start), prefix, yes_no(u->refuse_manual_stop), prefix, yes_no(u->default_dependencies), + prefix, job_mode_to_string(u->on_success_job_mode), prefix, job_mode_to_string(u->on_failure_job_mode), prefix, yes_no(u->ignore_on_isolate)); diff --git a/src/core/unit.c b/src/core/unit.c index ed6f87d9609..6892930d7bb 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -101,6 +101,7 @@ Unit* unit_new(Manager *m, size_t size) { u->unit_file_state = _UNIT_FILE_STATE_INVALID; u->unit_file_preset = -1; u->on_failure_job_mode = JOB_REPLACE; + u->on_success_job_mode = JOB_FAIL; u->cgroup_control_inotify_wd = -1; u->cgroup_memory_inotify_wd = -1; u->job_timeout = USEC_INFINITY; @@ -896,6 +897,7 @@ static void unit_maybe_warn_about_dependency( UNIT_CONFLICTED_BY, UNIT_BEFORE, UNIT_AFTER, + UNIT_ON_SUCCESS, UNIT_ON_FAILURE, UNIT_TRIGGERS, UNIT_TRIGGERED_BY)) @@ -1507,6 +1509,31 @@ static int unit_add_startup_units(Unit *u) { return set_ensure_put(&u->manager->startup_units, NULL, u); } +static int unit_validate_on_failure_job_mode( + Unit *u, + const char *job_mode_setting, + JobMode job_mode, + const char *dependency_name, + UnitDependencyAtom atom) { + + Unit *other, *found = NULL; + + if (job_mode != JOB_ISOLATE) + return 0; + + UNIT_FOREACH_DEPENDENCY(other, u, atom) { + if (!found) + found = other; + else if (found != other) + return log_unit_error_errno( + u, SYNTHETIC_ERRNO(ENOEXEC), + "More than one %s dependencies specified but %sisolate set. Refusing.", + dependency_name, job_mode_setting); + } + + return 0; +} + int unit_load(Unit *u) { int r; @@ -1560,19 +1587,13 @@ int unit_load(Unit *u) { if (r < 0) goto fail; - if (u->on_failure_job_mode == JOB_ISOLATE) { - Unit *other, *found = NULL; + r = unit_validate_on_failure_job_mode(u, "OnSuccessJobMode=", u->on_success_job_mode, "OnSuccess=", UNIT_ATOM_ON_SUCCESS); + if (r < 0) + goto fail; - UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_ON_FAILURE) { - if (!found) - found = other; - else if (found != other) { - r = log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOEXEC), - "More than one OnFailure= dependencies specified but OnFailureJobMode=isolate set. Refusing."); - goto fail; - } - } - } + r = unit_validate_on_failure_job_mode(u, "OnFailureJobMode=", u->on_failure_job_mode, "OnFailure=", UNIT_ATOM_ON_FAILURE); + if (r < 0) + goto fail; if (u->job_running_timeout != USEC_INFINITY && u->job_running_timeout > u->job_timeout) log_unit_warning(u, "JobRunningTimeoutSec= is greater than JobTimeoutSec=, it has no effect."); @@ -2082,25 +2103,39 @@ static void retroactively_stop_dependencies(Unit *u) { manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); } -void unit_start_on_failure(Unit *u) { +void unit_start_on_failure( + Unit *u, + const char *dependency_name, + UnitDependencyAtom atom, + JobMode job_mode) { + bool logged = false; Unit *other; int r; assert(u); + assert(dependency_name); + assert(IN_SET(atom, UNIT_ATOM_ON_SUCCESS, UNIT_ATOM_ON_FAILURE)); + + /* Act on OnFailure= and OnSuccess= dependencies */ - UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_ON_FAILURE) { + UNIT_FOREACH_DEPENDENCY(other, u, atom) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; if (!logged) { - log_unit_info(u, "Triggering OnFailure= dependencies."); + log_unit_info(u, "Triggering %s dependencies.", dependency_name); logged = true; } - r = manager_add_job(u->manager, JOB_START, other, u->on_failure_job_mode, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_START, other, job_mode, NULL, &error, NULL); if (r < 0) - log_unit_warning_errno(u, r, "Failed to enqueue OnFailure= job, ignoring: %s", bus_error_message(&error, r)); + log_unit_warning_errno( + u, r, "Failed to enqueue %s job, ignoring: %s", + dependency_name, bus_error_message(&error, r)); } + + if (logged) + log_unit_debug(u, "Triggering %s dependencies done.", dependency_name); } void unit_trigger_notify(Unit *u) { @@ -2568,7 +2603,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag log_unit_debug(u, "Unit entered failed state."); if (!(flags & UNIT_NOTIFY_WILL_AUTO_RESTART)) - unit_start_on_failure(u); + unit_start_on_failure(u, "OnFailure=", UNIT_ATOM_ON_FAILURE, u->on_failure_job_mode); } if (UNIT_IS_ACTIVE_OR_RELOADING(ns) && !UNIT_IS_ACTIVE_OR_RELOADING(os)) { @@ -2584,6 +2619,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag unit_emit_audit_stop(u, ns); unit_log_resources(u); } + + if (ns == UNIT_INACTIVE && !IN_SET(os, UNIT_FAILED, UNIT_INACTIVE, UNIT_MAINTENANCE) && + !(flags & UNIT_NOTIFY_WILL_AUTO_RESTART)) + unit_start_on_failure(u, "OnSuccess=", UNIT_ATOM_ON_SUCCESS, u->on_success_job_mode); } manager_recheck_journal(m); @@ -2871,6 +2910,8 @@ int unit_add_dependency( [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS, [UNIT_BEFORE] = UNIT_AFTER, [UNIT_AFTER] = UNIT_BEFORE, + [UNIT_ON_SUCCESS] = UNIT_ON_SUCCESS_OF, + [UNIT_ON_SUCCESS_OF] = UNIT_ON_SUCCESS, [UNIT_ON_FAILURE] = UNIT_ON_FAILURE_OF, [UNIT_ON_FAILURE_OF] = UNIT_ON_FAILURE, [UNIT_REFERENCES] = UNIT_REFERENCED_BY, diff --git a/src/core/unit.h b/src/core/unit.h index 6b3f48596f1..1a3aaa5020b 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -334,7 +334,8 @@ typedef struct Unit { * ones which might have appeared. */ sd_event_source *rewatch_pids_event_source; - /* How to start OnFailure units */ + /* How to start OnSuccess=/OnFailure= units */ + JobMode on_success_job_mode; JobMode on_failure_job_mode; /* Tweaking the GC logic */ @@ -828,7 +829,7 @@ bool unit_will_restart(Unit *u); int unit_add_default_target_dependency(Unit *u, Unit *target); -void unit_start_on_failure(Unit *u); +void unit_start_on_failure(Unit *u, const char *dependency_name, UnitDependencyAtom atom, JobMode job_mode); void unit_trigger_notify(Unit *u); UnitFileState unit_get_unit_file_state(Unit *u); diff --git a/test/fuzz/fuzz-unit-file/directives.service b/test/fuzz/fuzz-unit-file/directives.service index 95ab9a1c32a..50b4b4b7f6f 100644 --- a/test/fuzz/fuzz-unit-file/directives.service +++ b/test/fuzz/fuzz-unit-file/directives.service @@ -76,6 +76,8 @@ JoinsNamespaceOf= OnFailure= OnFailureIsolate= OnFailureJobMode= +OnSuccess= +OnSuccessJobMode= PartOf= PropagateReloadFrom= PropagateReloadTo= -- 2.47.3