diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index d3c2e39a18..83748c6dd8 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -109,6 +109,11 @@ node /org/freedesktop/systemd1 { out o unit_path, out s job_type, out a(uosos) affected_jobs); + EnqueueUnitJobMany(in as units, + in s job_type, + in s job_mode, + in t flags, + out a(uosos) jobs); KillUnit(in s name, in s whom, in i signal); @@ -593,8 +598,6 @@ node /org/freedesktop/systemd1 { - - @@ -871,6 +874,8 @@ node /org/freedesktop/systemd1 { + + @@ -1362,6 +1367,37 @@ node /org/freedesktop/systemd1 { the "Try" flavor is used in which case a service that is not running is not affected by the restart. The "ReloadOrRestart" flavors attempt a reload if the unit supports it and use a restart otherwise. + EnqueueUnitJob() is similar to the various + StartUnit()/StopUnit()/RestartUnit() + methods above, but provides a unified interface that operates on a single unit. The + job_type argument selects the job type and is one of + start, stop, reload, + restart, try-restart, + reload-or-restart or reload-or-try-restart. The + job_mode argument matches the mode argument of + StartUnit(). On reply the method returns the enqueued anchor job's + numeric id and object path, the affected unit's id and object path, the job type + actually queued (after type collapsing, e.g. reload-or-restart turns + into restart or reload), and an array of + additional jobs that were enqueued as part of the same transaction in order to satisfy + dependencies. + + EnqueueUnitJobMany() is similar to + EnqueueUnitJob(), but operates on multiple units at once. All + requested units are enqueued in a single transaction, which is necessary in order to + properly honour After=/Before= ordering between + units passed in the same call regardless of their order in the array. The + job_type argument is applied to each of the listed units, including + the magic values reload-or-restart and + reload-or-try-restart which are resolved per unit (units that + support reloading get a reload job, others get a restart). The flags + argument does not support any flags for now and must be passed as 0; + it is reserved for future extensions. On reply the method returns an array of tuples + describing the anchor job that was enqueued for each requested unit: the numeric job + id, the job object path, the unit id, the unit object path, and the job type that was + actually queued. Note that the order of tuples in the reply is unspecified and may not + correspond to the order of the input array. + EnqueueMarkedJobs() creates reload/restart jobs for units which have been appropriately marked, see Markers property above. This is equivalent to calling TryRestartUnit() or ReloadOrTryRestartUnit() for the marked @@ -12764,7 +12800,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ ReloadCount, EventLoopRateLimitIntervalUSec, and EventLoopRateLimitBurst were added in version 261. - KExecsCount was added in version 262. + KExecsCount, and + EnqueueUnitJobMany() were added in version 262. Unit Objects diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 69921ad626..2c4d809329 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -848,6 +848,105 @@ static int method_enqueue_unit_job(sd_bus_message *message, void *userdata, sd_b return method_generic_unit_operation(message, userdata, reterr_error, _UNIT_TYPE_INVALID, bus_unit_method_enqueue_job, GENERIC_UNIT_LOAD); } +static int method_enqueue_unit_job_many(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) { + Manager *m = ASSERT_PTR(userdata); + _cleanup_strv_free_ char **names = NULL; + _cleanup_set_free_ Set *jobs = NULL; + const char *jtype, *smode; + JobType type; + JobMode mode; + uint64_t flags; + bool reload_if_possible; + Job *j; + int r; + + assert(message); + + r = sd_bus_message_read_strv(message, &names); + if (r < 0) + return r; + + if (strv_isempty(names)) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "No units specified."); + + if (strv_length(names) > (unsigned) MANAGER_MAX_NAMES / 2) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Too many unit names requested."); + + r = sd_bus_message_read(message, "sst", &jtype, &smode, &flags); + if (r < 0) + return r; + + if (flags != 0) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags parameter, must be 0."); + + r = bus_unit_parse_job_type(jtype, &type, &reload_if_possible, reterr_error); + if (r < 0) + return r; + + mode = job_mode_from_string(smode); + if (mode < 0) + return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Job mode %s invalid", smode); + + r = mac_selinux_access_check(message, job_type_to_access_method(type), reterr_error); + if (r < 0) + return r; + + r = bus_verify_manage_units_async(m, message, reterr_error); + if (r < 0) + return r; + if (r == 0) + return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ + + jobs = set_new(NULL); + if (!jobs) + return -ENOMEM; + + r = manager_add_jobs(m, type, names, reload_if_possible, mode, + /* extra_flags= */ 0, /* affected_jobs= */ NULL, reterr_error, jobs); + if (r < 0) + return r; + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + r = sd_bus_message_new_method_return(message, &reply); + if (r < 0) + return r; + + r = sd_bus_message_open_container(reply, 'a', "(uosos)"); + if (r < 0) + return r; + + SET_FOREACH(j, jobs) { + _cleanup_free_ char *job_path = NULL, *unit_path = NULL; + + r = bus_job_track_sender(j, message); + if (r < 0) + return r; + + bus_job_send_pending_change_signal(j, true); + + job_path = job_dbus_path(j); + if (!job_path) + return -ENOMEM; + + unit_path = unit_dbus_path(j->unit); + if (!unit_path) + return -ENOMEM; + + r = sd_bus_message_append(reply, "(uosos)", + j->id, job_path, + j->unit->id, unit_path, + job_type_to_string(j->type)); + if (r < 0) + return r; + } + + r = sd_bus_message_close_container(reply); + if (r < 0) + return r; + + return sd_bus_message_send(reply); +} + static int method_start_unit_replace(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) { Manager *m = ASSERT_PTR(userdata); const char *old_name; @@ -3099,6 +3198,11 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_RESULT("u", job_id, "o", job_path, "s", unit_id, "o", unit_path, "s", job_type, "a(uosos)", affected_jobs), method_enqueue_unit_job, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_ARGS("EnqueueUnitJobMany", + SD_BUS_ARGS("as", units, "s", job_type, "s", job_mode, "t", flags), + SD_BUS_RESULT("a(uosos)", jobs), + method_enqueue_unit_job_many, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("KillUnit", SD_BUS_ARGS("s", name, "s", whom, "i", signal), SD_BUS_NO_RESULT, diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 1f9030f3e2..76f6441683 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -467,10 +467,44 @@ static int bus_unit_method_reload_or_try_restart(sd_bus_message *message, void * return bus_unit_method_start_generic(message, userdata, JOB_TRY_RESTART, true, reterr_error); } +int bus_unit_parse_job_type( + const char *jtype, + JobType *ret_type, + bool *ret_reload_if_possible, + sd_bus_error *reterr_error) { + + JobType type; + bool reload_if_possible = false; + + assert(jtype); + assert(ret_type); + assert(ret_reload_if_possible); + + /* Parses the job type string as accepted by the EnqueueUnitJob()/EnqueueUnitJobMany() bus methods. The + * two magic "reload-or-…" types are handled manually, the rest generically. The actual + * reload-vs-restart choice is unit-specific and applied per-unit later. */ + if (streq(jtype, "reload-or-restart")) { + type = JOB_RESTART; + reload_if_possible = true; + } else if (streq(jtype, "reload-or-try-restart")) { + type = JOB_TRY_RESTART; + reload_if_possible = true; + } else { + type = job_type_from_string(jtype); + if (type < 0) + return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Job type %s invalid", jtype); + } + + *ret_type = type; + *ret_reload_if_possible = reload_if_possible; + return 0; +} + int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) { BusUnitQueueFlags flags = BUS_UNIT_QUEUE_VERBOSE_REPLY; const char *jtype, *smode; Unit *u = ASSERT_PTR(userdata); + bool reload_if_possible; JobType type; JobMode mode; int r; @@ -481,19 +515,11 @@ int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_ if (r < 0) return r; - /* Parse the two magic reload types "reload-or-…" manually */ - if (streq(jtype, "reload-or-restart")) { - type = JOB_RESTART; + r = bus_unit_parse_job_type(jtype, &type, &reload_if_possible, reterr_error); + if (r < 0) + return r; + if (reload_if_possible) flags |= BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE; - } else if (streq(jtype, "reload-or-try-restart")) { - type = JOB_TRY_RESTART; - flags |= BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE; - } else { - /* And the rest generically */ - type = job_type_from_string(jtype); - if (type < 0) - return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Job type %s invalid", jtype); - } mode = job_mode_from_string(smode); if (mode < 0) diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h index 778f537672..a31a27dd54 100644 --- a/src/core/dbus-unit.h +++ b/src/core/dbus-unit.h @@ -15,6 +15,7 @@ void bus_unit_send_removed_signal(Unit *u); int bus_unit_method_start_generic(sd_bus_message *message, Unit *u, JobType job_type, bool reload_if_possible, sd_bus_error *reterr_error); int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error); +int bus_unit_parse_job_type(const char *jtype, JobType *ret_type, bool *ret_reload_if_possible, sd_bus_error *reterr_error); int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error); int bus_unit_method_kill_subgroup(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error); int bus_unit_method_reset_failed(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error); diff --git a/src/core/manager.c b/src/core/manager.c index 1ee2228c89..43042652c0 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2267,6 +2267,128 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, Hashmap *named_ return 0; } +int manager_add_jobs( + Manager *m, + JobType type, + char * const *names, + bool reload_if_possible, + JobMode mode, + TransactionAddFlags extra_flags, + Set *affected_jobs, + sd_bus_error *reterr_error, + Set *ret_jobs) { + + _cleanup_(transaction_abort_and_freep) Transaction *tr = NULL; + Job *j; + int r; + + assert(m); + assert(type >= 0 && type < _JOB_TYPE_MAX); + assert(!strv_isempty(names)); + 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(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start."); + + if (mode == JOB_TRIGGERING && type != JOB_STOP) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, + "--job-mode=triggering is only valid for stop."); + + if (mode == JOB_RESTART_DEPENDENCIES && type != JOB_START) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, + "--job-mode=restart-dependencies is only valid for start."); + + if (mode == JOB_ISOLATE && strv_length(names) > 1) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_NOT_SUPPORTED, + "Isolating more than one unit is not supported."); + + tr = transaction_new(mode == JOB_REPLACE_IRREVERSIBLY, ++m->last_transaction_id); + if (!tr) + return -ENOMEM; + + LOG_CONTEXT_PUSHF("TRANSACTION_ID=%" PRIu64, tr->id); + + STRV_FOREACH(name, names) { + Unit *u; + JobType t = type; + JobType merged_type; + + r = manager_load_unit(m, *name, NULL, reterr_error, &u); + if (r < 0) + return r; + + if (mode == JOB_ISOLATE && !u->allow_isolate) + return sd_bus_error_setf(reterr_error, BUS_ERROR_NO_ISOLATION, + "Operation refused, unit %s may not be isolated.", u->id); + + /* Per-unit validation and reload-if-possible mangling: units that can reload turn + * JOB_RESTART into JOB_RELOAD_OR_START and JOB_TRY_RESTART into JOB_TRY_RELOAD; others + * keep the original restart type. Also rejects manual start/stop on units that refuse + * it, etc. */ + r = unit_queue_job_check_and_mangle_type(u, &t, reload_if_possible, reterr_error); + if (r < 0) + return r; + + merged_type = job_type_collapse(t, u); + + log_unit_debug(u, "Trying to enqueue job %s/%s/%s", + u->id, job_type_to_string(merged_type), job_mode_to_string(mode)); + + r = transaction_add_job_and_dependencies( + tr, + merged_type, + u, + /* by= */ NULL, + 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) | + extra_flags, + reterr_error); + if (r < 0) + return r; + + if (mode == JOB_TRIGGERING) { + r = transaction_add_triggering_jobs(tr, u); + if (r < 0) + return r; + } + } + + if (mode == JOB_ISOLATE) { + r = transaction_add_isolate_jobs(tr, m); + if (r < 0) + return r; + } + + r = transaction_activate(tr, m, mode, affected_jobs, reterr_error); + if (r < 0) + return r; + + SET_FOREACH(j, tr->anchor_jobs) + log_unit_debug(j->unit, + "Enqueued job %s/%s as %u", + j->unit->id, job_type_to_string(j->type), (unsigned) j->id); + + if (ret_jobs) { + /* The anchor_jobs set would be destroyed anyway, so steal the contents. */ + r = set_move(ret_jobs, tr->anchor_jobs); + if (r < 0) { + /* On failure, still clear anchor_jobs so the cleanup handler doesn't trip the + * empty-set assertion in transaction_free(). */ + set_clear(tr->anchor_jobs); + return r; + } + } else + /* tr->anchor_jobs tracks pointers to jobs that are now installed in the manager; clear + * it so transaction_free() doesn't trip its empty-set assertion. */ + set_clear(tr->anchor_jobs); + + tr = transaction_free(tr); + return 0; +} + int manager_add_job_full( Manager *m, JobType type, @@ -2338,12 +2460,19 @@ int manager_add_job_full( if (r < 0) return r; + Job *anchor = ASSERT_PTR(set_first(tr->anchor_jobs)); + assert(set_size(tr->anchor_jobs) == 1); + log_unit_debug(unit, "Enqueued job %s/%s as %u", unit->id, - job_type_to_string(type), (unsigned) tr->anchor_job->id); + job_type_to_string(type), (unsigned) anchor->id); if (ret) - *ret = tr->anchor_job; + *ret = anchor; + + /* anchor_jobs tracks pointers to jobs that are now installed in the manager; clear it so + * transaction_free() doesn't trip its empty-set assertion. */ + set_clear(tr->anchor_jobs); tr = transaction_free(tr); return 0; @@ -2417,7 +2546,7 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error transaction_add_propagate_reload_jobs( tr, unit, - tr->anchor_job, + set_first(tr->anchor_jobs), mode == JOB_IGNORE_DEPENDENCIES ? TRANSACTION_IGNORE_ORDER : 0); /* Only activate the transaction if it contains jobs other than NOP anchor. @@ -2429,6 +2558,9 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error if (r < 0) return r; + /* tr->anchor_jobs tracks pointers to jobs that are now installed in the manager, clear it */ + set_clear(tr->anchor_jobs); + tr = transaction_free(tr); return 0; } diff --git a/src/core/manager.h b/src/core/manager.h index e655e168c6..8986da4e7d 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -11,6 +11,7 @@ #include "log.h" #include "path-lookup.h" #include "show-status.h" +#include "transaction.h" #include "unit.h" struct libmnt_monitor; @@ -577,6 +578,17 @@ int manager_dispatch_external_fd_to_unit(Manager *m, const char *unit_id, const 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_jobs( + Manager *m, + JobType type, + char * const *names, + bool reload_if_possible, + JobMode mode, + TransactionAddFlags extra_flags, + Set *affected_jobs, + sd_bus_error *reterr_error, + Set *ret_jobs); + int manager_add_job_full( Manager *m, JobType type, diff --git a/src/core/transaction.c b/src/core/transaction.c index 90cc8ee41f..8939f13271 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -20,6 +20,7 @@ static bool job_matters_to_anchor(Job *job); static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependencies); +static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, unsigned generation); static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependencies) { assert(tr); @@ -28,6 +29,7 @@ static void transaction_delete_job(Transaction *tr, Job *j, bool delete_dependen /* Deletes one job from the transaction. */ transaction_unlink_job(tr, j, delete_dependencies); + (void) set_remove(tr->anchor_jobs, j); job_free(j); } @@ -52,7 +54,7 @@ static void transaction_abort(Transaction *tr) { assert(hashmap_isempty(tr->jobs)); } -static void transaction_find_jobs_that_matter_to_anchor(Job *j, unsigned generation) { +static void transaction_find_jobs_that_matter_to_anchor_one(Job *j, unsigned generation) { assert(j); /* A recursive sweep through the graph that marks all units that matter to the anchor job, i.e. are @@ -72,10 +74,22 @@ static void transaction_find_jobs_that_matter_to_anchor(Job *j, unsigned generat if (l->object->generation == generation) continue; - transaction_find_jobs_that_matter_to_anchor(l->object, generation); + transaction_find_jobs_that_matter_to_anchor_one(l->object, generation); } } +static void transaction_find_jobs_that_matter_to_anchor(Transaction *tr, unsigned generation) { + Job *j; + + assert(tr); + + /* All anchor jobs (and reachable jobs) get marked in the same generation. The recursion uses the + * generation only to avoid re-traversing already-visited nodes, so reusing the generation across + * anchors is correct: nodes already marked by a previous anchor are skipped. */ + SET_FOREACH(j, tr->anchor_jobs) + transaction_find_jobs_that_matter_to_anchor_one(j, generation); +} + static void transaction_merge_and_delete_job(Transaction *tr, Job *j, Job *other, JobType t) { JobDependency *last; @@ -275,7 +289,7 @@ static int transaction_merge_jobs(Transaction *tr, sd_bus_error *e) { Job *k; while ((k = j->transaction_next)) { - if (tr->anchor_job == k) { + if (set_contains(tr->anchor_jobs, k)) { transaction_merge_and_delete_job(tr, k, j, t); j = k; } else @@ -306,7 +320,7 @@ static void transaction_drop_redundant(Transaction *tr) { bool keep = false; LIST_FOREACH(transaction, k, j) - if (tr->anchor_job == k || + if (set_contains(tr->anchor_jobs, k) || !job_type_is_redundant(k->type, unit_active_state(k->unit)) || (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type))) { keep = true; @@ -524,7 +538,7 @@ static void transaction_collect_garbage(Transaction *tr) { again = false; HASHMAP_FOREACH(j, tr->jobs) { - if (tr->anchor_job == j) + if (set_contains(tr->anchor_jobs, j)) continue; if (!j->object_list) { @@ -557,11 +571,20 @@ static int transaction_is_destructive(Transaction *tr, JobMode mode, sd_bus_erro assert(!j->transaction_next); if (j->unit->job && (IN_SET(mode, JOB_FAIL, JOB_LENIENT) || j->unit->job->irreversible) && - job_type_is_conflicting(j->unit->job->type, j->type)) + job_type_is_conflicting(j->unit->job->type, j->type)) { + _cleanup_free_ char *anchors = NULL; + Job *a; + + SET_FOREACH(a, tr->anchor_jobs) + if (strextendf_with_separator(&anchors, ", ", "%s/%s", + a->unit->id, job_type_to_string(a->type)) < 0) + return -ENOMEM; + return sd_bus_error_setf(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, - "Transaction for %s/%s is destructive (%s has '%s' job queued, but '%s' is included in transaction).", - tr->anchor_job->unit->id, job_type_to_string(tr->anchor_job->type), + "Transaction for %s is destructive (%s has '%s' job queued, but '%s' is included in transaction).", + strna(anchors), j->unit->id, job_type_to_string(j->unit->job->type), job_type_to_string(j->type)); + } } return 0; @@ -680,8 +703,16 @@ static int transaction_apply( installed_job = job_install(j); if (installed_job != j) { /* j has been merged into a previously installed job. */ - if (tr->anchor_job == j) - tr->anchor_job = installed_job; + if (set_contains(tr->anchor_jobs, j)) { + r = set_remove_and_put(tr->anchor_jobs, j, installed_job); + if (r == -EEXIST) + /* installed_job is already an anchor; just drop the + * about-to-be-freed key so it doesn't linger. */ + (void) set_remove(tr->anchor_jobs, j); + else + /* Old key was present and remove -> put cannot fail */ + assert(r >= 0); + } hashmap_remove_value(m->jobs, UINT32_TO_PTR(j->id), j); free_and_replace_full(j, installed_job, job_free); @@ -731,7 +762,7 @@ int transaction_activate( j->generation = 0; /* First step: figure out which jobs matter. */ - transaction_find_jobs_that_matter_to_anchor(tr->anchor_job, generation++); + transaction_find_jobs_that_matter_to_anchor(tr, generation++); /* Second step: Try not to stop any running services if we don't have to. Don't try to reverse * running jobs if we don't have to. */ @@ -1024,9 +1055,10 @@ int transaction_add_job_and_dependencies( if (!job_dependency_new(by, job, FLAGS_SET(flags, TRANSACTION_MATTERS), FLAGS_SET(flags, TRANSACTION_CONFLICTS))) return -ENOMEM; } else { - /* If the job has no parent job, it is the anchor job. */ - assert(!tr->anchor_job); - tr->anchor_job = job; + /* If the job has no parent job, it is an anchor job. */ + r = set_put(tr->anchor_jobs, job); + if (r < 0 && r != -EEXIST) + return r; if (FLAGS_SET(flags, TRANSACTION_REENQUEUE_ANCHOR)) job->refuse_late_merge = true; @@ -1210,6 +1242,7 @@ int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { assert(tr); assert(m); + assert(set_size(tr->anchor_jobs) == 1); HASHMAP_FOREACH_KEY(u, k, m->units) { _cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL; @@ -1225,7 +1258,9 @@ int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { if (!shall_stop_on_isolate(tr, u)) continue; - r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, tr->anchor_job, TRANSACTION_MATTERS, &e); + /* JOB_ISOLATE only ever has a single anchor (we reject multi-anchor isolate in the + * manager) so picking the first anchor is correct. */ + r = transaction_add_job_and_dependencies(tr, JOB_STOP, u, set_first(tr->anchor_jobs), TRANSACTION_MATTERS, &e); if (r < 0) log_unit_warning_errno(u, r, "Cannot add isolate job, ignoring: %s", bus_error_message(&e, r)); } @@ -1235,11 +1270,17 @@ int transaction_add_isolate_jobs(Transaction *tr, Manager *m) { int transaction_add_triggering_jobs(Transaction *tr, Unit *u) { Unit *trigger; + Job *by; int r; assert(tr); assert(u); + /* The unit's own anchor job is the head of the list in tr->jobs. We pull in JOB_STOP for all + * triggers and link them to that anchor, so they are correctly grouped per anchor when there are + * multiple anchors in the transaction. */ + by = ASSERT_PTR(hashmap_get(tr->jobs, u)); + UNIT_FOREACH_DEPENDENCY_SAFE(trigger, u, UNIT_ATOM_TRIGGERED_BY) { _cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL; @@ -1251,7 +1292,7 @@ int transaction_add_triggering_jobs(Transaction *tr, Unit *u) { if (hashmap_contains(tr->jobs, trigger)) continue; - r = transaction_add_job_and_dependencies(tr, JOB_STOP, trigger, tr->anchor_job, TRANSACTION_MATTERS, &e); + r = transaction_add_job_and_dependencies(tr, JOB_STOP, trigger, by, TRANSACTION_MATTERS, &e); if (r < 0) log_unit_warning_errno(u, r, "Cannot add triggered by job, ignoring: %s", bus_error_message(&e, r)); } @@ -1270,11 +1311,15 @@ Transaction* transaction_new(bool irreversible, uint64_t id) { *tr = (Transaction) { .jobs = hashmap_new(NULL), + .anchor_jobs = set_new(NULL), .irreversible = irreversible, .id = id, }; - if (!tr->jobs) + if (!tr->jobs || !tr->anchor_jobs) { + hashmap_free(tr->jobs); + set_free(tr->anchor_jobs); return NULL; + } return TAKE_PTR(tr); } @@ -1284,6 +1329,9 @@ Transaction* transaction_free(Transaction *tr) { return NULL; assert(hashmap_isempty(tr->jobs)); + assert(set_isempty(tr->anchor_jobs)); + + set_free(tr->anchor_jobs); hashmap_free(tr->jobs); return mfree(tr); diff --git a/src/core/transaction.h b/src/core/transaction.h index 275dc5984f..ee39b2600a 100644 --- a/src/core/transaction.h +++ b/src/core/transaction.h @@ -6,7 +6,7 @@ typedef struct Transaction { /* Jobs to be added */ Hashmap *jobs; /* Unit object => Job object list 1:1 */ - Job *anchor_job; /* The job the user asked for */ + Set *anchor_jobs; /* the jobs the user asked for */ bool irreversible; uint64_t id;