From ca543871b743423c5de28927895fe6021e6665b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 17:15:59 +0200 Subject: [PATCH 1/7] set: add new helper set_make() which is like set_new() + multiple set_put() in vararg --- src/basic/meson.build | 1 + src/basic/set.c | 61 +++++++++++++++++++++++++++++++++++++++++++ src/basic/set.h | 2 ++ src/test/test-set.c | 45 +++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 src/basic/set.c diff --git a/src/basic/meson.build b/src/basic/meson.build index 994336fde2..1ddefb7fbb 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -145,6 +145,7 @@ basic_sources_plain = files(''' securebits.h selinux-util.c selinux-util.h + set.c set.h sigbus.c sigbus.h diff --git a/src/basic/set.c b/src/basic/set.c new file mode 100644 index 0000000000..fd398b8212 --- /dev/null +++ b/src/basic/set.c @@ -0,0 +1,61 @@ +/*** + This file is part of systemd. + + Copyright 2017 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "set.h" + +int set_make(Set **ret, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS, void *add, ...) { + _cleanup_set_free_ Set *s = NULL; + int r; + + assert(ret); + + s = set_new(hash_ops HASHMAP_DEBUG_PASS_ARGS); + if (!s) + return -ENOMEM; + + if (add) { + va_list ap; + + r = set_put(s, add); + if (r < 0) + return r; + + va_start(ap, add); + + for(;;) { + void *arg = va_arg(ap, void*); + + if (!arg) + break; + + r = set_put(s, arg); + if (r < 0) { + va_end(ap); + return r; + } + } + + va_end(ap); + } + + *ret = s; + s = NULL; + + return 0; +} diff --git a/src/basic/set.h b/src/basic/set.h index a5f8beb0c4..12d0fda1ca 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -136,3 +136,5 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free); #define _cleanup_set_free_ _cleanup_(set_freep) #define _cleanup_set_free_free_ _cleanup_(set_free_freep) + +int set_make(Set **ret, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS, void *add, ...); diff --git a/src/test/test-set.c b/src/test/test-set.c index 0ee5ddcc9f..3fab350cf6 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -55,9 +55,54 @@ static void test_set_put(void) { assert_se(set_put(m, (void*) "22") == 0); } +static void test_set_make(void) { + _cleanup_set_free_ Set *s = NULL; + + assert_se(set_make(&s, NULL, UINT_TO_PTR(4), UINT_TO_PTR(6), UINT_TO_PTR(8), NULL) == 0); + assert_se(set_size(s) == 3); + assert_se(!set_contains(s, UINT_TO_PTR(0))); + assert_se(!set_contains(s, UINT_TO_PTR(1))); + assert_se(!set_contains(s, UINT_TO_PTR(2))); + assert_se(!set_contains(s, UINT_TO_PTR(3))); + assert_se(set_contains(s, UINT_TO_PTR(4))); + assert_se(!set_contains(s, UINT_TO_PTR(5))); + assert_se(set_contains(s, UINT_TO_PTR(6))); + assert_se(!set_contains(s, UINT_TO_PTR(7))); + assert_se(set_contains(s, UINT_TO_PTR(8))); + assert_se(!set_contains(s, UINT_TO_PTR(9))); + s = set_free(s); + + assert_se(set_make(&s, NULL, NULL) == 0); + assert_se(set_size(s) == 0); + assert_se(!set_contains(s, UINT_TO_PTR(0))); + assert_se(!set_contains(s, UINT_TO_PTR(4))); + assert_se(!set_contains(s, UINT_TO_PTR(6))); + assert_se(!set_contains(s, UINT_TO_PTR(8))); + s = set_free(s); + + assert_se(set_make(&s, NULL, UINT_TO_PTR(3), NULL) == 0); + assert_se(set_size(s) == 1); + assert_se(!set_contains(s, UINT_TO_PTR(0))); + assert_se(!set_contains(s, UINT_TO_PTR(1))); + assert_se(!set_contains(s, UINT_TO_PTR(2))); + assert_se(set_contains(s, UINT_TO_PTR(3))); + assert_se(!set_contains(s, UINT_TO_PTR(4))); + + assert_se(set_make(&s, NULL, UINT_TO_PTR(2), UINT_TO_PTR(5), NULL) == 0); + assert_se(set_size(s) == 2); + assert_se(!set_contains(s, UINT_TO_PTR(0))); + assert_se(!set_contains(s, UINT_TO_PTR(1))); + assert_se(set_contains(s, UINT_TO_PTR(2))); + assert_se(!set_contains(s, UINT_TO_PTR(3))); + assert_se(!set_contains(s, UINT_TO_PTR(4))); + assert_se(set_contains(s, UINT_TO_PTR(5))); + assert_se(!set_contains(s, UINT_TO_PTR(6))); +} + int main(int argc, const char *argv[]) { test_set_steal_first(); test_set_put(); + test_set_make(); return 0; } From a6951a50791943cca7a10a8c0a751d73db502faa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 17:17:30 +0200 Subject: [PATCH 2/7] service: rework service_kill_control_processes() Let's make sure we explicitly also kill any control process we know of, given that it might have moved outside of our control group. --- src/core/service.c | 50 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 21fc4e2abe..38a8010232 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1631,7 +1631,6 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f s->main_pid, s->control_pid, s->main_pid_alien); - if (r < 0) goto fail; @@ -1786,13 +1785,39 @@ fail: } static void service_kill_control_processes(Service *s) { - char *p; + int r; - if (!UNIT(s)->cgroup_path) - return; + assert(s); - p = strjoina(UNIT(s)->cgroup_path, "/control"); - cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, p, SIGKILL, CGROUP_SIGCONT|CGROUP_IGNORE_SELF|CGROUP_REMOVE, NULL, NULL, NULL); + if (s->control_pid > 0) { + r = kill_and_sigcont(s->control_pid, SIGKILL); + if (r < 0) { + _cleanup_free_ char *comm = NULL; + + (void) get_process_comm(s->control_pid, &comm); + + log_unit_debug_errno(UNIT(s), r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", + s->control_pid, strna(comm)); + } + } + + if (UNIT(s)->cgroup_path) { + _cleanup_set_free_ Set *pid_set = NULL; + char *p; + + if (s->control_pid > 0) { + r = set_make(&pid_set, PID_TO_PTR(s->control_pid), NULL); + if (r < 0) { + log_oom(); + return; + } + } + + p = strjoina(UNIT(s)->cgroup_path, "/control"); + r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, p, SIGKILL, CGROUP_SIGCONT|CGROUP_IGNORE_SELF|CGROUP_REMOVE, pid_set, NULL, NULL); + if (r < 0) + log_unit_debug_errno(UNIT(s), r, "Failed to send SIGKILL to processes of control group %s: %m", p); + } } static void service_enter_start(Service *s) { @@ -3411,9 +3436,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) } if (!streq_ptr(s->status_text, t)) { - free_and_replace(s->status_text, t); - notify_dbus = true; } } @@ -3523,10 +3546,11 @@ static void service_bus_name_owner_change( } else if (new_owner && s->main_pid <= 0 && - (s->state == SERVICE_START || - s->state == SERVICE_START_POST || - s->state == SERVICE_RUNNING || - s->state == SERVICE_RELOAD)) { + IN_SET(s->state, + SERVICE_START, + SERVICE_START_POST, + SERVICE_RUNNING, + SERVICE_RELOAD)) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; pid_t pid; @@ -3609,6 +3633,8 @@ static void service_reset_failed(Unit *u) { static int service_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { Service *s = SERVICE(u); + assert(s); + return unit_kill_common(u, who, signo, s->main_pid, s->control_pid, error); } From 850b741084fd58573f5f804a434e2fa9ee7bd1c8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 18:56:19 +0200 Subject: [PATCH 3/7] mount: clean up reload_result management a bit Let's only collect the first failure in the load result, and let's clear it explicitly when we are about to enter a new reload operation. This makes it more alike the handling of the main result value (which also only stores the first failure), and also the handling of service.c's reload state. --- src/core/mount.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 46bcf37ae0..780e050daf 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -653,7 +653,6 @@ static void mount_set_state(Mount *m, MountState state) { log_unit_debug(UNIT(m), "Changed %s -> %s", mount_state_to_string(old_state), mount_state_to_string(state)); unit_notify(UNIT(m), state_translation_table[old_state], state_translation_table[state], m->reload_result == MOUNT_SUCCESS); - m->reload_result = MOUNT_SUCCESS; } static int mount_coldplug(Unit *u) { @@ -964,12 +963,25 @@ fail: mount_enter_dead(m, MOUNT_FAILURE_RESOURCES); } +static void mount_set_reload_result(Mount *m, MountResult result) { + assert(m); + + /* Only store the first error we encounter */ + if (m->reload_result != MOUNT_SUCCESS) + return; + + m->reload_result = result; +} + static void mount_enter_remounting(Mount *m) { int r; MountParameters *p; assert(m); + /* Reset reload result when we are about to start a new remount operation */ + m->reload_result = MOUNT_SUCCESS; + m->control_command_id = MOUNT_EXEC_REMOUNT; m->control_command = m->exec_command + MOUNT_EXEC_REMOUNT; @@ -1007,7 +1019,7 @@ static void mount_enter_remounting(Mount *m) { fail: log_unit_warning_errno(UNIT(m), r, "Failed to run 'remount' task: %m"); - m->reload_result = MOUNT_FAILURE_RESOURCES; + mount_set_reload_result(m, MOUNT_FAILURE_RESOURCES); mount_enter_mounted(m, MOUNT_SUCCESS); } @@ -1210,7 +1222,9 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { else assert_not_reached("Unknown code"); - if (m->result == MOUNT_SUCCESS) + if (IN_SET(m->state, MOUNT_REMOUNTING, MOUNT_REMOUNTING_SIGKILL, MOUNT_REMOUNTING_SIGTERM)) + mount_set_reload_result(m, f); + else if (m->result == MOUNT_SUCCESS) m->result = f; if (m->control_command) { From 22af0e5873978efbb3c210dc4c889041c7d56447 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 19:13:37 +0200 Subject: [PATCH 4/7] mount: rework mount state engine This changes the mount unit state engine in the following ways: 1. The MOUNT_MOUNTING_SIGTERM and MOUNT_MOUNTING_SIGKILL are removed. They have been pretty much equivalent to MOUNT_UNMOUNTING_SIGTERM and MOUNT_UNMOUNTING_SIGKILL in what they do, and the outcome has been the same as well: the unit is stopped. Hence, let's simplify things a bit, and merge them. Note that we keep MOUNT_REMOUNTING_{SIGTERM|SIGKILL} however, as those states have a different outcome: the unit remains started. 2. mount_enter_signal() will now honour the SendSIGKILL= option of the mount unit if it was set. This was previously done already when we entered the signal states through a timeout, and was simply missing here. 3. A new helper function mount_enter_dead_or_mounted() is added that places the mount unit in either MOUNT_DEAD or MOUNT_MOUNTED, depending on what the kernel thinks about the mount's state. This function is called at various places now, wherever we finished an operation, and want to make sure our own state reflects again what the kernel thinks. Previously we had very similar code in a number of places and in other places didn't recheck the kernel state. Let's do that with the same logic and function at all relevant places now. 4. Rework mount_stop(): never forget about running control processes. Instead: when we have a start (i.e. a /bin/mount) process running, and are asked to stop, then enter the kill states for it, so that it gets cleaned up. This fixes #6048. Moreover, when we have a reload process running convert the possible states into the relevant unmounting states, so that we can properly execute the requested operation. Fixes #6048 --- src/basic/unit-name.c | 2 - src/basic/unit-name.h | 2 - src/core/automount.c | 2 - src/core/mount.c | 263 ++++++++++++++++++++---------------------- 4 files changed, 127 insertions(+), 142 deletions(-) diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 08db164aa9..06380f4b6e 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -897,8 +897,6 @@ static const char* const mount_state_table[_MOUNT_STATE_MAX] = { [MOUNT_MOUNTED] = "mounted", [MOUNT_REMOUNTING] = "remounting", [MOUNT_UNMOUNTING] = "unmounting", - [MOUNT_MOUNTING_SIGTERM] = "mounting-sigterm", - [MOUNT_MOUNTING_SIGKILL] = "mounting-sigkill", [MOUNT_REMOUNTING_SIGTERM] = "remounting-sigterm", [MOUNT_REMOUNTING_SIGKILL] = "remounting-sigkill", [MOUNT_UNMOUNTING_SIGTERM] = "unmounting-sigterm", diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index f40fff3fe1..55f6372ee5 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -89,8 +89,6 @@ typedef enum MountState { MOUNT_MOUNTED, MOUNT_REMOUNTING, MOUNT_UNMOUNTING, - MOUNT_MOUNTING_SIGTERM, - MOUNT_MOUNTING_SIGKILL, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL, MOUNT_UNMOUNTING_SIGTERM, diff --git a/src/core/automount.c b/src/core/automount.c index 96de933e23..5ee0937fa7 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -529,7 +529,6 @@ static void automount_trigger_notify(Unit *u, Unit *other) { if (IN_SET(MOUNT(other)->state, MOUNT_MOUNTING, MOUNT_MOUNTING_DONE, MOUNT_MOUNTED, MOUNT_REMOUNTING, - MOUNT_MOUNTING_SIGTERM, MOUNT_MOUNTING_SIGKILL, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL, MOUNT_UNMOUNTING_SIGTERM, MOUNT_UNMOUNTING_SIGKILL, MOUNT_FAILED)) { @@ -543,7 +542,6 @@ static void automount_trigger_notify(Unit *u, Unit *other) { /* The mount is in some unhappy state now, let's unfreeze any waiting clients */ if (IN_SET(MOUNT(other)->state, MOUNT_DEAD, MOUNT_UNMOUNTING, - MOUNT_MOUNTING_SIGTERM, MOUNT_MOUNTING_SIGKILL, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL, MOUNT_UNMOUNTING_SIGTERM, MOUNT_UNMOUNTING_SIGKILL, MOUNT_FAILED)) { diff --git a/src/core/mount.c b/src/core/mount.c index 780e050daf..949d14e55c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -58,8 +58,6 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { [MOUNT_MOUNTED] = UNIT_ACTIVE, [MOUNT_REMOUNTING] = UNIT_RELOADING, [MOUNT_UNMOUNTING] = UNIT_DEACTIVATING, - [MOUNT_MOUNTING_SIGTERM] = UNIT_DEACTIVATING, - [MOUNT_MOUNTING_SIGKILL] = UNIT_DEACTIVATING, [MOUNT_REMOUNTING_SIGTERM] = UNIT_RELOADING, [MOUNT_REMOUNTING_SIGKILL] = UNIT_RELOADING, [MOUNT_UNMOUNTING_SIGTERM] = UNIT_DEACTIVATING, @@ -126,13 +124,11 @@ static bool mount_state_active(MountState state) { MOUNT_MOUNTING, MOUNT_MOUNTING_DONE, MOUNT_REMOUNTING, - MOUNT_UNMOUNTING, - MOUNT_MOUNTING_SIGTERM, - MOUNT_MOUNTING_SIGKILL, - MOUNT_UNMOUNTING_SIGTERM, - MOUNT_UNMOUNTING_SIGKILL, MOUNT_REMOUNTING_SIGTERM, - MOUNT_REMOUNTING_SIGKILL); + MOUNT_REMOUNTING_SIGKILL, + MOUNT_UNMOUNTING, + MOUNT_UNMOUNTING_SIGTERM, + MOUNT_UNMOUNTING_SIGKILL); } static bool mount_is_bound_to_device(const Mount *m) { @@ -821,6 +817,35 @@ static void mount_enter_mounted(Mount *m, MountResult f) { mount_set_state(m, MOUNT_MOUNTED); } +static void mount_enter_dead_or_mounted(Mount *m, MountResult f) { + assert(m); + + /* Enter DEAD or MOUNTED state, depending on what the kernel currently says about the mount point. We use this + * whenever we executed an operation, so that our internal state reflects what the kernel says again, after all + * ultimately we just mirror the kernel's internal state on this. */ + + if (m->from_proc_self_mountinfo) + mount_enter_mounted(m, f); + else + mount_enter_dead(m, f); +} + +static int state_to_kill_operation(MountState state) { + switch (state) { + + case MOUNT_REMOUNTING_SIGTERM: + case MOUNT_UNMOUNTING_SIGTERM: + return KILL_TERMINATE; + + case MOUNT_REMOUNTING_SIGKILL: + case MOUNT_UNMOUNTING_SIGKILL: + return KILL_KILL; + + default: + return _KILL_OPERATION_INVALID; + } +} + static void mount_enter_signal(Mount *m, MountState state, MountResult f) { int r; @@ -832,8 +857,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { r = unit_kill_context( UNIT(m), &m->kill_context, - (state != MOUNT_MOUNTING_SIGTERM && state != MOUNT_UNMOUNTING_SIGTERM && state != MOUNT_REMOUNTING_SIGTERM) ? - KILL_KILL : KILL_TERMINATE, + state_to_kill_operation(state), -1, m->control_pid, false); @@ -846,26 +870,20 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { goto fail; mount_set_state(m, state); - } else if (state == MOUNT_REMOUNTING_SIGTERM) + } else if (state == MOUNT_REMOUNTING_SIGTERM && m->kill_context.send_sigkill) mount_enter_signal(m, MOUNT_REMOUNTING_SIGKILL, MOUNT_SUCCESS); - else if (state == MOUNT_REMOUNTING_SIGKILL) + else if (IN_SET(state, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL)) mount_enter_mounted(m, MOUNT_SUCCESS); - else if (state == MOUNT_MOUNTING_SIGTERM) - mount_enter_signal(m, MOUNT_MOUNTING_SIGKILL, MOUNT_SUCCESS); - else if (state == MOUNT_UNMOUNTING_SIGTERM) + else if (state == MOUNT_UNMOUNTING_SIGTERM && m->kill_context.send_sigkill) mount_enter_signal(m, MOUNT_UNMOUNTING_SIGKILL, MOUNT_SUCCESS); else - mount_enter_dead(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); return; fail: log_unit_warning_errno(UNIT(m), r, "Failed to kill processes: %m"); - - if (IN_SET(state, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL)) - mount_enter_mounted(m, MOUNT_FAILURE_RESOURCES); - else - mount_enter_dead(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } static void mount_enter_unmounting(Mount *m) { @@ -903,7 +921,7 @@ static void mount_enter_unmounting(Mount *m) { fail: log_unit_warning_errno(UNIT(m), r, "Failed to run 'umount' task: %m"); - mount_enter_mounted(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } static void mount_enter_mounting(Mount *m) { @@ -944,7 +962,6 @@ static void mount_enter_mounting(Mount *m) { r = exec_command_append(m->control_command, "-o", opts, NULL); } else r = -ENOENT; - if (r < 0) goto fail; @@ -960,7 +977,7 @@ static void mount_enter_mounting(Mount *m) { fail: log_unit_warning_errno(UNIT(m), r, "Failed to run 'mount' task: %m"); - mount_enter_dead(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } static void mount_set_reload_result(Mount *m, MountResult result) { @@ -1003,7 +1020,6 @@ static void mount_enter_remounting(Mount *m) { r = exec_command_append(m->control_command, "-t", p->fstype, NULL); } else r = -ENOENT; - if (r < 0) goto fail; @@ -1020,7 +1036,7 @@ static void mount_enter_remounting(Mount *m) { fail: log_unit_warning_errno(UNIT(m), r, "Failed to run 'remount' task: %m"); mount_set_reload_result(m, MOUNT_FAILURE_RESOURCES); - mount_enter_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); } static int mount_start(Unit *u) { @@ -1034,9 +1050,7 @@ static int mount_start(Unit *u) { if (IN_SET(m->state, MOUNT_UNMOUNTING, MOUNT_UNMOUNTING_SIGTERM, - MOUNT_UNMOUNTING_SIGKILL, - MOUNT_MOUNTING_SIGTERM, - MOUNT_MOUNTING_SIGKILL)) + MOUNT_UNMOUNTING_SIGKILL)) return -EAGAIN; /* Already on it! */ @@ -1068,25 +1082,38 @@ static int mount_stop(Unit *u) { assert(m); - /* Already on it */ - if (IN_SET(m->state, - MOUNT_UNMOUNTING, - MOUNT_UNMOUNTING_SIGKILL, - MOUNT_UNMOUNTING_SIGTERM, - MOUNT_MOUNTING_SIGTERM, - MOUNT_MOUNTING_SIGKILL)) + switch (m->state) { + + case MOUNT_UNMOUNTING: + case MOUNT_UNMOUNTING_SIGKILL: + case MOUNT_UNMOUNTING_SIGTERM: + /* Already on it */ return 0; - assert(IN_SET(m->state, - MOUNT_MOUNTING, - MOUNT_MOUNTING_DONE, - MOUNT_MOUNTED, - MOUNT_REMOUNTING, - MOUNT_REMOUNTING_SIGTERM, - MOUNT_REMOUNTING_SIGKILL)); + case MOUNT_MOUNTING: + case MOUNT_MOUNTING_DONE: + case MOUNT_REMOUNTING: + /* If we are still waiting for /bin/mount, we go directly into kill mode. */ + mount_enter_signal(m, MOUNT_UNMOUNTING_SIGTERM, MOUNT_SUCCESS); + return 0; - mount_enter_unmounting(m); - return 1; + case MOUNT_REMOUNTING_SIGTERM: + /* If we are already waiting for a hung remount, convert this to the matching unmounting state */ + mount_set_state(m, MOUNT_UNMOUNTING_SIGTERM); + return 0; + + case MOUNT_REMOUNTING_SIGKILL: + /* as above */ + mount_set_state(m, MOUNT_UNMOUNTING_SIGKILL); + return 0; + + case MOUNT_MOUNTED: + mount_enter_unmounting(m); + return 1; + + default: + assert_not_reached("Unexpected state."); + } } static int mount_reload(Unit *u) { @@ -1094,12 +1121,13 @@ static int mount_reload(Unit *u) { assert(m); - if (m->state == MOUNT_MOUNTING_DONE) + if (m->state == MOUNT_MOUNTING_DONE) /* not yet ready to reload, try again */ return -EAGAIN; assert(m->state == MOUNT_MOUNTED); mount_enter_remounting(m); + return 1; } @@ -1237,19 +1265,15 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { log_unit_full(u, f == MOUNT_SUCCESS ? LOG_DEBUG : LOG_NOTICE, 0, "Mount process exited, code=%s status=%i", sigchld_code_to_string(code), status); - /* Note that mount(8) returning and the kernel sending us a - * mount table change event might happen out-of-order. If an - * operation succeed we assume the kernel will follow soon too - * and already change into the resulting state. If it fails - * we check if the kernel still knows about the mount. and - * change state accordingly. */ + /* Note that mount(8) returning and the kernel sending us a mount table change event might happen + * out-of-order. If an operation succeed we assume the kernel will follow soon too and already change into the + * resulting state. If it fails we check if the kernel still knows about the mount. and change state + * accordingly. */ switch (m->state) { case MOUNT_MOUNTING: case MOUNT_MOUNTING_DONE: - case MOUNT_MOUNTING_SIGKILL: - case MOUNT_MOUNTING_SIGTERM: if (f == MOUNT_SUCCESS || m->from_proc_self_mountinfo) /* If /bin/mount returned success, or if we see the mount point in /proc/self/mountinfo we are @@ -1261,49 +1285,32 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { break; case MOUNT_REMOUNTING: - case MOUNT_REMOUNTING_SIGKILL: case MOUNT_REMOUNTING_SIGTERM: - - m->reload_result = f; - if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, MOUNT_SUCCESS); - else - mount_enter_dead(m, MOUNT_SUCCESS); - + case MOUNT_REMOUNTING_SIGKILL: + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); break; case MOUNT_UNMOUNTING: case MOUNT_UNMOUNTING_SIGKILL: case MOUNT_UNMOUNTING_SIGTERM: - if (f == MOUNT_SUCCESS) { + if (m->from_proc_self_mountinfo) { - if (m->from_proc_self_mountinfo) { + /* Still a mount point? If so, let's try again. Most likely there were multiple mount points + * stacked on top of each other. Note that due to the io event priority logic we can be sure + * the new mountinfo is loaded before we process the SIGCHLD for the mount command. */ - /* Still a mount point? If so, let's - * try again. Most likely there were - * multiple mount points stacked on - * top of each other. Note that due to - * the io event priority logic we can - * be sure the new mountinfo is loaded - * before we process the SIGCHLD for - * the mount command. */ - - if (m->n_retry_umount < RETRY_UMOUNT_MAX) { - log_unit_debug(u, "Mount still present, trying again."); - m->n_retry_umount++; - mount_enter_unmounting(m); - } else { - log_unit_debug(u, "Mount still present after %u attempts to unmount, giving up.", m->n_retry_umount); - mount_enter_mounted(m, f); - } - } else - mount_enter_dead(m, f); - - } else if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, f); - else + if (m->n_retry_umount < RETRY_UMOUNT_MAX) { + log_unit_debug(u, "Mount still present, trying again."); + m->n_retry_umount++; + mount_enter_unmounting(m); + } else { + log_unit_debug(u, "Mount still present after %u attempts to unmount, giving up.", m->n_retry_umount); + mount_enter_mounted(m, f); + } + } else mount_enter_dead(m, f); + break; default: @@ -1324,14 +1331,33 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user case MOUNT_MOUNTING: case MOUNT_MOUNTING_DONE: - log_unit_warning(UNIT(m), "Mounting timed out. Stopping."); - mount_enter_signal(m, MOUNT_MOUNTING_SIGTERM, MOUNT_FAILURE_TIMEOUT); + log_unit_warning(UNIT(m), "Mounting timed out. Terminating."); + mount_enter_signal(m, MOUNT_UNMOUNTING_SIGTERM, MOUNT_FAILURE_TIMEOUT); break; case MOUNT_REMOUNTING: - log_unit_warning(UNIT(m), "Remounting timed out. Stopping."); - m->reload_result = MOUNT_FAILURE_TIMEOUT; - mount_enter_mounted(m, MOUNT_SUCCESS); + log_unit_warning(UNIT(m), "Remounting timed out. Killing remount process."); + mount_set_reload_result(m, MOUNT_FAILURE_TIMEOUT); + mount_enter_signal(m, MOUNT_REMOUNTING_SIGTERM, MOUNT_SUCCESS); + break; + + case MOUNT_REMOUNTING_SIGTERM: + mount_set_reload_result(m, MOUNT_FAILURE_TIMEOUT); + + if (m->kill_context.send_sigkill) { + log_unit_warning(UNIT(m), "Remounting timed out. Killing."); + mount_enter_signal(m, MOUNT_REMOUNTING_SIGKILL, MOUNT_SUCCESS); + } else { + log_unit_warning(UNIT(m), "Remounting timed out. Skipping SIGKILL. Ignoring."); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + } + break; + + case MOUNT_REMOUNTING_SIGKILL: + mount_set_reload_result(m, MOUNT_FAILURE_TIMEOUT); + + log_unit_warning(UNIT(m), "Mount process still around after SIGKILL. Ignoring."); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); break; case MOUNT_UNMOUNTING: @@ -1339,57 +1365,19 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user mount_enter_signal(m, MOUNT_UNMOUNTING_SIGTERM, MOUNT_FAILURE_TIMEOUT); break; - case MOUNT_MOUNTING_SIGTERM: - if (m->kill_context.send_sigkill) { - log_unit_warning(UNIT(m), "Mounting timed out. Killing."); - mount_enter_signal(m, MOUNT_MOUNTING_SIGKILL, MOUNT_FAILURE_TIMEOUT); - } else { - log_unit_warning(UNIT(m), "Mounting timed out. Skipping SIGKILL. Ignoring."); - - if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, MOUNT_FAILURE_TIMEOUT); - else - mount_enter_dead(m, MOUNT_FAILURE_TIMEOUT); - } - break; - - case MOUNT_REMOUNTING_SIGTERM: - if (m->kill_context.send_sigkill) { - log_unit_warning(UNIT(m), "Remounting timed out. Killing."); - mount_enter_signal(m, MOUNT_REMOUNTING_SIGKILL, MOUNT_FAILURE_TIMEOUT); - } else { - log_unit_warning(UNIT(m), "Remounting timed out. Skipping SIGKILL. Ignoring."); - - if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, MOUNT_FAILURE_TIMEOUT); - else - mount_enter_dead(m, MOUNT_FAILURE_TIMEOUT); - } - break; - case MOUNT_UNMOUNTING_SIGTERM: if (m->kill_context.send_sigkill) { - log_unit_warning(UNIT(m), "Unmounting timed out. Killing."); + log_unit_warning(UNIT(m), "Mount process timed out. Killing."); mount_enter_signal(m, MOUNT_UNMOUNTING_SIGKILL, MOUNT_FAILURE_TIMEOUT); } else { - log_unit_warning(UNIT(m), "Unmounting timed out. Skipping SIGKILL. Ignoring."); - - if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, MOUNT_FAILURE_TIMEOUT); - else - mount_enter_dead(m, MOUNT_FAILURE_TIMEOUT); + log_unit_warning(UNIT(m), "Mount process timed out. Skipping SIGKILL. Ignoring."); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT); } break; - case MOUNT_MOUNTING_SIGKILL: - case MOUNT_REMOUNTING_SIGKILL: case MOUNT_UNMOUNTING_SIGKILL: - log_unit_warning(UNIT(m),"Mount process still around after SIGKILL. Ignoring."); - - if (m->from_proc_self_mountinfo) - mount_enter_mounted(m, MOUNT_FAILURE_TIMEOUT); - else - mount_enter_dead(m, MOUNT_FAILURE_TIMEOUT); + log_unit_warning(UNIT(m), "Mount process still around after SIGKILL. Ignoring."); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT); break; default: @@ -1647,7 +1635,6 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) { } static void mount_shutdown(Manager *m) { - assert(m); m->mount_event_source = sd_event_source_unref(m->mount_event_source); @@ -1918,6 +1905,10 @@ static void mount_reset_failed(Unit *u) { } static int mount_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { + Mount *m = MOUNT(u); + + assert(m); + return unit_kill_common(u, who, signo, -1, MOUNT(u)->control_pid, error); } From c634f3d2fc0f9f682af69e2d38b07847e5bd4f14 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 19:25:50 +0200 Subject: [PATCH 5/7] =?UTF-8?q?mount:=20rename=20mount=5Fstate=5Factive()?= =?UTF-8?q?=20=E2=86=92=20MOUNT=5FSTATE=5FWITH=5FPROCESS()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function returns true for all states that have a control process running, and each time we call it that's what we want to know, hence let's rename it accordingly. Moreover, the more generic unit states have an ACTIVE state, and it is defined quite differently from the set of states this function returns true for, hence let's avoid confusion and not reuse the word "ACTIVE" here in a different context. Finally, let's uppercase this, since in most ways it's pretty much identical to a macro --- src/core/mount.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 949d14e55c..22b3608a2f 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -68,6 +68,18 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); +static bool MOUNT_STATE_WITH_PROCESS(MountState state) { + return IN_SET(state, + MOUNT_MOUNTING, + MOUNT_MOUNTING_DONE, + MOUNT_REMOUNTING, + MOUNT_REMOUNTING_SIGTERM, + MOUNT_REMOUNTING_SIGKILL, + MOUNT_UNMOUNTING, + MOUNT_UNMOUNTING_SIGTERM, + MOUNT_UNMOUNTING_SIGKILL); +} + static bool mount_needs_network(const char *options, const char *fstype) { if (fstab_test_option(options, "_netdev\0")) return true; @@ -119,18 +131,6 @@ static bool mount_is_automount(const MountParameters *p) { "x-systemd.automount\0"); } -static bool mount_state_active(MountState state) { - return IN_SET(state, - MOUNT_MOUNTING, - MOUNT_MOUNTING_DONE, - MOUNT_REMOUNTING, - MOUNT_REMOUNTING_SIGTERM, - MOUNT_REMOUNTING_SIGKILL, - MOUNT_UNMOUNTING, - MOUNT_UNMOUNTING_SIGTERM, - MOUNT_UNMOUNTING_SIGKILL); -} - static bool mount_is_bound_to_device(const Mount *m) { const MountParameters *p; @@ -638,7 +638,7 @@ static void mount_set_state(Mount *m, MountState state) { old_state = m->state; m->state = state; - if (!mount_state_active(state)) { + if (!MOUNT_STATE_WITH_PROCESS(state)) { m->timer_event_source = sd_event_source_unref(m->timer_event_source); mount_unwatch_control_pid(m); m->control_command = NULL; @@ -669,7 +669,7 @@ static int mount_coldplug(Unit *u) { if (m->control_pid > 0 && pid_is_unwaited(m->control_pid) && - mount_state_active(new_state)) { + MOUNT_STATE_WITH_PROCESS(new_state)) { r = unit_watch_pid(UNIT(m), m->control_pid); if (r < 0) From 50864457e1bc5f7a4ab2fd02e1565bc5d135d2f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 19:53:19 +0200 Subject: [PATCH 6/7] swap: adjust swap.c in a similar way to what we just did to mount.c Also drop the redundant states and make all similar changes too. Thankfully the swap.c state engine is much simpler than mount.c's, hence this should be easier to digest. --- src/basic/unit-name.c | 2 - src/basic/unit-name.h | 2 - src/core/swap.c | 96 ++++++++++++++++++++----------------------- 3 files changed, 45 insertions(+), 55 deletions(-) diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 06380f4b6e..5e3f307dcd 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -978,8 +978,6 @@ static const char* const swap_state_table[_SWAP_STATE_MAX] = { [SWAP_ACTIVATING_DONE] = "activating-done", [SWAP_ACTIVE] = "active", [SWAP_DEACTIVATING] = "deactivating", - [SWAP_ACTIVATING_SIGTERM] = "activating-sigterm", - [SWAP_ACTIVATING_SIGKILL] = "activating-sigkill", [SWAP_DEACTIVATING_SIGTERM] = "deactivating-sigterm", [SWAP_DEACTIVATING_SIGKILL] = "deactivating-sigkill", [SWAP_FAILED] = "failed" diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index 55f6372ee5..15558b4fbd 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -170,8 +170,6 @@ typedef enum SwapState { SWAP_ACTIVATING_DONE, /* /sbin/swapon is running, and the swap is done. */ SWAP_ACTIVE, SWAP_DEACTIVATING, - SWAP_ACTIVATING_SIGTERM, - SWAP_ACTIVATING_SIGKILL, SWAP_DEACTIVATING_SIGTERM, SWAP_DEACTIVATING_SIGKILL, SWAP_FAILED, diff --git a/src/core/swap.c b/src/core/swap.c index d58f68458b..0cf03fc354 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -49,8 +49,6 @@ static const UnitActiveState state_translation_table[_SWAP_STATE_MAX] = { [SWAP_ACTIVATING_DONE] = UNIT_ACTIVE, [SWAP_ACTIVE] = UNIT_ACTIVE, [SWAP_DEACTIVATING] = UNIT_DEACTIVATING, - [SWAP_ACTIVATING_SIGTERM] = UNIT_DEACTIVATING, - [SWAP_ACTIVATING_SIGKILL] = UNIT_DEACTIVATING, [SWAP_DEACTIVATING_SIGTERM] = UNIT_DEACTIVATING, [SWAP_DEACTIVATING_SIGKILL] = UNIT_DEACTIVATING, [SWAP_FAILED] = UNIT_FAILED @@ -489,8 +487,6 @@ static void swap_set_state(Swap *s, SwapState state) { if (!IN_SET(state, SWAP_ACTIVATING, - SWAP_ACTIVATING_SIGTERM, - SWAP_ACTIVATING_SIGKILL, SWAP_ACTIVATING_DONE, SWAP_DEACTIVATING, SWAP_DEACTIVATING_SIGTERM, @@ -536,8 +532,6 @@ static int swap_coldplug(Unit *u) { pid_is_unwaited(s->control_pid) && IN_SET(new_state, SWAP_ACTIVATING, - SWAP_ACTIVATING_SIGTERM, - SWAP_ACTIVATING_SIGKILL, SWAP_ACTIVATING_DONE, SWAP_DEACTIVATING, SWAP_DEACTIVATING_SIGTERM, @@ -693,6 +687,15 @@ static void swap_enter_active(Swap *s, SwapResult f) { swap_set_state(s, SWAP_ACTIVE); } +static void swap_enter_dead_or_active(Swap *s, SwapResult f) { + assert(s); + + if (s->from_proc_swaps) + swap_enter_active(s, f); + else + swap_enter_dead(s, f); +} + static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { int r; KillOperation kop; @@ -702,7 +705,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { if (s->result == SWAP_SUCCESS) s->result = f; - if (IN_SET(state, SWAP_ACTIVATING_SIGTERM, SWAP_DEACTIVATING_SIGTERM)) + if (state == SWAP_DEACTIVATING_SIGTERM) kop = KILL_TERMINATE; else kop = KILL_KILL; @@ -717,18 +720,16 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { goto fail; swap_set_state(s, state); - } else if (state == SWAP_ACTIVATING_SIGTERM) - swap_enter_signal(s, SWAP_ACTIVATING_SIGKILL, SWAP_SUCCESS); - else if (state == SWAP_DEACTIVATING_SIGTERM) + } else if (state == SWAP_DEACTIVATING_SIGTERM && s->kill_context.send_sigkill) swap_enter_signal(s, SWAP_DEACTIVATING_SIGKILL, SWAP_SUCCESS); else - swap_enter_dead(s, SWAP_SUCCESS); + swap_enter_dead_or_active(s, SWAP_SUCCESS); return; fail: log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); - swap_enter_dead(s, SWAP_FAILURE_RESOURCES); + swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } static void swap_enter_activating(Swap *s) { @@ -786,7 +787,7 @@ static void swap_enter_activating(Swap *s) { fail: log_unit_warning_errno(UNIT(s), r, "Failed to run 'swapon' task: %m"); - swap_enter_dead(s, SWAP_FAILURE_RESOURCES); + swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } static void swap_enter_deactivating(Swap *s) { @@ -816,7 +817,7 @@ static void swap_enter_deactivating(Swap *s) { fail: log_unit_warning_errno(UNIT(s), r, "Failed to run 'swapoff' task: %m"); - swap_enter_active(s, SWAP_FAILURE_RESOURCES); + swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } static int swap_start(Unit *u) { @@ -825,17 +826,14 @@ static int swap_start(Unit *u) { assert(s); - /* We cannot fulfill this request right now, try again later - * please! */ - + /* We cannot fulfill this request right now, try again later please! */ if (IN_SET(s->state, SWAP_DEACTIVATING, SWAP_DEACTIVATING_SIGTERM, - SWAP_DEACTIVATING_SIGKILL, - SWAP_ACTIVATING_SIGTERM, - SWAP_ACTIVATING_SIGKILL)) + SWAP_DEACTIVATING_SIGKILL)) return -EAGAIN; + /* Already on it! */ if (s->state == SWAP_ACTIVATING) return 0; @@ -873,21 +871,30 @@ static int swap_stop(Unit *u) { assert(s); - if (IN_SET(s->state, - SWAP_DEACTIVATING, - SWAP_DEACTIVATING_SIGTERM, - SWAP_DEACTIVATING_SIGKILL, - SWAP_ACTIVATING_SIGTERM, - SWAP_ACTIVATING_SIGKILL)) + switch (s->state) { + + case SWAP_DEACTIVATING: + case SWAP_DEACTIVATING_SIGTERM: + case SWAP_DEACTIVATING_SIGKILL: + /* Already on it */ return 0; - assert(IN_SET(s->state, SWAP_ACTIVATING, SWAP_ACTIVATING_DONE, SWAP_ACTIVE)); + case SWAP_ACTIVATING: + case SWAP_ACTIVATING_DONE: + /* There's a control process pending, directly enter kill mode */ + swap_enter_signal(s, SWAP_DEACTIVATING_SIGTERM, SWAP_SUCCESS); + return 0; - if (detect_container() > 0) - return -EPERM; + case SWAP_ACTIVE: + if (detect_container() > 0) + return -EPERM; - swap_enter_deactivating(s); - return 1; + swap_enter_deactivating(s); + return 1; + + default: + assert_not_reached("Unexpected state."); + } } static int swap_serialize(Unit *u, FILE *f, FDSet *fds) { @@ -1015,10 +1022,8 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { case SWAP_ACTIVATING: case SWAP_ACTIVATING_DONE: - case SWAP_ACTIVATING_SIGTERM: - case SWAP_ACTIVATING_SIGKILL: - if (f == SWAP_SUCCESS) + if (f == SWAP_SUCCESS || s->from_proc_swaps) swap_enter_active(s, f); else swap_enter_dead(s, f); @@ -1028,7 +1033,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { case SWAP_DEACTIVATING_SIGKILL: case SWAP_DEACTIVATING_SIGTERM: - swap_enter_dead(s, f); + swap_enter_dead_or_active(s, f); break; default: @@ -1050,7 +1055,7 @@ static int swap_dispatch_timer(sd_event_source *source, usec_t usec, void *userd case SWAP_ACTIVATING: case SWAP_ACTIVATING_DONE: log_unit_warning(UNIT(s), "Activation timed out. Stopping."); - swap_enter_signal(s, SWAP_ACTIVATING_SIGTERM, SWAP_FAILURE_TIMEOUT); + swap_enter_signal(s, SWAP_DEACTIVATING_SIGTERM, SWAP_FAILURE_TIMEOUT); break; case SWAP_DEACTIVATING: @@ -1058,30 +1063,19 @@ static int swap_dispatch_timer(sd_event_source *source, usec_t usec, void *userd swap_enter_signal(s, SWAP_DEACTIVATING_SIGTERM, SWAP_FAILURE_TIMEOUT); break; - case SWAP_ACTIVATING_SIGTERM: - if (s->kill_context.send_sigkill) { - log_unit_warning(UNIT(s), "Activation timed out. Killing."); - swap_enter_signal(s, SWAP_ACTIVATING_SIGKILL, SWAP_FAILURE_TIMEOUT); - } else { - log_unit_warning(UNIT(s), "Activation timed out. Skipping SIGKILL. Ignoring."); - swap_enter_dead(s, SWAP_FAILURE_TIMEOUT); - } - break; - case SWAP_DEACTIVATING_SIGTERM: if (s->kill_context.send_sigkill) { - log_unit_warning(UNIT(s), "Deactivation timed out. Killing."); + log_unit_warning(UNIT(s), "Swap process timed out. Killing."); swap_enter_signal(s, SWAP_DEACTIVATING_SIGKILL, SWAP_FAILURE_TIMEOUT); } else { - log_unit_warning(UNIT(s), "Deactivation timed out. Skipping SIGKILL. Ignoring."); - swap_enter_dead(s, SWAP_FAILURE_TIMEOUT); + log_unit_warning(UNIT(s), "Swap process timed out. Skipping SIGKILL. Ignoring."); + swap_enter_dead_or_active(s, SWAP_FAILURE_TIMEOUT); } break; - case SWAP_ACTIVATING_SIGKILL: case SWAP_DEACTIVATING_SIGKILL: log_unit_warning(UNIT(s), "Swap process still around after SIGKILL. Ignoring."); - swap_enter_dead(s, SWAP_FAILURE_TIMEOUT); + swap_enter_dead_or_active(s, SWAP_FAILURE_TIMEOUT); break; default: From dedf37190986be8bc1447d2a1ab8a877e9dce2af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Sep 2017 19:57:52 +0200 Subject: [PATCH 7/7] swap: introduce SWAP_STATE_WITH_PROCESS() similar to MOUNT_STATE_WITH_PROCESS() --- src/core/swap.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/core/swap.c b/src/core/swap.c index 0cf03fc354..6db1c0df69 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -57,6 +57,15 @@ static const UnitActiveState state_translation_table[_SWAP_STATE_MAX] = { static int swap_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int swap_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); +static bool SWAP_STATE_WITH_PROCESS(SwapState state) { + return IN_SET(state, + SWAP_ACTIVATING, + SWAP_ACTIVATING_DONE, + SWAP_DEACTIVATING, + SWAP_DEACTIVATING_SIGTERM, + SWAP_DEACTIVATING_SIGKILL); +} + static void swap_unset_proc_swaps(Swap *s) { assert(s); @@ -485,12 +494,7 @@ static void swap_set_state(Swap *s, SwapState state) { old_state = s->state; s->state = state; - if (!IN_SET(state, - SWAP_ACTIVATING, - SWAP_ACTIVATING_DONE, - SWAP_DEACTIVATING, - SWAP_DEACTIVATING_SIGTERM, - SWAP_DEACTIVATING_SIGKILL)) { + if (!SWAP_STATE_WITH_PROCESS(state)) { s->timer_event_source = sd_event_source_unref(s->timer_event_source); swap_unwatch_control_pid(s); s->control_command = NULL; @@ -530,12 +534,7 @@ static int swap_coldplug(Unit *u) { if (s->control_pid > 0 && pid_is_unwaited(s->control_pid) && - IN_SET(new_state, - SWAP_ACTIVATING, - SWAP_ACTIVATING_DONE, - SWAP_DEACTIVATING, - SWAP_DEACTIVATING_SIGTERM, - SWAP_DEACTIVATING_SIGKILL)) { + SWAP_STATE_WITH_PROCESS(new_state)) { r = unit_watch_pid(UNIT(s), s->control_pid); if (r < 0)