From 50cbaba4fe5a32850998682699322d012e597e4a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Mar 2019 20:57:30 +0100 Subject: [PATCH 1/8] core: add new API for enqueing a job with returning the transaction data --- src/analyze/analyze-verify.c | 2 +- src/core/automount.c | 4 +- src/core/dbus-manager.c | 23 ++++- src/core/dbus-unit.c | 157 +++++++++++++++++++++++++++++++---- src/core/dbus-unit.h | 8 +- src/core/dbus.c | 2 +- src/core/device.c | 2 +- src/core/emergency-action.c | 7 +- src/core/main.c | 4 +- src/core/manager.c | 38 +++++---- src/core/manager.h | 6 +- src/core/path.c | 2 +- src/core/service.c | 2 +- src/core/socket.c | 4 +- src/core/timer.c | 2 +- src/core/transaction.c | 22 ++++- src/core/transaction.h | 2 +- src/core/unit.c | 16 ++-- src/test/test-engine.c | 20 ++--- 19 files changed, 247 insertions(+), 76 deletions(-) diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 1d8a1ed7b3..16b07cc856 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -202,7 +202,7 @@ static int verify_unit(Unit *u, bool check_man) { unit_dump(u, stdout, "\t"); log_unit_debug(u, "Creating %s/start job", u->id); - r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, &err, NULL); + r = manager_add_job(u->manager, JOB_START, u, JOB_REPLACE, NULL, &err, NULL); if (r < 0) log_unit_error_errno(u, r, "Failed to create %s/start: %s", u->id, bus_error_message(&err, r)); diff --git a/src/core/automount.c b/src/core/automount.c index 8ffdad18c7..28786158ee 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -781,7 +781,7 @@ static void automount_enter_running(Automount *a) { goto fail; } - r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(a)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); if (r < 0) { log_unit_warning(UNIT(a), "Failed to queue mount startup job: %s", bus_error_message(&error, r)); goto fail; @@ -1035,7 +1035,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo goto fail; } - r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(a)->manager, JOB_STOP, trigger, JOB_REPLACE, NULL, &error, NULL); if (r < 0) { log_unit_warning(UNIT(a), "Failed to queue umount startup job: %s", bus_error_message(&error, r)); goto fail; diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 88e4c6bb95..f0029081b6 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -574,6 +574,26 @@ static int method_reload_or_try_restart_unit(sd_bus_message *message, void *user return method_start_unit_generic(message, userdata, JOB_TRY_RESTART, true, error); } +static int method_enqueue_unit_job(sd_bus_message *message, void *userdata, sd_bus_error *error) { + Manager *m = userdata; + const char *name; + Unit *u; + int r; + + assert(message); + assert(m); + + r = sd_bus_message_read(message, "s", &name); + if (r < 0) + return r; + + r = manager_load_unit(m, name, NULL, error, &u); + if (r < 0) + return r; + + return bus_unit_method_enqueue_job(message, u, error); +} + static int method_start_unit_replace(sd_bus_message *message, void *userdata, sd_bus_error *error) { Manager *m = userdata; const char *old_name; @@ -955,7 +975,7 @@ static int method_start_transient_unit(sd_bus_message *message, void *userdata, return r; /* Finally, start it */ - return bus_unit_queue_job(message, u, JOB_START, mode, false, error); + return bus_unit_queue_job(message, u, JOB_START, mode, 0, error); } static int method_get_job(sd_bus_message *message, void *userdata, sd_bus_error *error) { @@ -2553,6 +2573,7 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_METHOD("TryRestartUnit", "ss", "o", method_try_restart_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ReloadOrRestartUnit", "ss", "o", method_reload_or_restart_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ReloadOrTryRestartUnit", "ss", "o", method_reload_or_try_restart_unit, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("EnqueueUnitJob", "sss", "uososa(uosos)", method_enqueue_unit_job, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("KillUnit", "ssi", NULL, method_kill_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ResetFailedUnit", "s", NULL, method_reset_failed_unit, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("SetUnitProperties", "sba(sv)", NULL, method_set_unit_properties, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index e1b4af1050..6f1a74d6b5 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -312,6 +312,14 @@ static int bus_verify_manage_units_async_full( error); } +static const char *const polkit_message_for_job[_JOB_TYPE_MAX] = { + [JOB_START] = N_("Authentication is required to start '$(unit)'."), + [JOB_STOP] = N_("Authentication is required to stop '$(unit)'."), + [JOB_RELOAD] = N_("Authentication is required to reload '$(unit)'."), + [JOB_RESTART] = N_("Authentication is required to restart '$(unit)'."), + [JOB_TRY_RESTART] = N_("Authentication is required to restart '$(unit)'."), +}; + int bus_unit_method_start_generic( sd_bus_message *message, Unit *u, @@ -321,13 +329,6 @@ int bus_unit_method_start_generic( const char *smode, *verb; JobMode mode; - static const char *const polkit_message_for_job[_JOB_TYPE_MAX] = { - [JOB_START] = N_("Authentication is required to start '$(unit)'."), - [JOB_STOP] = N_("Authentication is required to stop '$(unit)'."), - [JOB_RELOAD] = N_("Authentication is required to reload '$(unit)'."), - [JOB_RESTART] = N_("Authentication is required to restart '$(unit)'."), - [JOB_TRY_RESTART] = N_("Authentication is required to restart '$(unit)'."), - }; int r; assert(message); @@ -367,7 +368,8 @@ int bus_unit_method_start_generic( if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - return bus_unit_queue_job(message, u, job_type, mode, reload_if_possible, error); + return bus_unit_queue_job(message, u, job_type, mode, + reload_if_possible ? BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE : 0, error); } static int method_start(sd_bus_message *message, void *userdata, sd_bus_error *error) { @@ -398,6 +400,62 @@ static int method_reload_or_try_restart(sd_bus_message *message, void *userdata, return bus_unit_method_start_generic(message, userdata, JOB_TRY_RESTART, true, error); } +int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_error *error) { + BusUnitQueueFlags flags = BUS_UNIT_QUEUE_VERBOSE_REPLY; + const char *jtype, *smode; + Unit *u = userdata; + JobType type; + JobMode mode; + int r; + + assert(message); + assert(u); + + r = sd_bus_message_read(message, "ss", &jtype, &smode); + 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; + 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(error, SD_BUS_ERROR_INVALID_ARGS, "Job type %s invalid", jtype); + } + + mode = job_mode_from_string(smode); + if (mode < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Job mode %s invalid", smode); + + r = mac_selinux_unit_access_check( + u, message, + job_type_to_access_method(type), + error); + if (r < 0) + return r; + + r = bus_verify_manage_units_async_full( + u, + jtype, + CAP_SYS_ADMIN, + polkit_message_for_job[type], + true, + message, + 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 */ + + return bus_unit_queue_job(message, u, type, mode, flags, error); +} + int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error) { Unit *u = userdata; const char *swho; @@ -683,6 +741,7 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_METHOD("TryRestart", "s", "o", method_try_restart, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ReloadOrRestart", "s", "o", method_reload_or_restart, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ReloadOrTryRestart", "s", "o", method_reload_or_try_restart, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("EnqueueJob", "ss", "uososa(uosos)", bus_unit_method_enqueue_job, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Kill", "si", NULL, bus_unit_method_kill, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ResetFailed", NULL, NULL, bus_unit_method_reset_failed, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("SetProperties", "ba(sv)", NULL, bus_unit_method_set_properties, SD_BUS_VTABLE_UNPRIVILEGED), @@ -1269,11 +1328,14 @@ int bus_unit_queue_job( Unit *u, JobType type, JobMode mode, - bool reload_if_possible, + BusUnitQueueFlags flags, sd_bus_error *error) { - _cleanup_free_ char *path = NULL; - Job *j; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_free_ char *job_path = NULL, *unit_path = NULL; + _cleanup_(set_freep) Set *affected = NULL; + Iterator i; + Job *j, *a; int r; assert(message); @@ -1288,7 +1350,7 @@ int bus_unit_queue_job( if (r < 0) return r; - if (reload_if_possible && unit_can_reload(u)) { + if (FLAGS_SET(flags, BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE) && unit_can_reload(u)) { if (type == JOB_RESTART) type = JOB_RELOAD_OR_START; else if (type == JOB_TRY_RESTART) @@ -1306,7 +1368,13 @@ int bus_unit_queue_job( (type == JOB_RELOAD_OR_START && job_type_collapse(type, u) == JOB_START && u->refuse_manual_start)) return sd_bus_error_setf(error, BUS_ERROR_ONLY_BY_DEPENDENCY, "Operation refused, unit %s may be requested by dependency only (it is configured to refuse manual start/stop).", u->id); - r = manager_add_job(u->manager, type, u, mode, error, &j); + if (FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) { + affected = set_new(NULL); + if (!affected) + return -ENOMEM; + } + + r = manager_add_job(u->manager, type, u, mode, affected, error, &j); if (r < 0) return r; @@ -1314,14 +1382,67 @@ int bus_unit_queue_job( if (r < 0) return r; - path = job_dbus_path(j); - if (!path) - return -ENOMEM; - /* Before we send the method reply, force out the announcement JobNew for this job */ bus_job_send_pending_change_signal(j, true); - return sd_bus_reply_method_return(message, "o", path); + job_path = job_dbus_path(j); + if (!job_path) + return -ENOMEM; + + /* The classic response is just a job object path */ + if (!FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) + return sd_bus_reply_method_return(message, "o", job_path); + + /* In verbose mode respond with the anchor job plus everything that has been affected */ + r = sd_bus_message_new_method_return(message, &reply); + if (r < 0) + return r; + + 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_open_container(reply, 'a', "(uosos)"); + if (r < 0) + return r; + + SET_FOREACH(a, affected, i) { + + if (a->id == j->id) + continue; + + /* Free paths from previous iteration */ + job_path = mfree(job_path); + unit_path = mfree(unit_path); + + job_path = job_dbus_path(a); + if (!job_path) + return -ENOMEM; + + unit_path = unit_dbus_path(a->unit); + if (!unit_path) + return -ENOMEM; + + r = sd_bus_message_append(reply, "(uosos)", + a->id, job_path, + a->unit->id, unit_path, + job_type_to_string(a->type)); + if (r < 0) + return r; + } + + r = sd_bus_message_close_container(reply); + if (r < 0) + return r; + + return sd_bus_send(NULL, reply, NULL); } static int bus_unit_set_live_property( diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h index 345345e3eb..740bb1c86b 100644 --- a/src/core/dbus-unit.h +++ b/src/core/dbus-unit.h @@ -15,6 +15,7 @@ void bus_unit_send_pending_change_signal(Unit *u, bool including_new); 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 *error); +int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_reset_failed(sd_bus_message *message, void *userdata, sd_bus_error *error); @@ -25,7 +26,12 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd int bus_unit_method_ref(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error *error); -int bus_unit_queue_job(sd_bus_message *message, Unit *u, JobType type, JobMode mode, bool reload_if_possible, sd_bus_error *error); +typedef enum BusUnitQueueFlags { + BUS_UNIT_QUEUE_RELOAD_IF_POSSIBLE = 1 << 0, + BUS_UNIT_QUEUE_VERBOSE_REPLY = 1 << 1, +} BusUnitQueueFlags; + +int bus_unit_queue_job(sd_bus_message *message, Unit *u, JobType type, JobMode mode, BusUnitQueueFlags flags, sd_bus_error *error); int bus_unit_validate_load_state(Unit *u, sd_bus_error *error); int bus_unit_track_add_name(Unit *u, const char *name); diff --git a/src/core/dbus.c b/src/core/dbus.c index 255b86e7a4..74b1c51c1d 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -177,7 +177,7 @@ static int signal_activation_request(sd_bus_message *message, void *userdata, sd goto failed; } - r = manager_add_job(m, JOB_START, u, JOB_REPLACE, &error, NULL); + r = manager_add_job(m, JOB_START, u, JOB_REPLACE, NULL, &error, NULL); if (r < 0) goto failed; diff --git a/src/core/device.c b/src/core/device.c index 506bf74478..771239f53b 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -433,7 +433,7 @@ static int device_add_udev_wants(Unit *u, sd_device *dev) { if (strv_contains(d->wants_property, *i)) /* Was this unit already listed before? */ continue; - r = manager_add_job_by_name(u->manager, JOB_START, *i, JOB_FAIL, &error, NULL); + r = manager_add_job_by_name(u->manager, JOB_START, *i, JOB_FAIL, NULL, &error, NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue SYSTEMD_WANTS= job, ignoring: %s", bus_error_message(&error, r)); } diff --git a/src/core/emergency-action.c b/src/core/emergency-action.c index 09f8d74092..4330c0f2c1 100644 --- a/src/core/emergency-action.c +++ b/src/core/emergency-action.c @@ -48,8 +48,7 @@ void emergency_action( log_and_status(m, warn, "Rebooting", reason); (void) update_reboot_parameter_and_warn(reboot_arg, true); - (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_REBOOT_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL); - + (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_REBOOT_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL, NULL); break; case EMERGENCY_ACTION_REBOOT_FORCE: @@ -82,7 +81,7 @@ void emergency_action( if (MANAGER_IS_USER(m) || detect_container() > 0) { log_and_status(m, warn, "Exiting", reason); - (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_EXIT_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL); + (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_EXIT_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL, NULL); break; } @@ -91,7 +90,7 @@ void emergency_action( case EMERGENCY_ACTION_POWEROFF: log_and_status(m, warn, "Powering off", reason); - (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_POWEROFF_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL); + (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_POWEROFF_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL, NULL); break; case EMERGENCY_ACTION_EXIT_FORCE: diff --git a/src/core/main.c b/src/core/main.c index 6f6f4e5c5e..0ba22f815d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2121,13 +2121,13 @@ static int do_queue_default_job( assert(target->load_state == UNIT_LOADED); - r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, &error, &default_unit_job); + r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, NULL, &error, &default_unit_job); if (r == -EPERM) { log_debug_errno(r, "Default target could not be isolated, starting instead: %s", bus_error_message(&error, r)); sd_bus_error_free(&error); - r = manager_add_job(m, JOB_START, target, JOB_REPLACE, &error, &default_unit_job); + r = manager_add_job(m, JOB_START, target, JOB_REPLACE, NULL, &error, &default_unit_job); if (r < 0) { *ret_error_message = "Failed to start default target"; return log_emergency_errno(r, "Failed to start default target: %s", bus_error_message(&error, r)); diff --git a/src/core/manager.c b/src/core/manager.c index 7f4f71b9e9..f8a8ac346b 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1253,7 +1253,7 @@ static unsigned manager_dispatch_stop_when_unneeded_queue(Manager *m) { } /* Ok, nobody needs us anymore. Sniff. Then let's commit suicide */ - r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, NULL); + r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, NULL, &error, NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r)); } @@ -1730,9 +1730,17 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { return 0; } -int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_error *e, Job **_ret) { - int r; +int manager_add_job( + Manager *m, + JobType type, + Unit *unit, + JobMode mode, + Set *affected_jobs, + sd_bus_error *error, + Job **ret) { + Transaction *tr; + int r; assert(m); assert(type < _JOB_TYPE_MAX); @@ -1740,10 +1748,10 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_e assert(mode < _JOB_MODE_MAX); if (mode == JOB_ISOLATE && type != JOB_START) - return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start."); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start."); if (mode == JOB_ISOLATE && !unit->allow_isolate) - return sd_bus_error_setf(e, BUS_ERROR_NO_ISOLATION, "Operation refused, unit may not be isolated."); + return sd_bus_error_setf(error, BUS_ERROR_NO_ISOLATION, "Operation refused, unit may not be isolated."); log_unit_debug(unit, "Trying to enqueue job %s/%s/%s", unit->id, job_type_to_string(type), job_mode_to_string(mode)); @@ -1755,7 +1763,7 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_e r = transaction_add_job_and_dependencies(tr, type, unit, NULL, true, false, IN_SET(mode, JOB_IGNORE_DEPENDENCIES, JOB_IGNORE_REQUIREMENTS), - mode == JOB_IGNORE_DEPENDENCIES, e); + mode == JOB_IGNORE_DEPENDENCIES, error); if (r < 0) goto tr_abort; @@ -1765,7 +1773,7 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_e goto tr_abort; } - r = transaction_activate(tr, m, mode, e); + r = transaction_activate(tr, m, mode, affected_jobs, error); if (r < 0) goto tr_abort; @@ -1773,8 +1781,8 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_e "Enqueued job %s/%s as %u", unit->id, job_type_to_string(type), (unsigned) tr->anchor_job->id); - if (_ret) - *_ret = tr->anchor_job; + if (ret) + *ret = tr->anchor_job; transaction_free(tr); return 0; @@ -1785,7 +1793,7 @@ tr_abort: return r; } -int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, sd_bus_error *e, Job **ret) { +int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **ret) { Unit *unit = NULL; /* just to appease gcc, initialization is not really necessary */ int r; @@ -1799,10 +1807,10 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode return r; assert(unit); - return manager_add_job(m, type, unit, mode, e, ret); + return manager_add_job(m, type, unit, mode, affected_jobs, e, ret); } -int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Job **ret) { +int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; @@ -1811,7 +1819,7 @@ int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, assert(name); assert(mode < _JOB_MODE_MAX); - r = manager_add_job_by_name(m, type, name, mode, &error, ret); + r = manager_add_job_by_name(m, type, name, mode, affected_jobs, &error, ret); if (r < 0) return log_warning_errno(r, "Failed to enqueue %s job for %s: %s", job_mode_to_string(mode), name, bus_error_message(&error, r)); @@ -1839,7 +1847,7 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error /* Failure in adding individual dependencies is ignored, so this always succeeds. */ transaction_add_propagate_reload_jobs(tr, unit, tr->anchor_job, mode == JOB_IGNORE_DEPENDENCIES, e); - r = transaction_activate(tr, m, mode, e); + r = transaction_activate(tr, m, mode, NULL, e); if (r < 0) goto tr_abort; @@ -2549,7 +2557,7 @@ static void manager_start_target(Manager *m, const char *name, JobMode mode) { log_debug("Activating special unit %s", name); - r = manager_add_job_by_name(m, JOB_START, name, mode, &error, NULL); + r = manager_add_job_by_name(m, JOB_START, name, mode, NULL, &error, NULL); if (r < 0) log_error("Failed to enqueue %s job: %s", name, bus_error_message(&error, r)); } diff --git a/src/core/manager.h b/src/core/manager.h index c43c587a79..8c7bd7e231 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -425,9 +425,9 @@ int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_err int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret); int manager_load_unit_from_dbus_path(Manager *m, const char *s, sd_bus_error *e, Unit **_u); -int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_error *e, Job **_ret); -int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, sd_bus_error *e, Job **_ret); -int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Job **ret); +int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret); +int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **_ret); +int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, Job **ret); int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error *e); void manager_dump_units(Manager *s, FILE *f, const char *prefix); diff --git a/src/core/path.c b/src/core/path.c index 3fe14ef2bc..5a00150e9b 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -474,7 +474,7 @@ static void path_enter_running(Path *p) { return; } - r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); if (r < 0) goto fail; diff --git a/src/core/service.c b/src/core/service.c index 89029b6d11..d05d7c6691 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2181,7 +2181,7 @@ static void service_enter_restart(Service *s) { * restarted. We use JOB_RESTART (instead of the more obvious * JOB_START) here so that those dependency jobs will be added * as well. */ - r = manager_add_job(UNIT(s)->manager, JOB_RESTART, UNIT(s), JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(s)->manager, JOB_RESTART, UNIT(s), JOB_REPLACE, NULL, &error, NULL); if (r < 0) goto fail; diff --git a/src/core/socket.c b/src/core/socket.c index 1396842b26..95976ed44d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2310,7 +2310,7 @@ static void socket_enter_running(Socket *s, int cfd) { goto fail; } - r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, NULL, &error, NULL); if (r < 0) goto fail; } @@ -2385,7 +2385,7 @@ static void socket_enter_running(Socket *s, int cfd) { service->peer = TAKE_PTR(p); /* Pass ownership of the peer reference */ - r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(service), JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(service), JOB_REPLACE, NULL, &error, NULL); if (r < 0) { /* We failed to activate the new service, but it still exists. Let's make sure the service * closes and forgets the connection fd again, immediately. */ diff --git a/src/core/timer.c b/src/core/timer.c index d1e351c30d..acf4417f4c 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -575,7 +575,7 @@ static void timer_enter_running(Timer *t) { return; } - r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, &error, NULL); + r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, NULL); if (r < 0) goto fail; diff --git a/src/core/transaction.c b/src/core/transaction.c index 2418332b9a..e0ba3c845f 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -589,7 +589,12 @@ rescan: } } -static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { +static int transaction_apply( + Transaction *tr, + Manager *m, + JobMode mode, + Set *affected_jobs) { + Iterator i; Job *j; int r; @@ -646,6 +651,11 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) { job_add_to_dbus_queue(j); job_start_timer(j, false); job_shutdown_magic(j); + + /* When 'affected' is specified, let's track all in it all jobs that were touched because of + * this transaction. */ + if (affected_jobs) + (void) set_put(affected_jobs, j); } return 0; @@ -658,7 +668,13 @@ rollback: return r; } -int transaction_activate(Transaction *tr, Manager *m, JobMode mode, sd_bus_error *e) { +int transaction_activate( + Transaction *tr, + Manager *m, + JobMode mode, + Set *affected_jobs, + sd_bus_error *e) { + Iterator i; Job *j; int r; @@ -735,7 +751,7 @@ int transaction_activate(Transaction *tr, Manager *m, JobMode mode, sd_bus_error return log_notice_errno(r, "Requested transaction contradicts existing jobs: %s", bus_error_message(e, r)); /* Tenth step: apply changes */ - r = transaction_apply(tr, m, mode); + r = transaction_apply(tr, m, mode, affected_jobs); if (r < 0) return log_warning_errno(r, "Failed to apply transaction: %m"); diff --git a/src/core/transaction.h b/src/core/transaction.h index 70d74a4ccb..4b5620f5c8 100644 --- a/src/core/transaction.h +++ b/src/core/transaction.h @@ -29,6 +29,6 @@ int transaction_add_job_and_dependencies( bool ignore_requirements, bool ignore_order, sd_bus_error *e); -int transaction_activate(Transaction *tr, Manager *m, JobMode mode, sd_bus_error *e); +int transaction_activate(Transaction *tr, Manager *m, JobMode mode, Set *affected, sd_bus_error *e); int transaction_add_isolate_jobs(Transaction *tr, Manager *m); void transaction_abort(Transaction *tr); diff --git a/src/core/unit.c b/src/core/unit.c index 2d7cb1457c..35c268cd3b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2035,7 +2035,7 @@ static void unit_check_binds_to(Unit *u) { log_unit_info(u, "Unit is bound to inactive unit %s. Stopping, too.", other->id); /* A unit we need to run is gone. Sniff. Let's stop this. */ - r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, NULL); + r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, NULL, &error, NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r)); } @@ -2051,25 +2051,25 @@ static void retroactively_start_dependencies(Unit *u) { HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUIRES], i) if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL); + manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL, NULL); HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], i) if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL); + manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL, NULL); HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_WANTS], i) if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_START, other, JOB_FAIL, NULL, NULL); + manager_add_job(u->manager, JOB_START, other, JOB_FAIL, NULL, NULL, NULL); HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_CONFLICTS], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); + manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_CONFLICTED_BY], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); + manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); } static void retroactively_stop_dependencies(Unit *u) { @@ -2083,7 +2083,7 @@ static void retroactively_stop_dependencies(Unit *u) { /* Pull down units which are bound to us recursively if enabled */ HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BOUND_BY], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) - manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); + manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL, NULL); } void unit_start_on_failure(Unit *u) { @@ -2102,7 +2102,7 @@ void unit_start_on_failure(Unit *u) { HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_ON_FAILURE], i) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - r = manager_add_job(u->manager, JOB_START, other, u->on_failure_job_mode, &error, NULL); + r = manager_add_job(u->manager, JOB_START, other, u->on_failure_job_mode, NULL, &error, NULL); if (r < 0) log_unit_warning_errno(u, r, "Failed to enqueue OnFailure= job, ignoring: %s", bus_error_message(&error, r)); } diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 0673d36b62..701594fe53 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -40,7 +40,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, "\t"); printf("Test1: (Trivial)\n"); - r = manager_add_job(m, JOB_START, c, JOB_REPLACE, &err, &j); + r = manager_add_job(m, JOB_START, c, JOB_REPLACE, NULL, &err, &j); if (sd_bus_error_is_set(&err)) log_error("error: %s: %s", err.name, err.message); assert_se(r == 0); @@ -53,15 +53,15 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, "\t"); printf("Test2: (Cyclic Order, Unfixable)\n"); - assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, NULL, NULL, &j) == -EDEADLK); manager_dump_jobs(m, stdout, "\t"); printf("Test3: (Cyclic Order, Fixable, Garbage Collector)\n"); - assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); printf("Test4: (Identical transaction)\n"); - assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); printf("Load3:\n"); @@ -69,21 +69,21 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, "\t"); printf("Test5: (Colliding transaction, fail)\n"); - assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK); printf("Test6: (Colliding transaction, replace)\n"); - assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); printf("Test7: (Unmergeable job type, fail)\n"); - assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, &j) == -EDEADLK); + assert_se(manager_add_job(m, JOB_STOP, g, JOB_FAIL, NULL, NULL, &j) == -EDEADLK); printf("Test8: (Mergeable job type, fail)\n"); - assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_RESTART, g, JOB_FAIL, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); printf("Test9: (Unmergeable job type, replace)\n"); - assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_STOP, g, JOB_REPLACE, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); printf("Load4:\n"); @@ -91,7 +91,7 @@ int main(int argc, char *argv[]) { manager_dump_units(m, stdout, "\t"); printf("Test10: (Unmergeable job type of auxiliary job, fail)\n"); - assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, &j) == 0); + assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], b)); From c45e5fb877033c9e3f9b79121644ed71032af379 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 15:20:26 +0100 Subject: [PATCH 2/8] systemctl: replace switch statement by table of structures --- src/systemctl/systemctl.c | 68 ++++++++++++--------------------------- 1 file changed, 21 insertions(+), 47 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d162ddee5a..232d93fb02 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3279,64 +3279,38 @@ static int logind_set_wall_message(void) { } #endif -/* Ask systemd-logind, which might grant access to unprivileged users - * through polkit */ +/* Ask systemd-logind, which might grant access to unprivileged users through polkit */ static int logind_reboot(enum action a) { #if ENABLE_LOGIND + static const struct { + const char *method; + const char *description; + } actions[_ACTION_MAX] = { + [ACTION_POWEROFF] = { "PowerOff", "power off system" }, + [ACTION_REBOOT] = { "Reboot", "reboot system" }, + [ACTION_HALT] = { "Halt", "halt system" }, + [ACTION_SUSPEND] = { "Suspend", "suspend system" }, + [ACTION_HIBERNATE] = { "Hibernate", "hibernate system" }, + [ACTION_HYBRID_SLEEP] = { "HybridSleep", "put system into hybrid sleep" }, + [ACTION_SUSPEND_THEN_HIBERNATE] = { "SuspendThenHibernate", "suspend system, hibernate later" }, + }; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - const char *method, *description; sd_bus *bus; int r; + if (a < 0 || a >= _ACTION_MAX || !actions[a].method) + return -EINVAL; + r = acquire_bus(BUS_FULL, &bus); if (r < 0) return r; - switch (a) { - - case ACTION_POWEROFF: - method = "PowerOff"; - description = "power off system"; - break; - - case ACTION_REBOOT: - method = "Reboot"; - description = "reboot system"; - break; - - case ACTION_HALT: - method = "Halt"; - description = "halt system"; - break; - - case ACTION_SUSPEND: - method = "Suspend"; - description = "suspend system"; - break; - - case ACTION_HIBERNATE: - method = "Hibernate"; - description = "hibernate system"; - break; - - case ACTION_HYBRID_SLEEP: - method = "HybridSleep"; - description = "put system into hybrid sleep"; - break; - - case ACTION_SUSPEND_THEN_HIBERNATE: - method = "SuspendThenHibernate"; - description = "put system into suspend followed by hibernate"; - break; - - default: - return -EINVAL; - } - polkit_agent_open_maybe(); (void) logind_set_wall_message(); - log_debug("%s org.freedesktop.login1.Manager %s dbus call.", arg_dry_run ? "Would execute" : "Executing", method); + log_debug("%s org.freedesktop.login1.Manager %s dbus call.", arg_dry_run ? "Would execute" : "Executing", actions[a].method); + if (arg_dry_run) return 0; @@ -3345,12 +3319,12 @@ static int logind_reboot(enum action a) { "org.freedesktop.login1", "/org/freedesktop/login1", "org.freedesktop.login1.Manager", - method, + actions[a].method, &error, NULL, "b", arg_ask_password); if (r < 0) - return log_error_errno(r, "Failed to %s via logind: %s", description, bus_error_message(&error, r)); + return log_error_errno(r, "Failed to %s via logind: %s", actions[a].description, bus_error_message(&error, r)); return 0; #else From ee4296496ffa27e344b7f79356e56b0e5be841aa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 15:25:48 +0100 Subject: [PATCH 3/8] systemctl: more SYNTHETIC_ERRNO() conversion --- src/systemctl/systemctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 232d93fb02..15d431242f 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3545,8 +3545,8 @@ static int prepare_boot_loader_entry(void) { return 0; #else - log_error("Booting into boot loader entry not supported."); - return -ENOSYS; + return log_error_errno(SYNTHETIC_ERRNO(ENOSYS), + "Booting into boot loader entry not supported."); #endif } From 5fd77930ad9980af5257f9f871556d6973db736c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 15:49:52 +0100 Subject: [PATCH 4/8] systemctl: reindent table --- src/systemctl/systemctl.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 15d431242f..7fafe13515 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3072,21 +3072,21 @@ static const struct { const char *verb; const char *mode; } action_table[_ACTION_MAX] = { - [ACTION_HALT] = { SPECIAL_HALT_TARGET, "halt", "replace-irreversibly" }, - [ACTION_POWEROFF] = { SPECIAL_POWEROFF_TARGET, "poweroff", "replace-irreversibly" }, - [ACTION_REBOOT] = { SPECIAL_REBOOT_TARGET, "reboot", "replace-irreversibly" }, - [ACTION_KEXEC] = { SPECIAL_KEXEC_TARGET, "kexec", "replace-irreversibly" }, - [ACTION_RUNLEVEL2] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, - [ACTION_RUNLEVEL3] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, - [ACTION_RUNLEVEL4] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, - [ACTION_RUNLEVEL5] = { SPECIAL_GRAPHICAL_TARGET, NULL, "isolate" }, - [ACTION_RESCUE] = { SPECIAL_RESCUE_TARGET, "rescue", "isolate" }, - [ACTION_EMERGENCY] = { SPECIAL_EMERGENCY_TARGET, "emergency", "isolate" }, - [ACTION_DEFAULT] = { SPECIAL_DEFAULT_TARGET, "default", "isolate" }, - [ACTION_EXIT] = { SPECIAL_EXIT_TARGET, "exit", "replace-irreversibly" }, - [ACTION_SUSPEND] = { SPECIAL_SUSPEND_TARGET, "suspend", "replace-irreversibly" }, - [ACTION_HIBERNATE] = { SPECIAL_HIBERNATE_TARGET, "hibernate", "replace-irreversibly" }, - [ACTION_HYBRID_SLEEP] = { SPECIAL_HYBRID_SLEEP_TARGET, "hybrid-sleep", "replace-irreversibly" }, + [ACTION_HALT] = { SPECIAL_HALT_TARGET, "halt", "replace-irreversibly" }, + [ACTION_POWEROFF] = { SPECIAL_POWEROFF_TARGET, "poweroff", "replace-irreversibly" }, + [ACTION_REBOOT] = { SPECIAL_REBOOT_TARGET, "reboot", "replace-irreversibly" }, + [ACTION_KEXEC] = { SPECIAL_KEXEC_TARGET, "kexec", "replace-irreversibly" }, + [ACTION_RUNLEVEL2] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, + [ACTION_RUNLEVEL3] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, + [ACTION_RUNLEVEL4] = { SPECIAL_MULTI_USER_TARGET, NULL, "isolate" }, + [ACTION_RUNLEVEL5] = { SPECIAL_GRAPHICAL_TARGET, NULL, "isolate" }, + [ACTION_RESCUE] = { SPECIAL_RESCUE_TARGET, "rescue", "isolate" }, + [ACTION_EMERGENCY] = { SPECIAL_EMERGENCY_TARGET, "emergency", "isolate" }, + [ACTION_DEFAULT] = { SPECIAL_DEFAULT_TARGET, "default", "isolate" }, + [ACTION_EXIT] = { SPECIAL_EXIT_TARGET, "exit", "replace-irreversibly" }, + [ACTION_SUSPEND] = { SPECIAL_SUSPEND_TARGET, "suspend", "replace-irreversibly" }, + [ACTION_HIBERNATE] = { SPECIAL_HIBERNATE_TARGET, "hibernate", "replace-irreversibly" }, + [ACTION_HYBRID_SLEEP] = { SPECIAL_HYBRID_SLEEP_TARGET, "hybrid-sleep", "replace-irreversibly" }, [ACTION_SUSPEND_THEN_HIBERNATE] = { SPECIAL_SUSPEND_THEN_HIBERNATE_TARGET, "suspend-then-hibernate", "replace-irreversibly" }, }; From 94369fc0663255bbd327f97dba288ececf51a514 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 16:19:35 +0100 Subject: [PATCH 5/8] systemctl: split out extra args generation into helper function of its own --- src/systemctl/systemctl.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 7fafe13515..cd16c855e4 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3100,6 +3100,25 @@ static enum action verb_to_action(const char *verb) { return _ACTION_INVALID; } +static const char** make_extra_args(const char *extra_args[static 4]) { + size_t n = 0; + + if (arg_scope != UNIT_FILE_SYSTEM) + extra_args[n++] = "--user"; + + if (arg_transport == BUS_TRANSPORT_REMOTE) { + extra_args[n++] = "-H"; + extra_args[n++] = arg_host; + } else if (arg_transport == BUS_TRANSPORT_MACHINE) { + extra_args[n++] = "-M"; + extra_args[n++] = arg_host; + } else + assert(arg_transport == BUS_TRANSPORT_LOCAL); + + extra_args[n] = NULL; + return extra_args; +} + static int start_unit(int argc, char *argv[], void *userdata) { _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; _cleanup_(wait_context_free) WaitContext wait_context = {}; @@ -3205,22 +3224,9 @@ static int start_unit(int argc, char *argv[], void *userdata) { } if (!arg_no_block) { - const char* extra_args[4] = {}; - int arg_count = 0; + const char* extra_args[4]; - if (arg_scope != UNIT_FILE_SYSTEM) - extra_args[arg_count++] = "--user"; - - assert(IN_SET(arg_transport, BUS_TRANSPORT_LOCAL, BUS_TRANSPORT_REMOTE, BUS_TRANSPORT_MACHINE)); - if (arg_transport == BUS_TRANSPORT_REMOTE) { - extra_args[arg_count++] = "-H"; - extra_args[arg_count++] = arg_host; - } else if (arg_transport == BUS_TRANSPORT_MACHINE) { - extra_args[arg_count++] = "-M"; - extra_args[arg_count++] = arg_host; - } - - r = bus_wait_for_jobs(w, arg_quiet, extra_args); + r = bus_wait_for_jobs(w, arg_quiet, make_extra_args(extra_args)); if (r < 0) return r; From 85d9b5981ba6b7ee3955f95fa6cf3bb8cdf3444d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Mar 2019 20:58:13 +0100 Subject: [PATCH 6/8] systemctl: add new --show-transaction switch This new switch uses the new method call EnqueueUnitJob() for enqueuing a job and showing the jobs it enqueued. Fixes: #2297 --- src/systemctl/systemctl.c | 182 ++++++++++++++++++++++++++------------ 1 file changed, 127 insertions(+), 55 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index cd16c855e4..8c8e496da5 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -114,6 +114,7 @@ static bool arg_dry_run = false; static bool arg_quiet = false; static bool arg_full = false; static bool arg_recursive = false; +static bool arg_show_transaction = false; static int arg_force = 0; static bool arg_ask_password = false; static bool arg_runtime = false; @@ -728,7 +729,6 @@ static int get_unit_list_recursive( *_machines = NULL; *_unit_infos = TAKE_PTR(unit_infos); - *_replies = TAKE_PTR(replies); return c; @@ -2741,25 +2741,26 @@ static int check_triggering_units(sd_bus *bus, const char *name) { } static const struct { - const char *verb; - const char *method; + const char *verb; /* systemctl verb */ + const char *method; /* Name of the specific D-Bus method */ + const char *job_type; /* Job type when passing to the generic EnqueueUnitJob() method */ } unit_actions[] = { - { "start", "StartUnit" }, - { "stop", "StopUnit" }, - { "condstop", "StopUnit" }, - { "reload", "ReloadUnit" }, - { "restart", "RestartUnit" }, - { "try-restart", "TryRestartUnit" }, - { "condrestart", "TryRestartUnit" }, - { "reload-or-restart", "ReloadOrRestartUnit" }, - { "try-reload-or-restart", "ReloadOrTryRestartUnit" }, - { "reload-or-try-restart", "ReloadOrTryRestartUnit" }, - { "condreload", "ReloadOrTryRestartUnit" }, - { "force-reload", "ReloadOrTryRestartUnit" } + { "start", "StartUnit", "start" }, + { "stop", "StopUnit", "stop" }, + { "condstop", "StopUnit", "stop" }, /* legacy alias */ + { "reload", "ReloadUnit", "reload" }, + { "restart", "RestartUnit", "restart" }, + { "try-restart", "TryRestartUnit", "try-restart" }, + { "condrestart", "TryRestartUnit", "try-restart" }, /* legacy alias */ + { "reload-or-restart", "ReloadOrRestartUnit", "reload-or-restart" }, + { "try-reload-or-restart", "ReloadOrTryRestartUnit", "reload-or-try-restart" }, + { "reload-or-try-restart", "ReloadOrTryRestartUnit", "reload-or-try-restart" }, /* legacy alias */ + { "condreload", "ReloadOrTryRestartUnit", "reload-or-try-restart" }, /* legacy alias */ + { "force-reload", "ReloadOrTryRestartUnit", "reload-or-try-restart" }, /* legacy alias */ }; static const char *verb_to_method(const char *verb) { - uint i; + size_t i; for (i = 0; i < ELEMENTSOF(unit_actions); i++) if (streq_ptr(unit_actions[i].verb, verb)) @@ -2768,14 +2769,14 @@ static const char *verb_to_method(const char *verb) { return "StartUnit"; } -static const char *method_to_verb(const char *method) { - uint i; +static const char *verb_to_job_type(const char *verb) { + size_t i; for (i = 0; i < ELEMENTSOF(unit_actions); i++) - if (streq_ptr(unit_actions[i].method, method)) - return unit_actions[i].verb; + if (streq_ptr(unit_actions[i].verb, verb)) + return unit_actions[i].job_type; - return "n/a"; + return "start"; } typedef struct { @@ -2932,7 +2933,8 @@ static int wait_context_watch( static int start_unit_one( sd_bus *bus, - const char *method, + const char *method, /* When using classic per-job bus methods */ + const char *job_type, /* When using new-style EnqueueUnitJob() */ const char *name, const char *mode, sd_bus_error *error, @@ -2941,6 +2943,7 @@ static int start_unit_one( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; const char *path; + bool done = false; int r; assert(method); @@ -2957,44 +2960,80 @@ static int start_unit_one( log_debug("%s dbus call org.freedesktop.systemd1.Manager %s(%s, %s)", arg_dry_run ? "Would execute" : "Executing", method, name, mode); + if (arg_dry_run) return 0; - r = sd_bus_call_method( - bus, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - method, - error, - &reply, - "ss", name, mode); - if (r < 0) { - const char *verb; + if (arg_show_transaction) { + _cleanup_(sd_bus_error_free) sd_bus_error enqueue_error = SD_BUS_ERROR_NULL; - /* There's always a fallback possible for legacy actions. */ - if (arg_action != ACTION_SYSTEMCTL) - return r; + /* Use the new, fancy EnqueueUnitJob() API if the user wants us to print the transaction */ + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "EnqueueUnitJob", + &enqueue_error, + &reply, + "sss", + name, job_type, mode); + if (r < 0) { + if (!sd_bus_error_has_name(&enqueue_error, SD_BUS_ERROR_UNKNOWN_METHOD)) { + (void) sd_bus_error_move(error, &enqueue_error); + goto fail; + } - verb = method_to_verb(method); + /* Hmm, the API is not yet available. Let's use the classic API instead (see below). */ + log_notice("--show-transaction not supported by this service manager, proceeding without."); + } else { + const char *u, *jt; + uint32_t id; - log_error("Failed to %s %s: %s", verb, name, bus_error_message(error, r)); + r = sd_bus_message_read(reply, "uosos", &id, &path, &u, NULL, &jt); + if (r < 0) + return bus_log_parse_error(r); - if (!sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) && - !sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED) && - !sd_bus_error_has_name(error, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE)) - log_error("See %s logs and 'systemctl%s status%s %s' for details.", - arg_scope == UNIT_FILE_SYSTEM ? "system" : "user", - arg_scope == UNIT_FILE_SYSTEM ? "" : " --user", - name[0] == '-' ? " --" : "", - name); + log_info("Enqueued anchor job %" PRIu32 " %s/%s.", id, u, jt); - return r; + r = sd_bus_message_enter_container(reply, 'a', "(uosos)"); + if (r < 0) + return bus_log_parse_error(r); + for (;;) { + r = sd_bus_message_read(reply, "(uosos)", &id, NULL, &u, NULL, &jt); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; + + log_info("Enqueued auxiliary job %" PRIu32 " %s/%s.", id, u, jt); + } + + r = sd_bus_message_exit_container(reply); + if (r < 0) + return bus_log_parse_error(r); + + done = true; + } } - r = sd_bus_message_read(reply, "o", &path); - if (r < 0) - return bus_log_parse_error(r); + if (!done) { + r = sd_bus_call_method( + bus, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + method, + error, + &reply, + "ss", name, mode); + if (r < 0) + goto fail; + + r = sd_bus_message_read(reply, "o", &path); + if (r < 0) + return bus_log_parse_error(r); + } if (need_daemon_reload(bus, name) > 0) warn_unit_file_changed(name); @@ -3007,6 +3046,24 @@ static int start_unit_one( } return 0; + +fail: + /* There's always a fallback possible for legacy actions. */ + if (arg_action != ACTION_SYSTEMCTL) + return r; + + log_error_errno(r, "Failed to %s %s: %s", job_type, name, bus_error_message(error, r)); + + if (!sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) && + !sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED) && + !sd_bus_error_has_name(error, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE)) + log_error("See %s logs and 'systemctl%s status%s %s' for details.", + arg_scope == UNIT_FILE_SYSTEM ? "system" : "user", + arg_scope == UNIT_FILE_SYSTEM ? "" : " --user", + name[0] == '-' ? " --" : "", + name); + + return r; } static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***ret) { @@ -3063,7 +3120,6 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r } *ret = TAKE_PTR(mangled); - return 0; } @@ -3122,7 +3178,7 @@ static const char** make_extra_args(const char *extra_args[static 4]) { static int start_unit(int argc, char *argv[], void *userdata) { _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; _cleanup_(wait_context_free) WaitContext wait_context = {}; - const char *method, *mode, *one_name, *suffix = 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 **names = NULL; int r, ret = EXIT_SUCCESS; @@ -3148,27 +3204,34 @@ static int start_unit(int argc, char *argv[], void *userdata) { action = verb_to_action(argv[0]); if (action != _ACTION_INVALID) { + /* A command in style "systemctl reboot", "systemctl poweroff", … */ method = "StartUnit"; + job_type = "start"; mode = action_table[action].mode; one_name = action_table[action].target; } else { if (streq(argv[0], "isolate")) { + /* A "systemctl isolate …" command */ method = "StartUnit"; + job_type = "start"; mode = "isolate"; - suffix = ".target"; } else { + /* A command in style of "systemctl start …", "sysemctl stop …" and so on */ method = verb_to_method(argv[0]); + job_type = verb_to_job_type(argv[0]); mode = arg_job_mode; } one_name = NULL; } } else { + /* A SysV legacy command such as "halt", "reboot", "poweroff", … */ assert(arg_action >= 0 && arg_action < _ACTION_MAX); assert(action_table[arg_action].target); assert(action_table[arg_action].mode); method = "StartUnit"; + job_type = "start"; mode = action_table[arg_action].mode; one_name = action_table[arg_action].target; } @@ -3201,9 +3264,11 @@ static int start_unit(int argc, char *argv[], void *userdata) { NULL); if (r < 0) return log_error_errno(r, "Failed to enable subscription: %m"); + r = sd_event_default(&wait_context.event); if (r < 0) return log_error_errno(r, "Failed to allocate event loop: %m"); + r = sd_bus_attach_event(bus, wait_context.event, 0); if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop: %m"); @@ -3212,7 +3277,7 @@ static int start_unit(int argc, char *argv[], void *userdata) { STRV_FOREACH(name, names) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - r = start_unit_one(bus, method, *name, mode, &error, w, arg_wait ? &wait_context : NULL); + r = start_unit_one(bus, method, job_type, *name, mode, &error, w, arg_wait ? &wait_context : NULL); if (ret == EXIT_SUCCESS && r < 0) ret = translate_bus_error_to_exit_status(r, &error); @@ -7534,6 +7599,8 @@ static int systemctl_help(void) { " --reverse Show reverse dependencies with 'list-dependencies'\n" " --job-mode=MODE Specify how to deal with already queued jobs, when\n" " queueing a new job\n" + " -T --show-transaction\n" + " When enqueuing a unit job, show full transaction\n" " --show-types When showing sockets, explicitly show their type\n" " --value When showing properties, only print the value\n" " -i --ignore-inhibitors\n" @@ -7941,6 +8008,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { { "boot-loader-entry", required_argument, NULL, ARG_BOOT_LOADER_ENTRY }, { "now", no_argument, NULL, ARG_NOW }, { "message", required_argument, NULL, ARG_MESSAGE }, + { "show-transaction", no_argument, NULL, 'T' }, {} }; @@ -7952,7 +8020,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { /* we default to allowing interactive authorization only in systemctl (not in the legacy commands) */ arg_ask_password = true; - while ((c = getopt_long(argc, argv, "ht:p:alqfs:H:M:n:o:ir.::", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "ht:p:alqfs:H:M:n:o:iTr.::", options, NULL)) >= 0) switch (c) { @@ -8289,6 +8357,10 @@ static int systemctl_parse_argv(int argc, char *argv[]) { return log_oom(); break; + case 'T': + arg_show_transaction = true; + break; + case '.': /* Output an error mimicking getopt, and print a hint afterwards */ log_error("%s: invalid option -- '.'", program_invocation_name); From f087c7e072bb338d5c7c0781c9fbc900612efd18 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 17:39:36 +0100 Subject: [PATCH 7/8] test: add some basic testing that "systemctl start -T" does something --- test/TEST-03-JOBS/test-jobs.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/TEST-03-JOBS/test-jobs.sh b/test/TEST-03-JOBS/test-jobs.sh index e66ea53621..42190cf478 100755 --- a/test/TEST-03-JOBS/test-jobs.sh +++ b/test/TEST-03-JOBS/test-jobs.sh @@ -26,6 +26,13 @@ grep 'sleep\.service.*running' /root/list-jobs.txt grep 'hello\.service' /root/list-jobs.txt && exit 1 systemctl stop sleep.service hello-after-sleep.target +# Some basic testing that --show-transaction does something useful +! systemctl is-active systemd-importd +systemctl -T start systemd-importd +systemctl is-active systemd-importd +systemctl --show-transaction stop systemd-importd +! systemctl is-active systemd-importd + # Test for a crash when enqueuing a JOB_NOP when other job already exists systemctl start --no-block hello-after-sleep.target # hello.service should still be waiting, so these try-restarts will collapse From df4a7cb7323c8cf00553d766913312c5b7ccd508 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Mar 2019 18:02:49 +0100 Subject: [PATCH 8/8] man: document the new systemctl --show-transaction option --- man/systemctl.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/man/systemctl.xml b/man/systemctl.xml index b5b9e077ee..d991e979f1 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -299,6 +299,20 @@ + + + + + + When enqueuing a unit job (for example as effect of a systemctl start + invocation or similar), show brief information about all jobs enqueued, covering both the requested + job and any added because of unit dependencies. Note that the output will only include jobs + immediately part of the transaction requested. It is possible that service start-up program code + run as effect of the enqueued jobs might request further jobs to be pulled in. This means that + completion of the listed jobs might ultimately entail more jobs than the listed ones. + + +