diff options
author | Mike Yuan <me@yhndnzj.com> | 2024-10-24 19:44:10 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-24 19:44:10 +0200 |
commit | 4e69da071deb1822e2488231ebfd1e57297819ad (patch) | |
tree | 69bc4ad405b925b02aa0b4547c50674af64d9cf8 /src/core | |
parent | man: insert a comma before 'and' (diff) | |
parent | core: clean up errors for live mounting (diff) | |
download | systemd-4e69da071deb1822e2488231ebfd1e57297819ad.tar.xz systemd-4e69da071deb1822e2488231ebfd1e57297819ad.zip |
Merge pull request #34799 from YHNdnzj/service-followups
core: follow-ups for live mount
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/service.c | 140 | ||||
-rw-r--r-- | src/core/service.h | 2 | ||||
-rw-r--r-- | src/core/unit.c | 21 | ||||
-rw-r--r-- | src/core/unit.h | 6 |
4 files changed, 82 insertions, 87 deletions
diff --git a/src/core/service.c b/src/core/service.c index dfad574e65..1ac158f1ea 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -64,6 +64,7 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_RELOAD] = UNIT_RELOADING, [SERVICE_RELOAD_SIGNAL] = UNIT_RELOADING, [SERVICE_RELOAD_NOTIFY] = UNIT_RELOADING, + [SERVICE_MOUNTING] = UNIT_REFRESHING, [SERVICE_STOP] = UNIT_DEACTIVATING, [SERVICE_STOP_WATCHDOG] = UNIT_DEACTIVATING, [SERVICE_STOP_SIGTERM] = UNIT_DEACTIVATING, @@ -79,7 +80,6 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_AUTO_RESTART] = UNIT_ACTIVATING, [SERVICE_AUTO_RESTART_QUEUED] = UNIT_ACTIVATING, [SERVICE_CLEANING] = UNIT_MAINTENANCE, - [SERVICE_MOUNTING] = UNIT_REFRESHING, }; /* For Type=idle we never want to delay any other jobs, hence we @@ -95,6 +95,7 @@ static const UnitActiveState state_translation_table_idle[_SERVICE_STATE_MAX] = [SERVICE_RELOAD] = UNIT_RELOADING, [SERVICE_RELOAD_SIGNAL] = UNIT_RELOADING, [SERVICE_RELOAD_NOTIFY] = UNIT_RELOADING, + [SERVICE_MOUNTING] = UNIT_REFRESHING, [SERVICE_STOP] = UNIT_DEACTIVATING, [SERVICE_STOP_WATCHDOG] = UNIT_DEACTIVATING, [SERVICE_STOP_SIGTERM] = UNIT_DEACTIVATING, @@ -110,7 +111,6 @@ static const UnitActiveState state_translation_table_idle[_SERVICE_STATE_MAX] = [SERVICE_AUTO_RESTART] = UNIT_ACTIVATING, [SERVICE_AUTO_RESTART_QUEUED] = UNIT_ACTIVATING, [SERVICE_CLEANING] = UNIT_MAINTENANCE, - [SERVICE_MOUNTING] = UNIT_REFRESHING, }; static int service_dispatch_inotify_io(sd_event_source *source, int fd, uint32_t events, void *userdata); @@ -136,9 +136,10 @@ static bool SERVICE_STATE_WITH_CONTROL_PROCESS(ServiceState state) { SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, + SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, - SERVICE_CLEANING, SERVICE_MOUNTING); + SERVICE_CLEANING); } static void service_init(Unit *u) { @@ -982,8 +983,8 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) { prefix, service_state_to_string(s->state), prefix, service_result_to_string(s->result), prefix, service_result_to_string(s->reload_result), - prefix, service_result_to_string(s->clean_result), prefix, service_result_to_string(s->live_mount_result), + prefix, service_result_to_string(s->clean_result), prefix, yes_no(s->permissions_start_only), prefix, yes_no(s->root_directory_start_only), prefix, yes_no(s->remain_after_exit), @@ -1261,7 +1262,7 @@ static void service_search_main_pid(Service *s) { r = unit_watch_pidref(UNIT(s), &s->main_pid, /* exclusive= */ false); if (r < 0) /* FIXME: we need to do something here */ - log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" from: %m", s->main_pid.pid); + log_unit_warning_errno(UNIT(s), r, "Failed to watch main PID "PID_FMT": %m", s->main_pid.pid); } static void service_set_state(Service *s, ServiceState state) { @@ -1283,10 +1284,10 @@ static void service_set_state(Service *s, ServiceState state) { SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, + SERVICE_MOUNTING, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, SERVICE_AUTO_RESTART, - SERVICE_MOUNTING, SERVICE_CLEANING)) s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); @@ -1315,6 +1316,9 @@ static void service_set_state(Service *s, ServiceState state) { if (!IN_SET(state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_MOUNTING)) service_stop_watchdog(s); + if (state != SERVICE_MOUNTING) /* Just in case */ + s->mount_request = sd_bus_message_unref(s->mount_request); + if (state == SERVICE_EXITED && !MANAGER_IS_RELOADING(u->manager)) { /* For the inactive states unit_notify() will trim the cgroup. But for exit we have to * do that ourselves... */ @@ -1336,9 +1340,6 @@ static void service_set_state(Service *s, ServiceState state) { unit_destroy_runtime_data(u, &s->exec_context); } - if (state != SERVICE_MOUNTING) /* Just in case */ - s->mount_request = sd_bus_message_unref(s->mount_request); - if (old_state != state) log_unit_debug(u, "Changed %s -> %s", service_state_to_string(old_state), service_state_to_string(state)); @@ -1357,6 +1358,7 @@ static usec_t service_coldplug_timeout(Service *s) { case SERVICE_RELOAD: case SERVICE_RELOAD_SIGNAL: case SERVICE_RELOAD_NOTIFY: + case SERVICE_MOUNTING: return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_start_usec); case SERVICE_RUNNING: @@ -1380,9 +1382,6 @@ static usec_t service_coldplug_timeout(Service *s) { case SERVICE_CLEANING: return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->exec_context.timeout_clean_usec); - case SERVICE_MOUNTING: - return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_start_usec); - default: return USEC_INFINITY; } @@ -2889,14 +2888,16 @@ static int service_start(Unit *u) { return 1; } -static void service_mount_request_reply(Service *s, bool success, const char *error) { +static void service_live_mount_finish(Service *s, ServiceResult f, const char *error) { assert(s); assert(error); + s->live_mount_result = f; + if (!s->mount_request) return; - if (success) { + if (f == SERVICE_SUCCESS) { (void) sd_bus_reply_method_return(s->mount_request, NULL); log_unit_debug(UNIT(s), "'%s' method succeeded", @@ -2941,7 +2942,7 @@ static int service_stop(Unit *u) { case SERVICE_MOUNTING: service_kill_control_process(s); - service_mount_request_reply(s, /* success= */ false, BUS_ERROR_UNIT_INACTIVE); + service_live_mount_finish(s, SERVICE_FAILURE_PROTOCOL, BUS_ERROR_UNIT_INACTIVE); _fallthrough_; case SERVICE_CONDITION: case SERVICE_START_PRE: @@ -3087,6 +3088,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item(f, "state", service_state_to_string(s->state)); (void) serialize_item(f, "result", service_result_to_string(s->result)); (void) serialize_item(f, "reload-result", service_result_to_string(s->reload_result)); + (void) serialize_item(f, "live-mount-result", service_result_to_string(s->live_mount_result)); (void) serialize_pidref(f, fds, "control-pid", &s->control_pid); if (s->main_pid_known) @@ -3315,7 +3317,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, state = service_state_from_string(value); if (state < 0) - log_unit_debug(u, "Failed to parse state value: %s", value); + log_unit_debug_errno(u, state, "Failed to parse state value: %s", value); else s->deserialized_state = state; } else if (streq(key, "result")) { @@ -3323,7 +3325,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, f = service_result_from_string(value); if (f < 0) - log_unit_debug(u, "Failed to parse result value: %s", value); + log_unit_debug_errno(u, f, "Failed to parse result value: %s", value); else if (f != SERVICE_SUCCESS) s->result = f; @@ -3332,10 +3334,19 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, f = service_result_from_string(value); if (f < 0) - log_unit_debug(u, "Failed to parse reload result value: %s", value); + log_unit_debug_errno(u, f, "Failed to parse reload result value: %s", value); else if (f != SERVICE_SUCCESS) s->reload_result = f; + } else if (streq(key, "live-mount-result")) { + ServiceResult f; + + f = service_result_from_string(value); + if (f < 0) + log_unit_debug_errno(u, f, "Failed to parse live mount result value: %s", value); + else if (f != SERVICE_SUCCESS) + s->live_mount_result = f; + } else if (streq(key, "control-pid")) { if (!pidref_is_set(&s->control_pid)) @@ -4185,6 +4196,12 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { service_enter_running(s, SERVICE_SUCCESS); break; + case SERVICE_MOUNTING: + service_live_mount_finish(s, f, SD_BUS_ERROR_FAILED); + + service_enter_running(s, SERVICE_SUCCESS); + break; + case SERVICE_STOP: service_enter_signal(s, SERVICE_STOP_SIGTERM, f); break; @@ -4219,14 +4236,6 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { service_enter_dead(s, SERVICE_SUCCESS, false); break; - case SERVICE_MOUNTING: - s->live_mount_result = f; - - service_mount_request_reply(s, f == SERVICE_SUCCESS, SD_BUS_ERROR_FAILED); - - service_enter_running(s, SERVICE_SUCCESS); - break; - default: assert_not_reached(); } @@ -4303,8 +4312,7 @@ static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *us case SERVICE_MOUNTING: log_unit_warning(UNIT(s), "Mount operation timed out. Killing mount process."); service_kill_control_process(s); - s->live_mount_result = SERVICE_FAILURE_TIMEOUT; - service_mount_request_reply(s, /* success= */ false, SD_BUS_ERROR_TIMEOUT); + service_live_mount_finish(s, SERVICE_FAILURE_TIMEOUT, SD_BUS_ERROR_TIMEOUT); service_enter_running(s, SERVICE_SUCCESS); break; @@ -5013,8 +5021,8 @@ static void service_reset_failed(Unit *u) { s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; - s->clean_result = SERVICE_SUCCESS; s->live_mount_result = SERVICE_SUCCESS; + s->clean_result = SERVICE_SUCCESS; s->n_restarts = 0; (void) unit_set_debug_invocation(u, /* enable= */ false); @@ -5159,7 +5167,8 @@ static int service_can_clean(Unit *u, ExecCleanMask *ret) { return 0; } -static int service_live_mount(Unit *u, +static int service_live_mount( + Unit *u, const char *src, const char *dst, sd_bus_message *message, @@ -5167,9 +5176,8 @@ static int service_live_mount(Unit *u, const MountOptions *options, sd_bus_error *error) { - _cleanup_(pidref_done) PidRef worker = PIDREF_NULL; Service *s = ASSERT_PTR(SERVICE(u)); - const char *propagate_directory; + _cleanup_(pidref_done) PidRef worker = PIDREF_NULL; int r; assert(u); @@ -5180,7 +5188,7 @@ static int service_live_mount(Unit *u, assert(!s->mount_request); if (s->state != SERVICE_RUNNING || !pidref_is_set(&s->main_pid)) { - log_unit_warning(u, "Service is not running, cannot live mount"); + log_unit_warning(u, "Service is not running, cannot live mount."); return sd_bus_error_setf( error, BUS_ERROR_UNIT_INACTIVE, @@ -5191,7 +5199,7 @@ static int service_live_mount(Unit *u, } if (mount_point_is_credentials(u->manager->prefix[EXEC_DIRECTORY_RUNTIME], dst)) { - log_unit_warning(u, "Refusing to live mount over credential mount '%s'", dst); + log_unit_warning(u, "Refusing to live mount over credential mount '%s'.", dst); return sd_bus_error_setf( error, SD_BUS_ERROR_INVALID_ARGS, @@ -5202,7 +5210,7 @@ static int service_live_mount(Unit *u, } if (path_startswith_strv(dst, s->exec_context.inaccessible_paths)) { - log_unit_warning(u, "%s is not accessible to this unit, cannot live mount", dst); + log_unit_warning(u, "%s is not accessible to this unit, cannot live mount.", dst); return sd_bus_error_setf( error, SD_BUS_ERROR_INVALID_ARGS, @@ -5219,18 +5227,14 @@ static int service_live_mount(Unit *u, r = service_arm_timer(s, /* relative= */ true, s->timeout_start_usec); if (r < 0) { - log_unit_warning_errno(u, r, "Failed to install timer: %m"); - sd_bus_error_set_errnof( - error, - r, - "Live mounting '%s' on '%s' for unit '%s' cannot be scheduled: failed to install timer", - src, - dst, - u->id); + log_unit_error_errno(u, r, "Failed to install timer: %m"); + sd_bus_error_set_errnof(error, r, + "Live mounting '%s' on '%s' for unit '%s': failed to install timer: %m", + src, dst, u->id); goto fail; } - propagate_directory = strjoina("/run/systemd/propagate/", u->id); + const char *propagate_directory = strjoina("/run/systemd/propagate/", u->id); /* Given we are running from PID1, avoid doing potentially heavy I/O operations like opening images * directly, and instead fork a worker process. We record the D-Bus message, so that we can reply @@ -5238,19 +5242,12 @@ static int service_live_mount(Unit *u, * resource is available (or the operation failed) once they receive the response. */ r = unit_fork_helper_process(u, "(sd-mount-in-ns)", /* into_cgroup= */ false, &worker); if (r < 0) { - log_unit_warning_errno( - u, - r, - "Failed to fork process to mount '%s' on '%s' in unit's namespace: %m", - src, - dst); - sd_bus_error_set_errnof( - error, - r, - "Live mounting '%s' on '%s' for unit '%s' cannot be scheduled: failed to fork process", - src, - dst, - u->id); + log_unit_error_errno(u, r, + "Failed to fork process to mount '%s' on '%s' in unit's namespace: %m", + src, dst); + sd_bus_error_set_errnof(error, r, + "Live mounting '%s' on '%s' for unit '%s': failed to fork off helper process into namespace: %m", + src, dst, u->id); goto fail; } if (r == 0) { @@ -5271,26 +5268,21 @@ static int service_live_mount(Unit *u, src, dst, flags); if (r < 0) - log_unit_warning_errno( - u, - r, - "Failed to mount '%s' on '%s' in unit's namespace: %m", - src, - dst); + log_unit_error_errno(u, r, + "Failed to mount '%s' on '%s' in unit's namespace: %m", + src, dst); else log_unit_debug(u, "Mounted '%s' on '%s' in unit's namespace", src, dst); + _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } r = unit_watch_pidref(u, &worker, /* exclusive= */ true); if (r < 0) { - sd_bus_error_set_errnof( - error, - r, - "Live mounting '%s' on '%s' for unit '%s' failed: failed to watch worker process", - src, - dst, - u->id); + log_unit_warning_errno(u, r, "Failed to watch live mount helper process: %m"); + sd_bus_error_set_errnof(error, r, + "Live mounting '%s' on '%s' for unit '%s': failed to watch live mount helper process: %m", + src, dst, u->id); goto fail; } @@ -5301,15 +5293,15 @@ static int service_live_mount(Unit *u, fail: s->live_mount_result = SERVICE_FAILURE_RESOURCES; - s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); + service_enter_running(s, SERVICE_SUCCESS); return r; } -static int service_can_live_mount(const Unit *u, sd_bus_error *error) { - assert(u); +static int service_can_live_mount(Unit *u, sd_bus_error *error) { + Service *s = ASSERT_PTR(SERVICE(u)); /* Ensure that the unit runs in a private mount namespace */ - if (!exec_needs_mount_namespace(unit_get_exec_context(u), /* params= */ NULL, unit_get_exec_runtime(u))) + if (!exec_needs_mount_namespace(&s->exec_context, /* params= */ NULL, s->exec_runtime)) return sd_bus_error_setf( error, SD_BUS_ERROR_INVALID_ARGS, diff --git a/src/core/service.h b/src/core/service.h index 74b2b1c55e..68be9075c1 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -193,8 +193,8 @@ struct Service { /* If we shut down, remember why */ ServiceResult result; ServiceResult reload_result; - ServiceResult clean_result; ServiceResult live_mount_result; + ServiceResult clean_result; bool main_pid_known:1; bool main_pid_alien:1; diff --git a/src/core/unit.c b/src/core/unit.c index 64ce5967f4..dda98bb861 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2043,7 +2043,11 @@ int unit_reload(Unit *u) { return -EBADR; state = unit_active_state(u); - if (state == UNIT_RELOADING) + if (IN_SET(state, UNIT_RELOADING, UNIT_REFRESHING)) + /* "refreshing" means some resources in the unit namespace is being updated. Unlike reload, + * the unit processes aren't made aware of refresh. Let's put the job back to queue + * in both cases, as refresh typically takes place before reload and it's better to wait + * for it rather than failing. */ return -EAGAIN; if (state != UNIT_ACTIVE) @@ -6392,22 +6396,21 @@ Condition *unit_find_failed_condition(Unit *u) { return failed_trigger && !has_succeeded_trigger ? failed_trigger : NULL; } -int unit_can_live_mount(const Unit *u, sd_bus_error *error) { +int unit_can_live_mount(Unit *u, sd_bus_error *error) { assert(u); if (!UNIT_VTABLE(u)->live_mount) return sd_bus_error_setf( error, - SD_BUS_ERROR_INVALID_ARGS, - "Live mounting not supported for unit type '%s' of unit '%s'.", - unit_type_to_string(u->type), - u->id); + SD_BUS_ERROR_NOT_SUPPORTED, + "Live mounting not supported by unit type '%s'", + unit_type_to_string(u->type)); if (u->load_state != UNIT_LOADED) return sd_bus_error_setf( error, BUS_ERROR_NO_SUCH_UNIT, - "Unit '%s' not loaded, cannot live mount.", + "Unit '%s' not loaded, cannot live mount", u->id); if (!UNIT_VTABLE(u)->can_live_mount) @@ -6429,7 +6432,7 @@ int unit_live_mount( assert(UNIT_VTABLE(u)->live_mount); if (!UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u))) { - log_unit_debug(u, "Unit not active"); + log_unit_debug(u, "Unit not active, cannot perform live mount."); return sd_bus_error_setf( error, BUS_ERROR_UNIT_INACTIVE, @@ -6440,7 +6443,7 @@ int unit_live_mount( } if (unit_active_state(u) == UNIT_REFRESHING) { - log_unit_debug(u, "Unit already live mounting"); + log_unit_debug(u, "Unit already live mounting, refusing further requests."); return sd_bus_error_setf( error, BUS_ERROR_UNIT_BUSY, diff --git a/src/core/unit.h b/src/core/unit.h index 5ffd82b67e..01e1adf961 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -587,7 +587,7 @@ typedef struct UnitVTable { /* Add a bind/image mount into the unit namespace while it is running. */ int (*live_mount)(Unit *u, const char *src, const char *dst, sd_bus_message *message, MountInNamespaceFlags flags, const MountOptions *options, sd_bus_error *error); - int (*can_live_mount)(const Unit *u, sd_bus_error *error); + int (*can_live_mount)(Unit *u, sd_bus_error *error); /* Serialize state and file descriptors that should be carried over into the new * instance after reexecution. */ @@ -650,7 +650,7 @@ typedef struct UnitVTable { int (*bus_commit_properties)(Unit *u); /* Return the unit this unit is following */ - Unit *(*following)(Unit *u); + Unit* (*following)(Unit *u); /* Return the set of units that are following each other */ int (*following_set)(Unit *u, Set **s); @@ -1047,7 +1047,7 @@ void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret_ne void unit_set_freezer_state(Unit *u, FreezerState state); void unit_freezer_complete(Unit *u, FreezerState kernel_state); -int unit_can_live_mount(const Unit *u, sd_bus_error *error); +int unit_can_live_mount(Unit *u, sd_bus_error *error); int unit_live_mount(Unit *u, const char *src, const char *dst, sd_bus_message *message, MountInNamespaceFlags flags, const MountOptions *options, sd_bus_error *error); Condition *unit_find_failed_condition(Unit *u); |