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/man/systemctl.xml b/man/systemctl.xml index 122d58d66d..8b64b179ea 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -418,6 +418,13 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err Start (activate) one or more units specified on the command line. + When more than one unit is specified, the units are enqueued in a single job transaction. + This ensures that ordering dependencies (After=/Before=) + configured between the listed units are honored, regardless of the order in which they are given + on the command line. The same applies to stop, restart and + the reload/restart variants below. If a single transaction cannot be put together, separate + transactions will be submitted separately instead as a fallback. + Note that unit glob patterns expand to names of units currently in memory. Units which are not active and are not in a failed state usually are not in memory, and will not be matched by any pattern. In addition, in case of instantiated units, systemd is often unaware of the instance 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 d89b1eae0d..015f575ac1 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2288,6 +2288,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, @@ -2359,12 +2481,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; @@ -2438,7 +2567,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. @@ -2450,6 +2579,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 304124de04..a2f0644ef0 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; @@ -576,6 +577,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; diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index 9286a6fbb3..831b454372 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -678,22 +678,18 @@ static int maybe_enable_disable(sd_bus *bus, const char *path, bool enable) { return 0; } -static int maybe_start_stop_restart(sd_bus *bus, const char *path, const char *method, BusWaitForJobs *wait) { +static int maybe_start_stop_restart(sd_bus *bus, const char *name, const char *method, BusWaitForJobs *wait) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_free_ char *name = NULL; const char *job = NULL; int r; + assert(name); assert(STR_IN_SET(method, "StartUnit", "StopUnit", "RestartUnit")); if (!arg_now) return 0; - r = path_extract_filename(path, &name); - if (r < 0) - return log_error_errno(r, "Failed to extract file name from '%s': %m", path); - r = bus_call_method( bus, bus_systemd_mgr, @@ -704,7 +700,7 @@ static int maybe_start_stop_restart(sd_bus *bus, const char *path, const char *m if (r < 0) return log_error_errno(r, "Failed to call %s on the portable service %s: %s", method, - path, + name, bus_error_message(&error, r)); r = sd_bus_message_read(reply, "o", &job); @@ -724,8 +720,92 @@ static int maybe_start_stop_restart(sd_bus *bus, const char *path, const char *m return 0; } +static int maybe_start_stop_restart_units( + sd_bus *bus, + char * const *names, + const char *job_type, + BusWaitForJobs *wait) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + const char *method; + int r; + + assert(bus); + assert(STR_IN_SET(job_type, "start", "stop", "restart")); + + if (!arg_now || strv_isempty(names)) + return 0; + + method = streq(job_type, "start") ? "StartUnit" : + streq(job_type, "stop") ? "StopUnit" : + "RestartUnit"; + + /* Prefer the new EnqueueUnitJobMany() method which submits all units in a single transaction. + * Falls back to per-unit calls on older managers (UnknownMethod) or when the new method rejects + * something about the request (InvalidArgs). */ + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "EnqueueUnitJobMany"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_strv(m, (char**) names); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append(m, "sst", job_type, "replace", UINT64_C(0)); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_call(bus, m, 0, &error, &reply); + if (r >= 0) { + r = sd_bus_message_enter_container(reply, 'a', "(uosos)"); + if (r < 0) + return bus_log_parse_error(r); + + for (;;) { + const char *path, *unit_id; + uint32_t id; + + r = sd_bus_message_read(reply, "(uosos)", &id, &path, &unit_id, NULL, NULL); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; + + if (!arg_quiet) + log_info("Queued %s to call %s on portable service %s.", path, method, unit_id); + + if (wait) { + r = bus_wait_for_jobs_add(wait, path); + if (r < 0) + return log_error_errno(r, "Failed to watch %s job to call %s on %s: %m", + path, method, unit_id); + } + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return bus_log_parse_error(r); + + return 0; + } + + if (!sd_bus_error_has_names(&error, SD_BUS_ERROR_UNKNOWN_METHOD, SD_BUS_ERROR_INVALID_ARGS)) + return log_error_errno(r, "Failed to enqueue jobs for portable services: %s", + bus_error_message(&error, r)); + + log_debug_errno(r, "EnqueueUnitJobMany() not supported (%s), falling back to per-unit calls.", + bus_error_message(&error, r)); + + STRV_FOREACH(name, names) + (void) maybe_start_stop_restart(bus, *name, method, wait); + + return 0; +} + static int maybe_enable_start(sd_bus *bus, sd_bus_message *reply) { _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL; + _cleanup_strv_free_ char **start_names = NULL; int r; if (!arg_enable && !arg_now) @@ -755,7 +835,15 @@ static int maybe_enable_start(sd_bus *bus, sd_bus_message *reply) { if (STR_IN_SET(type, "symlink", "copy") && is_portable_managed(path)) { (void) maybe_enable_disable(bus, path, true); - (void) maybe_start_stop_restart(bus, path, "StartUnit", wait); + + _cleanup_free_ char *name = NULL; + r = path_extract_filename(path, &name); + if (r < 0) + return log_error_errno(r, "Failed to extract file name from '%s': %m", path); + + r = strv_consume(&start_names, TAKE_PTR(name)); + if (r < 0) + return log_oom(); } } @@ -763,6 +851,8 @@ static int maybe_enable_start(sd_bus *bus, sd_bus_message *reply) { if (r < 0) return r; + (void) maybe_start_stop_restart_units(bus, start_names, "start", wait); + if (!arg_no_block) { r = bus_wait_for_jobs(wait, arg_quiet, NULL); if (r < 0) @@ -774,6 +864,7 @@ static int maybe_enable_start(sd_bus *bus, sd_bus_message *reply) { static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL; + _cleanup_strv_free_ char **stop_names = NULL, **restart_names = NULL; int r; if (!arg_enable && !arg_now) @@ -804,14 +895,25 @@ static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { if (r == 0) break; - if (streq(type, "unlink") && is_portable_managed(path)) - (void) maybe_start_stop_restart(bus, path, "StopUnit", wait); + if (streq(type, "unlink") && is_portable_managed(path) && arg_now) { + _cleanup_free_ char *name = NULL; + + r = path_extract_filename(path, &name); + if (r < 0) + return log_error_errno(r, "Failed to extract file name from '%s': %m", path); + + r = strv_consume(&stop_names, TAKE_PTR(name)); + if (r < 0) + return log_oom(); + } } r = sd_bus_message_exit_container(reply); if (r < 0) return r; + (void) maybe_start_stop_restart_units(bus, stop_names, "stop", wait); + /* Then we get a list of units that were either added or changed, so that we can * enable them and/or restart them if the user asked us to. */ r = sd_bus_message_enter_container(reply, 'a', "(sss)"); @@ -829,7 +931,15 @@ static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { if (STR_IN_SET(type, "symlink", "copy") && is_portable_managed(path)) { (void) maybe_enable_disable(bus, path, true); - (void) maybe_start_stop_restart(bus, path, "RestartUnit", wait); + + _cleanup_free_ char *name = NULL; + r = path_extract_filename(path, &name); + if (r < 0) + return log_error_errno(r, "Failed to extract file name from '%s': %m", path); + + r = strv_consume(&restart_names, TAKE_PTR(name)); + if (r < 0) + return log_oom(); } } @@ -837,6 +947,8 @@ static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { if (r < 0) return r; + (void) maybe_start_stop_restart_units(bus, restart_names, "restart", wait); + if (!arg_no_block) { r = bus_wait_for_jobs(wait, arg_quiet, NULL); if (r < 0) @@ -940,9 +1052,6 @@ static int maybe_stop_disable_clean(sd_bus *bus, char *image, char *argv[]) { if (r < 0) return bus_log_parse_error(r); - (void) maybe_start_stop_restart(bus, name, "StopUnit", wait); - (void) maybe_enable_disable(bus, name, false); - r = strv_extend(&units, name); if (r < 0) return log_oom(); @@ -952,6 +1061,12 @@ static int maybe_stop_disable_clean(sd_bus *bus, char *image, char *argv[]) { if (r < 0) return bus_log_parse_error(r); + (void) maybe_start_stop_restart_units(bus, units, "stop", wait); + + /* Disable after stopping to match the idiomatic stop-then-disable lifecycle order. */ + STRV_FOREACH(name, units) + (void) maybe_enable_disable(bus, *name, false); + /* Stopping must always block or the detach will fail if the unit is still running */ r = bus_wait_for_jobs(wait, arg_quiet, NULL); if (r < 0) diff --git a/src/systemctl/systemctl-start-unit.c b/src/systemctl/systemctl-start-unit.c index 1b6dbd8b14..9bc924a07d 100644 --- a/src/systemctl/systemctl-start-unit.c +++ b/src/systemctl/systemctl-start-unit.c @@ -2,7 +2,6 @@ #include "sd-bus.h" -#include "alloc-util.h" #include "ansi-color.h" #include "bus-common-errors.h" #include "bus-error.h" @@ -192,6 +191,95 @@ fail: return r; } +static int start_units_one_transaction( + sd_bus *bus, + char * const *names, + const char *job_type, + const char *mode, + sd_bus_error *error, + BusWaitForJobs *w, + BusWaitForUnits *wu, + char ***ret_stopped_units) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; + int r; + + assert(bus); + assert(!strv_isempty(names)); + assert(job_type); + assert(mode); + assert(error); + + log_debug("Executing dbus call org.freedesktop.systemd1.Manager EnqueueUnitJobMany(%s, %s) for %zu units", + job_type, mode, strv_length(names)); + + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "EnqueueUnitJobMany"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_strv(m, (char**) names); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append(m, "sst", job_type, mode, UINT64_C(0)); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_call(bus, m, /* usec= */ 0, error, &reply); + if (r < 0) + return r; + + r = sd_bus_message_enter_container(reply, 'a', "(uosos)"); + if (r < 0) + return bus_log_parse_error(r); + + for (;;) { + const char *path, *unit_id, *jt; + uint32_t id; + + r = sd_bus_message_read(reply, "(uosos)", &id, &path, &unit_id, NULL, &jt); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; + + log_debug("Enqueued job %" PRIu32 " for %s/%s as %s", id, unit_id, jt, path); + + if (need_daemon_reload(bus, unit_id) > 0) + warn_unit_file_changed(unit_id); + + if (w) { + log_debug("Adding %s to the set", path); + r = bus_wait_for_jobs_add(w, path); + if (r < 0) + return log_error_errno(r, "Failed to watch job for %s: %m", unit_id); + } + + if (wu) { + r = bus_wait_for_units_add_unit(wu, unit_id, BUS_WAIT_FOR_INACTIVE|BUS_WAIT_NO_JOB, NULL, NULL); + if (r < 0) + return log_error_errno(r, "Failed to watch unit %s: %m", unit_id); + } + + /* Only record units for which the daemon actually enqueued a stop job. The + * server may optimize away stops for units that are already inactive, in + * which case the corresponding entry has a different job type (or is + * omitted), and we must not warn about "unit can still be triggered by…" + * for those. */ + if (ret_stopped_units && streq(jt, "stop")) { + r = strv_extend(ret_stopped_units, unit_id); + if (r < 0) + return log_oom(); + } + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return bus_log_parse_error(r); + + return 0; +} + static int enqueue_marked_jobs( sd_bus *bus, BusWaitForJobs *w) { @@ -293,7 +381,7 @@ int verb_start(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(bus_wait_for_units_freep) BusWaitForUnits *wu = NULL; _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; const char *method, *job_type, *mode, *one_name, *suffix = NULL; - _cleanup_free_ char **stopped_units = NULL; /* Do not use _cleanup_strv_free_ */ + _cleanup_strv_free_ char **stopped_units = NULL; _cleanup_strv_free_ char **names = NULL; bool is_enqueue_marked_jobs = false; int r, ret = EXIT_SUCCESS; @@ -401,22 +489,59 @@ int verb_start(int argc, char *argv[], uintptr_t _data, void *userdata) { if (is_enqueue_marked_jobs) ret = enqueue_marked_jobs(bus, w); else { + bool fallback = true; + if (arg_verbose) (void) journal_fork(arg_runtime_scope, names, arg_output, &journal_pid); - STRV_FOREACH(name, names) { + /* When operating on multiple units in one go, prefer the new EnqueueUnitJobMany() method + * which builds a single transaction. This ensures After= ordering is honored regardless + * of the order on the command line (see issue #8102). For show-transaction or dry-run we + * stick to per-unit calls. */ + if (arg_action == ACTION_SYSTEMCTL && + strv_length(names) > 1 && + !arg_show_transaction && + !arg_dry_run && + !streq(mode, "isolate")) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - r = start_unit_one(bus, method, job_type, *name, mode, &error, w, wu); - if (ret == EXIT_SUCCESS && r < 0) + r = start_units_one_transaction(bus, names, job_type, mode, &error, w, wu, + streq(method, "StopUnit") ? &stopped_units : NULL); + if (r >= 0) + fallback = false; + else if (sd_bus_error_has_names(&error, + SD_BUS_ERROR_UNKNOWN_METHOD, + SD_BUS_ERROR_INVALID_ARGS)) { + log_debug_errno(r, "EnqueueUnitJobMany() unavailable or unsupported (%s), falling back to per-unit calls.", + bus_error_message(&error, r)); + } else if (sd_bus_error_has_name(&error, BUS_ERROR_UNIT_MASKED) && + STR_IN_SET(method, "TryRestartUnit", "ReloadOrTryRestartUnit")) { + /* try-restart variants silently ignore masked units in the per-unit + * path; preserve that by falling back so each unit is evaluated + * individually. */ + log_debug_errno(r, "Batched %s aborted due to masked unit (%s), falling back to per-unit calls.", + method, bus_error_message(&error, r)); + } else { + log_error_errno(r, "Failed to enqueue jobs: %s", bus_error_message(&error, r)); ret = translate_bus_error_to_exit_status(r, &error); - - if (r >= 0 && streq(method, "StopUnit")) { - r = strv_push(&stopped_units, *name); - if (r < 0) - return log_oom(); + fallback = false; /* Real error, don't fall back. */ } } + + if (fallback) + STRV_FOREACH(name, names) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + + r = start_unit_one(bus, method, job_type, *name, mode, &error, w, wu); + if (ret == EXIT_SUCCESS && r < 0) + ret = translate_bus_error_to_exit_status(r, &error); + + if (r >= 0 && streq(method, "StopUnit")) { + r = strv_extend(&stopped_units, *name); + if (r < 0) + return log_oom(); + } + } } if (!arg_no_block) { diff --git a/test/units/TEST-07-PID1.multi-units-transaction.sh b/test/units/TEST-07-PID1.multi-units-transaction.sh new file mode 100755 index 0000000000..d8bf794447 --- /dev/null +++ b/test/units/TEST-07-PID1.multi-units-transaction.sh @@ -0,0 +1,375 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eux +set -o pipefail + +# Operate multiple units in a single transaction. +# Issue: https://github.com/systemd/systemd/issues/8102 +# +# When 'systemctl start ' is executed, both units must be enqueued in a +# single transaction so that After= ordering is honoured regardless of the order +# of the arguments. Previously each unit was sent to PID1 in its own D-Bus +# request and thus its own transaction, which meant the ordering dependency was +# only effective when the dependee was queued before the dependent. + +MARKER_DIR="$(mktemp -d /tmp/issue8102.marker.XXXXXX)" +MARKER="$MARKER_DIR/done" +SOCK_DIR="$(mktemp -d /tmp/issue8102.sock.XXXXXX)" +SOCK_PATH="$SOCK_DIR/sock" + +at_exit() { + set +e + + systemctl stop issue8102-second.service issue8102-first.service + systemctl stop issue8102-sock-foo.service issue8102-sock-foo.socket + systemctl stop 'issue8102-many@*.service' + systemctl stop issue8102-conflict-a.service issue8102-conflict-b.service + systemctl reset-failed issue8102-second.service issue8102-first.service + systemctl reset-failed issue8102-sock-foo.service issue8102-sock-foo.socket + systemctl reset-failed 'issue8102-many@*.service' + systemctl reset-failed issue8102-conflict-a.service issue8102-conflict-b.service + rm -f /run/systemd/system/issue8102-{first,second}.service + rm -f /run/systemd/system/issue8102-sock-foo.{service,socket} + rm -f /run/systemd/system/issue8102-many@.service + rm -f /run/systemd/system/issue8102-conflict-{a,b}.service + rm -rf "$MARKER_DIR" "$SOCK_DIR" + systemctl daemon-reload +} + +trap at_exit EXIT + +mkdir -p /run/systemd/system + +cat >/run/systemd/system/issue8102-first.service < "$MARKER"' +EOF + +cat >/run/systemd/system/issue8102-second.service </run/systemd/system/issue8102-sock-foo.socket </run/systemd/system/issue8102-sock-foo.service </dev/null +# Wait for the units to come back up after the restart. +# shellcheck disable=SC2016 +timeout 30s bash -c ' + while [[ "$(systemctl show -P ActiveState issue8102-sock-foo.service)" != active ]] || + [[ "$(systemctl show -P ActiveState issue8102-sock-foo.socket)" != active ]]; do + sleep 0.5 + done +' +test -S "$SOCK_PATH" + +# --------------------------------------------------------------------------- +# Argument validation corner cases for EnqueueUnitJobMany(). +# --------------------------------------------------------------------------- + +# Empty units array → the handler must reject the call with an INVALID_ARGS +# error before doing anything. +out=$(busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 0 \ + start replace \ + 0 2>&1) && { echo 'busctl unexpectedly succeeded'; exit 1; } +echo "$out" | grep -F "No units specified" >/dev/null + +# Non-zero flags parameter is reserved and must be rejected. +out=$(busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 1 issue8102-first.service \ + start replace \ + 1 2>&1) && { echo 'busctl unexpectedly succeeded'; exit 1; } +echo "$out" | grep -F "Invalid flags parameter" >/dev/null + +# Bogus job type → rejected before any job is constructed. +out=$(busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 1 issue8102-first.service \ + not-a-real-job-type replace \ + 0 2>&1) && { echo 'busctl unexpectedly succeeded'; exit 1; } +echo "$out" | grep -F "Job type not-a-real-job-type invalid" >/dev/null + +# Bogus job mode → rejected before any job is constructed. +out=$(busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 1 issue8102-first.service \ + start not-a-real-mode \ + 0 2>&1) && { echo 'busctl unexpectedly succeeded'; exit 1; } +echo "$out" | grep -F "Job mode not-a-real-mode invalid" >/dev/null + +# Unknown unit must be reported as an error and no partial state should remain +# in the job queue: list one valid + one bogus unit, ensure the call fails and +# that the valid unit has no pending start job afterwards. +systemctl stop issue8102-first.service +[[ "$(systemctl show -P ActiveState issue8102-first.service)" == inactive ]] + +(! busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 2 issue8102-first.service issue8102-does-not-exist.service \ + start replace \ + 0 2>/dev/null) + +# busctl is synchronous: by the time the call returns with an error, PID1 has +# already rejected the transaction. Verify the valid unit was not started +# behind our back (the transaction must be all-or-nothing). +[[ "$(systemctl show -P ActiveState issue8102-first.service)" == inactive ]] + +# --------------------------------------------------------------------------- +# Many units in a single transaction. +# --------------------------------------------------------------------------- +# Build a template unit and instantiate a fair number of instances, all +# enqueued via a single EnqueueUnitJobMany() call to exercise the strv path with +# a transaction that anchors many units at once. + +cat >/run/systemd/system/issue8102-many@.service <<'EOF' +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=/bin/true +EOF +systemctl daemon-reload + +MANY_COUNT=20 +mapfile -t MANY_UNITS < <(for i in $(seq 1 "$MANY_COUNT"); do printf 'issue8102-many@%d.service\n' "$i"; done) +MANY_BUSCTL_ARGS=("$MANY_COUNT" "${MANY_UNITS[@]}") + +busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + "${MANY_BUSCTL_ARGS[@]}" \ + start replace \ + 0 >/dev/null + +# Wait for every instance to become active. +# shellcheck disable=SC2016 +timeout 30s bash -c ' + for u in "$@"; do + while [[ "$(systemctl show -P ActiveState "$u")" != active ]]; do + sleep 0.2 + done + done +' bash "${MANY_UNITS[@]}" + +# Stop them all in one transaction too, verifying the stop path scales as well. +busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + "${MANY_BUSCTL_ARGS[@]}" \ + stop replace \ + 0 >/dev/null + +# shellcheck disable=SC2016 +timeout 30s bash -c ' + for u in "$@"; do + while [[ "$(systemctl show -P ActiveState "$u")" != inactive ]]; do + sleep 0.2 + done + done +' bash "${MANY_UNITS[@]}" + +# --------------------------------------------------------------------------- +# Incompatible transaction: two units that Conflict= with each other cannot be +# started together. Both start anchors would force the other unit to stop, so +# the transaction is unsatisfiable regardless of the chosen job mode. The +# handler must report an error and roll back, so that neither unit ends up +# active. +# --------------------------------------------------------------------------- + +cat >/run/systemd/system/issue8102-conflict-a.service </run/systemd/system/issue8102-conflict-b.service </dev/null) + +# Atomicity: neither unit must end up activated by the failed call. busctl is +# synchronous so by the time it returns PID1 has already rolled back. +[[ "$(systemctl show -P ActiveState issue8102-conflict-a.service)" == inactive ]] +[[ "$(systemctl show -P ActiveState issue8102-conflict-b.service)" == inactive ]] + +# --------------------------------------------------------------------------- +# reload-or-try-restart with a mix of reloadable and non-reloadable units. +# The socket cannot reload so it must be restarted, while a unit that does +# implement reload would be reloaded. Both must be active afterwards. +# --------------------------------------------------------------------------- + +systemctl start issue8102-sock-foo.service issue8102-sock-foo.socket +[[ "$(systemctl show -P ActiveState issue8102-sock-foo.service)" == active ]] +[[ "$(systemctl show -P ActiveState issue8102-sock-foo.socket)" == active ]] + +busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 2 issue8102-sock-foo.service issue8102-sock-foo.socket \ + reload-or-try-restart replace \ + 0 >/dev/null + +# shellcheck disable=SC2016 +timeout 30s bash -c ' + while [[ "$(systemctl show -P ActiveState issue8102-sock-foo.service)" != active ]] || + [[ "$(systemctl show -P ActiveState issue8102-sock-foo.socket)" != active ]]; do + sleep 0.5 + done +' +test -S "$SOCK_PATH" + +# try-restart on a unit that is not currently running must be a no-op (no error) +# and must not start it. +systemctl stop issue8102-first.service +[[ "$(systemctl show -P ActiveState issue8102-first.service)" == inactive ]] + +busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + EnqueueUnitJobMany \ + assst \ + 1 issue8102-first.service \ + try-restart replace \ + 0 >/dev/null + +[[ "$(systemctl show -P ActiveState issue8102-first.service)" == inactive ]]