From 1391f149f0eea6e9cdd1df7fd25fa2cfd6f21c2c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 1 Aug 2024 02:23:14 +0200 Subject: [PATCH] core/service: actually allow to "hurry up" auto restarts unit_start() advertises that start requests don't get suppressed, so that it could be used to manually speed up auto restarts. However, service_start() so far rejected this, stating that clients should issue restart request in order to trigger BindsTo=/OnFailure=. That seems to be a red herring though, because for a long time the service states between auto-restarts were buggy (#27594). With the introduction of RestartMode=direct, the behavior is sane again and customizable, hence I see no reason to refuse this anymore. Whether those deps are triggered solely depends on RestartMode= now. Plus, filter out some intermediate states that should never be seen in service_start(). Fixes #33890 --- src/core/service.c | 30 ++++++++++++++++++------------ test/units/TEST-03-JOBS.sh | 28 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 2c6d83babe5..49ed3b088a7 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2567,13 +2567,16 @@ fail: service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); } -static void service_enter_restart(Service *s) { +static void service_enter_restart(Service *s, bool shortcut) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; + /* shortcut: a manual start request is received, restart immediately */ + assert(s); + assert(s->state == SERVICE_AUTO_RESTART); - if (unit_has_job_type(UNIT(s), JOB_STOP)) { + if (!shortcut && unit_has_job_type(UNIT(s), JOB_STOP)) { /* Don't restart things if we are going down anyway */ log_unit_info(UNIT(s), "Stop job pending for unit, skipping automatic restart."); return; @@ -2598,7 +2601,8 @@ static void service_enter_restart(Service *s) { "MESSAGE_ID=" SD_MESSAGE_UNIT_RESTART_SCHEDULED_STR, LOG_UNIT_INVOCATION_ID(UNIT(s)), LOG_UNIT_MESSAGE(UNIT(s), - "Scheduled restart job, restart counter is at %u.", s->n_restarts), + "Scheduled restart job%s, restart counter is at %u.", + shortcut ? " immediately on client request" : "", s->n_restarts), "N_RESTARTS=%u", s->n_restarts); service_set_state(s, SERVICE_AUTO_RESTART_QUEUED); @@ -2758,11 +2762,9 @@ static void service_run_next_main(Service *s) { } static int service_start(Unit *u) { - Service *s = SERVICE(u); + Service *s = ASSERT_PTR(SERVICE(u)); int r; - assert(s); - /* We cannot fulfill this request right now, try again later * please! */ if (IN_SET(s->state, @@ -2774,13 +2776,17 @@ static int service_start(Unit *u) { if (IN_SET(s->state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST)) return 0; - /* A service that will be restarted must be stopped first to trigger BindsTo and/or OnFailure - * dependencies. If a user does not want to wait for the holdoff time to elapse, the service should - * be manually restarted, not started. We simply return EAGAIN here, so that any start jobs stay - * queued, and assume that the auto restart timer will eventually trigger the restart. */ - if (IN_SET(s->state, SERVICE_AUTO_RESTART, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART)) + if (s->state == SERVICE_AUTO_RESTART) { + /* As mentioned in unit_start(), we allow manual starts to act as "hurry up" signals + * for auto restart. We need to re-enqueue the job though, as the job type has changed + * (JOB_RESTART_DEPENDENCIES). */ + + service_enter_restart(s, /* shortcut = */ true); return -EAGAIN; + } + /* SERVICE_*_BEFORE_AUTO_RESTART are not to be expected here, as those are intermediate states + * that should never be seen outside of service_enter_dead(). */ assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_RESOURCES_PINNED, SERVICE_AUTO_RESTART_QUEUED)); r = unit_acquire_invocation_id(u); @@ -4290,7 +4296,7 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us log_unit_debug(UNIT(s), "Service has no hold-off time (RestartSec=0), scheduling restart."); - service_enter_restart(s); + service_enter_restart(s, /* shortcut = */ false); break; case SERVICE_CLEANING: diff --git a/test/units/TEST-03-JOBS.sh b/test/units/TEST-03-JOBS.sh index 115b941f926..067f2ce67de 100755 --- a/test/units/TEST-03-JOBS.sh +++ b/test/units/TEST-03-JOBS.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash # SPDX-License-Identifier: LGPL-2.1-or-later +# shellcheck disable=SC2016 + set -eux set -o pipefail @@ -166,4 +168,30 @@ assert_rc 3 systemctl --quiet is-active succeeds-on-restart.target systemctl start fails-on-restart.target || : assert_rc 3 systemctl --quiet is-active fails-on-restart.target +# Test shortcutting auto restart + +export UNIT_NAME="TEST-03-JOBS-shortcut-restart.service" +TMP_FILE="/tmp/test-03-shortcut-restart-test$RANDOM" + +cat >"/run/systemd/system/$UNIT_NAME" <