core: clean up OnFailure= and OnSuccess= handling a bit

- Replace "on_failure" in function names with "on_termination"
- Only pass UnitDependencyAtom in, as other info can be
  determined from that

diff --git a/src/core/job.c b/src/core/job.c
index 2f19468a9544f8cc7cfa4e4dcd3197f3099c223b..fb32223e1843e64a40c52e94030187a27363d686 100644 (file)
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -1056,8 +1056,9 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr
             !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u)))
                 job_fail_dependencies(u, UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE);
 
-        /* Trigger OnFailure= dependencies that are not generated by the unit itself. We don't treat
-         * JOB_CANCELED as failure in this context. And JOB_FAILURE is already handled by the unit itself. */
+        /* Trigger OnFailure= dependencies manually here. We need to do that because a failed job might not
+         * cause a unit state change. Note that we don't treat JOB_CANCELED as failure in this context.
+         * And JOB_FAILURE is already handled by the unit itself (unit_notify). */
         if (IN_SET(result, JOB_TIMEOUT, JOB_DEPENDENCY)) {
                 log_unit_struct(u, LOG_NOTICE,
                                 "JOB_TYPE=%s", job_type_to_string(t),
@@ -1067,7 +1068,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, "OnFailure=", UNIT_ATOM_ON_FAILURE, u->on_failure_job_mode);
+                unit_start_on_termination_deps(u, UNIT_ATOM_ON_FAILURE);
         }
 
         unit_trigger_notify(u);

diff --git a/src/core/unit.c b/src/core/unit.c
index def621ef6d11c6f824d0200bc6642a5b7c3c34c2..7e5f56cb21e0348f836e53c56e4f6372c0823e7a 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1600,26 +1600,36 @@ 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) {
+static const struct {
+        UnitDependencyAtom atom;
+        size_t job_mode_offset;
+        const char *dependency_name;
+        const char *job_mode_setting_name;
+} on_termination_settings[] = {
+        { UNIT_ATOM_ON_SUCCESS, offsetof(Unit, on_success_job_mode), "OnSuccess=", "OnSuccessJobMode=" },
+        { UNIT_ATOM_ON_FAILURE, offsetof(Unit, on_failure_job_mode), "OnFailure=", "OnFailureJobMode=" },
+};
 
-        Unit *other, *found = NULL;
+static int unit_validate_on_termination_job_modes(Unit *u) {
+        assert(u);
 
-        if (job_mode != JOB_ISOLATE)
-                return 0;
+        /* Verify that if On{Success,Failure}JobMode=isolate, only one unit gets specified. */
 
-        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);
+        FOREACH_ELEMENT(setting, on_termination_settings) {
+                JobMode job_mode = *(JobMode*) ((uint8_t*) u + setting->job_mode_offset);
+
+                if (job_mode != JOB_ISOLATE)
+                        continue;
+
+                Unit *other, *found = NULL;
+                UNIT_FOREACH_DEPENDENCY(other, u, setting->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.",
+                                                            setting->dependency_name, setting->job_mode_setting_name);
+                }
         }
 
         return 0;
@@ -1678,11 +1688,7 @@ int unit_load(Unit *u) {
                 if (r < 0)
                         goto fail;
 
-                r = unit_validate_on_failure_job_mode(u, "OnSuccessJobMode=", u->on_success_job_mode, "OnSuccess=", UNIT_ATOM_ON_SUCCESS);
-                if (r < 0)
-                        goto fail;
-
-                r = unit_validate_on_failure_job_mode(u, "OnFailureJobMode=", u->on_failure_job_mode, "OnFailure=", UNIT_ATOM_ON_FAILURE);
+                r = unit_validate_on_termination_job_modes(u);
                 if (r < 0)
                         goto fail;
 
@@ -2246,40 +2252,43 @@ static void retroactively_stop_dependencies(Unit *u) {
                         (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL);
 }
 
-void unit_start_on_failure(
-                Unit *u,
-                const char *dependency_name,
-                UnitDependencyAtom atom,
-                JobMode job_mode) {
-
-        int n_jobs = -1;
-        Unit *other;
+void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom) {
+        const char *dependency_name = NULL;
+        JobMode job_mode;
+        unsigned n_jobs = 0;
         int r;
 
+        /* Act on OnFailure= and OnSuccess= dependencies */
+
         assert(u);
-        assert(dependency_name);
+        assert(u->manager);
         assert(IN_SET(atom, UNIT_ATOM_ON_SUCCESS, UNIT_ATOM_ON_FAILURE));
 
-        /* Act on OnFailure= and OnSuccess= dependencies */
+        FOREACH_ELEMENT(setting, on_termination_settings)
+                if (atom == setting->atom) {
+                        job_mode = *(JobMode*) ((uint8_t*) u + setting->job_mode_offset);
+                        dependency_name = setting->dependency_name;
+                        break;
+                }
+
+        assert(dependency_name);
 
+        Unit *other;
         UNIT_FOREACH_DEPENDENCY(other, u, atom) {
                 _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
 
-                if (n_jobs < 0) {
+                if (n_jobs == 0)
                         log_unit_info(u, "Triggering %s dependencies.", dependency_name);
-                        n_jobs = 0;
-                }
 
                 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 %s job, ignoring: %s",
-                                        dependency_name, bus_error_message(&error, r));
+                        log_unit_warning_errno(u, r, "Failed to enqueue %s%s job, ignoring: %s",
+                                               dependency_name, other->id, bus_error_message(&error, r));
                 n_jobs++;
         }
 
-        if (n_jobs >= 0)
-                log_unit_debug(u, "Triggering %s dependencies done (%i %s).",
+        if (n_jobs > 0)
+                log_unit_debug(u, "Triggering %s dependencies done (%u %s).",
                                dependency_name, n_jobs, n_jobs == 1 ? "job" : "jobs");
 }
 
@@ -2688,9 +2697,9 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
                 }
 
                 if (ns == UNIT_INACTIVE && !IN_SET(os, UNIT_FAILED, UNIT_INACTIVE, UNIT_MAINTENANCE))
-                        unit_start_on_failure(u, "OnSuccess=", UNIT_ATOM_ON_SUCCESS, u->on_success_job_mode);
+                        unit_start_on_termination_deps(u, UNIT_ATOM_ON_SUCCESS);
                 else if (ns != os && ns == UNIT_FAILED)
-                        unit_start_on_failure(u, "OnFailure=", UNIT_ATOM_ON_FAILURE, u->on_failure_job_mode);
+                        unit_start_on_termination_deps(u, UNIT_ATOM_ON_FAILURE);
         }
 
         manager_recheck_journal(m);

diff --git a/src/core/unit.h b/src/core/unit.h
index b135fecc51425e38aeb0317995e8eedf3bb0ee3d..b0ffbbcebe28940f5adfc2ac93e7ee4dc8568f20 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -916,7 +916,7 @@ bool unit_will_restart(Unit *u);
 
 int unit_add_default_target_dependency(Unit *u, Unit *target);
 
-void unit_start_on_failure(Unit *u, const char *dependency_name, UnitDependencyAtom atom, JobMode job_mode);
+void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom);
 void unit_trigger_notify(Unit *u);
 
 UnitFileState unit_get_unit_file_state(Unit *u);