From: Mike Yuan Date: Fri, 7 Jun 2024 22:02:26 +0000 (+0200) Subject: core: clean up OnFailure= and OnSuccess= handling a bit X-Git-Tag: v257-rc1~1170^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9faeb2d024540ac4b94dedb00a13606ec262cc31;p=thirdparty%2Fsystemd.git 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 2f19468a954..fb32223e184 100644 --- 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 def621ef6d1..7e5f56cb21e 100644 --- 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 b135fecc514..b0ffbbcebe2 100644 --- 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);