diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/analyze/analyze-verify-util.c | 2 | ||||
-rw-r--r-- | src/core/automount.c | 4 | ||||
-rw-r--r-- | src/core/dbus-unit.c | 2 | ||||
-rw-r--r-- | src/core/dbus.c | 2 | ||||
-rw-r--r-- | src/core/job.c | 61 | ||||
-rw-r--r-- | src/core/job.h | 30 | ||||
-rw-r--r-- | src/core/main.c | 4 | ||||
-rw-r--r-- | src/core/manager.c | 19 | ||||
-rw-r--r-- | src/core/manager.h | 24 | ||||
-rw-r--r-- | src/core/path.c | 2 | ||||
-rw-r--r-- | src/core/service.c | 27 | ||||
-rw-r--r-- | src/core/socket.c | 4 | ||||
-rw-r--r-- | src/core/timer.c | 2 | ||||
-rw-r--r-- | src/core/transaction.c | 27 | ||||
-rw-r--r-- | src/core/transaction.h | 24 | ||||
-rw-r--r-- | src/core/unit-dependency-atom.c | 12 | ||||
-rw-r--r-- | src/core/unit-dependency-atom.h | 38 | ||||
-rw-r--r-- | src/core/unit.c | 10 | ||||
-rw-r--r-- | src/test/test-engine.c | 24 |
19 files changed, 182 insertions, 136 deletions
diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c index 017d4b4672..97f5449b0b 100644 --- a/src/analyze/analyze-verify-util.c +++ b/src/analyze/analyze-verify-util.c @@ -251,7 +251,7 @@ static int verify_unit(Unit *u, bool check_man, const char *root) { unit_dump(u, stdout, "\t"); log_unit_debug(u, "Creating %s/start job", u->id); - r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) log_unit_error_errno(u, r, "Failed to create %s/start: %s", u->id, bus_error_message(&error, r)); diff --git a/src/core/automount.c b/src/core/automount.c index 9d0f7a1f6f..ac81875442 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -781,7 +781,7 @@ static void automount_enter_running(Automount *a) { goto fail; } - r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) { log_unit_warning(UNIT(a), "Failed to queue mount startup job: %s", bus_error_message(&error, r)); goto fail; @@ -997,7 +997,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo goto fail; } - r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) { log_unit_warning(UNIT(a), "Failed to queue unmount job: %s", bus_error_message(&error, r)); goto fail; diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 2db473fbed..03f25402a2 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1864,7 +1864,7 @@ int bus_unit_queue_job_one( return -ENOMEM; } - r = manager_add_job(u->manager, type, u, mode, affected, error, &j); + r = manager_add_job_full(u->manager, type, u, mode, /* extra_flags = */ 0, affected, error, &j); if (r < 0) return r; diff --git a/src/core/dbus.c b/src/core/dbus.c index 2692dde151..744b97f991 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -175,7 +175,7 @@ static int signal_activation_request(sd_bus_message *message, void *userdata, sd goto failed; } - r = manager_add_job(m, JOB_START, u, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(m, JOB_START, u, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) goto failed; diff --git a/src/core/job.c b/src/core/job.c index 468571ae71..8fbbe757fe 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -162,6 +162,7 @@ static void job_set_state(Job *j, JobState state) { void job_uninstall(Job *j) { Job **pj; + assert(j); assert(j->installed); job_set_state(j, JOB_WAITING); @@ -185,25 +186,37 @@ void job_uninstall(Job *j) { j->installed = false; } -static bool job_type_allows_late_merge(JobType t) { - /* Tells whether it is OK to merge a job of type 't' with an already - * running job. - * Reloads cannot be merged this way. Think of the sequence: - * 1. Reload of a daemon is in progress; the daemon has already loaded - * its config file, but hasn't completed the reload operation yet. +static bool jobs_may_late_merge(const Job *j, const Job *uj) { + assert(j); + assert(!j->installed); + assert(uj); + assert(uj->installed); + assert(uj->state == JOB_RUNNING); + + /* Tells whether it is OK to merge a job with an already running job. */ + + if (j->refuse_late_merge) /* refused when constructing transaction? */ + return false; + + /* Reloads cannot be merged this way. Think of the sequence: + * 1. Reload of a daemon is in progress; the daemon has already loaded its config file, but hasn't + * completed the reload operation yet. * 2. Edit foo's config file. * 3. Trigger another reload to have the daemon use the new config. - * Should the second reload job be merged into the first one, the daemon - * would not know about the new config. - * JOB_RESTART jobs on the other hand can be merged, because they get - * patched into JOB_START after stopping the unit. So if we see a - * JOB_RESTART running, it means the unit hasn't stopped yet and at - * this time the merge is still allowed. */ - return t != JOB_RELOAD; + * Should the second reload job be merged into the first one, the daemon would not know about the new config. + * JOB_RESTART jobs on the other hand can be merged, because they get patched into JOB_START + * after stopping the unit. So if we see a JOB_RESTART running, it means the unit hasn't stopped yet + * and at this time the merge is still allowed. */ + if (j->type == JOB_RELOAD) + return false; + + return job_type_is_superset(uj->type, j->type); } static void job_merge_into_installed(Job *j, Job *other) { + assert(j); assert(j->installed); + assert(other); assert(j->unit == other->unit); if (j->type != JOB_NOP) { @@ -219,13 +232,13 @@ static void job_merge_into_installed(Job *j, Job *other) { j->ignore_order = j->ignore_order || other->ignore_order; } -Job* job_install(Job *j, bool refuse_late_merge) { +Job* job_install(Job *j) { Job **pj; Job *uj; assert(j); assert(!j->installed); - assert(j->type < _JOB_TYPE_MAX_IN_TRANSACTION); + assert(j->type >= 0 && j->type < _JOB_TYPE_MAX_IN_TRANSACTION); assert(j->state == JOB_WAITING); pj = j->type == JOB_NOP ? &j->unit->nop_job : &j->unit->job; @@ -237,8 +250,7 @@ Job* job_install(Job *j, bool refuse_late_merge) { else { /* not conflicting, i.e. mergeable */ - if (uj->state == JOB_WAITING || - (!refuse_late_merge && job_type_allows_late_merge(j->type) && job_type_is_superset(uj->type, j->type))) { + if (uj->state == JOB_WAITING || jobs_may_late_merge(j, uj)) { job_merge_into_installed(uj, j); log_unit_debug(uj->unit, "Merged %s/%s into installed job %s/%s as %"PRIu32, @@ -329,14 +341,16 @@ JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool * this means the 'anchor' job (i.e. the one the user * explicitly asked for) is the requester. */ - l = new0(JobDependency, 1); + l = new(JobDependency, 1); if (!l) return NULL; - l->subject = subject; - l->object = object; - l->matters = matters; - l->conflicts = conflicts; + *l = (JobDependency) { + .subject = subject, + .object = object, + .matters = matters, + .conflicts = conflicts, + }; if (subject) LIST_PREPEND(subject, subject->subject_list, l); @@ -494,6 +508,7 @@ JobType job_type_collapse(JobType t, Unit *u) { return JOB_RELOAD; default: + assert(t >= 0 && t < _JOB_TYPE_MAX_IN_TRANSACTION); return t; } } @@ -1243,7 +1258,7 @@ int job_deserialize(Job *j, FILE *f) { for (;;) { _cleanup_free_ char *l = NULL; size_t k; - char *v; + const char *v; r = deserialize_read_line(f, &l); if (r < 0) diff --git a/src/core/job.h b/src/core/job.h index 680b4a6016..f3daf9d094 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -127,7 +127,7 @@ struct Job { LIST_HEAD(JobDependency, object_list); /* Used for graph algs as a "I have been here" marker */ - Job* marker; + Job *marker; unsigned generation; uint32_t id; @@ -135,6 +135,10 @@ struct Job { JobType type; JobState state; + JobResult result; + + unsigned run_queue_idx; + sd_event_source *timer_event_source; usec_t begin_usec; usec_t begin_running_usec; @@ -149,31 +153,35 @@ struct Job { sd_bus_track *bus_track; char **deserialized_clients; - JobResult result; - - unsigned run_queue_idx; - /* If the job had a specific trigger that needs to be advertised (eg: a path unit), store it. */ ActivationDetails *activation_details; bool installed:1; bool in_run_queue:1; + bool matters_to_anchor:1; - bool in_dbus_queue:1; - bool sent_dbus_new_signal:1; + bool refuse_late_merge:1; bool ignore_order:1; bool irreversible:1; - bool in_gc_queue:1; + + bool in_dbus_queue:1; + bool sent_dbus_new_signal:1; + bool ref_by_private_bus:1; + + bool in_gc_queue:1; }; Job* job_new(Unit *unit, JobType type); Job* job_new_raw(Unit *unit); void job_unlink(Job *job); Job* job_free(Job *job); -Job* job_install(Job *j, bool refuse_late_merge); +DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free); + +Job* job_install(Job *j); int job_install_deserialized(Job *j); void job_uninstall(Job *j); + void job_dump(Job *j, FILE *f, const char *prefix); int job_serialize(Job *j, FILE *f); int job_deserialize(Job *j, FILE *f); @@ -182,8 +190,6 @@ int job_coldplug(Job *j); JobDependency* job_dependency_new(Job *subject, Job *object, bool matters, bool conflicts); void job_dependency_free(JobDependency *l); -int job_merge(Job *j, Job *other); - JobType job_type_lookup_merge(JobType a, JobType b) _pure_; _pure_ static inline bool job_type_is_mergeable(JobType a, JobType b) { @@ -231,8 +237,6 @@ void job_add_to_gc_queue(Job *j); int job_get_before(Job *j, Job*** ret); int job_get_after(Job *j, Job*** ret); -DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free); - const char* job_type_to_string(JobType t) _const_; JobType job_type_from_string(const char *s) _pure_; diff --git a/src/core/main.c b/src/core/main.c index 0c171a788d..93a1e221f7 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2582,13 +2582,13 @@ static int do_queue_default_job( assert(target->load_state == UNIT_LOADED); - r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, NULL, &error, &job); + r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, &error, &job); if (r == -EPERM) { log_debug_errno(r, "Default target could not be isolated, starting instead: %s", bus_error_message(&error, r)); sd_bus_error_free(&error); - r = manager_add_job(m, JOB_START, target, JOB_REPLACE, NULL, &error, &job); + r = manager_add_job(m, JOB_START, target, JOB_REPLACE, &error, &job); if (r < 0) { *ret_error_message = "Failed to start default target"; return log_struct_errno(LOG_EMERG, r, diff --git a/src/core/manager.c b/src/core/manager.c index 57b7c3cbd8..f58bc547a6 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1553,7 +1553,7 @@ static unsigned manager_dispatch_stop_when_unneeded_queue(Manager *m) { } /* Ok, nobody needs us anymore. Sniff. Then let's commit suicide */ - r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, /* ret = */ NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r)); } @@ -1594,7 +1594,7 @@ static unsigned manager_dispatch_start_when_upheld_queue(Manager *m) { continue; } - r = manager_add_job(u->manager, JOB_START, u, JOB_FAIL, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_START, u, JOB_FAIL, &error, /* ret = */ NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue start job, ignoring: %s", bus_error_message(&error, r)); } @@ -1635,7 +1635,7 @@ static unsigned manager_dispatch_stop_when_bound_queue(Manager *m) { continue; } - r = manager_add_job(u->manager, JOB_STOP, u, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_STOP, u, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r)); } @@ -2109,11 +2109,12 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo return 0; } -int manager_add_job( +int manager_add_job_full( Manager *m, JobType type, Unit *unit, JobMode mode, + TransactionAddFlags extra_flags, Set *affected_jobs, sd_bus_error *error, Job **ret) { @@ -2122,9 +2123,10 @@ int manager_add_job( int r; assert(m); - assert(type < _JOB_TYPE_MAX); + assert(type >= 0 && type < _JOB_TYPE_MAX); assert(unit); - assert(mode < _JOB_MODE_MAX); + assert(mode >= 0 && mode < _JOB_MODE_MAX); + assert((extra_flags & ~_TRANSACTION_FLAGS_MASK_PUBLIC) == 0); if (mode == JOB_ISOLATE && type != JOB_START) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start."); @@ -2154,7 +2156,8 @@ int manager_add_job( TRANSACTION_MATTERS | (IN_SET(mode, JOB_IGNORE_DEPENDENCIES, JOB_IGNORE_REQUIREMENTS) ? TRANSACTION_IGNORE_REQUIREMENTS : 0) | (mode == JOB_IGNORE_DEPENDENCIES ? TRANSACTION_IGNORE_ORDER : 0) | - (mode == JOB_RESTART_DEPENDENCIES ? TRANSACTION_PROPAGATE_START_AS_RESTART : 0), + (mode == JOB_RESTART_DEPENDENCIES ? TRANSACTION_PROPAGATE_START_AS_RESTART : 0) | + extra_flags, error); if (r < 0) return r; @@ -2200,7 +2203,7 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode return r; assert(unit); - return manager_add_job(m, type, unit, mode, affected_jobs, e, ret); + return manager_add_job_full(m, type, unit, mode, /* extra_flags = */ 0, affected_jobs, e, ret); } int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret) { diff --git a/src/core/manager.h b/src/core/manager.h index abf02bf079..c1f7f8c083 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -139,6 +139,7 @@ typedef enum WatchdogType { #include "job.h" #include "path-lookup.h" #include "show-status.h" +#include "transaction.h" #include "unit-name.h" #include "unit.h" @@ -554,9 +555,26 @@ int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_err int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret); int manager_load_unit_from_dbus_path(Manager *m, const char *s, sd_bus_error *e, Unit **_u); -int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret); -int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret); -int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret); +int manager_add_job_full( + Manager *m, + JobType type, + Unit *unit, + JobMode mode, + TransactionAddFlags extra_flags, + Set *affected_jobs, + sd_bus_error *error, + Job **ret); +static inline int manager_add_job( + Manager *m, + JobType type, + Unit *unit, + JobMode mode, + sd_bus_error *error, + Job **ret) { + return manager_add_job_full(m, type, unit, mode, 0, NULL, error, ret); +} +int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **ret); +int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret); int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error *e); void manager_clear_jobs(Manager *m); diff --git a/src/core/path.c b/src/core/path.c index 50f6db1470..e0dc93aabd 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -546,7 +546,7 @@ static void path_enter_running(Path *p, char *trigger_path) { goto fail; } - r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); + r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, &error, &job); if (r < 0) { log_unit_warning(UNIT(p), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); goto fail; diff --git a/src/core/service.c b/src/core/service.c index f48e15c2eb..737dc9905a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -47,6 +47,7 @@ #include "string-table.h" #include "string-util.h" #include "strv.h" +#include "transaction.h" #include "unit-name.h" #include "unit.h" #include "utf8.h" @@ -2645,13 +2646,23 @@ static void service_enter_restart(Service *s, bool shortcut) { return; } - /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves - * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. */ - r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, NULL, &error, NULL); + /* Any units that are bound to this service must also be restarted, unless RestartMode=direct. + * We use JOB_START for ourselves but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART + * for those dependency jobs in the former case, plain JOB_REPLACE when RestartMode=direct. + * + * Also, when RestartMode=direct is used, the service being restarted don't enter the inactive/failed state, + * i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous job might still + * be running (especially for Type=oneshot services). + * We need to refuse late merge and re-enqueue the anchor job. */ + r = manager_add_job_full(UNIT(s)->manager, + JOB_START, UNIT(s), + s->restart_mode == SERVICE_RESTART_MODE_DIRECT ? JOB_REPLACE : JOB_RESTART_DEPENDENCIES, + TRANSACTION_REENQUEUE_ANCHOR, + /* affected_jobs = */ NULL, + &error, /* ret = */ NULL); if (r < 0) { log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r)); - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false); - return; + return service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false); } /* Count the jobs we enqueue for restarting. This counter is maintained as long as the unit isn't @@ -4955,7 +4966,7 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s e = sd_bus_message_get_error(reply); if (e) { r = sd_bus_error_get_errno(e); - log_warning_errno(r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r)); + log_unit_warning_errno(UNIT(s), r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r)); return 1; } @@ -4967,7 +4978,7 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s r = pidref_set_pid(&pidref, pid); if (r < 0) { - log_debug_errno(r, "GetConnectionUnixProcessID() returned invalid PID: %m"); + log_unit_debug_errno(UNIT(s), r, "GetConnectionUnixProcessID() returned invalid PID: %m"); return 1; } @@ -5014,7 +5025,7 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) { "s", s->bus_name); if (r < 0) - log_debug_errno(r, "Failed to request owner PID of service name, ignoring: %m"); + log_unit_debug_errno(u, r, "Failed to request owner PID of service name, ignoring: %m"); } } diff --git a/src/core/socket.c b/src/core/socket.c index fce50e28e4..e4a19285c9 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2345,7 +2345,7 @@ static void socket_enter_running(Socket *s, int cfd_in) { goto fail; } - r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) goto queue_error; } @@ -2413,7 +2413,7 @@ static void socket_enter_running(Socket *s, int cfd_in) { s->n_connections++; - r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, NULL, &error, NULL); + r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, &error, /* ret = */ NULL); if (r < 0) { /* We failed to activate the new service, but it still exists. Let's make sure the * service closes and forgets the connection fd again, immediately. */ diff --git a/src/core/timer.c b/src/core/timer.c index e44298d5e8..b37a67f310 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -618,7 +618,7 @@ static void timer_enter_running(Timer *t) { goto fail; } - r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); + r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, &error, &job); if (r < 0) { log_unit_warning(UNIT(t), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); goto fail; diff --git a/src/core/transaction.c b/src/core/transaction.c index 9d48768d87..377ea05983 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -620,6 +620,9 @@ static int transaction_apply( Job *j; int r; + assert(tr); + assert(m); + /* Moves the transaction jobs to the set of active jobs */ if (IN_SET(mode, JOB_ISOLATE, JOB_FLUSH)) { @@ -658,12 +661,7 @@ static int transaction_apply( /* Clean the job dependencies */ transaction_unlink_job(tr, j, false); - /* When RestartMode=direct is used, the service being restarted don't enter the inactive/failed - * state, i.e. unit_process_job -> job_finish_and_invalidate is never called, and the previous - * job might still be running (especially for Type=oneshot services). We need to refuse - * late merge and re-enqueue the anchor job. */ - installed_job = job_install(j, - /* refuse_late_merge = */ mode == JOB_RESTART_DEPENDENCIES && j == tr->anchor_job); + installed_job = job_install(j); if (installed_job != j) { /* j has been merged into a previously installed job */ if (tr->anchor_job == j) @@ -705,10 +703,10 @@ int transaction_activate( int r; unsigned generation = 1; - assert(tr); + /* This applies the changes recorded in tr->jobs to the actual list of jobs, if possible. */ - /* This applies the changes recorded in tr->jobs to - * the actual list of jobs, if possible. */ + assert(tr); + assert(m); /* Reset the generation counter of all installed jobs. The detection of cycles * looks at installed jobs. If they had a non-zero generation from some previous @@ -739,7 +737,6 @@ int transaction_activate( r = transaction_verify_order(tr, &generation, e); if (r >= 0) break; - if (r != -EAGAIN) return log_warning_errno(r, "Requested transaction contains an unfixable cyclic ordering dependency: %s", bus_error_message(e, r)); @@ -754,7 +751,6 @@ int transaction_activate( r = transaction_merge_jobs(tr, e); if (r >= 0) break; - if (r != -EAGAIN) return log_warning_errno(r, "Requested transaction contains unmergeable jobs: %s", bus_error_message(e, r)); @@ -817,9 +813,6 @@ static Job* transaction_add_one_job(Transaction *tr, JobType type, Unit *unit, b if (!j) return NULL; - j->generation = 0; - j->marker = NULL; - j->matters_to_anchor = false; j->irreversible = tr->irreversible; LIST_PREPEND(transaction, f, j); @@ -944,6 +937,7 @@ int transaction_add_job_and_dependencies( int r; assert(tr); + assert(type >= 0); assert(type < _JOB_TYPE_MAX); assert(type < _JOB_TYPE_MAX_IN_TRANSACTION); assert(unit); @@ -1008,6 +1002,9 @@ int transaction_add_job_and_dependencies( /* If the job has no parent job, it is the anchor job. */ assert(!tr->anchor_job); tr->anchor_job = ret; + + if (FLAGS_SET(flags, TRANSACTION_REENQUEUE_ANCHOR)) + ret->refuse_late_merge = true; } if (!is_new || FLAGS_SET(flags, TRANSACTION_IGNORE_REQUIREMENTS) || type == JOB_NOP) @@ -1087,7 +1084,7 @@ int transaction_add_job_and_dependencies( if (IN_SET(type, JOB_RESTART, JOB_STOP) || (type == JOB_START && FLAGS_SET(flags, TRANSACTION_PROPAGATE_START_AS_RESTART))) { bool is_stop = type == JOB_STOP; - UNIT_FOREACH_DEPENDENCY(dep, ret->unit, is_stop ? UNIT_ATOM_PROPAGATE_STOP : UNIT_ATOM_PROPAGATE_RESTART) { + UNIT_FOREACH_DEPENDENCY(dep, ret->unit, UNIT_ATOM_PROPAGATE_STOP) { /* We propagate RESTART only as TRY_RESTART, in order not to start dependencies that * are not around. */ JobType nt; diff --git a/src/core/transaction.h b/src/core/transaction.h index 151e02dd60..b6728a35d7 100644 --- a/src/core/transaction.h +++ b/src/core/transaction.h @@ -2,6 +2,7 @@ #pragma once typedef struct Transaction Transaction; +typedef enum TransactionAddFlags TransactionAddFlags; #include "hashmap.h" #include "job.h" @@ -10,14 +11,14 @@ typedef struct Transaction Transaction; struct Transaction { /* Jobs to be added */ - Hashmap *jobs; /* Unit object => Job object list 1:1 */ - Job *anchor_job; /* the job the user asked for */ + Hashmap *jobs; /* Unit object => Job object list 1:1 */ + Job *anchor_job; /* The job the user asked for */ bool irreversible; }; -Transaction *transaction_new(bool irreversible); -Transaction *transaction_free(Transaction *tr); -Transaction *transaction_abort_and_free(Transaction *tr); +Transaction* transaction_new(bool irreversible); +Transaction* transaction_free(Transaction *tr); +Transaction* transaction_abort_and_free(Transaction *tr); DEFINE_TRIVIAL_CLEANUP_FUNC(Transaction*, transaction_abort_and_free); typedef enum TransactionAddFlags { @@ -31,13 +32,22 @@ typedef enum TransactionAddFlags { /* Indicate that we're in the recursion for processing UNIT_ATOM_PROPAGATE_STOP_GRACEFUL units */ TRANSACTION_PROCESS_PROPAGATE_STOP_GRACEFUL = 1 << 5, + + /* Always re-enqueue anchor job (refuse late merge) */ + TRANSACTION_REENQUEUE_ANCHOR = 1 << 6, + + _TRANSACTION_FLAGS_MASK_PUBLIC = TRANSACTION_REENQUEUE_ANCHOR, } TransactionAddFlags; void transaction_add_propagate_reload_jobs( Transaction *tr, - Unit *unit, Job *by, + Unit *unit, + Job *by, TransactionAddFlags flags); +int transaction_add_isolate_jobs(Transaction *tr, Manager *m); +int transaction_add_triggering_jobs(Transaction *tr, Unit *u); + int transaction_add_job_and_dependencies( Transaction *tr, JobType type, @@ -47,5 +57,3 @@ int transaction_add_job_and_dependencies( sd_bus_error *e); int transaction_activate(Transaction *tr, Manager *m, JobMode mode, Set *affected, sd_bus_error *e); -int transaction_add_isolate_jobs(Transaction *tr, Manager *m); -int transaction_add_triggering_jobs(Transaction *tr, Unit *u); diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c index 35b279b5c7..aba58612f7 100644 --- a/src/core/unit-dependency-atom.c +++ b/src/core/unit-dependency-atom.c @@ -33,8 +33,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { UNIT_ATOM_ADD_STOP_WHEN_UNNEEDED_QUEUE | UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE, - [UNIT_PART_OF] = UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE, - [UNIT_UPHOLDS] = UNIT_ATOM_PULL_IN_START_IGNORED | UNIT_ATOM_RETROACTIVE_START_REPLACE | UNIT_ATOM_ADD_START_WHEN_UPHELD_QUEUE | @@ -42,13 +40,11 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE, [UNIT_REQUIRED_BY] = UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART | UNIT_ATOM_PROPAGATE_START_FAILURE | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED | UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES, [UNIT_REQUISITE_OF] = UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART | UNIT_ATOM_PROPAGATE_START_FAILURE | UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED | @@ -59,7 +55,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { [UNIT_BOUND_BY] = UNIT_ATOM_RETROACTIVE_STOP_ON_STOP | UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART | UNIT_ATOM_PROPAGATE_START_FAILURE | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED | UNIT_ATOM_ADD_CANNOT_BE_ACTIVE_WITHOUT_QUEUE | @@ -69,9 +64,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED, - [UNIT_CONSISTS_OF] = UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART, - [UNIT_CONFLICTS] = UNIT_ATOM_PULL_IN_STOP | UNIT_ATOM_RETROACTIVE_STOP_ON_START, @@ -89,6 +81,8 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { [UNIT_ON_SUCCESS_OF] = UNIT_ATOM_ON_SUCCESS_OF, [UNIT_BEFORE] = UNIT_ATOM_BEFORE, [UNIT_AFTER] = UNIT_ATOM_AFTER, + [UNIT_PART_OF] = UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE, + [UNIT_CONSISTS_OF] = UNIT_ATOM_PROPAGATE_STOP, [UNIT_TRIGGERS] = UNIT_ATOM_TRIGGERS, [UNIT_TRIGGERED_BY] = UNIT_ATOM_TRIGGERED_BY, [UNIT_PROPAGATES_RELOAD_TO] = UNIT_ATOM_PROPAGATES_RELOAD_TO, @@ -160,7 +154,6 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { return UNIT_UPHOLDS; case UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART | UNIT_ATOM_PROPAGATE_START_FAILURE | UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED | @@ -170,7 +163,6 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { case UNIT_ATOM_RETROACTIVE_STOP_ON_STOP | UNIT_ATOM_PROPAGATE_STOP | - UNIT_ATOM_PROPAGATE_RESTART | UNIT_ATOM_PROPAGATE_START_FAILURE | UNIT_ATOM_PINS_STOP_WHEN_UNNEEDED | UNIT_ATOM_ADD_CANNOT_BE_ACTIVE_WITHOUT_QUEUE | diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h index 96f00ca39a..89a3eae1bf 100644 --- a/src/core/unit-dependency-atom.h +++ b/src/core/unit-dependency-atom.h @@ -56,35 +56,33 @@ typedef enum UnitDependencyAtom { /* If our start job succeeded but the unit is inactive then (think: oneshot units), propagate this as * failure to the other unit. */ UNIT_ATOM_PROPAGATE_INACTIVE_START_AS_FAILURE = UINT64_C(1) << 17, - /* When putting together a transaction, propagate JOB_STOP from our unit to the other. */ + /* When putting together a transaction, propagate JOB_STOP/JOB_RESTART from our unit to the other. */ UNIT_ATOM_PROPAGATE_STOP = UINT64_C(1) << 18, /* Like UNIT_ATOM_PROPAGATE_STOP, but enqueues a restart job if there's already a start job (avoids * job type conflict). */ UNIT_ATOM_PROPAGATE_STOP_GRACEFUL = UINT64_C(1) << 19, - /* When putting together a transaction, propagate JOB_RESTART from our unit to the other. */ - UNIT_ATOM_PROPAGATE_RESTART = UINT64_C(1) << 20, /* Add the other unit to the default target dependency queue */ - UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 21, + UNIT_ATOM_ADD_DEFAULT_TARGET_DEPENDENCY_QUEUE = UINT64_C(1) << 20, /* Recheck default target deps on other units (which are target units) */ - UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES = UINT64_C(1) << 22, + UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES = UINT64_C(1) << 21, /* The remaining atoms map 1:1 to the equally named high-level deps */ - UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 23, - UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 24, - UNIT_ATOM_ON_FAILURE_OF = UINT64_C(1) << 25, - UNIT_ATOM_ON_SUCCESS_OF = UINT64_C(1) << 26, - UNIT_ATOM_BEFORE = UINT64_C(1) << 27, - UNIT_ATOM_AFTER = UINT64_C(1) << 28, - UNIT_ATOM_TRIGGERS = UINT64_C(1) << 29, - UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 30, - UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 31, - UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 32, - UNIT_ATOM_REFERENCES = UINT64_C(1) << 33, - UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 34, - UNIT_ATOM_IN_SLICE = UINT64_C(1) << 35, - UNIT_ATOM_SLICE_OF = UINT64_C(1) << 36, - _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 37) - 1, + UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 22, + UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 23, + UNIT_ATOM_ON_FAILURE_OF = UINT64_C(1) << 24, + UNIT_ATOM_ON_SUCCESS_OF = UINT64_C(1) << 25, + UNIT_ATOM_BEFORE = UINT64_C(1) << 26, + UNIT_ATOM_AFTER = UINT64_C(1) << 27, + UNIT_ATOM_TRIGGERS = UINT64_C(1) << 28, + UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 29, + UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 30, + UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 31, + UNIT_ATOM_REFERENCES = UINT64_C(1) << 32, + UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 33, + UNIT_ATOM_IN_SLICE = UINT64_C(1) << 34, + UNIT_ATOM_SLICE_OF = UINT64_C(1) << 35, + _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 36) - 1, _UNIT_DEPENDENCY_ATOM_INVALID = -EINVAL, } UnitDependencyAtom; diff --git a/src/core/unit.c b/src/core/unit.c index dda98bb861..eec08a2fbf 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2217,16 +2217,16 @@ static void retroactively_start_dependencies(Unit *u) { UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_START_REPLACE) /* Requires= + BindsTo= */ if (!unit_has_dependency(u, UNIT_ATOM_AFTER, other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) - (void) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL, NULL); + (void) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL); UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_START_FAIL) /* Wants= */ if (!unit_has_dependency(u, UNIT_ATOM_AFTER, other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) - (void) manager_add_job(u->manager, JOB_START, other, JOB_FAIL, NULL, NULL, NULL); + (void) manager_add_job(u->manager, JOB_START, other, JOB_FAIL, /* error = */ NULL, /* ret = */ NULL); UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_STOP_ON_START) /* Conflicts= (and inverse) */ if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) - (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); + (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL); } static void retroactively_stop_dependencies(Unit *u) { @@ -2238,7 +2238,7 @@ static void retroactively_stop_dependencies(Unit *u) { /* Pull down units which are bound to us recursively if enabled */ UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_RETROACTIVE_STOP_ON_STOP) /* BoundBy= */ if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) - (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); + (void) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, /* error = */ NULL, /* ret = */ NULL); } void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom) { @@ -2269,7 +2269,7 @@ void unit_start_on_termination_deps(Unit *u, UnitDependencyAtom atom) { if (n_jobs == 0) log_unit_info(u, "Triggering %s dependencies.", dependency_name); - r = manager_add_job(u->manager, JOB_START, other, job_mode, NULL, &error, NULL); + r = manager_add_job(u->manager, JOB_START, other, job_mode, &error, /* ret = */ NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue %s%s job, ignoring: %s", dependency_name, other->id, bus_error_message(&error, r)); diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 0169f8320f..57774d04c5 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -106,7 +106,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test1: (Trivial)\n"); - r = manager_add_job(m, JOB_START, c, JOB_REPLACE, NULL, &err, &j); + r = manager_add_job(m, JOB_START, c, JOB_REPLACE, &err, &j); if (sd_bus_error_is_set(&err)) log_error("error: %s: %s", err.name, err.message); assert_se(r == 0); @@ -119,15 +119,15 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test2: (Cyclic Order, Unfixable)\n"); - assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, &j) == -EDEADLK); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Test3: (Cyclic Order, Fixable, Garbage Collector)\n"); - assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Test4: (Identical transaction)\n"); - assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Load3:\n"); @@ -135,21 +135,21 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test5: (Colliding transaction, fail)\n"); - assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, &j) == -EDEADLK); printf("Test6: (Colliding transaction, replace)\n"); - assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Test7: (Unmergeable job type, fail)\n"); - assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, &j) == -EDEADLK); printf("Test8: (Mergeable job type, fail)\n"); - assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Test9: (Unmergeable job type, replace)\n"); - assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Load4:\n"); @@ -157,7 +157,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test10: (Unmergeable job type of auxiliary job, fail)\n"); - assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, &j) == 0); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); printf("Load5:\n"); @@ -168,7 +168,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test11: (Start/stop job ordering, execution cycle)\n"); - assert_se(manager_add_job(m, JOB_START, i, JOB_FAIL, NULL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, i, JOB_FAIL, NULL, &j) == 0); assert_se(unit_has_job_type(a, JOB_STOP)); assert_se(unit_has_job_type(d, JOB_STOP)); assert_se(unit_has_job_type(b, JOB_START)); @@ -181,7 +181,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, /* patterns= */ NULL, "\t"); printf("Test12: (Trivial cycle, Unfixable)\n"); - assert_se(manager_add_job(m, JOB_START, a_conj, JOB_REPLACE, NULL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_START, a_conj, JOB_REPLACE, NULL, &j) == -EDEADLK); manager_dump_jobs(m, stdout, /* patterns= */ NULL, "\t"); assert_se(!hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b)); |