From: Luca Boccassi Date: Thu, 29 Aug 2024 12:17:13 +0000 (+0100) Subject: core: add method to enqueue multiple jobs in a single call X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7d3b32daef3125e70dd3f1689fb563a06b0c6753;p=thirdparty%2Fsystemd.git core: add method to enqueue multiple jobs in a single call Currently only a single job for a single unit can be enqueued atomically, so there is no guarantee that, e.g., starting a unit and its socket at the same time will happen in the same transaction. That forces callers to 'know' the right order in which to start new units being installed, or failures will occur. It also means some ordering constraints are ignored, in case the separate calls are done in the wrong manual order. Add a new EnqueueUnitJobMany() D-Bus method that takes a list of units to start. Fixes https://github.com/systemd/systemd/issues/8102 Co-authored-by: Michal Koutný --- diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index d3c2e39a18c..83748c6dd81 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 69921ad6263..2c4d809329f 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 1f9030f3e2e..76f64416831 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; - flags |= BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE; - } else if (streq(jtype, "reload-or-try-restart")) { - type = JOB_TRY_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 { - /* 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 778f537672c..a31a27dd546 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 1ee2228c899..43042652c04 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 e655e168c60..8986da4e7d3 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 90cc8ee41f1..8939f13271f 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 275dc5984f8..ee39b2600a1 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;