From 10691b9e50df326891cca974bdca686883cd7869 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 13:09:42 +0200 Subject: [PATCH 01/12] service: clean up logging a bit This rearranges various cases of "goto fail" in service.c: sometimes the whole "goto fail" logic was redundant, since only jumped to form a single place. Sometimes the log message was generated in the fail section, instead of the place jumped to from, which resulted in duplicate or misleading error messages. No real codeflow changes, just refactoring primarily around log messages. --- src/core/service.c | 159 +++++++++++++++++++++------------------------ 1 file changed, 73 insertions(+), 86 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index edb39c6df6a..237edd6355c 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2037,8 +2037,11 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) restart_usec_next = service_restart_usec_next(s); r = service_arm_timer(s, /* relative= */ true, restart_usec_next); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install restart timer: %m"); + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false); + return; + } log_unit_debug(UNIT(s), "Next restart interval calculated as: %s", FORMAT_TIMESPAN(restart_usec_next, 0)); @@ -2082,12 +2085,6 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) /* Reset TTY ownership if necessary */ exec_context_revert_tty(&s->exec_context); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run install restart timer: %m"); - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, false); } static void service_enter_stop_post(Service *s, ServiceResult f) { @@ -2110,18 +2107,15 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { s->timeout_stop_usec, EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_IS_CONTROL|EXEC_SETENV_RESULT|EXEC_CONTROL_CGROUP, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop-post' task: %m"); + service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_FAILURE_RESOURCES); + return; + } service_set_state(s, SERVICE_STOP_POST); } else service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_SUCCESS); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'stop-post' task: %m"); - service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_FAILURE_RESOURCES); } static int state_to_kill_operation(Service *s, ServiceState state) { @@ -2171,14 +2165,18 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f &s->main_pid, &s->control_pid, s->main_pid_alien); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); goto fail; + } if (r > 0) { r = service_arm_timer(s, /* relative= */ true, kill_operation == KILL_WATCHDOG ? service_timeout_abort_usec(s) : s->timeout_stop_usec); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; + } service_set_state(s, state); } else if (IN_SET(state, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM) && s->kill_context.send_sigkill) @@ -2188,17 +2186,15 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f else if (IN_SET(state, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM) && s->kill_context.send_sigkill) service_enter_signal(s, SERVICE_FINAL_SIGKILL, SERVICE_SUCCESS); else - service_enter_dead(s, SERVICE_SUCCESS, true); + service_enter_dead(s, SERVICE_SUCCESS, /* allow_restart= */ true); return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); - if (IN_SET(state, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL)) service_enter_stop_post(s, SERVICE_FAILURE_RESOURCES); else - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, true); + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); } static void service_enter_stop_by_notify(Service *s) { @@ -2233,18 +2229,15 @@ static void service_enter_stop(Service *s, ServiceResult f) { s->timeout_stop_usec, EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_SETENV_RESULT|EXEC_CONTROL_CGROUP, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop' task: %m"); + service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); + return; + } service_set_state(s, SERVICE_STOP); } else service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_SUCCESS); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'stop' task: %m"); - service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); } static bool service_good(Service *s) { @@ -2312,18 +2305,15 @@ static void service_enter_start_post(Service *s) { s->timeout_start_usec, EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_CONTROL_CGROUP, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-post' task: %m"); + service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + return; + } service_set_state(s, SERVICE_START_POST); } else service_enter_running(s, SERVICE_SUCCESS); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-post' task: %m"); - service_enter_stop(s, SERVICE_FAILURE_RESOURCES); } static void service_kill_control_process(Service *s) { @@ -2425,8 +2415,10 @@ static void service_enter_start(Service *s) { timeout, EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG|EXEC_WRITE_CREDENTIALS|EXEC_SETENV_MONITOR_RESULT, &pidref); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start' task: %m"); goto fail; + } if (IN_SET(s->type, SERVICE_SIMPLE, SERVICE_IDLE)) { /* For simple services we immediately start @@ -2459,7 +2451,6 @@ static void service_enter_start(Service *s) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'start' task: %m"); service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); } @@ -2484,8 +2475,10 @@ static void service_enter_start_pre(Service *s) { s->timeout_start_usec, EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_APPLY_TTY_STDIN|EXEC_SETENV_MONITOR_RESULT|EXEC_WRITE_CREDENTIALS, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-pre' task: %m"); goto fail; + } service_set_state(s, SERVICE_START_PRE); } else @@ -2494,8 +2487,7 @@ static void service_enter_start_pre(Service *s) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-pre' task: %m"); - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, true); + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); } static void service_enter_condition(Service *s) { @@ -2521,8 +2513,10 @@ static void service_enter_condition(Service *s) { EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_APPLY_TTY_STDIN, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'exec-condition' task: %m"); goto fail; + } service_set_state(s, SERVICE_CONDITION); } else @@ -2531,8 +2525,7 @@ static void service_enter_condition(Service *s) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'exec-condition' task: %m"); - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, true); + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); } static void service_enter_restart(Service *s) { @@ -2550,8 +2543,11 @@ static void service_enter_restart(Service *s) { /* Any units that are bound to this service must also be restarted. We use JOB_START for ourselves * but then set JOB_RESTART_DEPENDENCIES which will enqueue JOB_RESTART for those dependency jobs. */ r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(s), JOB_RESTART_DEPENDENCIES, NULL, &error, NULL); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r)); + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ false); + return; + } /* Count the jobs we enqueue for restarting. This counter is maintained as long as the unit isn't * fully stopped, i.e. as long as it remains up or remains in auto-start states. The user can reset @@ -2572,11 +2568,6 @@ static void service_enter_restart(Service *s) { /* Notify clients about changed restart counter */ unit_add_to_dbus_queue(UNIT(s)); - return; - -fail: - log_unit_warning(UNIT(s), "Failed to schedule restart job: %s", bus_error_message(&error, r)); - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, false); } static void service_enter_reload_by_notify(Service *s) { @@ -2626,7 +2617,7 @@ static void service_enter_reload(Service *s) { EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_CONTROL_CGROUP, &s->control_pid); if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to run 'reload' task: %m"); + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'reload' task: %m"); goto fail; } @@ -2681,23 +2672,19 @@ static void service_run_next_control(Service *s) { (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_START) ? EXEC_SETENV_MONITOR_RESULT : 0)| (IN_SET(s->control_command_id, SERVICE_EXEC_START_POST, SERVICE_EXEC_RELOAD, SERVICE_EXEC_STOP, SERVICE_EXEC_STOP_POST) ? EXEC_CONTROL_CGROUP : 0), &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn next control task: %m"); - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run next control task: %m"); - - if (IN_SET(s->state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START_POST, SERVICE_STOP)) - service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); - else if (s->state == SERVICE_STOP_POST) - service_enter_dead(s, SERVICE_FAILURE_RESOURCES, true); - else if (s->state == SERVICE_RELOAD) { - s->reload_result = SERVICE_FAILURE_RESOURCES; - service_enter_running(s, SERVICE_SUCCESS); - } else - service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + if (IN_SET(s->state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START_POST, SERVICE_STOP)) + service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); + else if (s->state == SERVICE_STOP_POST) + service_enter_dead(s, SERVICE_FAILURE_RESOURCES, /* allow_restart= */ true); + else if (s->state == SERVICE_RELOAD) { + s->reload_result = SERVICE_FAILURE_RESOURCES; + service_enter_running(s, SERVICE_SUCCESS); + } else + service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + } } static void service_run_next_main(Service *s) { @@ -2717,15 +2704,13 @@ static void service_run_next_main(Service *s) { s->timeout_start_usec, EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG|EXEC_SETENV_MONITOR_RESULT|EXEC_WRITE_CREDENTIALS, &pidref); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn next main task: %m"); + service_enter_stop(s, SERVICE_FAILURE_RESOURCES); + return; + } (void) service_set_main_pidref(s, &pidref); - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run next main task: %m"); - service_enter_stop(s, SERVICE_FAILURE_RESOURCES); } static int service_start(Unit *u) { @@ -3487,18 +3472,17 @@ static int service_watch_pid_file(Service *s) { log_unit_debug(UNIT(s), "Setting watch for PID file %s", s->pid_file_pathspec->path); r = path_spec_watch(s->pid_file_pathspec, service_dispatch_inotify_io); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_error_errno(UNIT(s), r, "Failed to set a watch for PID file %s: %m", s->pid_file_pathspec->path); + service_unwatch_pid_file(s); + return r; + } /* the pidfile might have appeared just before we set the watch */ log_unit_debug(UNIT(s), "Trying to read PID file %s in case it changed", s->pid_file_pathspec->path); service_retry_pid_file(s); return 0; -fail: - log_unit_error_errno(UNIT(s), r, "Failed to set a watch for PID file %s: %m", s->pid_file_pathspec->path); - service_unwatch_pid_file(s); - return r; } static int service_demand_pid_file(Service *s) { @@ -4929,18 +4913,21 @@ static int service_clean(Unit *u, ExecCleanMask mask) { s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; r = service_arm_timer(s, /* relative= */ true, s->exec_context.timeout_clean_usec); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; + } r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to spawn cleaning task: %m"); goto fail; + } service_set_state(s, SERVICE_CLEANING); return 0; fail: - log_unit_warning_errno(u, r, "Failed to initiate cleaning: %m"); s->clean_result = SERVICE_FAILURE_RESOURCES; s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); return r; From 3dde96d85471ed9f33d40cb40f7a3797185ff4c6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 13:11:39 +0200 Subject: [PATCH 02/12] service: add error handling for all service_arm_timer() invocations Let's clean this up a bit, and catch all errors and do something reasonable in case this happens. --- src/core/service.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 237edd6355c..f2f6dffc02b 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2198,11 +2198,18 @@ fail: } static void service_enter_stop_by_notify(Service *s) { + int r; + assert(s); (void) unit_enqueue_rewatch_pids(UNIT(s)); - service_arm_timer(s, /* relative= */ true, s->timeout_stop_usec); + r = service_arm_timer(s, /* relative= */ true, s->timeout_stop_usec); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); + return; + } /* The service told us it's stopping, so it's as if we SIGTERM'd it. */ service_set_state(s, SERVICE_STOP_SIGTERM); @@ -2261,6 +2268,8 @@ static bool service_good(Service *s) { } static void service_enter_running(Service *s, ServiceResult f) { + int r; + assert(s); if (s->result == SERVICE_SUCCESS) @@ -2279,7 +2288,13 @@ static void service_enter_running(Service *s, ServiceResult f) { service_enter_stop_by_notify(s); else { service_set_state(s, SERVICE_RUNNING); - service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); + + r = service_arm_timer(s, /* relative= */ false, service_running_timeout(s)); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + service_enter_running(s, SERVICE_FAILURE_RESOURCES); + return; + } } } else if (s->remain_after_exit) @@ -2576,13 +2591,20 @@ static void service_enter_reload_by_notify(Service *s) { assert(s); - service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + s->reload_result = SERVICE_FAILURE_RESOURCES; + service_enter_running(s, SERVICE_SUCCESS); + return; + } + service_set_state(s, SERVICE_RELOAD_NOTIFY); /* service_enter_reload_by_notify is never called during a reload, thus no loops are possible. */ r = manager_propagate_reload(UNIT(s)->manager, UNIT(s), JOB_FAIL, &error); if (r < 0) - log_unit_warning(UNIT(s), "Failed to schedule propagation of reload: %s", bus_error_message(&error, r)); + log_unit_warning(UNIT(s), "Failed to schedule propagation of reload, ignoring: %s", bus_error_message(&error, r)); } static void service_enter_reload(Service *s) { @@ -2623,7 +2645,12 @@ static void service_enter_reload(Service *s) { service_set_state(s, SERVICE_RELOAD); } else if (killed) { - service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); + goto fail; + } + service_set_state(s, SERVICE_RELOAD_SIGNAL); } else { service_enter_running(s, SERVICE_SUCCESS); From 529f768ab85917b5404754334ed0212076e562eb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Sep 2023 17:21:51 +0200 Subject: [PATCH 03/12] socket: drop redundant TAKE_FD(cfd) line In the only two codepaths we reach this place we know that cfd is already invalidated. In the Accept=yes case there's already a TAKE_FD() a few lines further up, and in the Accept=no case there is no connection fd anyway. --- src/core/socket.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/socket.c b/src/core/socket.c index 43f412bc895..0b5bfd26487 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2432,7 +2432,6 @@ static void socket_enter_running(Socket *s, int cfd_in) { unit_add_to_dbus_queue(UNIT(s)); } - TAKE_FD(cfd); return; refuse: From a951c1aea02ba313cb11a281db87a8a0a4a94de7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 15:07:26 +0200 Subject: [PATCH 04/12] socket: modernize socket_acquire_peer() a bit --- src/core/socket.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 0b5bfd26487..967e3f08457 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -506,7 +506,7 @@ static SocketPeer *socket_peer_free(SocketPeer *p) { DEFINE_TRIVIAL_REF_UNREF_FUNC(SocketPeer, socket_peer, socket_peer_free); -int socket_acquire_peer(Socket *s, int fd, SocketPeer **p) { +int socket_acquire_peer(Socket *s, int fd, SocketPeer **ret) { _cleanup_(socket_peer_unrefp) SocketPeer *remote = NULL; SocketPeer sa = { .peer_salen = sizeof(union sockaddr_union), @@ -515,22 +515,19 @@ int socket_acquire_peer(Socket *s, int fd, SocketPeer **p) { assert(fd >= 0); assert(s); + assert(ret); if (getpeername(fd, &sa.peer.sa, &sa.peer_salen) < 0) - return log_unit_error_errno(UNIT(s), errno, "getpeername failed: %m"); + return log_unit_error_errno(UNIT(s), errno, "getpeername() failed: %m"); if (!IN_SET(sa.peer.sa.sa_family, AF_INET, AF_INET6, AF_VSOCK)) { - *p = NULL; + *ret = NULL; return 0; } - r = set_ensure_allocated(&s->peers_by_address, &peer_address_hash_ops); - if (r < 0) - return r; - i = set_get(s->peers_by_address, &sa); if (i) { - *p = socket_peer_ref(i); + *ret = socket_peer_ref(i); return 1; } @@ -541,13 +538,13 @@ int socket_acquire_peer(Socket *s, int fd, SocketPeer **p) { remote->peer = sa.peer; remote->peer_salen = sa.peer_salen; - r = set_put(s->peers_by_address, remote); + r = set_ensure_put(&s->peers_by_address, &peer_address_hash_ops, remote); if (r < 0) - return r; + return log_unit_error_errno(UNIT(s), r, "Failed to insert peer info into hash table: %m"); remote->socket = s; - *p = TAKE_PTR(remote); + *ret = TAKE_PTR(remote); return 1; } From a237a93e6cc46b51b987f813e4fc293145e89e1d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 15:09:00 +0200 Subject: [PATCH 05/12] socket: clean up error message generation/fail paths also for the socket state engine --- src/core/socket.c | 119 +++++++++++++++++++++------------------------- 1 file changed, 55 insertions(+), 64 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 967e3f08457..715ebcbd89f 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2087,18 +2087,15 @@ static void socket_enter_stop_post(Socket *s, SocketResult f) { pidref_done(&s->control_pid); r = socket_spawn(s, s->control_command, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop-post' task: %m"); + socket_enter_signal(s, SOCKET_FINAL_SIGTERM, SOCKET_FAILURE_RESOURCES); + return; + } socket_set_state(s, SOCKET_STOP_POST); } else socket_enter_signal(s, SOCKET_FINAL_SIGTERM, SOCKET_SUCCESS); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'stop-post' task: %m"); - socket_enter_signal(s, SOCKET_FINAL_SIGTERM, SOCKET_FAILURE_RESOURCES); } static int state_to_kill_operation(Socket *s, SocketState state) { @@ -2126,13 +2123,17 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { /* main_pid= */ NULL, &s->control_pid, /* main_pid_alien= */ false); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); goto fail; + } if (r > 0) { r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; + } socket_set_state(s, state); } else if (state == SOCKET_STOP_PRE_SIGTERM) @@ -2147,8 +2148,6 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); - if (IN_SET(state, SOCKET_STOP_PRE_SIGTERM, SOCKET_STOP_PRE_SIGKILL)) socket_enter_stop_post(s, SOCKET_FAILURE_RESOURCES); else @@ -2170,18 +2169,15 @@ static void socket_enter_stop_pre(Socket *s, SocketResult f) { pidref_done(&s->control_pid); r = socket_spawn(s, s->control_command, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'stop-pre' task: %m"); + socket_enter_stop_post(s, SOCKET_FAILURE_RESOURCES); + return; + } socket_set_state(s, SOCKET_STOP_PRE); } else socket_enter_stop_post(s, SOCKET_SUCCESS); - - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'stop-pre' task: %m"); - socket_enter_stop_post(s, SOCKET_FAILURE_RESOURCES); } static void socket_enter_listening(Socket *s) { @@ -2196,14 +2192,11 @@ static void socket_enter_listening(Socket *s) { r = socket_watch_fds(s); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to watch sockets: %m"); - goto fail; + socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); + return; } socket_set_state(s, SOCKET_LISTENING); - return; - -fail: - socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); } static void socket_enter_start_post(Socket *s) { @@ -2219,18 +2212,14 @@ static void socket_enter_start_post(Socket *s) { r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-post' task: %m"); - goto fail; + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-post' task: %m"); + socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); + return; } socket_set_state(s, SOCKET_START_POST); } else socket_enter_listening(s); - - return; - -fail: - socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); } static void socket_enter_start_chown(Socket *s) { @@ -2252,7 +2241,7 @@ static void socket_enter_start_chown(Socket *s) { r = socket_chown(s, &s->control_pid); if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to fork 'start-chown' task: %m"); + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-chown' task: %m"); goto fail; } @@ -2282,18 +2271,14 @@ static void socket_enter_start_pre(Socket *s) { r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { - log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-pre' task: %m"); - goto fail; + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'start-pre' task: %m"); + socket_enter_dead(s, SOCKET_FAILURE_RESOURCES); + return; } socket_set_state(s, SOCKET_START_PRE); } else socket_enter_start_chown(s); - - return; - -fail: - socket_enter_dead(s, SOCKET_FAILURE_RESOURCES); } static void flush_ports(Socket *s) { @@ -2339,7 +2324,7 @@ static void socket_enter_running(Socket *s, int cfd_in) { goto refuse; } - if (cfd < 0) { + if (cfd < 0) { /* Accept=no case */ bool pending = false; Unit *other; @@ -2352,18 +2337,18 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (!pending) { if (!UNIT_ISSET(s->service)) { - r = log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOENT), - "Service to activate vanished, refusing activation."); + r = log_unit_warning_errno(UNIT(s), SYNTHETIC_ERRNO(ENOENT), + "Service to activate vanished, refusing activation."); goto fail; } r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_REPLACE, NULL, &error, NULL); if (r < 0) - goto fail; + goto queue_error; } socket_set_state(s, SOCKET_RUNNING); - } else { + } else { /* Accept=yes case */ _cleanup_(socket_peer_unrefp) SocketPeer *p = NULL; Unit *service; @@ -2397,13 +2382,17 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (r < 0) { if (ERRNO_IS_DISCONNECT(r)) return; + + log_unit_warning_errno(UNIT(s), r, "Failed to load connection service unit: %m"); goto fail; } r = unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, service, false, UNIT_DEPENDENCY_IMPLICIT); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to add Before=/Triggers= dependencies on connection unit: %m"); goto fail; + } s->n_accepted++; @@ -2411,6 +2400,8 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (r < 0) { if (ERRNO_IS_DISCONNECT(r)) return; + + log_unit_warning_errno(UNIT(s), r, "Failed to set socket on service: %m"); goto fail; } @@ -2422,7 +2413,7 @@ static void socket_enter_running(Socket *s, int cfd_in) { /* We failed to activate the new service, but it still exists. Let's make sure the * service closes and forgets the connection fd again, immediately. */ service_release_socket_fd(SERVICE(service)); - goto fail; + goto queue_error; } /* Notify clients about changed counters */ @@ -2435,7 +2426,7 @@ refuse: s->n_refused++; return; -fail: +queue_error: if (ERRNO_IS_RESOURCE(r)) log_unit_warning(UNIT(s), "Failed to queue service startup job: %s", bus_error_message(&error, r)); @@ -2444,6 +2435,7 @@ fail: cfd >= 0 ? "template" : "non-template", bus_error_message(&error, r)); +fail: socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); } @@ -2461,20 +2453,16 @@ static void socket_run_next(Socket *s) { pidref_done(&s->control_pid); r = socket_spawn(s, s->control_command, &s->control_pid); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn next task: %m"); - return; - -fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run next task: %m"); - - if (s->state == SOCKET_START_POST) - socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); - else if (s->state == SOCKET_STOP_POST) - socket_enter_dead(s, SOCKET_FAILURE_RESOURCES); - else - socket_enter_signal(s, SOCKET_FINAL_SIGTERM, SOCKET_FAILURE_RESOURCES); + if (s->state == SOCKET_START_POST) + socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES); + else if (s->state == SOCKET_STOP_POST) + socket_enter_dead(s, SOCKET_FAILURE_RESOURCES); + else + socket_enter_signal(s, SOCKET_FINAL_SIGTERM, SOCKET_FAILURE_RESOURCES); + } } static int socket_start(Unit *u) { @@ -3443,18 +3431,21 @@ static int socket_clean(Unit *u, ExecCleanMask mask) { s->control_command_id = _SOCKET_EXEC_COMMAND_INVALID; r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->exec_context.timeout_clean_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; + } r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to spawn cleaning task: %m"); goto fail; + } socket_set_state(s, SOCKET_CLEANING); return 0; fail: - log_unit_warning_errno(u, r, "Failed to initiate cleaning: %m"); s->clean_result = SOCKET_FAILURE_RESOURCES; s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); return r; From d30eb0b5d96692baaee09bc2b9e9e0ab677d0730 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 15:59:24 +0200 Subject: [PATCH 06/12] mount: also rework log message generation --- src/core/mount.c | 148 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 108 insertions(+), 40 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 08c2ddf9156..3e60ab48d85 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1039,13 +1039,17 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { /* main_pid= */ NULL, &m->control_pid, /* main_pid_alien= */ false); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to kill processes: %m"); goto fail; + } if (r > 0) { r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to install timer: %m"); goto fail; + } mount_set_state(m, state); } else if (state == MOUNT_REMOUNTING_SIGTERM && m->kill_context.send_sigkill) @@ -1060,10 +1064,34 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { return; fail: - log_unit_warning_errno(UNIT(m), r, "Failed to kill processes: %m"); mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } +static int mount_set_umount_command(Mount *m, ExecCommand *c) { + int r; + + assert(m); + assert(c); + + r = exec_command_set(c, UMOUNT_PATH, m->where, "-c", NULL); + if (r < 0) + return r; + + if (m->lazy_unmount) { + r = exec_command_append(c, "-l", NULL); + if (r < 0) + return r; + } + + if (m->force_unmount) { + r = exec_command_append(c, "-f", NULL); + if (r < 0) + return r; + } + + return 0; +} + static void mount_enter_unmounting(Mount *m) { int r; @@ -1079,29 +1107,71 @@ static void mount_enter_unmounting(Mount *m) { m->control_command_id = MOUNT_EXEC_UNMOUNT; m->control_command = m->exec_command + MOUNT_EXEC_UNMOUNT; - r = exec_command_set(m->control_command, UMOUNT_PATH, m->where, "-c", NULL); - if (r >= 0 && m->lazy_unmount) - r = exec_command_append(m->control_command, "-l", NULL); - if (r >= 0 && m->force_unmount) - r = exec_command_append(m->control_command, "-f", NULL); - if (r < 0) + r = mount_set_umount_command(m, m->control_command); + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to prepare umount command line: %m"); goto fail; + } mount_unwatch_control_pid(m); r = mount_spawn(m, m->control_command, &m->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to spawn 'umount' task: %m"); goto fail; + } mount_set_state(m, MOUNT_UNMOUNTING); return; fail: - log_unit_warning_errno(UNIT(m), r, "Failed to run 'umount' task: %m"); mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } +static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParameters *p) { + int r; + + assert(m); + assert(c); + assert(p); + + r = exec_command_set(c, MOUNT_PATH, p->what, m->where, NULL); + if (r < 0) + return r; + + if (m->sloppy_options) { + r = exec_command_append(c, "-s", NULL); + if (r < 0) + return r; + } + + if (m->read_write_only) { + r = exec_command_append(c, "-w", NULL); + if (r < 0) + return r; + } + + if (p->fstype) { + r = exec_command_append(c, "-t", p->fstype, NULL); + if (r < 0) + return r; + } + + _cleanup_free_ char *opts = NULL; + r = fstab_filter_options(p->options, "nofail\0" "noauto\0" "auto\0", NULL, NULL, NULL, &opts); + if (r < 0) + return r; + + if (!isempty(opts)) { + r = exec_command_append(c, "-o", opts, NULL); + if (r < 0) + return r; + } + + return 0; +} + static void mount_enter_mounting(Mount *m) { int r; MountParameters *p; @@ -1148,38 +1218,28 @@ static void mount_enter_mounting(Mount *m) { } if (p) { - _cleanup_free_ char *opts = NULL; - - r = fstab_filter_options(p->options, "nofail\0" "noauto\0" "auto\0", NULL, NULL, NULL, &opts); - if (r < 0) + r = mount_set_mount_command(m, m->control_command, p); + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to prepare mount command line: %m"); goto fail; - - r = exec_command_set(m->control_command, MOUNT_PATH, p->what, m->where, NULL); - if (r >= 0 && m->sloppy_options) - r = exec_command_append(m->control_command, "-s", NULL); - if (r >= 0 && m->read_write_only) - r = exec_command_append(m->control_command, "-w", NULL); - if (r >= 0 && p->fstype) - r = exec_command_append(m->control_command, "-t", p->fstype, NULL); - if (r >= 0 && !isempty(opts)) - r = exec_command_append(m->control_command, "-o", opts, NULL); - } else - r = -ENOENT; - if (r < 0) + } + } else { + r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); goto fail; + } mount_unwatch_control_pid(m); r = mount_spawn(m, m->control_command, &m->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to spawn 'mount' task: %m"); goto fail; + } mount_set_state(m, MOUNT_MOUNTING); - return; fail: - log_unit_warning_errno(UNIT(m), r, "Failed to run 'mount' task: %m"); mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); } @@ -1223,23 +1283,28 @@ static void mount_enter_remounting(Mount *m) { r = exec_command_append(m->control_command, "-w", NULL); if (r >= 0 && p->fstype) r = exec_command_append(m->control_command, "-t", p->fstype, NULL); - } else - r = -ENOENT; - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to prepare remount command line: %m"); + goto fail; + } + + } else { + r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); goto fail; + } mount_unwatch_control_pid(m); r = mount_spawn(m, m->control_command, &m->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to spawn 'remount' task: %m"); goto fail; + } mount_set_state(m, MOUNT_REMOUNTING); - return; fail: - log_unit_warning_errno(UNIT(m), r, "Failed to run 'remount' task: %m"); mount_set_reload_result(m, MOUNT_FAILURE_RESOURCES); mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); } @@ -2241,18 +2306,21 @@ static int mount_clean(Unit *u, ExecCleanMask mask) { m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID; r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->exec_context.timeout_clean_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; + } r = unit_fork_and_watch_rm_rf(u, l, &m->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to spawn cleaning task: %m"); goto fail; + } mount_set_state(m, MOUNT_CLEANING); return 0; fail: - log_unit_warning_errno(u, r, "Failed to initiate cleaning: %m"); m->clean_result = MOUNT_FAILURE_RESOURCES; m->timer_event_source = sd_event_source_disable_unref(m->timer_event_source); return r; From b0ff583705554ef7bbc1be764697812778ff325a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 16:46:28 +0200 Subject: [PATCH 07/12] swap: also modernize state engine log message generation --- src/core/swap.c | 61 ++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/core/swap.c b/src/core/swap.c index aa04b994622..58715b10ef3 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -435,12 +435,14 @@ static int swap_setup_unit( delete = true; r = unit_new_for_name(m, sizeof(Swap), e, &u); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to load swap unit: %m"); goto fail; + } SWAP(u)->what = strdup(what); if (!SWAP(u)->what) { - r = -ENOMEM; + r = log_oom(); goto fail; } @@ -453,7 +455,7 @@ static int swap_setup_unit( if (!p->what) { p->what = strdup(what_proc_swaps); if (!p->what) { - r = -ENOMEM; + r = log_oom(); goto fail; } } @@ -479,8 +481,6 @@ static int swap_setup_unit( return 0; fail: - log_unit_warning_errno(u, r, "Failed to load swap unit: %m"); - if (delete) unit_free(u); @@ -770,13 +770,17 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { /* main_pid= */ NULL, &s->control_pid, /* main_pid_alien= */ false); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); goto fail; + } if (r > 0) { r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; + } swap_set_state(s, state); } else if (state == SWAP_DEACTIVATING_SIGTERM && s->kill_context.send_sigkill) @@ -787,7 +791,6 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } @@ -817,38 +820,45 @@ static void swap_enter_activating(Swap *s) { else r = asprintf(&opts, "pri=%i", s->parameters_fragment.priority); if (r < 0) { - r = -ENOMEM; + r = log_oom(); goto fail; } } } r = exec_command_set(s->control_command, "/sbin/swapon", "--fixpgsz", NULL); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to initialize swapon command line: %m"); goto fail; + } if (s->parameters_fragment.options || opts) { r = exec_command_append(s->control_command, "-o", opts ?: s->parameters_fragment.options, NULL); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to prepare swapon command line: %m"); goto fail; + } } r = exec_command_append(s->control_command, s->what, NULL); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to prepare swapon command line: %m"); goto fail; + } swap_unwatch_control_pid(s); r = swap_spawn(s, s->control_command, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'swapon' task: %m"); goto fail; + } swap_set_state(s, SWAP_ACTIVATING); return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'swapon' task: %m"); swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } @@ -864,21 +874,23 @@ static void swap_enter_deactivating(Swap *s) { "/sbin/swapoff", s->what, NULL); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to prepare swapoff command line: %m"); goto fail; + } swap_unwatch_control_pid(s); r = swap_spawn(s, s->control_command, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to spawn 'swapoff' task: %m"); goto fail; + } swap_set_state(s, SWAP_DEACTIVATING); - return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to run 'swapoff' task: %m"); swap_enter_dead_or_active(s, SWAP_FAILURE_RESOURCES); } @@ -1179,7 +1191,7 @@ static int swap_load_proc_swaps(Manager *m, bool set_flags) { if (k == EOF) break; - log_warning("Failed to parse /proc/swaps:%u.", i); + log_warning("Failed to parse /proc/swaps:%u, skipping.", i); continue; } @@ -1202,8 +1214,6 @@ static int swap_process_proc_swaps(Manager *m) { r = swap_load_proc_swaps(m, true); if (r < 0) { - log_error_errno(r, "Failed to reread /proc/swaps: %m"); - /* Reset flags, just in case, for late calls */ LIST_FOREACH(units_by_type, u, m->units_by_type[UNIT_SWAP]) { Swap *swap = SWAP(u); @@ -1540,18 +1550,21 @@ static int swap_clean(Unit *u, ExecCleanMask mask) { s->control_command_id = _SWAP_EXEC_COMMAND_INVALID; r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->exec_context.timeout_clean_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; + } r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(u, r, "Failed to spawn cleaning task: %m"); goto fail; + } swap_set_state(s, SWAP_CLEANING); return 0; fail: - log_unit_warning_errno(u, r, "Failed to initiate cleaning: %m"); s->clean_result = SWAP_FAILURE_RESOURCES; s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); return r; From bfeb10911e58bf8890eb7687cf12eddb09ab0c5d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 17:00:52 +0200 Subject: [PATCH 08/12] automount: also modernize log logic --- src/core/automount.c | 45 ++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 52fb4b6948c..8864208288d 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -82,7 +82,7 @@ static void unmount_autofs(Automount *a) { if (a->where) { r = repeat_unmount(a->where, MNT_DETACH|UMOUNT_NOFOLLOW); if (r < 0) - log_error_errno(r, "Failed to unmount: %m"); + log_unit_error_errno(UNIT(a), r, "Failed to unmount: %m"); } } } @@ -341,6 +341,7 @@ static void automount_enter_dead(Automount *a, AutomountResult f) { static int open_dev_autofs(Manager *m) { struct autofs_dev_ioctl param; + int r; assert(m); @@ -354,9 +355,10 @@ static int open_dev_autofs(Manager *m) { return log_error_errno(errno, "Failed to open /dev/autofs: %m"); init_autofs_dev_ioctl(¶m); - if (ioctl(m->dev_autofs_fd, AUTOFS_DEV_IOCTL_VERSION, ¶m) < 0) { + r = RET_NERRNO(ioctl(m->dev_autofs_fd, AUTOFS_DEV_IOCTL_VERSION, ¶m)); + if (r < 0) { m->dev_autofs_fd = safe_close(m->dev_autofs_fd); - return -errno; + return log_error_errno(r, "Failed to issue AUTOFS_DEV_IOCTL_VERSION ioctl: %m"); } log_debug("Autofs kernel version %u.%u", param.ver_major, param.ver_minor); @@ -556,8 +558,8 @@ static void automount_trigger_notify(Unit *u, Unit *other) { } static void automount_enter_waiting(Automount *a) { + _cleanup_close_pair_ int pipe_fd[2] = PIPE_EBADF; _cleanup_close_ int ioctl_fd = -EBADF; - int pipe_fd[2] = PIPE_EBADF; char name[STRLEN("systemd-") + DECIMAL_STR_MAX(pid_t) + 1]; _cleanup_free_ char *options = NULL; bool mounted = false; @@ -585,12 +587,14 @@ static void automount_enter_waiting(Automount *a) { } if (pipe2(pipe_fd, O_CLOEXEC) < 0) { - r = -errno; + r = log_unit_warning_errno(UNIT(a), errno, "Failed to allocate autofs pipe: %m"); goto fail; } r = fd_nonblock(pipe_fd[0], true); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(a), r, "Failed to make read side of pipe non-blocking: %m"); goto fail; + } if (asprintf( &options, @@ -599,12 +603,12 @@ static void automount_enter_waiting(Automount *a) { getpgrp(), isempty(a->extra_options) ? "" : ",", strempty(a->extra_options)) < 0) { - r = -ENOMEM; + r = log_oom(); goto fail; } xsprintf(name, "systemd-"PID_FMT, getpid_cached()); - r = mount_nofollow(name, a->where, "autofs", 0, options); + r = mount_nofollow_verbose(LOG_WARNING, name, a->where, "autofs", 0, options); if (r < 0) goto fail; @@ -613,46 +617,47 @@ static void automount_enter_waiting(Automount *a) { pipe_fd[1] = safe_close(pipe_fd[1]); if (stat(a->where, &st) < 0) { - r = -errno; + r = log_unit_warning_errno(UNIT(a), errno, "Failed to stat new automount point '%s': %m", a->where); goto fail; } ioctl_fd = open_ioctl_fd(dev_autofs_fd, a->where, st.st_dev); if (ioctl_fd < 0) { - r = ioctl_fd; + r = log_unit_warning_errno(UNIT(a), ioctl_fd, "Failed to open automount ioctl fd for '%s': %m", a->where); goto fail; } r = autofs_protocol(dev_autofs_fd, ioctl_fd); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(a), r, "Failed to validate autofs protocol for '%s': %m", a->where); goto fail; + } r = autofs_set_timeout(dev_autofs_fd, ioctl_fd, a->timeout_idle_usec); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(a), r, "Failed to set autofs timeout for '%s': %m", a->where); goto fail; + } r = sd_event_add_io(UNIT(a)->manager->event, &a->pipe_event_source, pipe_fd[0], EPOLLIN, automount_dispatch_io, a); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(a), r, "Failed to allocate IO event source for autofs mount '%s': %m", a->where); goto fail; + } (void) sd_event_source_set_description(a->pipe_event_source, "automount-io"); - a->pipe_fd = pipe_fd[0]; + a->pipe_fd = TAKE_FD(pipe_fd[0]); a->dev_id = st.st_dev; automount_set_state(a, AUTOMOUNT_WAITING); - return; fail: - log_unit_error_errno(UNIT(a), r, "Failed to initialize automounter: %m"); - - safe_close_pair(pipe_fd); - if (mounted) { r = repeat_unmount(a->where, MNT_DETACH|UMOUNT_NOFOLLOW); if (r < 0) - log_error_errno(r, "Failed to unmount, ignoring: %m"); + log_unit_warning_errno(UNIT(a), r, "Failed to unmount, ignoring: %m"); } automount_enter_dead(a, AUTOMOUNT_FAILURE_RESOURCES); From e7912a08b4fc60a2836cb04c6c3b7d51a0bd2bf2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 17:04:41 +0200 Subject: [PATCH 09/12] timer: also modernize timer state machine error logging --- src/core/timer.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/core/timer.c b/src/core/timer.c index 2df75a18218..ba18691cec8 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -379,8 +379,7 @@ static void timer_enter_waiting(Timer *t, bool time_change) { trigger = UNIT_TRIGGER(UNIT(t)); if (!trigger) { log_unit_error(UNIT(t), "Unit to trigger vanished."); - timer_enter_dead(t, TIMER_FAILURE_RESOURCES); - return; + goto fail; } triple_timestamp_get(&ts); @@ -505,12 +504,16 @@ static void timer_enter_waiting(Timer *t, bool time_change) { if (t->monotonic_event_source) { r = sd_event_source_set_time(t->monotonic_event_source, t->next_elapse_monotonic_or_boottime); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to reschedule monotonic event source: %m"); goto fail; + } r = sd_event_source_set_enabled(t->monotonic_event_source, SD_EVENT_ONESHOT); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to enable monotonic event source: %m"); goto fail; + } } else { r = sd_event_add_time( @@ -519,8 +522,10 @@ static void timer_enter_waiting(Timer *t, bool time_change) { t->wake_system ? CLOCK_BOOTTIME_ALARM : CLOCK_MONOTONIC, t->next_elapse_monotonic_or_boottime, t->accuracy_usec, timer_dispatch, t); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to add monotonic event source: %m"); goto fail; + } (void) sd_event_source_set_description(t->monotonic_event_source, "timer-monotonic"); } @@ -528,8 +533,10 @@ static void timer_enter_waiting(Timer *t, bool time_change) { } else if (t->monotonic_event_source) { r = sd_event_source_set_enabled(t->monotonic_event_source, SD_EVENT_OFF); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to disable monotonic event source: %m"); goto fail; + } } if (found_realtime) { @@ -539,12 +546,16 @@ static void timer_enter_waiting(Timer *t, bool time_change) { if (t->realtime_event_source) { r = sd_event_source_set_time(t->realtime_event_source, t->next_elapse_realtime); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to reschedule realtime event source: %m"); goto fail; + } r = sd_event_source_set_enabled(t->realtime_event_source, SD_EVENT_ONESHOT); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to enable realtime event source: %m"); goto fail; + } } else { r = sd_event_add_time( UNIT(t)->manager->event, @@ -552,8 +563,10 @@ static void timer_enter_waiting(Timer *t, bool time_change) { t->wake_system ? CLOCK_REALTIME_ALARM : CLOCK_REALTIME, t->next_elapse_realtime, t->accuracy_usec, timer_dispatch, t); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to add realtime event source: %m"); goto fail; + } (void) sd_event_source_set_description(t->realtime_event_source, "timer-realtime"); } @@ -561,15 +574,16 @@ static void timer_enter_waiting(Timer *t, bool time_change) { } else if (t->realtime_event_source) { r = sd_event_source_set_enabled(t->realtime_event_source, SD_EVENT_OFF); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(t), r, "Failed to disable realtime event source: %m"); goto fail; + } } timer_set_state(t, TIMER_WAITING); return; fail: - log_unit_warning_errno(UNIT(t), r, "Failed to enter waiting state: %m"); timer_enter_dead(t, TIMER_FAILURE_RESOURCES); } @@ -589,19 +603,20 @@ static void timer_enter_running(Timer *t) { trigger = UNIT_TRIGGER(UNIT(t)); if (!trigger) { log_unit_error(UNIT(t), "Unit to trigger vanished."); - timer_enter_dead(t, TIMER_FAILURE_RESOURCES); - return; + goto fail; } details = activation_details_new(UNIT(t)); if (!details) { - r = -ENOMEM; + log_oom(); goto fail; } r = manager_add_job(UNIT(t)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); - if (r < 0) + if (r < 0) { + log_unit_warning(UNIT(t), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); goto fail; + } dual_timestamp_get(&t->last_trigger); ACTIVATION_DETAILS_TIMER(details)->last_trigger = t->last_trigger; @@ -615,7 +630,6 @@ static void timer_enter_running(Timer *t) { return; fail: - log_unit_warning(UNIT(t), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); timer_enter_dead(t, TIMER_FAILURE_RESOURCES); } From 4d7da557e28aa55f396f72e8129c583a121c38f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 17:07:32 +0200 Subject: [PATCH 10/12] path: also modernize path state machine logging --- src/core/path.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/path.c b/src/core/path.c index d5651cd9d83..3ffa0acc625 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -530,23 +530,26 @@ static void path_enter_running(Path *p, char *trigger_path) { trigger = UNIT_TRIGGER(UNIT(p)); if (!trigger) { log_unit_error(UNIT(p), "Unit to trigger vanished."); - path_enter_dead(p, PATH_FAILURE_RESOURCES); - return; + goto fail; } details = activation_details_new(UNIT(p)); if (!details) { - r = -ENOMEM; + log_oom(); goto fail; } r = free_and_strdup(&(ACTIVATION_DETAILS_PATH(details))->trigger_path_filename, trigger_path); - if (r < 0) + if (r < 0) { + log_oom(); goto fail; + } r = manager_add_job(UNIT(p)->manager, JOB_START, trigger, JOB_REPLACE, NULL, &error, &job); - if (r < 0) + if (r < 0) { + log_unit_warning(UNIT(p), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); goto fail; + } job_set_activation_details(job, details); @@ -556,7 +559,6 @@ static void path_enter_running(Path *p, char *trigger_path) { return; fail: - log_unit_warning(UNIT(p), "Failed to queue unit startup job: %s", bus_error_message(&error, r)); path_enter_dead(p, PATH_FAILURE_RESOURCES); } @@ -594,8 +596,11 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify) } r = path_watch(p); - if (r < 0) - goto fail; + if (r < 0) { + log_unit_warning_errno(UNIT(p), r, "Failed to enter waiting state: %m"); + path_enter_dead(p, PATH_FAILURE_RESOURCES); + return; + } /* Hmm, so now we have created inotify watches, but the file * might have appeared/been removed by now, so we must @@ -608,11 +613,6 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify) } path_set_state(p, PATH_WAITING); - return; - -fail: - log_unit_warning_errno(UNIT(p), r, "Failed to enter waiting state: %m"); - path_enter_dead(p, PATH_FAILURE_RESOURCES); } static void path_mkdir(Path *p) { From c5acfe18fbeb39fb8f78b3047dd2ad60983f571a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 17:38:46 +0200 Subject: [PATCH 11/12] scope: also modernize state machine logging --- src/core/scope.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/scope.c b/src/core/scope.c index 1664d49c94b..78aa638ed2c 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -348,14 +348,18 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { /* main_pid= */ NULL, /* control_pid= */ NULL, /* main_pid_alien= */ false); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); goto fail; + } } if (r > 0) { r = scope_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); - if (r < 0) + if (r < 0) { + log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; + } scope_set_state(s, state); } else if (state == SCOPE_STOP_SIGTERM) @@ -366,8 +370,6 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { return; fail: - log_unit_warning_errno(UNIT(s), r, "Failed to kill processes: %m"); - scope_enter_dead(s, SCOPE_FAILURE_RESOURCES); } From e92768004ef8627ecede5107d21c8f8c6ca976d0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Sep 2023 17:38:23 +0200 Subject: [PATCH 12/12] core: generalize service_arm_timer() for all unit types --- src/core/mount.c | 36 ++++++------------------------------ src/core/scope.c | 36 ++++++------------------------------ src/core/service.c | 26 +------------------------- src/core/socket.c | 38 +++++++------------------------------- src/core/swap.c | 36 ++++++------------------------------ src/core/unit.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/core/unit.h | 2 ++ 7 files changed, 71 insertions(+), 146 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 3e60ab48d85..e7a18d13b79 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -191,34 +191,10 @@ static void mount_init(Unit *u) { u->ignore_on_isolate = true; } -static int mount_arm_timer(Mount *m, usec_t usec) { - int r; - +static int mount_arm_timer(Mount *m, bool relative, usec_t usec) { assert(m); - if (usec == USEC_INFINITY) - return sd_event_source_set_enabled(m->timer_event_source, SD_EVENT_OFF); - - if (m->timer_event_source) { - r = sd_event_source_set_time(m->timer_event_source, usec); - if (r < 0) - return r; - - return sd_event_source_set_enabled(m->timer_event_source, SD_EVENT_ONESHOT); - } - - r = sd_event_add_time( - UNIT(m)->manager->event, - &m->timer_event_source, - CLOCK_MONOTONIC, - usec, 0, - mount_dispatch_timer, m); - if (r < 0) - return r; - - (void) sd_event_source_set_description(m->timer_event_source, "mount-timer"); - - return 0; + return unit_arm_timer(UNIT(m), &m->timer_event_source, relative, usec, mount_dispatch_timer); } static void mount_unwatch_control_pid(Mount *m) { @@ -809,7 +785,7 @@ static int mount_coldplug(Unit *u) { if (r < 0) return r; - r = mount_arm_timer(m, usec_add(u->state_change_timestamp.monotonic, m->timeout_usec)); + r = mount_arm_timer(m, /* relative= */ false, usec_add(u->state_change_timestamp.monotonic, m->timeout_usec)); if (r < 0) return r; } @@ -932,7 +908,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, PidRef *ret_pid) { if (r < 0) return r; - r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec)); + r = mount_arm_timer(m, /* relative= */ true, m->timeout_usec); if (r < 0) return r; @@ -1045,7 +1021,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { } if (r > 0) { - r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->timeout_usec)); + r = mount_arm_timer(m, /* relative= */ true, m->timeout_usec); if (r < 0) { log_unit_warning_errno(UNIT(m), r, "Failed to install timer: %m"); goto fail; @@ -2305,7 +2281,7 @@ static int mount_clean(Unit *u, ExecCleanMask mask) { m->control_command = NULL; m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID; - r = mount_arm_timer(m, usec_add(now(CLOCK_MONOTONIC), m->exec_context.timeout_clean_usec)); + r = mount_arm_timer(m, /* relative= */ true, m->exec_context.timeout_clean_usec); if (r < 0) { log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; diff --git a/src/core/scope.c b/src/core/scope.c index 78aa638ed2c..cd6fafe0de2 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -76,34 +76,10 @@ static usec_t scope_running_timeout(Scope *s) { delta); } -static int scope_arm_timer(Scope *s, usec_t usec) { - int r; - +static int scope_arm_timer(Scope *s, bool relative, usec_t usec) { assert(s); - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, usec); - if (r < 0) - return r; - - return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); - } - - if (usec == USEC_INFINITY) - return 0; - - r = sd_event_add_time( - UNIT(s)->manager->event, - &s->timer_event_source, - CLOCK_MONOTONIC, - usec, 0, - scope_dispatch_timer, s); - if (r < 0) - return r; - - (void) sd_event_source_set_description(s->timer_event_source, "scope-timer"); - - return 0; + return unit_arm_timer(UNIT(s), &s->timer_event_source, relative, usec, scope_dispatch_timer); } static void scope_set_state(Scope *s, ScopeState state) { @@ -260,7 +236,7 @@ static int scope_coldplug(Unit *u) { if (s->deserialized_state == s->state) return 0; - r = scope_arm_timer(s, scope_coldplug_timeout(s)); + r = scope_arm_timer(s, /* relative= */ false, scope_coldplug_timeout(s)); if (r < 0) return r; @@ -355,7 +331,7 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { } if (r > 0) { - r = scope_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); + r = scope_arm_timer(s, /* relative= */ true, s->timeout_stop_usec); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; @@ -381,7 +357,7 @@ static int scope_enter_start_chown(Scope *s) { assert(s); assert(s->user); - r = scope_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), u->manager->defaults.timeout_start_usec)); + r = scope_arm_timer(s, /* relative= */ true, u->manager->defaults.timeout_start_usec); if (r < 0) return r; @@ -464,7 +440,7 @@ static int scope_enter_running(Scope *s) { scope_set_state(s, SCOPE_RUNNING); /* Set the maximum runtime timeout. */ - scope_arm_timer(s, scope_running_timeout(s)); + scope_arm_timer(s, /* relative= */ false, scope_running_timeout(s)); /* On unified we use proper notifications hence we can unwatch the PIDs * we just attached to the scope. This can also be done on legacy as diff --git a/src/core/service.c b/src/core/service.c index f2f6dffc02b..fc2380c5a95 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -620,33 +620,9 @@ static usec_t service_running_timeout(Service *s) { } static int service_arm_timer(Service *s, bool relative, usec_t usec) { - int r; - assert(s); - if (s->timer_event_source) { - r = (relative ? sd_event_source_set_time_relative : sd_event_source_set_time)(s->timer_event_source, usec); - if (r < 0) - return r; - - return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); - } - - if (usec == USEC_INFINITY) - return 0; - - r = (relative ? sd_event_add_time_relative : sd_event_add_time)( - UNIT(s)->manager->event, - &s->timer_event_source, - CLOCK_MONOTONIC, - usec, 0, - service_dispatch_timer, s); - if (r < 0) - return r; - - (void) sd_event_source_set_description(s->timer_event_source, "service-timer"); - - return 0; + return unit_arm_timer(UNIT(s), &s->timer_event_source, relative, usec, service_dispatch_timer); } static int service_verify(Service *s) { diff --git a/src/core/socket.c b/src/core/socket.c index 715ebcbd89f..03c41b4049a 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -182,34 +182,10 @@ static void socket_done(Unit *u) { s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); } -static int socket_arm_timer(Socket *s, usec_t usec) { - int r; - +static int socket_arm_timer(Socket *s, bool relative, usec_t usec) { assert(s); - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, usec); - if (r < 0) - return r; - - return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); - } - - if (usec == USEC_INFINITY) - return 0; - - r = sd_event_add_time( - UNIT(s)->manager->event, - &s->timer_event_source, - CLOCK_MONOTONIC, - usec, 0, - socket_dispatch_timer, s); - if (r < 0) - return r; - - (void) sd_event_source_set_description(s->timer_event_source, "socket-timer"); - - return 0; + return unit_arm_timer(UNIT(s), &s->timer_event_source, relative, usec, socket_dispatch_timer); } static bool have_non_accept_socket(Socket *s) { @@ -1885,7 +1861,7 @@ static int socket_coldplug(Unit *u) { if (r < 0) return r; - r = socket_arm_timer(s, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); + r = socket_arm_timer(s, /* relative= */ false, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); if (r < 0) return r; } @@ -1948,7 +1924,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, PidRef *ret_pid) { if (r < 0) return r; - r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); + r = socket_arm_timer(s, /* relative= */ true, s->timeout_usec); if (r < 0) return r; @@ -1984,7 +1960,7 @@ static int socket_chown(Socket *s, PidRef *ret_pid) { assert(s); - r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); + r = socket_arm_timer(s, /* relative= */ true, s->timeout_usec); if (r < 0) return r; @@ -2129,7 +2105,7 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { } if (r > 0) { - r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); + r = socket_arm_timer(s, /* relative= */ true, s->timeout_usec); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; @@ -3430,7 +3406,7 @@ static int socket_clean(Unit *u, ExecCleanMask mask) { s->control_command = NULL; s->control_command_id = _SOCKET_EXEC_COMMAND_INVALID; - r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->exec_context.timeout_clean_usec)); + r = socket_arm_timer(s, /* relative= */ true, s->exec_context.timeout_clean_usec); if (r < 0) { log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; diff --git a/src/core/swap.c b/src/core/swap.c index 58715b10ef3..e849e33a421 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -181,34 +181,10 @@ static void swap_done(Unit *u) { s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); } -static int swap_arm_timer(Swap *s, usec_t usec) { - int r; - +static int swap_arm_timer(Swap *s, bool relative, usec_t usec) { assert(s); - if (s->timer_event_source) { - r = sd_event_source_set_time(s->timer_event_source, usec); - if (r < 0) - return r; - - return sd_event_source_set_enabled(s->timer_event_source, SD_EVENT_ONESHOT); - } - - if (usec == USEC_INFINITY) - return 0; - - r = sd_event_add_time( - UNIT(s)->manager->event, - &s->timer_event_source, - CLOCK_MONOTONIC, - usec, 0, - swap_dispatch_timer, s); - if (r < 0) - return r; - - (void) sd_event_source_set_description(s->timer_event_source, "swap-timer"); - - return 0; + return unit_arm_timer(UNIT(s), &s->timer_event_source, relative, usec, swap_dispatch_timer); } static SwapParameters* swap_get_parameters(Swap *s) { @@ -588,7 +564,7 @@ static int swap_coldplug(Unit *u) { if (r < 0) return r; - r = swap_arm_timer(s, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); + r = swap_arm_timer(s, /* relative= */ false, usec_add(u->state_change_timestamp.monotonic, s->timeout_usec)); if (r < 0) return r; } @@ -675,7 +651,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, PidRef *ret_pid) { if (r < 0) return r; - r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); + r = swap_arm_timer(s, /* relative= */ true, s->timeout_usec); if (r < 0) return r; @@ -776,7 +752,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { } if (r > 0) { - r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); + r = swap_arm_timer(s, /* relative= */ true, s->timeout_usec); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to install timer: %m"); goto fail; @@ -1549,7 +1525,7 @@ static int swap_clean(Unit *u, ExecCleanMask mask) { s->control_command = NULL; s->control_command_id = _SWAP_EXEC_COMMAND_INVALID; - r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->exec_context.timeout_clean_usec)); + r = swap_arm_timer(s, /* relative= */ true, s->exec_context.timeout_clean_usec); if (r < 0) { log_unit_warning_errno(u, r, "Failed to install timer: %m"); goto fail; diff --git a/src/core/unit.c b/src/core/unit.c index 5fd66e7b6e0..f6b6d2f499e 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6233,6 +6233,49 @@ int unit_get_transitive_dependency_set(Unit *u, UnitDependencyAtom atom, Set **r return 0; } +int unit_arm_timer( + Unit *u, + sd_event_source **source, + bool relative, + usec_t usec, + sd_event_time_handler_t handler) { + + int r; + + assert(u); + assert(source); + assert(handler); + + if (*source) { + if (usec == USEC_INFINITY) + return sd_event_source_set_enabled(*source, SD_EVENT_OFF); + + r = (relative ? sd_event_source_set_time_relative : sd_event_source_set_time)(*source, usec); + if (r < 0) + return r; + + return sd_event_source_set_enabled(*source, SD_EVENT_ONESHOT); + } + + if (usec == USEC_INFINITY) + return 0; + + r = (relative ? sd_event_add_time_relative : sd_event_add_time)( + u->manager->event, + source, + CLOCK_MONOTONIC, + usec, 0, + handler, + u); + if (r < 0) + return r; + + const char *d = strjoina(unit_type_to_string(u->type), "-timer"); + (void) sd_event_source_set_description(*source, d); + + return 0; +} + const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX] = { [UNIT_PATH] = &activation_details_path_vtable, [UNIT_TIMER] = &activation_details_timer_vtable, diff --git a/src/core/unit.h b/src/core/unit.h index f1a80cc8912..808d9c4ceaa 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1087,6 +1087,8 @@ int unit_thaw_vtable_common(Unit *u); Condition *unit_find_failed_condition(Unit *u); +int unit_arm_timer(Unit *u, sd_event_source **source, bool relative, usec_t usec, sd_event_time_handler_t handler); + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full_errno_zerook(unit, level, error, ...) \