From 072993504e3e4206ae1019f5461a0372f7d82ddf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 13:01:26 +0200 Subject: core: move enforcement of the start limit into per-unit-type code again Let's move the enforcement of the per-unit start limit from unit.c into the type-specific files again. For unit types that know a concept of "result" codes this allows us to hook up the start limit condition to it with an explicit result code. Also, this makes sure that the state checks in clal like service_start() may be done before the start limit is checked, as the start limit really should be checked last, right before everything has been verified to be in order. The generic start limit logic is left in unit.c, but the invocation of it is moved into the per-type files, in the various xyz_start() functions, so that they may place the check at the right location. Note that this change drops the enforcement entirely from device, slice, target and scope units, since these unit types generally may not fail activation, or may only be activated a single time. This is also documented now. Note that restores the "start-limit-hit" result code that existed before 6bf0f408e4833152197fb38fb10a9989c89f3a59 already in the service code. However, it's not introduced for all units that have a result code concept. Fixes #3166. --- src/core/automount.c | 10 +++++++++- src/core/automount.h | 1 + src/core/busname.c | 8 ++++++++ src/core/busname.h | 1 + src/core/mount.c | 10 +++++++++- src/core/mount.h | 1 + src/core/path.c | 8 ++++++++ src/core/path.h | 1 + src/core/service.c | 9 +++++++++ src/core/service.h | 1 + src/core/socket.c | 8 ++++++++ src/core/socket.h | 1 + src/core/swap.c | 10 +++++++++- src/core/swap.h | 1 + src/core/timer.c | 10 +++++++++- src/core/timer.h | 1 + src/core/unit.c | 8 +------- src/core/unit.h | 2 ++ 18 files changed, 80 insertions(+), 11 deletions(-) (limited to 'src/core') diff --git a/src/core/automount.c b/src/core/automount.c index 7c55d7bc49..c871b02a94 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -751,6 +751,7 @@ fail: static int automount_start(Unit *u) { Automount *a = AUTOMOUNT(u); Unit *trigger; + int r; assert(a); assert(a->state == AUTOMOUNT_DEAD || a->state == AUTOMOUNT_FAILED); @@ -766,6 +767,12 @@ static int automount_start(Unit *u) { return -ENOENT; } + r = unit_start_limit_test(u); + if (r < 0) { + automount_enter_dead(a, AUTOMOUNT_FAILURE_START_LIMIT_HIT); + return r; + } + a->result = AUTOMOUNT_SUCCESS; automount_enter_waiting(a); return 1; @@ -1037,7 +1044,8 @@ static bool automount_supported(void) { static const char* const automount_result_table[_AUTOMOUNT_RESULT_MAX] = { [AUTOMOUNT_SUCCESS] = "success", - [AUTOMOUNT_FAILURE_RESOURCES] = "resources" + [AUTOMOUNT_FAILURE_RESOURCES] = "resources", + [AUTOMOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(automount_result, AutomountResult); diff --git a/src/core/automount.h b/src/core/automount.h index cf5b1cf994..414717e5b8 100644 --- a/src/core/automount.h +++ b/src/core/automount.h @@ -26,6 +26,7 @@ typedef struct Automount Automount; typedef enum AutomountResult { AUTOMOUNT_SUCCESS, AUTOMOUNT_FAILURE_RESOURCES, + AUTOMOUNT_FAILURE_START_LIMIT_HIT, _AUTOMOUNT_RESULT_MAX, _AUTOMOUNT_RESULT_INVALID = -1 } AutomountResult; diff --git a/src/core/busname.c b/src/core/busname.c index f4f433340c..5600d1ac90 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -607,6 +607,7 @@ fail: static int busname_start(Unit *u) { BusName *n = BUSNAME(u); + int r; assert(n); @@ -632,6 +633,12 @@ static int busname_start(Unit *u) { assert(IN_SET(n->state, BUSNAME_DEAD, BUSNAME_FAILED)); + r = unit_start_limit_test(u); + if (r < 0) { + busname_enter_dead(n, BUSNAME_FAILURE_START_LIMIT_HIT); + return r; + } + n->result = BUSNAME_SUCCESS; busname_enter_making(n); @@ -1014,6 +1021,7 @@ static const char* const busname_result_table[_BUSNAME_RESULT_MAX] = { [BUSNAME_FAILURE_EXIT_CODE] = "exit-code", [BUSNAME_FAILURE_SIGNAL] = "signal", [BUSNAME_FAILURE_CORE_DUMP] = "core-dump", + [BUSNAME_FAILURE_START_LIMIT_HIT] = "start-limit-hit", [BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit", }; diff --git a/src/core/busname.h b/src/core/busname.h index 52c4055dbb..a8562db458 100644 --- a/src/core/busname.h +++ b/src/core/busname.h @@ -32,6 +32,7 @@ typedef enum BusNameResult { BUSNAME_FAILURE_EXIT_CODE, BUSNAME_FAILURE_SIGNAL, BUSNAME_FAILURE_CORE_DUMP, + BUSNAME_FAILURE_START_LIMIT_HIT, BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT, _BUSNAME_RESULT_MAX, _BUSNAME_RESULT_INVALID = -1 diff --git a/src/core/mount.c b/src/core/mount.c index cc07873b24..aa48941f0d 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -984,6 +984,7 @@ fail: static int mount_start(Unit *u) { Mount *m = MOUNT(u); + int r; assert(m); @@ -1002,6 +1003,12 @@ static int mount_start(Unit *u) { assert(m->state == MOUNT_DEAD || m->state == MOUNT_FAILED); + r = unit_start_limit_test(u); + if (r < 0) { + mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); + return r; + } + m->result = MOUNT_SUCCESS; m->reload_result = MOUNT_SUCCESS; m->reset_cpu_usage = true; @@ -1821,7 +1828,8 @@ static const char* const mount_result_table[_MOUNT_RESULT_MAX] = { [MOUNT_FAILURE_TIMEOUT] = "timeout", [MOUNT_FAILURE_EXIT_CODE] = "exit-code", [MOUNT_FAILURE_SIGNAL] = "signal", - [MOUNT_FAILURE_CORE_DUMP] = "core-dump" + [MOUNT_FAILURE_CORE_DUMP] = "core-dump", + [MOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(mount_result, MountResult); diff --git a/src/core/mount.h b/src/core/mount.h index 3b343c6b1f..da529c44f4 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -39,6 +39,7 @@ typedef enum MountResult { MOUNT_FAILURE_EXIT_CODE, MOUNT_FAILURE_SIGNAL, MOUNT_FAILURE_CORE_DUMP, + MOUNT_FAILURE_START_LIMIT_HIT, _MOUNT_RESULT_MAX, _MOUNT_RESULT_INVALID = -1 } MountResult; diff --git a/src/core/path.c b/src/core/path.c index 5e7b3eb234..0dd0d375d8 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -560,6 +560,7 @@ static void path_mkdir(Path *p) { static int path_start(Unit *u) { Path *p = PATH(u); Unit *trigger; + int r; assert(p); assert(p->state == PATH_DEAD || p->state == PATH_FAILED); @@ -570,6 +571,12 @@ static int path_start(Unit *u) { return -ENOENT; } + r = unit_start_limit_test(u); + if (r < 0) { + path_enter_dead(p, PATH_FAILURE_START_LIMIT_HIT); + return r; + } + path_mkdir(p); p->result = PATH_SUCCESS; @@ -739,6 +746,7 @@ DEFINE_STRING_TABLE_LOOKUP(path_type, PathType); static const char* const path_result_table[_PATH_RESULT_MAX] = { [PATH_SUCCESS] = "success", [PATH_FAILURE_RESOURCES] = "resources", + [PATH_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult); diff --git a/src/core/path.h b/src/core/path.h index bbbcebd78e..4230c8fb99 100644 --- a/src/core/path.h +++ b/src/core/path.h @@ -62,6 +62,7 @@ static inline bool path_spec_owns_inotify_fd(PathSpec *s, int fd) { typedef enum PathResult { PATH_SUCCESS, PATH_FAILURE_RESOURCES, + PATH_FAILURE_START_LIMIT_HIT, _PATH_RESULT_MAX, _PATH_RESULT_INVALID = -1 } PathResult; diff --git a/src/core/service.c b/src/core/service.c index f7a3fcf2b9..7ebabca5d6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1957,6 +1957,7 @@ fail: static int service_start(Unit *u) { Service *s = SERVICE(u); + int r; assert(s); @@ -1983,6 +1984,13 @@ static int service_start(Unit *u) { assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)); + /* Make sure we don't enter a busy loop of some kind. */ + r = unit_start_limit_test(u); + if (r < 0) { + service_enter_dead(s, SERVICE_FAILURE_START_LIMIT_HIT, false); + return r; + } + s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; s->main_pid_known = false; @@ -3266,6 +3274,7 @@ static const char* const service_result_table[_SERVICE_RESULT_MAX] = { [SERVICE_FAILURE_SIGNAL] = "signal", [SERVICE_FAILURE_CORE_DUMP] = "core-dump", [SERVICE_FAILURE_WATCHDOG] = "watchdog", + [SERVICE_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(service_result, ServiceResult); diff --git a/src/core/service.h b/src/core/service.h index c7f1e81bdb..4af3d40439 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -86,6 +86,7 @@ typedef enum ServiceResult { SERVICE_FAILURE_SIGNAL, SERVICE_FAILURE_CORE_DUMP, SERVICE_FAILURE_WATCHDOG, + SERVICE_FAILURE_START_LIMIT_HIT, _SERVICE_RESULT_MAX, _SERVICE_RESULT_INVALID = -1 } ServiceResult; diff --git a/src/core/socket.c b/src/core/socket.c index 7eeed068bd..c500d122d8 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2057,6 +2057,7 @@ fail: static int socket_start(Unit *u) { Socket *s = SOCKET(u); + int r; assert(s); @@ -2101,6 +2102,12 @@ static int socket_start(Unit *u) { assert(s->state == SOCKET_DEAD || s->state == SOCKET_FAILED); + r = unit_start_limit_test(u); + if (r < 0) { + socket_enter_dead(s, SOCKET_FAILURE_START_LIMIT_HIT); + return r; + } + s->result = SOCKET_SUCCESS; s->reset_cpu_usage = true; @@ -2818,6 +2825,7 @@ static const char* const socket_result_table[_SOCKET_RESULT_MAX] = { [SOCKET_FAILURE_EXIT_CODE] = "exit-code", [SOCKET_FAILURE_SIGNAL] = "signal", [SOCKET_FAILURE_CORE_DUMP] = "core-dump", + [SOCKET_FAILURE_START_LIMIT_HIT] = "start-limit-hit", [SOCKET_FAILURE_TRIGGER_LIMIT_HIT] = "trigger-limit-hit", [SOCKET_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit" }; diff --git a/src/core/socket.h b/src/core/socket.h index 2a4b1bb674..0f1ac69c6f 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -52,6 +52,7 @@ typedef enum SocketResult { SOCKET_FAILURE_EXIT_CODE, SOCKET_FAILURE_SIGNAL, SOCKET_FAILURE_CORE_DUMP, + SOCKET_FAILURE_START_LIMIT_HIT, SOCKET_FAILURE_TRIGGER_LIMIT_HIT, SOCKET_FAILURE_SERVICE_START_LIMIT_HIT, _SOCKET_RESULT_MAX, diff --git a/src/core/swap.c b/src/core/swap.c index d8802470d2..300911866f 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -814,6 +814,7 @@ fail: static int swap_start(Unit *u) { Swap *s = SWAP(u), *other; + int r; assert(s); @@ -842,6 +843,12 @@ static int swap_start(Unit *u) { if (UNIT(other)->job && UNIT(other)->job->state == JOB_RUNNING) return -EAGAIN; + r = unit_start_limit_test(u); + if (r < 0) { + swap_enter_dead(s, SWAP_FAILURE_START_LIMIT_HIT); + return r; + } + s->result = SWAP_SUCCESS; s->reset_cpu_usage = true; @@ -1447,7 +1454,8 @@ static const char* const swap_result_table[_SWAP_RESULT_MAX] = { [SWAP_FAILURE_TIMEOUT] = "timeout", [SWAP_FAILURE_EXIT_CODE] = "exit-code", [SWAP_FAILURE_SIGNAL] = "signal", - [SWAP_FAILURE_CORE_DUMP] = "core-dump" + [SWAP_FAILURE_CORE_DUMP] = "core-dump", + [SWAP_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(swap_result, SwapResult); diff --git a/src/core/swap.h b/src/core/swap.h index ac7a63d81b..fbf66debdc 100644 --- a/src/core/swap.h +++ b/src/core/swap.h @@ -38,6 +38,7 @@ typedef enum SwapResult { SWAP_FAILURE_EXIT_CODE, SWAP_FAILURE_SIGNAL, SWAP_FAILURE_CORE_DUMP, + SWAP_FAILURE_START_LIMIT_HIT, _SWAP_RESULT_MAX, _SWAP_RESULT_INVALID = -1 } SwapResult; diff --git a/src/core/timer.c b/src/core/timer.c index f8f5f4b2e4..3206296f09 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -599,6 +599,7 @@ static int timer_start(Unit *u) { Timer *t = TIMER(u); TimerValue *v; Unit *trigger; + int r; assert(t); assert(t->state == TIMER_DEAD || t->state == TIMER_FAILED); @@ -609,6 +610,12 @@ static int timer_start(Unit *u) { return -ENOENT; } + r = unit_start_limit_test(u); + if (r < 0) { + timer_enter_dead(t, TIMER_FAILURE_START_LIMIT_HIT); + return r; + } + t->last_trigger = DUAL_TIMESTAMP_NULL; /* Reenable all timers that depend on unit activation time */ @@ -808,7 +815,8 @@ DEFINE_STRING_TABLE_LOOKUP(timer_base, TimerBase); static const char* const timer_result_table[_TIMER_RESULT_MAX] = { [TIMER_SUCCESS] = "success", - [TIMER_FAILURE_RESOURCES] = "resources" + [TIMER_FAILURE_RESOURCES] = "resources", + [TIMER_FAILURE_START_LIMIT_HIT] = "start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(timer_result, TimerResult); diff --git a/src/core/timer.h b/src/core/timer.h index 698e6da2f5..9c4b64f898 100644 --- a/src/core/timer.h +++ b/src/core/timer.h @@ -48,6 +48,7 @@ typedef struct TimerValue { typedef enum TimerResult { TIMER_SUCCESS, TIMER_FAILURE_RESOURCES, + TIMER_FAILURE_START_LIMIT_HIT, _TIMER_RESULT_MAX, _TIMER_RESULT_INVALID = -1 } TimerResult; diff --git a/src/core/unit.c b/src/core/unit.c index 64466e4fb4..fd9ecc36ce 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1462,7 +1462,7 @@ void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) { unit_status_print_starting_stopping(u, t); } -static int unit_start_limit_test(Unit *u) { +int unit_start_limit_test(Unit *u) { assert(u); if (ratelimit_test(&u->start_limit)) { @@ -1488,7 +1488,6 @@ static int unit_start_limit_test(Unit *u) { int unit_start(Unit *u) { UnitActiveState state; Unit *following; - int r; assert(u); @@ -1541,11 +1540,6 @@ int unit_start(Unit *u) { if (!UNIT_VTABLE(u)->start) return -EBADR; - /* Make sure we don't enter a busy loop of some kind. */ - r = unit_start_limit_test(u); - if (r < 0) - return r; - /* We don't suppress calls to ->start() here when we are * already starting, to allow this request to be used as a * "hurry up" call, for example when the unit is in some "auto diff --git a/src/core/unit.h b/src/core/unit.h index 5909652976..6ae1a8984a 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -617,6 +617,8 @@ static inline bool unit_supported(Unit *u) { void unit_warn_if_dir_nonempty(Unit *u, const char* where); int unit_fail_if_symlink(Unit *u, const char* where); +int unit_start_limit_test(Unit *u); + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full(unit, level, error, ...) \ -- cgit v1.2.3-54-g00ecf From 87ec20ef2089712e6c9a9f3ddfba6c5e312694fe Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 14:50:27 +0200 Subject: core: fix detection whether per-unit drop-ins changed This fixes fall-out from 6d10d308c6cd16528ef58fa4f5822aef936862d3. Until that commit, do determine whether a daemon reload was required we compare the mtime of the main unit file we loaded with the mtime of it on disk for equality, but for drop-ins we only stored the newest mtime of all of them and then did a "newer-than" comparison. This was brokeni with the above commit, when all checks where changed to be for equality. With this change all checks are now done as "newer-than", fixing the drop-in mtime case. Strictly speaking this will not detect a number of changes that the code before above commit detected, but given that the mtime is unlikely to go backwards, and this is just intended to be a helpful hint anyway, this looks OK in order to keep things simple. Fixes: #3123 --- src/core/unit.c | 10 +++++----- src/systemctl/systemctl.c | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'src/core') diff --git a/src/core/unit.c b/src/core/unit.c index fd9ecc36ce..b4d313a71b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2945,7 +2945,7 @@ int unit_coldplug(Unit *u) { return 0; } -static bool fragment_mtime_changed(const char *path, usec_t mtime) { +static bool fragment_mtime_newer(const char *path, usec_t mtime) { struct stat st; if (!path) @@ -2957,7 +2957,7 @@ static bool fragment_mtime_changed(const char *path, usec_t mtime) { if (mtime > 0) /* For non-empty files check the mtime */ - return timespec_load(&st.st_mtim) != mtime; + return timespec_load(&st.st_mtim) > mtime; else if (!null_or_empty(&st)) /* For masked files check if they are still so */ return true; @@ -2972,8 +2972,8 @@ bool unit_need_daemon_reload(Unit *u) { assert(u); - if (fragment_mtime_changed(u->fragment_path, u->fragment_mtime) || - fragment_mtime_changed(u->source_path, u->source_mtime)) + if (fragment_mtime_newer(u->fragment_path, u->fragment_mtime) || + fragment_mtime_newer(u->source_path, u->source_mtime)) return true; (void) unit_find_dropin_paths(u, &t); @@ -2986,7 +2986,7 @@ bool unit_need_daemon_reload(Unit *u) { if (strv_overlap(u->dropin_paths, t)) { STRV_FOREACH(path, u->dropin_paths) - if (fragment_mtime_changed(*path, u->dropin_mtime)) + if (fragment_mtime_newer(*path, u->dropin_mtime)) return true; return false; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9af25e22a4..bec4f31b39 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2334,6 +2334,8 @@ static int need_daemon_reload(sd_bus *bus, const char *unit) { } static void warn_unit_file_changed(const char *name) { + assert(name); + log_warning("%sWarning:%s %s changed on disk. Run 'systemctl%s daemon-reload' to reload units.", ansi_highlight_red(), ansi_normal(), -- cgit v1.2.3-54-g00ecf From ab932a622d57fd327ef95992c343fd4425324088 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 15:07:40 +0200 Subject: core: simplify unit_need_daemon_reload() a bit And let's make it more accurate: if we have acquire the list of unit drop-ins, then let's do a full comparison against the old list we already have, and if things differ in any way, we know we have to reload. This makes sure we detect changes to drop-in directories in more cases. --- src/core/unit.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) (limited to 'src/core') diff --git a/src/core/unit.c b/src/core/unit.c index b4d313a71b..93aead0489 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2968,32 +2968,24 @@ static bool fragment_mtime_newer(const char *path, usec_t mtime) { bool unit_need_daemon_reload(Unit *u) { _cleanup_strv_free_ char **t = NULL; char **path; - unsigned loaded_cnt, current_cnt; assert(u); - if (fragment_mtime_newer(u->fragment_path, u->fragment_mtime) || - fragment_mtime_newer(u->source_path, u->source_mtime)) + if (fragment_mtime_newer(u->fragment_path, u->fragment_mtime)) return true; - (void) unit_find_dropin_paths(u, &t); - loaded_cnt = strv_length(t); - current_cnt = strv_length(u->dropin_paths); - - if (loaded_cnt == current_cnt) { - if (loaded_cnt == 0) - return false; + if (fragment_mtime_newer(u->source_path, u->source_mtime)) + return true; - if (strv_overlap(u->dropin_paths, t)) { - STRV_FOREACH(path, u->dropin_paths) - if (fragment_mtime_newer(*path, u->dropin_mtime)) - return true; + (void) unit_find_dropin_paths(u, &t); + if (!strv_equal(u->dropin_paths, t)) + return true; - return false; - } - } + STRV_FOREACH(path, u->dropin_paths) + if (fragment_mtime_newer(*path, u->dropin_mtime)) + return true; - return true; + return false; } void unit_reset_failed(Unit *u) { -- cgit v1.2.3-54-g00ecf From 7b2fd9d51259f6cf350791434e640ac3519acc6c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 16:01:16 +0200 Subject: core: remove duplicate code in automount_update_mount() Also, fix indentation. --- src/core/automount.c | 39 +++++++++++---------------------------- 1 file changed, 11 insertions(+), 28 deletions(-) (limited to 'src/core') diff --git a/src/core/automount.c b/src/core/automount.c index c871b02a94..464d6a70c0 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -477,39 +477,22 @@ int automount_update_mount(Automount *a, MountState old_state, MountState state) if (r < 0) log_unit_warning_errno(UNIT(a), r, "Failed to start expiration timer, ignoring: %m"); break; - case MOUNT_DEAD: - case MOUNT_UNMOUNTING: - case MOUNT_MOUNTING_SIGTERM: - case MOUNT_MOUNTING_SIGKILL: - case MOUNT_REMOUNTING_SIGTERM: - case MOUNT_REMOUNTING_SIGKILL: - case MOUNT_UNMOUNTING_SIGTERM: - case MOUNT_UNMOUNTING_SIGKILL: - case MOUNT_FAILED: + + case MOUNT_DEAD: + case MOUNT_UNMOUNTING: + case MOUNT_MOUNTING_SIGTERM: + case MOUNT_MOUNTING_SIGKILL: + case MOUNT_REMOUNTING_SIGTERM: + case MOUNT_REMOUNTING_SIGKILL: + case MOUNT_UNMOUNTING_SIGTERM: + case MOUNT_UNMOUNTING_SIGKILL: + case MOUNT_FAILED: if (old_state != state) automount_send_ready(a, a->tokens, -ENODEV); + (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); break; - default: - break; - } - switch (state) { - case MOUNT_DEAD: - automount_send_ready(a, a->expire_tokens, 0); - break; - case MOUNT_MOUNTING: - case MOUNT_MOUNTING_DONE: - case MOUNT_MOUNTING_SIGTERM: - case MOUNT_MOUNTING_SIGKILL: - case MOUNT_REMOUNTING_SIGTERM: - case MOUNT_REMOUNTING_SIGKILL: - case MOUNT_UNMOUNTING_SIGTERM: - case MOUNT_UNMOUNTING_SIGKILL: - case MOUNT_FAILED: - if (old_state != state) - automount_send_ready(a, a->expire_tokens, -ENODEV); - break; default: break; } -- cgit v1.2.3-54-g00ecf From 9703a8adb5c2ef16fcbf0e92189494a5a3438b06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 16:01:39 +0200 Subject: automount: add debug message when we get notified about mount state changes --- src/core/automount.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/automount.c b/src/core/automount.c index 464d6a70c0..d2386d04f7 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -408,7 +408,7 @@ static int autofs_send_ready(int dev_autofs_fd, int ioctl_fd, uint32_t token, in init_autofs_dev_ioctl(¶m); param.ioctlfd = ioctl_fd; - if (status) { + if (status != 0) { param.fail.token = token; param.fail.status = status; } else @@ -435,7 +435,7 @@ static int automount_send_ready(Automount *a, Set *tokens, int status) { if (ioctl_fd < 0) return ioctl_fd; - if (status) + if (status != 0) log_unit_debug_errno(UNIT(a), status, "Sending failure: %m"); else log_unit_debug(UNIT(a), "Sending success."); @@ -469,7 +469,10 @@ int automount_update_mount(Automount *a, MountState old_state, MountState state) assert(a); + log_unit_debug(UNIT(a), "Got notified about mount unit state change %s → %s", mount_state_to_string(old_state), mount_state_to_string(state)); + switch (state) { + case MOUNT_MOUNTED: case MOUNT_REMOUNTING: automount_send_ready(a, a->tokens, 0); -- cgit v1.2.3-54-g00ecf From d14e3a0de913cd5ef52693a9466129820322cff3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 16:41:41 +0200 Subject: core: don't propagate service state to sockets as long as there's still a job for the service queued --- src/core/socket.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/core') diff --git a/src/core/socket.c b/src/core/socket.c index c500d122d8..d3d4866fe6 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2742,17 +2742,26 @@ static void socket_trigger_notify(Unit *u, Unit *other) { assert(u); assert(other); - /* Don't propagate state changes from the service if we are - already down or accepting connections */ - if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING) || s->accept) + /* Filter out invocations with bogus state */ + if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE) + return; + + /* Don't propagate state changes from the service if we are already down */ + if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING)) return; + /* We don't care for the service state if we are in Accept=yes mode */ + if (s->accept) + return; + + /* Propagate start limit hit state */ if (other->start_limit_hit) { socket_enter_stop_pre(s, SOCKET_FAILURE_SERVICE_START_LIMIT_HIT); return; } - if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE) + /* Don't propagate anything if there's still a job queued */ + if (other->job) return; if (IN_SET(SERVICE(other)->state, -- cgit v1.2.3-54-g00ecf From fae03ed32a77934a5c39ed8e338ec6c7a75857a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 16:51:45 +0200 Subject: automount: rework propagation between automount and mount units Port the progagation logic to the generic Unit->trigger_notify() callback logic in the unit vtable, that is called for a unit not only when the triggered unit of it changes state but also when a job for that unit finishes. This, firstly allows us to make the code a bit cleaner and more generic, but more importantly, allows us to notice correctly when a mount job fails, and propagate that back to autofs client processes. Fixes: #2181 --- src/core/automount.c | 65 +++++++++++++++++++++++++++++++++------------------- src/core/automount.h | 3 +-- src/core/mount.c | 19 --------------- 3 files changed, 42 insertions(+), 45 deletions(-) (limited to 'src/core') diff --git a/src/core/automount.c b/src/core/automount.c index d2386d04f7..5577c64255 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -464,43 +464,57 @@ static int automount_send_ready(Automount *a, Set *tokens, int status) { static int automount_start_expire(Automount *a); -int automount_update_mount(Automount *a, MountState old_state, MountState state) { +static void automount_trigger_notify(Unit *u, Unit *other) { + Automount *a = AUTOMOUNT(u); int r; assert(a); + assert(other); + + /* Filter out invocations with bogus state */ + if (other->load_state != UNIT_LOADED || other->type != UNIT_MOUNT) + return; + + /* Don't propagate state changes from the mount if we are already down */ + if (!IN_SET(a->state, AUTOMOUNT_WAITING, AUTOMOUNT_RUNNING)) + return; - log_unit_debug(UNIT(a), "Got notified about mount unit state change %s → %s", mount_state_to_string(old_state), mount_state_to_string(state)); + /* Propagate start limit hit state */ + if (other->start_limit_hit) { + automount_enter_dead(a, AUTOMOUNT_FAILURE_MOUNT_START_LIMIT_HIT); + return; + } - switch (state) { + /* Don't propagate anything if there's still a job queued */ + if (other->job) + return; + + /* The mount is successfully established */ + if (IN_SET(MOUNT(other)->state, MOUNT_MOUNTED, MOUNT_REMOUNTING)) { + (void) automount_send_ready(a, a->tokens, 0); - case MOUNT_MOUNTED: - case MOUNT_REMOUNTING: - automount_send_ready(a, a->tokens, 0); r = automount_start_expire(a); if (r < 0) log_unit_warning_errno(UNIT(a), r, "Failed to start expiration timer, ignoring: %m"); - break; - case MOUNT_DEAD: - case MOUNT_UNMOUNTING: - case MOUNT_MOUNTING_SIGTERM: - case MOUNT_MOUNTING_SIGKILL: - case MOUNT_REMOUNTING_SIGTERM: - case MOUNT_REMOUNTING_SIGKILL: - case MOUNT_UNMOUNTING_SIGTERM: - case MOUNT_UNMOUNTING_SIGKILL: - case MOUNT_FAILED: - if (old_state != state) - automount_send_ready(a, a->tokens, -ENODEV); + automount_set_state(a, AUTOMOUNT_RUNNING); + } - (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); - break; + /* 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)) { - default: - break; - } + (void) automount_send_ready(a, a->tokens, -ENODEV); - return 0; + if (a->expire_event_source) + (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); + + automount_set_state(a, AUTOMOUNT_WAITING); + } } static void automount_enter_waiting(Automount *a) { @@ -1032,6 +1046,7 @@ static const char* const automount_result_table[_AUTOMOUNT_RESULT_MAX] = { [AUTOMOUNT_SUCCESS] = "success", [AUTOMOUNT_FAILURE_RESOURCES] = "resources", [AUTOMOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit", + [AUTOMOUNT_FAILURE_MOUNT_START_LIMIT_HIT] = "mount-start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(automount_result, AutomountResult); @@ -1066,6 +1081,8 @@ const UnitVTable automount_vtable = { .check_gc = automount_check_gc, + .trigger_notify = automount_trigger_notify, + .reset_failed = automount_reset_failed, .bus_vtable = bus_automount_vtable, diff --git a/src/core/automount.h b/src/core/automount.h index 414717e5b8..76a201178e 100644 --- a/src/core/automount.h +++ b/src/core/automount.h @@ -27,6 +27,7 @@ typedef enum AutomountResult { AUTOMOUNT_SUCCESS, AUTOMOUNT_FAILURE_RESOURCES, AUTOMOUNT_FAILURE_START_LIMIT_HIT, + AUTOMOUNT_FAILURE_MOUNT_START_LIMIT_HIT, _AUTOMOUNT_RESULT_MAX, _AUTOMOUNT_RESULT_INVALID = -1 } AutomountResult; @@ -54,7 +55,5 @@ struct Automount { extern const UnitVTable automount_vtable; -int automount_update_mount(Automount *a, MountState old_state, MountState state); - const char* automount_result_to_string(AutomountResult i) _const_; AutomountResult automount_result_from_string(const char *s) _pure_; diff --git a/src/core/mount.c b/src/core/mount.c index aa48941f0d..037f3684c7 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -584,23 +584,6 @@ static int mount_load(Unit *u) { return mount_verify(m); } -static int mount_notify_automount(Mount *m, MountState old_state, MountState state) { - Unit *p; - int r; - Iterator i; - - assert(m); - - SET_FOREACH(p, UNIT(m)->dependencies[UNIT_TRIGGERED_BY], i) - if (p->type == UNIT_AUTOMOUNT) { - r = automount_update_mount(AUTOMOUNT(p), old_state, state); - if (r < 0) - return r; - } - - return 0; -} - static void mount_set_state(Mount *m, MountState state) { MountState old_state; assert(m); @@ -624,8 +607,6 @@ static void mount_set_state(Mount *m, MountState state) { m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID; } - mount_notify_automount(m, old_state, state); - if (state != old_state) log_unit_debug(UNIT(m), "Changed %s -> %s", mount_state_to_string(old_state), mount_state_to_string(state)); -- cgit v1.2.3-54-g00ecf From dbb0578edc5ab8e11641c8b2d29904d4f5f8e1e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 May 2016 17:12:35 +0200 Subject: automount: move resetting of expiry timeout to automount_set_state() that way we can be sure that there's no expiry timeout in place at any time when we aren't in the RUNNING state. --- src/core/automount.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'src/core') diff --git a/src/core/automount.c b/src/core/automount.c index 5577c64255..e2590a22bc 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -75,6 +75,9 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(struct expire_data*, expire_data_free); static int open_dev_autofs(Manager *m); static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, void *userdata); +static int automount_start_expire(Automount *a); +static void automount_stop_expire(Automount *a); +static int automount_send_ready(Automount *a, Set *tokens, int status); static void automount_init(Unit *u) { Automount *a = AUTOMOUNT(u); @@ -87,8 +90,6 @@ static void automount_init(Unit *u) { UNIT(a)->ignore_on_isolate = true; } -static int automount_send_ready(Automount *a, Set *tokens, int status); - static void unmount_autofs(Automount *a) { int r; @@ -235,6 +236,9 @@ static void automount_set_state(Automount *a, AutomountState state) { old_state = a->state; a->state = state; + if (state != AUTOMOUNT_RUNNING) + automount_stop_expire(a); + if (state != AUTOMOUNT_WAITING && state != AUTOMOUNT_RUNNING) unmount_autofs(a); @@ -462,8 +466,6 @@ static int automount_send_ready(Automount *a, Set *tokens, int status) { return r; } -static int automount_start_expire(Automount *a); - static void automount_trigger_notify(Unit *u, Unit *other) { Automount *a = AUTOMOUNT(u); int r; @@ -510,9 +512,6 @@ static void automount_trigger_notify(Unit *u, Unit *other) { (void) automount_send_ready(a, a->tokens, -ENODEV); - if (a->expire_event_source) - (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); - automount_set_state(a, AUTOMOUNT_WAITING); } } @@ -699,6 +698,15 @@ static int automount_start_expire(Automount *a) { return 0; } +static void automount_stop_expire(Automount *a) { + assert(a); + + if (!a->expire_event_source) + return; + + (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); +} + static void automount_enter_runnning(Automount *a) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; struct stat st; @@ -965,7 +973,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo case autofs_ptype_expire_direct: log_unit_debug(UNIT(a), "Got direct umount request on %s", a->where); - (void) sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF); + automount_stop_expire(a); r = set_ensure_allocated(&a->expire_tokens, NULL); if (r < 0) { -- cgit v1.2.3-54-g00ecf