From 758cd9ddac150342dbac49ed4c80c68531c169b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Jul 2023 17:54:59 +0200 Subject: [PATCH] manager: fix reloading in reload-or-restart --marked bus_unit_queue_job_one has two callers: - bus_unit_queue_job which would do the appropriate transormations to turn JOB_TRY_RESTART into JOB_TRY_RELOAD, - and method_enqueue_marked_jobs which did not. In effect, method_enqueue_marked_jobs() would queue restart jobs for units which has Markers= needs-reload or needs-restart. When the chunk of code which does the transformations is moved from bus_unit_queue_job to bus_unit_queue_job_one, there is no change for bus_unit_queue_job, and method_enqueue_marked_jobs is fixed. The additional checks that are done seem reasonable to do from method_enqueue_marked_jobs: we shouldn't be restarting units which are configured to not allow that, or force unwanted start of dbus-broker. (cherry picked from commit 8ea8e23f4013dbc4f4a66c81eb786f0505434f2e) Resolves: RHEL-40878 --- src/core/dbus-unit.c | 82 ++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 49eacc977c..b45b3fdb53 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1736,6 +1736,47 @@ int bus_unit_queue_job_one( Job *j, *a; int r; + 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) + type = JOB_TRY_RELOAD; + } + + if (type == JOB_STOP && + IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_ERROR, UNIT_BAD_SETTING) && + unit_active_state(u) == UNIT_INACTIVE) + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not loaded.", u->id); + + if ((type == JOB_START && u->refuse_manual_start) || + (type == JOB_STOP && u->refuse_manual_stop) || + (IN_SET(type, JOB_RESTART, JOB_TRY_RESTART) && (u->refuse_manual_start || u->refuse_manual_stop)) || + (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); + + /* dbus-broker issues StartUnit for activation requests, and Type=dbus services automatically + * gain dependency on dbus.socket. Therefore, if dbus has a pending stop job, the new start + * job that pulls in dbus again would cause job type conflict. Let's avoid that by rejecting + * job enqueuing early. + * + * Note that unlike signal_activation_request(), we can't use unit_inactive_or_pending() + * here. StartUnit is a more generic interface, and thus users are allowed to use e.g. systemctl + * to start Type=dbus services even when dbus is inactive. */ + if (type == JOB_START && u->type == UNIT_SERVICE && SERVICE(u)->type == SERVICE_DBUS) + FOREACH_STRING(dbus_unit, SPECIAL_DBUS_SOCKET, SPECIAL_DBUS_SERVICE) { + Unit *dbus; + + dbus = manager_get_unit(u->manager, dbus_unit); + if (dbus && unit_stop_pending(dbus)) + return sd_bus_error_setf(error, + BUS_ERROR_SHUTTING_DOWN, + "Operation for unit %s refused, D-Bus is shutting down.", + u->id); + } + if (FLAGS_SET(flags, BUS_UNIT_QUEUE_VERBOSE_REPLY)) { affected = set_new(NULL); if (!affected) @@ -1828,47 +1869,6 @@ int bus_unit_queue_job( if (r < 0) return r; - 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) - type = JOB_TRY_RELOAD; - } - - if (type == JOB_STOP && - IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_ERROR, UNIT_BAD_SETTING) && - unit_active_state(u) == UNIT_INACTIVE) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not loaded.", u->id); - - if ((type == JOB_START && u->refuse_manual_start) || - (type == JOB_STOP && u->refuse_manual_stop) || - (IN_SET(type, JOB_RESTART, JOB_TRY_RESTART) && (u->refuse_manual_start || u->refuse_manual_stop)) || - (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); - - /* dbus-broker issues StartUnit for activation requests, and Type=dbus services automatically - * gain dependency on dbus.socket. Therefore, if dbus has a pending stop job, the new start - * job that pulls in dbus again would cause job type conflict. Let's avoid that by rejecting - * job enqueuing early. - * - * Note that unlike signal_activation_request(), we can't use unit_inactive_or_pending() - * here. StartUnit is a more generic interface, and thus users are allowed to use e.g. systemctl - * to start Type=dbus services even when dbus is inactive. */ - if (type == JOB_START && u->type == UNIT_SERVICE && SERVICE(u)->type == SERVICE_DBUS) - FOREACH_STRING(dbus_unit, SPECIAL_DBUS_SOCKET, SPECIAL_DBUS_SERVICE) { - Unit *dbus; - - dbus = manager_get_unit(u->manager, dbus_unit); - if (dbus && unit_stop_pending(dbus)) - return sd_bus_error_setf(error, - BUS_ERROR_SHUTTING_DOWN, - "Operation for unit %s refused, D-Bus is shutting down.", - u->id); - } - r = sd_bus_message_new_method_return(message, &reply); if (r < 0) return r;