From 3282591dc30b2934a895c7403d2f0b0690260947 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 18:48:42 +0100 Subject: core: make sure "systemctl reload-or-try-restart is actually a noop if a unit is not running This makes sure we follow the same basic logic for try-restart if we have a try-reload. Fixes #688 --- src/core/dbus-unit.c | 2 +- src/core/job.c | 8 ++++++++ src/core/job.h | 3 +++ src/core/unit.c | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index e4d2c08972..386ea96d1b 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -987,7 +987,7 @@ int bus_unit_queue_job( if (type == JOB_RESTART) type = JOB_RELOAD_OR_START; else if (type == JOB_TRY_RESTART) - type = JOB_RELOAD; + type = JOB_TRY_RELOAD; } r = mac_selinux_unit_access_check( diff --git a/src/core/job.c b/src/core/job.c index 274c554da9..4e111ffb46 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -405,6 +405,13 @@ JobType job_type_collapse(JobType t, Unit *u) { return JOB_RESTART; + case JOB_TRY_RELOAD: + s = unit_active_state(u); + if (UNIT_IS_INACTIVE_OR_DEACTIVATING(s)) + return JOB_NOP; + + return JOB_RELOAD; + case JOB_RELOAD_OR_START: s = unit_active_state(u); if (UNIT_IS_INACTIVE_OR_DEACTIVATING(s)) @@ -1202,6 +1209,7 @@ static const char* const job_type_table[_JOB_TYPE_MAX] = { [JOB_RELOAD_OR_START] = "reload-or-start", [JOB_RESTART] = "restart", [JOB_TRY_RESTART] = "try-restart", + [JOB_TRY_RELOAD] = "try-reload", [JOB_NOP] = "nop", }; diff --git a/src/core/job.h b/src/core/job.h index 118b24e5b7..52866fdc48 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -66,6 +66,9 @@ enum JobType { * Thus we never need to merge it with anything. */ JOB_TRY_RESTART = _JOB_TYPE_MAX_IN_TRANSACTION, /* if running, stop and then start */ + /* Similar to JOB_TRY_RESTART but collapses to JOB_RELOAD or JOB_NOP */ + JOB_TRY_RELOAD, + /* JOB_RELOAD_OR_START won't enter into a transaction and cannot result * from transaction merging (there's no way for JOB_RELOAD and * JOB_START to meet in one transaction). It can result from a merge diff --git a/src/core/unit.c b/src/core/unit.c index 32267d95f5..b6fbf4e785 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1894,6 +1894,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su case JOB_RELOAD: case JOB_RELOAD_OR_START: + case JOB_TRY_RELOAD: if (u->job->state == JOB_RUNNING) { if (ns == UNIT_ACTIVE) @@ -2107,6 +2108,7 @@ bool unit_job_is_applicable(Unit *u, JobType j) { return unit_can_start(u); case JOB_RELOAD: + case JOB_TRY_RELOAD: return unit_can_reload(u); case JOB_RELOAD_OR_START: -- cgit v1.2.3-54-g00ecf From f0469b8c4abbeee9ca69678245cd08314adc24c0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 18:49:59 +0100 Subject: core: when determining system state, don't bother with JOB_TRY_RESTART When we determine the current system state we check whether units like emergency.target are running or a job that results in them being run is queued. However, this is not the case for JOB_TRY_RESTART, since that's a NOP if the unit has not been running before. Hence, don't bother with checking for that job type. --- src/core/manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/manager.c b/src/core/manager.c index a83a8b013a..e8fea376ff 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3097,18 +3097,18 @@ ManagerState manager_state(Manager *m) { /* Is the special shutdown target queued? If so, we are in shutdown state */ u = manager_get_unit(m, SPECIAL_SHUTDOWN_TARGET); - if (u && u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_TRY_RESTART, JOB_RELOAD_OR_START)) + if (u && u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)) return MANAGER_STOPPING; /* Are the rescue or emergency targets active or queued? If so we are in maintenance state */ u = manager_get_unit(m, SPECIAL_RESCUE_TARGET); if (u && (UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)) || - (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_TRY_RESTART, JOB_RELOAD_OR_START)))) + (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)))) return MANAGER_MAINTENANCE; u = manager_get_unit(m, SPECIAL_EMERGENCY_TARGET); if (u && (UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)) || - (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_TRY_RESTART, JOB_RELOAD_OR_START)))) + (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)))) return MANAGER_MAINTENANCE; /* Are there any failed units? If so, we are in degraded mode */ -- cgit v1.2.3-54-g00ecf From 75a77a6ba4dbccd3e8d002f5efd3f714e1b49de2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 18:51:42 +0100 Subject: core: when propagating reload jobs, downgrade them to try-reload Otherwise we might end up generating jobs that fail immediately. This follows the same logic that restart propagation follows. --- src/core/transaction.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/transaction.c b/src/core/transaction.c index 8b0ed74643..0d53e4bac0 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -1010,7 +1010,13 @@ int transaction_add_job_and_dependencies( if (type == JOB_RELOAD) { SET_FOREACH(dep, ret->unit->dependencies[UNIT_PROPAGATES_RELOAD_TO], i) { - r = transaction_add_job_and_dependencies(tr, JOB_RELOAD, dep, ret, false, false, false, ignore_order, e); + JobType nt; + + nt = job_type_collapse(JOB_TRY_RELOAD, dep); + if (nt == JOB_NOP) + continue; + + r = transaction_add_job_and_dependencies(tr, nt, dep, ret, false, false, false, ignore_order, e); if (r < 0) { log_unit_warning(dep, "Cannot add dependency reload job, ignoring: %s", -- cgit v1.2.3-54-g00ecf From 42f729c15e880fa2cff225ff9937c475e10493d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 18:53:34 +0100 Subject: systemctl: improve message when a job fails with a JOB_INVALID state This result can only happen if the job was a reload job for an inactive unit. Make the error message actually say that. --- src/shared/bus-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index b9a8ee4074..63fd9b9514 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -2100,7 +2100,7 @@ static int check_wait_response(BusWaitForJobs *d, bool quiet, const char *extra_ else if (streq(d->result, "dependency")) log_error("A dependency job for %s failed. See 'journalctl -xe' for details.", strna(d->name)); else if (streq(d->result, "invalid")) - log_error("Job for %s invalid.", strna(d->name)); + log_error("%s is not active, cannot reload.", strna(d->name)); else if (streq(d->result, "assert")) log_error("Assertion failed on job for %s.", strna(d->name)); else if (streq(d->result, "unsupported")) -- cgit v1.2.3-54-g00ecf From aabf5d42434082ae4e2d0c98d4705f58ee707567 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 18:57:04 +0100 Subject: systemctl: rename "reload-or-try-restart" verb to "try-reload-or-restart" But also keep the old name as (undocumented) compatibility around. The reload-or-try-restart was documented to be a NOP if the unit is not running, since the previous commits this is also implemented. The old name suggests that the "try" logic only applies to restarting. Fix this, by moving the "try-" to the front, to indicate that the whole option is a NOP if the service isn't running. --- man/systemctl.xml | 2 +- shell-completion/bash/systemctl.in | 2 +- shell-completion/zsh/_systemctl.in | 6 +++--- src/systemctl/systemctl.c | 5 +++-- 4 files changed, 8 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/man/systemctl.xml b/man/systemctl.xml index 6e5369f061..cce7861139 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -748,7 +748,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service - reload-or-try-restart PATTERN... + try-reload-or-restart PATTERN... Reload one or more units if they support it. If not, diff --git a/shell-completion/bash/systemctl.in b/shell-completion/bash/systemctl.in index 6ffab33e45..ef7dc6285a 100644 --- a/shell-completion/bash/systemctl.in +++ b/shell-completion/bash/systemctl.in @@ -170,7 +170,7 @@ _systemctl () { [STARTABLE_UNITS]='start' [STOPPABLE_UNITS]='stop condstop kill try-restart condrestart' [ISOLATABLE_UNITS]='isolate' - [RELOADABLE_UNITS]='reload condreload reload-or-try-restart force-reload' + [RELOADABLE_UNITS]='reload condreload try-reload-or-restart force-reload' [RESTARTABLE_UNITS]='restart reload-or-restart' [TARGET_AND_UNITS]='add-wants add-requires' [MASKED_UNITS]='unmask' diff --git a/shell-completion/zsh/_systemctl.in b/shell-completion/zsh/_systemctl.in index 58c88c9d98..667243eb53 100644 --- a/shell-completion/zsh/_systemctl.in +++ b/shell-completion/zsh/_systemctl.in @@ -17,7 +17,7 @@ "force-reload:Reload one or more units if possible, otherwise restart if active" "hibernate:Hibernate the system" "hybrid-sleep:Hibernate and suspend the system" - "reload-or-try-restart:Reload one or more units if possible, otherwise restart if active" + "try-reload-or-restart:Reload one or more units if possible, otherwise restart if active" "isolate:Start one unit and stop all others" "kill:Send signal to processes of a unit" "is-active:Check whether units are active" @@ -69,7 +69,7 @@ # Deal with any aliases case $cmd in condrestart) cmd="try-restart";; - force-reload) cmd="reload-or-try-restart";; + force-reload) cmd="try-reload-or-restart";; esac if (( $#cmd )); then @@ -230,7 +230,7 @@ done } # Completion functions for RELOADABLE_UNITS -for fun in reload reload-or-try-restart force-reload ; do +for fun in reload try-reload-or-restart force-reload ; do (( $+functions[_systemctl_$fun] )) || _systemctl_$fun() { local _sys_active_units; _systemctl_active_units diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 12209dc99b..f5703af241 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2538,6 +2538,7 @@ static const struct { { "try-restart", "TryRestartUnit" }, { "condrestart", "TryRestartUnit" }, { "reload-or-restart", "ReloadOrRestartUnit" }, + { "try-reload-or-restart", "ReloadOrTryRestartUnit" }, { "reload-or-try-restart", "ReloadOrTryRestartUnit" }, { "condreload", "ReloadOrTryRestartUnit" }, { "force-reload", "ReloadOrTryRestartUnit" } @@ -6234,8 +6235,8 @@ static void systemctl_help(void) { " try-restart NAME... Restart one or more units if active\n" " reload-or-restart NAME... Reload one or more units if possible,\n" " otherwise start or restart\n" - " reload-or-try-restart NAME... Reload one or more units if possible,\n" - " otherwise restart if active\n" + " try-reload-or-restart NAME... If active, reload one or more units,\n" + " if supported, otherwise restart\n" " isolate NAME Start one unit and stop all others\n" " kill NAME... Send signal to processes of a unit\n" " is-active PATTERN... Check whether units are active\n" -- cgit v1.2.3-54-g00ecf