From 6ac62d61db737b01ad3776a7688d8a4c57b3f7d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Mar 2023 21:52:41 +0200 Subject: [PATCH 1/9] service: release resources from a seperate queue, not unit_check_gc() The per-unit-type release_resources() hook (most prominent use: to release a service unit's fdstore once a unit is entirely dead and has no jobs more) was currently invoked as part of unit_check_gc(), whose primary purpose is to determine if a unit should be GC'ed. This was always a bit ugly, as release_resources() changes state of the unit, while unit_check_gc() is otherwise (and was before release_resources() was added) a "passive" function that just checks for a couple of conditions. unit_check_gc() is called at various places, including when we wonder if we should add a unit to the gc queue, and then again when we take it out of the gc queue to dtermine whether to really gc it now. The fact that these checks have side effects so far wasn't too problematic, as the state changes (primarily: that services would empty their fdstores) were relatively limited and scope. A later patch in this series is supposed to extend the service state engine with a separate state distinct from SERVICE_DEAD that is very much like it but indicates that the service still has active resources (specifically the fdstore). For cases like that the releasing of the fdstore would result in state changes (as we'd then return to a classic SERVICE_DEAD state). And this is where the fact that the release_resources() is called as side-effect becomes problematic: it would mean that unit state changes would instantly propagate to state changes elsewhere, though we usually want this to be done through the run queue for coalescing and avoidance of recursion. Hence, let's clean this up: let's move the release_resources() logic into a queue of its own, and then enqueue items into it from the general state change notification handle in unit_notify(). --- src/core/manager.c | 23 ++++++++++++++ src/core/manager.h | 3 ++ src/core/unit.c | 75 +++++++++++++++++++++++++++++++++++----------- src/core/unit.h | 7 +++++ 4 files changed, 91 insertions(+), 17 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 47a502df7f..b859ce58af 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1250,6 +1250,26 @@ static unsigned manager_dispatch_cleanup_queue(Manager *m) { return n; } +static unsigned manager_dispatch_release_resources_queue(Manager *m) { + unsigned n = 0; + Unit *u; + + assert(m); + + while ((u = m->release_resources_queue)) { + assert(u->in_release_resources_queue); + + LIST_REMOVE(release_resources_queue, m->release_resources_queue, u); + u->in_release_resources_queue = false; + + n++; + + unit_release_resources(u); + } + + return n; +} + enum { GC_OFFSET_IN_PATH, /* This one is on the path we were traveling */ GC_OFFSET_UNSURE, /* No clue */ @@ -3158,6 +3178,9 @@ int manager_loop(Manager *m) { if (manager_dispatch_stop_when_unneeded_queue(m) > 0) continue; + if (manager_dispatch_release_resources_queue(m) > 0) + continue; + if (manager_dispatch_dbus_queue(m) > 0) continue; diff --git a/src/core/manager.h b/src/core/manager.h index fc95b75120..486eda11b6 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -195,6 +195,9 @@ struct Manager { /* Units that have BindsTo= another unit, and might need to be shutdown because the bound unit is not active. */ LIST_HEAD(Unit, stop_when_bound_queue); + /* Units that have resources open, and where it might be good to check if they can be released now */ + LIST_HEAD(Unit, release_resources_queue); + sd_event *event; /* This maps PIDs we care about to units that are interested in. We allow multiple units to be interested in diff --git a/src/core/unit.c b/src/core/unit.c index 409801aed2..089213cf88 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -394,37 +394,54 @@ static bool unit_success_failure_handler_has_jobs(Unit *unit) { return false; } +void unit_release_resources(Unit *u) { + UnitActiveState state; + + assert(u); + + if (u->job || u->nop_job) + return; + + if (u->perpetual) + return; + + state = unit_active_state(u); + if (!IN_SET(state, UNIT_INACTIVE, UNIT_FAILED)) + return; + + if (unit_will_restart(u)) + return; + + if (UNIT_VTABLE(u)->release_resources) + UNIT_VTABLE(u)->release_resources(u); +} + bool unit_may_gc(Unit *u) { UnitActiveState state; int r; assert(u); - /* Checks whether the unit is ready to be unloaded for garbage collection. - * Returns true when the unit may be collected, and false if there's some - * reason to keep it loaded. + /* Checks whether the unit is ready to be unloaded for garbage collection. Returns true when the + * unit may be collected, and false if there's some reason to keep it loaded. * - * References from other units are *not* checked here. Instead, this is done - * in unit_gc_sweep(), but using markers to properly collect dependency loops. + * References from other units are *not* checked here. Instead, this is done in unit_gc_sweep(), but + * using markers to properly collect dependency loops. */ if (u->job || u->nop_job) return false; - state = unit_active_state(u); - - /* If the unit is inactive and failed and no job is queued for it, then release its runtime resources */ - if (UNIT_IS_INACTIVE_OR_FAILED(state) && - UNIT_VTABLE(u)->release_resources) - UNIT_VTABLE(u)->release_resources(u); - if (u->perpetual) return false; if (sd_bus_track_count(u->bus_track) > 0) return false; - /* But we keep the unit object around for longer when it is referenced or configured to not be gc'ed */ + state = unit_active_state(u); + + /* But we keep the unit object around for longer when it is referenced or configured to not be + * gc'ed */ switch (u->collect_mode) { case COLLECT_INACTIVE: @@ -458,10 +475,10 @@ bool unit_may_gc(Unit *u) { return false; } - if (UNIT_VTABLE(u)->may_gc && !UNIT_VTABLE(u)->may_gc(u)) - return false; + if (!UNIT_VTABLE(u)->may_gc) + return true; - return true; + return UNIT_VTABLE(u)->may_gc(u); } void unit_add_to_load_queue(Unit *u) { @@ -565,6 +582,25 @@ void unit_submit_to_stop_when_bound_queue(Unit *u) { u->in_stop_when_bound_queue = true; } +void unit_submit_to_release_resources_queue(Unit *u) { + assert(u); + + if (u->in_release_resources_queue) + return; + + if (u->job || u->nop_job) + return; + + if (u->perpetual) + return; + + if (!UNIT_VTABLE(u)->release_resources) + return; + + LIST_PREPEND(release_resources_queue, u->manager->release_resources_queue, u); + u->in_release_resources_queue = true; +} + static void unit_clear_dependencies(Unit *u) { assert(u); @@ -781,6 +817,9 @@ Unit* unit_free(Unit *u) { if (u->in_stop_when_bound_queue) LIST_REMOVE(stop_when_bound_queue, u->manager->stop_when_bound_queue, u); + if (u->in_release_resources_queue) + LIST_REMOVE(release_resources_queue, u->manager->release_resources_queue, u); + bpf_firewall_close(u); hashmap_free(u->bpf_foreign_by_key); @@ -2567,7 +2606,6 @@ static bool unit_process_job(Job *j, UnitActiveState ns, UnitNotifyFlags flags) assert(j); if (j->state == JOB_WAITING) - /* So we reached a different state for this job. Let's see if we can run it now if it failed previously * due to EAGAIN. */ job_add_to_run_queue(j); @@ -2771,6 +2809,9 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag /* Maybe the unit should be GC'ed now? */ unit_add_to_gc_queue(u); + + /* Maybe we can release some resources now? */ + unit_submit_to_release_resources_queue(u); } if (UNIT_IS_ACTIVE_OR_RELOADING(ns)) { diff --git a/src/core/unit.h b/src/core/unit.h index 420405b2b7..b13ac33018 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -317,6 +317,9 @@ typedef struct Unit { /* Queue of units that have a BindTo= dependency on some other unit, and should possibly be shut down */ LIST_FIELDS(Unit, stop_when_bound_queue); + /* Queue of units that should be checked if they can release resources now */ + LIST_FIELDS(Unit, release_resources_queue); + /* PIDs we keep an eye on. Note that a unit might have many * more, but these are the ones we care enough about to * process SIGCHLD for */ @@ -479,6 +482,7 @@ typedef struct Unit { bool in_stop_when_unneeded_queue:1; bool in_start_when_upheld_queue:1; bool in_stop_when_bound_queue:1; + bool in_release_resources_queue:1; bool sent_dbus_new_signal:1; @@ -832,6 +836,8 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c); int unit_choose_id(Unit *u, const char *name); int unit_set_description(Unit *u, const char *description); +void unit_release_resources(Unit *u); + bool unit_may_gc(Unit *u); static inline bool unit_is_extrinsic(Unit *u) { @@ -853,6 +859,7 @@ void unit_add_to_target_deps_queue(Unit *u); void unit_submit_to_stop_when_unneeded_queue(Unit *u); void unit_submit_to_start_when_upheld_queue(Unit *u); void unit_submit_to_stop_when_bound_queue(Unit *u); +void unit_submit_to_release_resources_queue(Unit *u); int unit_merge(Unit *u, Unit *other); int unit_merge_by_name(Unit *u, const char *other); From c25fac9a17b95271bb6f8d967d33c5a9aa9e4bc9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Mar 2023 22:06:39 +0200 Subject: [PATCH 2/9] service: rework how we release resources Let's normalize how we release service resources, i.e. the three types of fds we maintain for each service: 1. the fdstore 2. the socket fd for per-connection socket activated services 3. stdin/stdout/stderr The generic service_release_resources() hook now calls into service_release_fd_store() + service_close_socket_fd() service_release_stdio_fd() one after the other, releasing them all for the generic "release_resources" infra of the unit lifecycle. We do no longer close the socket fd from service_set_state(), moving this exclusively into service_release_resources(), so that all fds are closed the same way. --- src/core/service.c | 55 +++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 650741cc7c..934500abd6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -199,6 +199,11 @@ static int service_set_main_pid(Service *s, pid_t pid) { void service_close_socket_fd(Service *s) { assert(s); + if (s->socket_fd < 0 && !UNIT_ISSET(s->accept_socket) && !s->socket_peer) + return; + + log_unit_debug(UNIT(s), "Closing connection socket."); + /* Undo the effect of service_set_socket_fd(). */ s->socket_fd = asynchronous_close(s->socket_fd); @@ -387,34 +392,29 @@ static void service_fd_store_unlink(ServiceFDStore *fs) { static void service_release_fd_store(Service *s) { assert(s); + if (!s->fd_store) + return; + log_unit_debug(UNIT(s), "Releasing all stored fds"); + while (s->fd_store) service_fd_store_unlink(s->fd_store); assert(s->n_fd_store == 0); } -static void service_release_resources(Unit *u) { - Service *s = SERVICE(u); - +static void service_release_stdio_fd(Service *s) { assert(s); - /* Don't release resources if this is a transitionary failed/dead state */ - if (IN_SET(s->state, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART)) + if (s->stdin_fd < 0 && s->stdout_fd < 0 && s->stdout_fd < 0) return; - if (!s->fd_store && s->stdin_fd < 0 && s->stdout_fd < 0 && s->stderr_fd < 0) - return; - - log_unit_debug(u, "Releasing resources."); + log_unit_debug(UNIT(s), "Releasing stdin/stdout/stderr file descriptors."); s->stdin_fd = safe_close(s->stdin_fd); s->stdout_fd = safe_close(s->stdout_fd); s->stderr_fd = safe_close(s->stderr_fd); - - service_release_fd_store(s); } - static void service_done(Unit *u) { Service *s = SERVICE(u); @@ -459,7 +459,8 @@ static void service_done(Unit *u) { s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot); - service_release_resources(u); + service_release_fd_store(s); + service_release_stdio_fd(s); } static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *userdata) { @@ -1248,15 +1249,6 @@ static void service_set_state(Service *s, ServiceState state) { unit_dequeue_rewatch_pids(UNIT(s)); } - if (!IN_SET(state, - SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, - SERVICE_RUNNING, - SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, - SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, - SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL) && - !(state == SERVICE_DEAD && UNIT(s)->job)) - service_close_socket_fd(s); - if (state != SERVICE_START) s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source); @@ -4919,6 +4911,25 @@ static int service_can_start(Unit *u) { return 1; } +static void service_release_resources(Unit *u) { + Service *s = SERVICE(ASSERT_PTR(u)); + + /* Invoked by the unit state engine, whenever it realizes that unit is dead and there's no job + * anymore for it, and it hence is a good idea to release resources */ + + /* Don't release resources if this is a transitionary failed/dead state + * (i.e. SERVICE_DEAD_BEFORE_AUTO_RESTART/SERVICE_FAILED_BEFORE_AUTO_RESTART), insist on a permanent + * failure state. */ + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)) + return; + + log_unit_debug(u, "Releasing resources..."); + + service_close_socket_fd(s); + service_release_stdio_fd(s); + service_release_fd_store(s); +} + static const char* const service_restart_table[_SERVICE_RESTART_MAX] = { [SERVICE_RESTART_NO] = "no", [SERVICE_RESTART_ON_SUCCESS] = "on-success", From b9c1883a9cd9b5126fe648f3e198143dc19a222d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Mar 2023 22:07:22 +0200 Subject: [PATCH 3/9] service: add ability to pin fd store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oftentimes it is useful to allow the per-service fd store to survive longer than for a restart. This is useful in various scenarios: 1. An fd to some security relevant object needs to be stashed somewhere, that should not be cleaned automatically, because the security enforcement would be dropped then. 2. A user namespace fd should be allocated on first invocation and be kept around until the user logs out (i.e. systemd --user ends), á la #16328 (This does not implement what #16318 asks for, but should solve the use-case discussed there.) 3. There's interest in allow a concept of "userspace reboots" where the kernel stays running, and userspace is swapped out (i.e. all services exit, and the rootfs transitioned into a new version of it) while keeping some select resources pinned, very similar to how we implement a switch root. Thus it is useful to allow services to exit, while leaving their fds around till the very end. This is exposed through a new FileDescriptorStorePreserve= setting that is closely modelled after RuntimeDirectoryPreserve= (in fact it reused the same internal type), since we want similar behaviour in the end, and quite often they probably want to be used together. --- man/org.freedesktop.systemd1.xml | 6 ++++ man/systemd.service.xml | 21 ++++++++++-- src/basic/unit-def.c | 1 + src/basic/unit-def.h | 1 + src/core/dbus-execute.c | 8 ++--- src/core/dbus-execute.h | 2 ++ src/core/dbus-service.c | 4 +++ src/core/load-fragment-gperf.gperf.in | 3 +- src/core/load-fragment.c | 2 +- src/core/load-fragment.h | 2 +- src/core/service.c | 47 ++++++++++++++++++++------- src/core/service.h | 1 + src/shared/bus-unit-util.c | 3 +- 13 files changed, 80 insertions(+), 21 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index f39893f647..e462c60636 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -2619,6 +2619,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { readonly u FileDescriptorStoreMax = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly u NFileDescriptorStore = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly s FileDescriptorStorePreserve = '...'; readonly s StatusText = '...'; readonly i StatusErrno = ...; readonly s Result = '...'; @@ -3244,6 +3246,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -3830,6 +3834,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 1b116b8372..7de1350a59 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -1066,7 +1066,7 @@ FDSTORE=1 messages. This is useful for implementing services that can restart after an explicit request or a crash without losing state. Any open sockets and other file descriptors which should not be closed during the restart may be stored this way. Application state - can either be serialized to a file in /run/, or better, stored in a + can either be serialized to a file in RuntimeDirectory=, or stored in a memfd_create2 memory file descriptor. Defaults to 0, i.e. no file descriptors may be stored in the service manager. All file descriptors passed to the service manager from a specific service are passed back @@ -1075,7 +1075,8 @@ details about the precise protocol used and the order in which the file descriptors are passed). Any file descriptors passed to the service manager are automatically closed when POLLHUP or POLLERR is seen on them, or when the service is - fully stopped and no job is queued or being executed for it. If this option is used, + fully stopped and no job is queued or being executed for it (the latter can be tweaked with + FileDescriptorStorePreserve=, see below). If this option is used, NotifyAccess= (see above) should be set to open access to the notification socket provided by systemd. If NotifyAccess= is not set, it will be implicitly set to . @@ -1097,6 +1098,22 @@ details. + + FileDescriptorStorePreserve= + Takes one of no, yes, + restart and controls when to release the service's file descriptor store + (i.e. when to close the contained file descriptors, if any). If set to no the + file descriptor store is automatically released when the service is stopped; if + restart (the default) it is kept around as long as the unit is neither inactive + nor failed, or a job is queued for the service, or the service is expected to be restarted. If + yes the file descriptor store is kept around until the unit is removed from + memory (i.e. is not referenced anymore and inactive). The latter is useful to keep entries in the + file descriptor store pinned until the service manage exits. + + Use systemctl clean --what=fdstore … to release the file descriptor store + explicitly. + + USBFunctionDescriptors= Configure the location of a file containing diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c index a0fab46a19..86b66e2be0 100644 --- a/src/basic/unit-def.c +++ b/src/basic/unit-def.c @@ -201,6 +201,7 @@ static const char* const service_state_table[_SERVICE_STATE_MAX] = { [SERVICE_FAILED] = "failed", [SERVICE_DEAD_BEFORE_AUTO_RESTART] = "dead-before-auto-restart", [SERVICE_FAILED_BEFORE_AUTO_RESTART] = "failed-before-auto-restart", + [SERVICE_DEAD_RESOURCES_PINNED] = "dead-resources-pinned", [SERVICE_AUTO_RESTART] = "auto-restart", [SERVICE_CLEANING] = "cleaning", }; diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h index 2fab42e9c7..169e1f719e 100644 --- a/src/basic/unit-def.h +++ b/src/basic/unit-def.h @@ -146,6 +146,7 @@ typedef enum ServiceState { SERVICE_FAILED, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART, + SERVICE_DEAD_RESOURCES_PINNED, /* Like SERVICE_DEAD, but with pinned resources */ SERVICE_AUTO_RESTART, SERVICE_CLEANING, _SERVICE_STATE_MAX, diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index d77842bdab..ce20183a70 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -49,7 +49,7 @@ BUS_DEFINE_PROPERTY_GET_ENUM(bus_property_get_exec_output, exec_output, ExecOutput); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_exec_input, exec_input, ExecInput); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_exec_utmp_mode, exec_utmp_mode, ExecUtmpMode); -static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_exec_preserve_mode, exec_preserve_mode, ExecPreserveMode); +BUS_DEFINE_PROPERTY_GET_ENUM(bus_property_get_exec_preserve_mode, exec_preserve_mode, ExecPreserveMode); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_exec_keyring_mode, exec_keyring_mode, ExecKeyringMode); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_protect_proc, protect_proc, ProtectProc); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_proc_subset, proc_subset, ProcSubset); @@ -1318,7 +1318,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("LockPersonality", "b", bus_property_get_bool, offsetof(ExecContext, lock_personality), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RestrictAddressFamilies", "(bas)", property_get_address_families, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RuntimeDirectorySymlink", "a(sst)", bus_property_get_exec_dir_symlink, offsetof(ExecContext, directories[EXEC_DIRECTORY_RUNTIME]), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("RuntimeDirectoryPreserve", "s", property_get_exec_preserve_mode, offsetof(ExecContext, runtime_directory_preserve_mode), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RuntimeDirectoryPreserve", "s", bus_property_get_exec_preserve_mode, offsetof(ExecContext, runtime_directory_preserve_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RuntimeDirectoryMode", "u", bus_property_get_mode, offsetof(ExecContext, directories[EXEC_DIRECTORY_RUNTIME].mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RuntimeDirectory", "as", bus_property_get_exec_dir, offsetof(ExecContext, directories[EXEC_DIRECTORY_RUNTIME]), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("StateDirectorySymlink", "a(sst)", bus_property_get_exec_dir_symlink, offsetof(ExecContext, directories[EXEC_DIRECTORY_STATE]), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1695,7 +1695,7 @@ static BUS_DEFINE_SET_TRANSIENT_PARSE(protect_home, ProtectHome, protect_home_fr static BUS_DEFINE_SET_TRANSIENT_PARSE(keyring_mode, ExecKeyringMode, exec_keyring_mode_from_string); static BUS_DEFINE_SET_TRANSIENT_PARSE(protect_proc, ProtectProc, protect_proc_from_string); static BUS_DEFINE_SET_TRANSIENT_PARSE(proc_subset, ProcSubset, proc_subset_from_string); -static BUS_DEFINE_SET_TRANSIENT_PARSE(preserve_mode, ExecPreserveMode, exec_preserve_mode_from_string); +BUS_DEFINE_SET_TRANSIENT_PARSE(exec_preserve_mode, ExecPreserveMode, exec_preserve_mode_from_string); static BUS_DEFINE_SET_TRANSIENT_PARSE_PTR(personality, unsigned long, parse_personality); static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(secure_bits, "i", int32_t, int, "%" PRIi32, secure_bits_to_string_alloc_with_check); static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(capability, "t", uint64_t, uint64_t, "%" PRIu64, capability_set_to_string); @@ -2047,7 +2047,7 @@ int bus_exec_context_set_transient_property( return bus_set_transient_proc_subset(u, name, &c->proc_subset, message, flags, error); if (streq(name, "RuntimeDirectoryPreserve")) - return bus_set_transient_preserve_mode(u, name, &c->runtime_directory_preserve_mode, message, flags, error); + return bus_set_transient_exec_preserve_mode(u, name, &c->runtime_directory_preserve_mode, message, flags, error); if (streq(name, "UMask")) return bus_set_transient_mode_t(u, name, &c->umask, message, flags, error); diff --git a/src/core/dbus-execute.h b/src/core/dbus-execute.h index c53834140e..5926bdb4b1 100644 --- a/src/core/dbus-execute.h +++ b/src/core/dbus-execute.h @@ -28,6 +28,8 @@ int bus_property_get_exec_output(sd_bus *bus, const char *path, const char *inte int bus_property_get_exec_command(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); int bus_property_get_exec_command_list(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); int bus_property_get_exec_ex_command_list(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); +int bus_property_get_exec_preserve_mode(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); int bus_exec_context_set_transient_property(Unit *u, ExecContext *c, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_set_transient_exec_command(Unit *u, const char *name, ExecCommand **exec_command, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); +int bus_set_transient_exec_preserve_mode(Unit *u, const char *name, ExecPreserveMode *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index a563624fce..ecab3f479d 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -349,6 +349,7 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_PROPERTY("BusName", "s", NULL, offsetof(Service, bus_name), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("FileDescriptorStoreMax", "u", bus_property_get_unsigned, offsetof(Service, n_fd_store_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NFileDescriptorStore", "u", property_get_size_as_uint32, offsetof(Service, n_fd_store), 0), + SD_BUS_PROPERTY("FileDescriptorStorePreserve", "s", bus_property_get_exec_preserve_mode, offsetof(Service, fd_store_preserve_mode), 0), SD_BUS_PROPERTY("StatusText", "s", NULL, offsetof(Service, status_text), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("StatusErrno", "i", bus_property_get_int, offsetof(Service, status_errno), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Service, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), @@ -599,6 +600,9 @@ static int bus_service_set_transient_property( if (streq(name, "FileDescriptorStoreMax")) return bus_set_transient_unsigned(u, name, &s->n_fd_store_max, message, flags, error); + if (streq(name, "FileDescriptorStorePreserve")) + return bus_set_transient_exec_preserve_mode(u, name, &s->fd_store_preserve_mode, message, flags, error); + if (streq(name, "NotifyAccess")) return bus_set_transient_notify_access(u, name, &s->notify_access, message, flags, error); diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index f35c743655..ce0e2f0c5c 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -136,7 +136,7 @@ {{type}}.MountFlags, config_parse_exec_mount_propagation_flag, 0, offsetof({{type}}, exec_context.mount_propagation_flag) {{type}}.MountAPIVFS, config_parse_exec_mount_apivfs, 0, offsetof({{type}}, exec_context) {{type}}.Personality, config_parse_personality, 0, offsetof({{type}}, exec_context.personality) -{{type}}.RuntimeDirectoryPreserve, config_parse_runtime_preserve_mode, 0, offsetof({{type}}, exec_context.runtime_directory_preserve_mode) +{{type}}.RuntimeDirectoryPreserve, config_parse_exec_preserve_mode, 0, offsetof({{type}}, exec_context.runtime_directory_preserve_mode) {{type}}.RuntimeDirectoryMode, config_parse_mode, 0, offsetof({{type}}, exec_context.directories[EXEC_DIRECTORY_RUNTIME].mode) {{type}}.RuntimeDirectory, config_parse_exec_directories, 0, offsetof({{type}}, exec_context.directories[EXEC_DIRECTORY_RUNTIME]) {{type}}.StateDirectoryMode, config_parse_mode, 0, offsetof({{type}}, exec_context.directories[EXEC_DIRECTORY_STATE].mode) @@ -433,6 +433,7 @@ Service.SysVStartPriority, config_parse_warn_compat, Service.NonBlocking, config_parse_bool, 0, offsetof(Service, exec_context.non_blocking) Service.BusName, config_parse_bus_name, 0, offsetof(Service, bus_name) Service.FileDescriptorStoreMax, config_parse_unsigned, 0, offsetof(Service, n_fd_store_max) +Service.FileDescriptorStorePreserve, config_parse_exec_preserve_mode, 0, offsetof(Service, fd_store_preserve_mode) Service.NotifyAccess, config_parse_notify_access, 0, offsetof(Service, notify_access) Service.Sockets, config_parse_service_sockets, 0, 0 Service.BusPolicy, config_parse_warn_compat, DISABLED_LEGACY, 0 diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 686a72402d..cf0096263a 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -138,7 +138,7 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_job_mode, job_mode, JobMode, "Failed to pa DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, NotifyAccess, "Failed to parse notify access specifier"); DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_home, protect_home, ProtectHome, "Failed to parse protect home value"); DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_system, protect_system, ProtectSystem, "Failed to parse protect system value"); -DEFINE_CONFIG_PARSE_ENUM(config_parse_runtime_preserve_mode, exec_preserve_mode, ExecPreserveMode, "Failed to parse runtime directory preserve mode"); +DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_preserve_mode, exec_preserve_mode, ExecPreserveMode, "Failed to parse resource preserve mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_service_type, service_type, ServiceType, "Failed to parse service type"); DEFINE_CONFIG_PARSE_ENUM(config_parse_service_exit_type, service_exit_type, ServiceExitType, "Failed to parse service exit type"); DEFINE_CONFIG_PARSE_ENUM(config_parse_service_restart, service_restart, ServiceRestart, "Failed to parse service restart specifier"); diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index ab682ee23e..98adf5ae05 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -101,7 +101,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_exec_selinux_context); CONFIG_PARSER_PROTOTYPE(config_parse_exec_apparmor_profile); CONFIG_PARSER_PROTOTYPE(config_parse_exec_smack_process_label); CONFIG_PARSER_PROTOTYPE(config_parse_address_families); -CONFIG_PARSER_PROTOTYPE(config_parse_runtime_preserve_mode); +CONFIG_PARSER_PROTOTYPE(config_parse_exec_preserve_mode); CONFIG_PARSER_PROTOTYPE(config_parse_exec_directories); CONFIG_PARSER_PROTOTYPE(config_parse_set_credential); CONFIG_PARSER_PROTOTYPE(config_parse_load_credential); diff --git a/src/core/service.c b/src/core/service.c index 934500abd6..7130cbf4e6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -69,6 +69,7 @@ static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_FAILED] = UNIT_FAILED, [SERVICE_DEAD_BEFORE_AUTO_RESTART] = UNIT_INACTIVE, [SERVICE_FAILED_BEFORE_AUTO_RESTART] = UNIT_FAILED, + [SERVICE_DEAD_RESOURCES_PINNED] = UNIT_INACTIVE, [SERVICE_AUTO_RESTART] = UNIT_ACTIVATING, [SERVICE_CLEANING] = UNIT_MAINTENANCE, }; @@ -97,6 +98,7 @@ static const UnitActiveState state_translation_table_idle[_SERVICE_STATE_MAX] = [SERVICE_FAILED] = UNIT_FAILED, [SERVICE_DEAD_BEFORE_AUTO_RESTART] = UNIT_INACTIVE, [SERVICE_FAILED_BEFORE_AUTO_RESTART] = UNIT_FAILED, + [SERVICE_DEAD_RESOURCES_PINNED] = UNIT_INACTIVE, [SERVICE_AUTO_RESTART] = UNIT_ACTIVATING, [SERVICE_CLEANING] = UNIT_MAINTENANCE, }; @@ -139,6 +141,8 @@ static void service_init(Unit *u) { s->oom_policy = _OOM_POLICY_INVALID; s->reload_begin_usec = USEC_INFINITY; s->reload_signal = SIGHUP; + + s->fd_store_preserve_mode = EXEC_PRESERVE_RESTART; } static void service_unwatch_control_pid(Service *s) { @@ -1031,8 +1035,10 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) { if (s->n_fd_store_max > 0) fprintf(f, "%sFile Descriptor Store Max: %u\n" + "%sFile Descriptor Store Pin: %s\n" "%sFile Descriptor Store Current: %zu\n", prefix, s->n_fd_store_max, + prefix, exec_preserve_mode_to_string(s->fd_store_preserve_mode), prefix, s->n_fd_store); service_dump_fdstore(s, f, prefix); @@ -1244,7 +1250,8 @@ static void service_set_state(Service *s, ServiceState state) { if (IN_SET(state, SERVICE_DEAD, SERVICE_FAILED, - SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART, SERVICE_AUTO_RESTART)) { + SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART, SERVICE_AUTO_RESTART, + SERVICE_DEAD_RESOURCES_PINNED)) { unit_unwatch_all_pids(UNIT(s)); unit_dequeue_rewatch_pids(UNIT(s)); } @@ -1351,7 +1358,8 @@ static int service_coldplug(Unit *u) { if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART, SERVICE_AUTO_RESTART, - SERVICE_CLEANING)) { + SERVICE_CLEANING, + SERVICE_DEAD_RESOURCES_PINNED)) { (void) unit_enqueue_rewatch_pids(u); (void) unit_setup_exec_runtime(u); } @@ -1939,6 +1947,12 @@ static bool service_will_restart(Unit *u) { return unit_will_restart_default(u); } +static ServiceState service_determine_dead_state(Service *s) { + assert(s); + + return s->fd_store && s->fd_store_preserve_mode == EXEC_PRESERVE_YES ? SERVICE_DEAD_RESOURCES_PINNED : SERVICE_DEAD; +} + static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) { ServiceState end_state, restart_state; int r; @@ -1955,11 +1969,11 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) if (s->result == SERVICE_SUCCESS) { unit_log_success(UNIT(s)); - end_state = SERVICE_DEAD; + end_state = service_determine_dead_state(s); restart_state = SERVICE_DEAD_BEFORE_AUTO_RESTART; } else if (s->result == SERVICE_SKIP_CONDITION) { unit_log_skip(UNIT(s), service_result_to_string(s->result)); - end_state = SERVICE_DEAD; + end_state = service_determine_dead_state(s); restart_state = SERVICE_DEAD_BEFORE_AUTO_RESTART; } else { unit_log_failure(UNIT(s), service_result_to_string(s->result)); @@ -2023,6 +2037,10 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) /* Also, remove the runtime directory */ unit_destroy_runtime_data(UNIT(s), &s->exec_context); + /* Also get rid of the fd store, if that's configured. */ + if (s->fd_store_preserve_mode == EXEC_PRESERVE_NO) + service_release_fd_store(s); + /* Get rid of the IPC bits of the user */ unit_unref_uid_gid(UNIT(s), true); @@ -2701,7 +2719,7 @@ static int service_start(Unit *u) { if (IN_SET(s->state, SERVICE_AUTO_RESTART, SERVICE_DEAD_BEFORE_AUTO_RESTART, SERVICE_FAILED_BEFORE_AUTO_RESTART)) return -EAGAIN; - assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)); + assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_RESOURCES_PINNED)); r = unit_acquire_invocation_id(u); if (r < 0) @@ -2760,7 +2778,7 @@ static int service_stop(Unit *u) { case SERVICE_AUTO_RESTART: /* A restart will be scheduled or is in progress. */ - service_set_state(s, SERVICE_DEAD); + service_set_state(s, service_determine_dead_state(s)); return 0; case SERVICE_CONDITION: @@ -2789,6 +2807,7 @@ static int service_stop(Unit *u) { case SERVICE_FAILED_BEFORE_AUTO_RESTART: case SERVICE_DEAD: case SERVICE_FAILED: + case SERVICE_DEAD_RESOURCES_PINNED: default: /* Unknown state, or unit_stop() should already have handled these */ assert_not_reached(); @@ -3397,7 +3416,7 @@ static bool service_may_gc(Unit *u) { /* Only allow collection of actually dead services, i.e. not those that are in the transitionary * SERVICE_DEAD_BEFORE_AUTO_RESTART/SERVICE_FAILED_BEFORE_AUTO_RESTART states. */ - if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)) + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_RESOURCES_PINNED)) return false; return true; @@ -3624,6 +3643,7 @@ static void service_notify_cgroup_empty_event(Unit *u) { case SERVICE_DEAD_BEFORE_AUTO_RESTART: case SERVICE_FAILED_BEFORE_AUTO_RESTART: case SERVICE_AUTO_RESTART: + case SERVICE_DEAD_RESOURCES_PINNED: unit_prune_cgroup(u); break; @@ -4712,7 +4732,7 @@ int service_set_socket_fd( assert(!s->socket_peer); - if (s->state != SERVICE_DEAD) + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_DEAD_RESOURCES_PINNED)) return -EAGAIN; if (getpeername_pretty(fd, true, &peer_text) >= 0) { @@ -4749,7 +4769,7 @@ static void service_reset_failed(Unit *u) { assert(s); if (s->state == SERVICE_FAILED) - service_set_state(s, SERVICE_DEAD); + service_set_state(s, service_determine_dead_state(s)); s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; @@ -4920,14 +4940,19 @@ static void service_release_resources(Unit *u) { /* Don't release resources if this is a transitionary failed/dead state * (i.e. SERVICE_DEAD_BEFORE_AUTO_RESTART/SERVICE_FAILED_BEFORE_AUTO_RESTART), insist on a permanent * failure state. */ - if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)) + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_DEAD_RESOURCES_PINNED)) return; log_unit_debug(u, "Releasing resources..."); service_close_socket_fd(s); service_release_stdio_fd(s); - service_release_fd_store(s); + + if (s->fd_store_preserve_mode != EXEC_PRESERVE_YES) + service_release_fd_store(s); + + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + service_set_state(s, SERVICE_DEAD); } static const char* const service_restart_table[_SERVICE_RESTART_MAX] = { diff --git a/src/core/service.h b/src/core/service.h index 03edb18e31..1d1aa91fc6 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -206,6 +206,7 @@ struct Service { ServiceFDStore *fd_store; size_t n_fd_store; unsigned n_fd_store_max; + ExecPreserveMode fd_store_preserve_mode; char *usb_function_descriptors; char *usb_function_strings; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 1c991ae54f..ebbd1f7f28 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2198,7 +2198,8 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con "USBFunctionStrings", "OOMPolicy", "TimeoutStartFailureMode", - "TimeoutStopFailureMode")) + "TimeoutStopFailureMode", + "FileDescriptorStorePreserve")) return bus_append_string(m, field, eq); if (STR_IN_SET(field, "PermissionsStartOnly", From 4fb8f1e88322b94b0fa051d3c6fd19cac0227aaa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Mar 2023 22:10:01 +0200 Subject: [PATCH 4/9] service: allow freeing the fdstore via cleaning Now that we have a potentially pinned fdstore let's add a concept for cleaning it explicitly on user requested. Let's expose this via "systemctl clean", i.e. the same way as user directories are cleaned. --- man/systemctl.xml | 28 +++++++++++------- src/core/dbus-unit.c | 21 ++++++------- src/core/execute.c | 17 +++++++++++ src/core/execute.h | 5 +++- src/core/service.c | 36 ++++++++++++++++++++--- src/core/timer.c | 1 + src/systemctl/systemctl-clean-or-freeze.c | 2 +- src/systemctl/systemctl.c | 3 +- 8 files changed, 86 insertions(+), 27 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index 1a881d1049..705ed3e238 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -538,12 +538,16 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err StateDirectory=, CacheDirectory=, LogsDirectory= and RuntimeDirectory=, see systemd.exec5 - for details. For timer units this may be used to clear out the persistent timestamp data if + for details. It may also be used to clear the file decriptor store as enabled via + FileDescriptorStoreMax=, see + systemd.service5 + for details. For timer units this may be used to clear out the persistent timestamp data if Persistent= is used and is selected, see systemd.timer5. This command only applies to units that use either of these settings. If is - not specified, both the cache and runtime data are removed (as these two types of data are - generally redundant and reproducible on the next invocation of the unit). + not specified, the cache and runtime data as well as the file descriptor store are removed (as + these three types of resources are generally redundant and reproducible on the next invocation of + the unit). Note that the specified units must be stopped to invoke this operation. @@ -2193,13 +2197,17 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err Select what type of per-unit resources to remove when the clean command is - invoked, see below. Takes one of configuration, state, - cache, logs, runtime to select the - type of resource. This option may be specified more than once, in which case all specified resource - types are removed. Also accepts the special value all as a shortcut for - specifying all five resource types. If this option is not specified defaults to the combination of - cache and runtime, i.e. the two kinds of resources that - are generally considered to be redundant and can be reconstructed on next invocation. + invoked, see above. Takes one of configuration, state, + cache, logs, runtime, + fdstore to select the type of resource. This option may be specified more than + once, in which case all specified resource types are removed. Also accepts the special value + all as a shortcut for specifying all six resource types. If this option is not + specified defaults to the combination of cache, runtime + and fdstore, i.e. the three kinds of resources that are generally considered + to be redundant and can be reconstructed on next invocation. Note that the explicit removal of the + fdstore resource type is only useful if the + FileDescriptorStorePreserve= option is enabled, since the file descriptor store + is otherwise cleaned automatically when the unit is stopped. diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 3f083a8174..c01f41cb44 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -90,6 +90,12 @@ static int property_get_can_clean( return r; } + if (FLAGS_SET(mask, EXEC_CLEAN_FDSTORE)) { + r = sd_bus_message_append(reply, "s", "fdstore"); + if (r < 0) + return r; + } + return sd_bus_message_close_container(reply); } @@ -696,6 +702,7 @@ int bus_unit_method_clean(sd_bus_message *message, void *userdata, sd_bus_error return r; for (;;) { + ExecCleanMask m; const char *i; r = sd_bus_message_read(message, "s", &i); @@ -704,17 +711,11 @@ int bus_unit_method_clean(sd_bus_message *message, void *userdata, sd_bus_error if (r == 0) break; - if (streq(i, "all")) - mask |= EXEC_CLEAN_ALL; - else { - ExecDirectoryType t; + m = exec_clean_mask_from_string(i); + if (m < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid resource type: %s", i); - t = exec_resource_type_from_string(i); - if (t < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid resource type: %s", i); - - mask |= 1U << t; - } + mask |= m; } r = sd_bus_message_exit_container(message); diff --git a/src/core/execute.c b/src/core/execute.c index 8b09794089..879f1345b0 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -7582,6 +7582,23 @@ void exec_directory_sort(ExecDirectory *d) { } } +ExecCleanMask exec_clean_mask_from_string(const char *s) { + ExecDirectoryType t; + + assert(s); + + if (streq(s, "all")) + return EXEC_CLEAN_ALL; + if (streq(s, "fdstore")) + return EXEC_CLEAN_FDSTORE; + + t = exec_resource_type_from_string(s); + if (t < 0) + return (ExecCleanMask) t; + + return 1U << t; +} + DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(exec_set_credential_hash_ops, char, string_hash_func, string_compare_func, ExecSetCredential, exec_set_credential_free); DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(exec_load_credential_hash_ops, char, string_hash_func, string_compare_func, ExecLoadCredential, exec_load_credential_free); diff --git a/src/core/execute.h b/src/core/execute.h index 123fc1ec60..d2f5507405 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -161,8 +161,9 @@ typedef enum ExecCleanMask { EXEC_CLEAN_CACHE = 1U << EXEC_DIRECTORY_CACHE, EXEC_CLEAN_LOGS = 1U << EXEC_DIRECTORY_LOGS, EXEC_CLEAN_CONFIGURATION = 1U << EXEC_DIRECTORY_CONFIGURATION, + EXEC_CLEAN_FDSTORE = 1U << _EXEC_DIRECTORY_TYPE_MAX, EXEC_CLEAN_NONE = 0, - EXEC_CLEAN_ALL = (1U << _EXEC_DIRECTORY_TYPE_MAX) - 1, + EXEC_CLEAN_ALL = (1U << (_EXEC_DIRECTORY_TYPE_MAX+1)) - 1, _EXEC_CLEAN_MASK_INVALID = -EINVAL, } ExecCleanMask; @@ -520,6 +521,8 @@ void exec_directory_done(ExecDirectory *d); int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink); void exec_directory_sort(ExecDirectory *d); +ExecCleanMask exec_clean_mask_from_string(const char *s); + extern const struct hash_ops exec_set_credential_hash_ops; extern const struct hash_ops exec_load_credential_hash_ops; diff --git a/src/core/service.c b/src/core/service.c index 7130cbf4e6..dcd62fabdc 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4857,22 +4857,39 @@ static const char* service_status_text(Unit *u) { static int service_clean(Unit *u, ExecCleanMask mask) { _cleanup_strv_free_ char **l = NULL; + bool may_clean_fdstore = false; Service *s = SERVICE(u); int r; assert(s); assert(mask != 0); - if (s->state != SERVICE_DEAD) + if (!IN_SET(s->state, SERVICE_DEAD, SERVICE_DEAD_RESOURCES_PINNED)) return -EBUSY; + /* Determine if there's anything we could potentially clean */ r = exec_context_get_clean_directories(&s->exec_context, u->manager->prefix, mask, &l); if (r < 0) return r; - if (strv_isempty(l)) - return -EUNATCH; + if (mask & EXEC_CLEAN_FDSTORE) + may_clean_fdstore = s->n_fd_store > 0 || s->n_fd_store_max > 0; + if (strv_isempty(l) && !may_clean_fdstore) + return -EUNATCH; /* Nothing to potentially clean */ + + /* Let's clean the stuff we can clean quickly */ + if (may_clean_fdstore) + service_release_fd_store(s); + + /* If we are done, leave quickly */ + if (strv_isempty(l)) { + if (s->state == SERVICE_DEAD_RESOURCES_PINNED && !s->fd_store) + service_set_state(s, SERVICE_DEAD); + return 0; + } + + /* We need to clean disk stuff. This is slow, hence do it out of process, and change state */ service_unwatch_control_pid(s); s->clean_result = SERVICE_SUCCESS; s->control_command = NULL; @@ -4899,10 +4916,21 @@ fail: static int service_can_clean(Unit *u, ExecCleanMask *ret) { Service *s = SERVICE(u); + ExecCleanMask mask = 0; + int r; assert(s); + assert(ret); - return exec_context_get_clean_mask(&s->exec_context, ret); + r = exec_context_get_clean_mask(&s->exec_context, &mask); + if (r < 0) + return r; + + if (s->n_fd_store_max > 0) + mask |= EXEC_CLEAN_FDSTORE; + + *ret = mask; + return 0; } static const char *service_finished_job(Unit *u, JobType t, JobResult result) { diff --git a/src/core/timer.c b/src/core/timer.c index 371e42e0aa..419416b325 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -884,6 +884,7 @@ static int timer_can_clean(Unit *u, ExecCleanMask *ret) { Timer *t = TIMER(u); assert(t); + assert(ret); *ret = t->persistent ? EXEC_CLEAN_STATE : 0; return 0; diff --git a/src/systemctl/systemctl-clean-or-freeze.c b/src/systemctl/systemctl-clean-or-freeze.c index 5c15a9fba0..40d5f6d557 100644 --- a/src/systemctl/systemctl-clean-or-freeze.c +++ b/src/systemctl/systemctl-clean-or-freeze.c @@ -21,7 +21,7 @@ int verb_clean_or_freeze(int argc, char *argv[], void *userdata) { polkit_agent_open_maybe(); if (!arg_clean_what) { - arg_clean_what = strv_new("cache", "runtime"); + arg_clean_what = strv_new("cache", "runtime", "fdstore"); if (!arg_clean_what) return log_oom(); } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index df4cf68435..21e09536a6 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -937,7 +937,8 @@ static int systemctl_parse_argv(int argc, char *argv[]) { "state\n" "cache\n" "logs\n" - "configuration"); + "configuration\n" + "fdstore"); return 0; } From 99620f457ed0886852ba18c9093b59767299121c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Apr 2023 12:17:16 +0200 Subject: [PATCH 5/9] service: close fdstore asynchronously The file descriptors we keep in the fdstore might be basically anything, let's clean it up with our asynchronous closing feature, to not deadlock on close(). (Let's also do the same for stdin/stdout/stderr fds, since they might point to network services these days.) --- TODO | 2 -- src/core/service.c | 15 ++++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/TODO b/TODO index 29b1ac6601..19a2535e1b 100644 --- a/TODO +++ b/TODO @@ -1479,8 +1479,6 @@ Features: * maybe rework get_user_creds() to query the user database if $SHELL is used for root, but only then. -* be stricter with fds we receive for the fdstore: close them asynchronously - * calenderspec: add support for week numbers and day numbers within a year. This would allow us to define "bi-weekly" triggers safely. diff --git a/src/core/service.c b/src/core/service.c index dcd62fabdc..c1669da3a2 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -389,7 +389,7 @@ static void service_fd_store_unlink(ServiceFDStore *fs) { sd_event_source_disable_unref(fs->event_source); free(fs->fdname); - safe_close(fs->fd); + asynchronous_close(fs->fd); free(fs); } @@ -415,9 +415,9 @@ static void service_release_stdio_fd(Service *s) { log_unit_debug(UNIT(s), "Releasing stdin/stdout/stderr file descriptors."); - s->stdin_fd = safe_close(s->stdin_fd); - s->stdout_fd = safe_close(s->stdout_fd); - s->stderr_fd = safe_close(s->stderr_fd); + s->stdin_fd = asynchronous_close(s->stdin_fd); + s->stdout_fd = asynchronous_close(s->stdout_fd); + s->stderr_fd = asynchronous_close(s->stderr_fd); } static void service_done(Unit *u) { Service *s = SERVICE(u); @@ -500,7 +500,7 @@ static int service_add_fd_store(Service *s, int fd, const char *name, bool do_po if (r < 0) return r; if (r > 0) { - safe_close(fd); + asynchronous_close(fd); return 0; /* fd already included */ } } @@ -543,7 +543,7 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name, bo assert(s); while (fdset_size(fds) > 0) { - _cleanup_close_ int fd = -EBADF; + _cleanup_(asynchronous_closep) int fd = -EBADF; fd = fdset_steal_first(fds); if (fd < 0) @@ -558,7 +558,8 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name, bo return log_unit_error_errno(UNIT(s), r, "Failed to add fd to store: %m"); if (r > 0) log_unit_debug(UNIT(s), "Added fd %i (%s) to fd store.", fd, strna(name)); - fd = -EBADF; + + TAKE_FD(fd); } return 0; From 1ba84fef3c1e505cb1413ef446a85c0c7ec439c6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Apr 2023 13:42:08 +0200 Subject: [PATCH 6/9] core: move runtime directory removal into release_resource handler We already clear the various fds we keep from the release_resources() handler, let's also destroy the runtime dir from there if this preservation mode is selected. This makes a minor semantic change: previously we'd keep a runtime directory around if RuntimeDirectoryPreserve=restart is selected and at least one JOB_START job was around. With this logic we'll keep it around a tiny bit longer: as long as any job for the unit is around. --- src/core/unit.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 089213cf88..d632c34f98 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -396,6 +396,7 @@ static bool unit_success_failure_handler_has_jobs(Unit *unit) { void unit_release_resources(Unit *u) { UnitActiveState state; + ExecContext *ec; assert(u); @@ -412,6 +413,10 @@ void unit_release_resources(Unit *u) { if (unit_will_restart(u)) return; + ec = unit_get_exec_context(u); + if (ec && ec->runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART) + exec_context_destroy_runtime_directory(ec, u->manager->prefix[EXEC_DIRECTORY_RUNTIME]); + if (UNIT_VTABLE(u)->release_resources) UNIT_VTABLE(u)->release_resources(u); } @@ -582,6 +587,21 @@ void unit_submit_to_stop_when_bound_queue(Unit *u) { u->in_stop_when_bound_queue = true; } +static bool unit_can_release_resources(Unit *u) { + ExecContext *ec; + + assert(u); + + if (UNIT_VTABLE(u)->release_resources) + return true; + + ec = unit_get_exec_context(u); + if (ec && ec->runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART) + return true; + + return false; +} + void unit_submit_to_release_resources_queue(Unit *u) { assert(u); @@ -594,7 +614,7 @@ void unit_submit_to_release_resources_queue(Unit *u) { if (u->perpetual) return; - if (!UNIT_VTABLE(u)->release_resources) + if (!unit_can_release_resources(u)) return; LIST_PREPEND(release_resources_queue, u->manager->release_resources_queue, u); @@ -5863,8 +5883,8 @@ void unit_destroy_runtime_data(Unit *u, const ExecContext *context) { assert(u); assert(context); - if (context->runtime_directory_preserve_mode == EXEC_PRESERVE_NO || - (context->runtime_directory_preserve_mode == EXEC_PRESERVE_RESTART && !unit_will_restart(u))) + /* EXEC_PRESERVE_RESTART is handled via unit_release_resources()! */ + if (context->runtime_directory_preserve_mode == EXEC_PRESERVE_NO) exec_context_destroy_runtime_directory(context, u->manager->prefix[EXEC_DIRECTORY_RUNTIME]); exec_context_destroy_credentials(context, u->manager->prefix[EXEC_DIRECTORY_RUNTIME], u->id); From 81a1d6d6790ad6ae8ff63147dfab68e6835178c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Apr 2023 15:51:07 +0200 Subject: [PATCH 7/9] =?UTF-8?q?service:=20rename=20service=5Fclose=5Fsocke?= =?UTF-8?q?t=5Ffd()=20=E2=86=92=20service=5Frelease=5Fsocket=5Ffd()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just to match service_release_stdio_fd() and service_release_fd_store() in the name, since they do similar things. This follows the concept that we "release" resources, and this is all generically wrapped in "service_release_resources()". --- src/core/service.c | 9 ++++----- src/core/service.h | 2 +- src/core/socket.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index c1669da3a2..e59c579627 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -200,7 +200,7 @@ static int service_set_main_pid(Service *s, pid_t pid) { return 0; } -void service_close_socket_fd(Service *s) { +void service_release_socket_fd(Service *s) { assert(s); if (s->socket_fd < 0 && !UNIT_ISSET(s->accept_socket) && !s->socket_peer) @@ -454,8 +454,6 @@ static void service_done(Unit *u) { s->usb_function_descriptors = mfree(s->usb_function_descriptors); s->usb_function_strings = mfree(s->usb_function_strings); - service_close_socket_fd(s); - service_stop_watchdog(s); s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); @@ -463,8 +461,9 @@ static void service_done(Unit *u) { s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot); - service_release_fd_store(s); + service_release_socket_fd(s); service_release_stdio_fd(s); + service_release_fd_store(s); } static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *userdata) { @@ -4974,7 +4973,7 @@ static void service_release_resources(Unit *u) { log_unit_debug(u, "Releasing resources..."); - service_close_socket_fd(s); + service_release_socket_fd(s); service_release_stdio_fd(s); if (s->fd_store_preserve_mode != EXEC_PRESERVE_YES) diff --git a/src/core/service.h b/src/core/service.h index 1d1aa91fc6..5b7f67457e 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -244,7 +244,7 @@ static inline usec_t service_get_watchdog_usec(Service *s) { extern const UnitVTable service_vtable; int service_set_socket_fd(Service *s, int fd, struct Socket *socket, struct SocketPeer *peer, bool selinux_context_net); -void service_close_socket_fd(Service *s); +void service_release_socket_fd(Service *s); usec_t service_restart_usec_next(Service *s); diff --git a/src/core/socket.c b/src/core/socket.c index c26daced1a..891276fd50 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2393,7 +2393,7 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (r < 0) { /* 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_close_socket_fd(SERVICE(service)); + service_release_socket_fd(SERVICE(service)); goto fail; } From e8783d7620e4811738be078480aa5ffc9bbdcf9b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 12 Apr 2023 21:07:29 +0200 Subject: [PATCH 8/9] pid1: add some debug logging when stashing ds into the fdstore --- src/core/service.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index e59c579627..1c31782fab 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -481,6 +481,7 @@ static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *us } static int service_add_fd_store(Service *s, int fd, const char *name, bool do_poll) { + struct stat st; ServiceFDStore *fs; int r; @@ -489,16 +490,22 @@ static int service_add_fd_store(Service *s, int fd, const char *name, bool do_po assert(s); assert(fd >= 0); + if (fstat(fd, &st) < 0) + return -errno; + + log_unit_debug(UNIT(s), "Trying to stash fd for dev=" DEVNUM_FORMAT_STR "/inode=%" PRIu64, DEVNUM_FORMAT_VAL(st.st_dev), (uint64_t) st.st_ino); + if (s->n_fd_store >= s->n_fd_store_max) - return -EXFULL; /* Our store is full. - * Use this errno rather than E[NM]FILE to distinguish from - * the case where systemd itself hits the file limit. */ + /* Our store is full. Use this errno rather than E[NM]FILE to distinguish from the case + * where systemd itself hits the file limit. */ + return log_unit_debug_errno(UNIT(s), SYNTHETIC_ERRNO(EXFULL), "Hit fd store limit."); LIST_FOREACH(fd_store, i, s->fd_store) { r = same_fd(i->fd, fd); if (r < 0) return r; if (r > 0) { + log_unit_debug(UNIT(s), "Suppressing duplicate fd in fd store."); asynchronous_close(fd); return 0; /* fd already included */ } From 3540ce8587cbd21ce9c2dbec72ea7fa3d1b38a5f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Apr 2023 11:41:55 +0200 Subject: [PATCH 9/9] test: validate that fdstore pinning works --- test/testsuite-80.units/fdstore-nopin.service | 8 +++ test/testsuite-80.units/fdstore-pin.service | 8 +++ test/testsuite-80.units/fdstore-pin.sh | 47 +++++++++++++++++ test/testsuite-80.units/fdstore-pin.target | 3 ++ test/units/testsuite-80.service | 2 + test/units/testsuite-80.sh | 51 +++++++++++++++++++ 6 files changed, 119 insertions(+) create mode 100644 test/testsuite-80.units/fdstore-nopin.service create mode 100644 test/testsuite-80.units/fdstore-pin.service create mode 100755 test/testsuite-80.units/fdstore-pin.sh create mode 100644 test/testsuite-80.units/fdstore-pin.target diff --git a/test/testsuite-80.units/fdstore-nopin.service b/test/testsuite-80.units/fdstore-nopin.service new file mode 100644 index 0000000000..58a687a411 --- /dev/null +++ b/test/testsuite-80.units/fdstore-nopin.service @@ -0,0 +1,8 @@ +[Service] +Type=notify +NotifyAccess=all +FileDescriptorStoreMax=10 +FileDescriptorStorePreserve=restart +ExecStart=/usr/lib/systemd/tests/testdata/testsuite-80.units/fdstore-pin.sh 0 +StandardOutput=journal+console +StandardError=journal+console diff --git a/test/testsuite-80.units/fdstore-pin.service b/test/testsuite-80.units/fdstore-pin.service new file mode 100644 index 0000000000..bc78ee0a4c --- /dev/null +++ b/test/testsuite-80.units/fdstore-pin.service @@ -0,0 +1,8 @@ +[Service] +Type=notify +NotifyAccess=all +FileDescriptorStoreMax=10 +FileDescriptorStorePreserve=yes +ExecStart=/usr/lib/systemd/tests/testdata/testsuite-80.units/fdstore-pin.sh 1 +StandardOutput=journal+console +StandardError=journal+console diff --git a/test/testsuite-80.units/fdstore-pin.sh b/test/testsuite-80.units/fdstore-pin.sh new file mode 100755 index 0000000000..4cb041a284 --- /dev/null +++ b/test/testsuite-80.units/fdstore-pin.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eux +set -o pipefail + +PINNED="$1" +COUNTER="/tmp/fdstore-invoked.$PINNED" +FILE="/tmp/fdstore-data.$PINNED" + +# This script is called six times: thrice from a service unit where the fdstore +# is pinned, and thrice where it isn't. The second iteration of each series is +# a restart, the third a stop followed by a start + +if [ -e "$COUNTER" ] ; then + read -r N < "$COUNTER" +else + N=0 +fi + +echo "Invocation #$N with PINNED=$PINNED." + +if [ "$N" -eq 0 ] ; then + # First iteration + test "${LISTEN_FDS:-0}" -eq 0 + test ! -e "$FILE" + echo waldi > "$FILE" + systemd-notify --fd=3 --fdname="fd-$N-$PINNED" 3< "$FILE" +elif [ "$N" -eq 1 ] || { [ "$N" -eq 2 ] && [ "$PINNED" -eq 1 ]; } ; then + # Second iteration, or iteration with pinning on + test "${LISTEN_FDS:-0}" -eq 1 + # We reopen fd #3 here, so that the read offset is at zero each time (hence no <&3 here…) + read -r word < /proc/self/fd/3 + test "$word" = "waldi" +else + test "${LISTEN_FDS:-0}" -eq 0 + test -e "$FILE" +fi + +if [ "$N" -ge 2 ] ; then + rm "$COUNTER" "$FILE" +else + echo $((N + 1)) > "$COUNTER" +fi + +systemd-notify --ready --status="Ready" + +exec sleep infinity diff --git a/test/testsuite-80.units/fdstore-pin.target b/test/testsuite-80.units/fdstore-pin.target new file mode 100644 index 0000000000..319b7e1da8 --- /dev/null +++ b/test/testsuite-80.units/fdstore-pin.target @@ -0,0 +1,3 @@ +[Unit] +After=fdstore-pin.service fdstore-nopin.service +Wants=fdstore-pin.service fdstore-nopin.service diff --git a/test/units/testsuite-80.service b/test/units/testsuite-80.service index 4c7f5d5ebd..82b08a1c8b 100644 --- a/test/units/testsuite-80.service +++ b/test/units/testsuite-80.service @@ -6,3 +6,5 @@ Description=TEST-80-NOTIFYACCESS ExecStartPre=rm -f /failed /testok ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh Type=oneshot +StandardOutput=journal+console +StandardError=journal+console diff --git a/test/units/testsuite-80.sh b/test/units/testsuite-80.sh index 15ea36bf88..7440b241be 100755 --- a/test/units/testsuite-80.sh +++ b/test/units/testsuite-80.sh @@ -48,6 +48,8 @@ assert_eq "$(systemctl show notify.service -p NotifyAccess --value)" "all" rm /tmp/syncfifo1 /tmp/syncfifo2 +# Now test basic fdstore behaviour + MYSCRIPT="/tmp/myscript$RANDOM.sh" cat >> "$MYSCRIPT" <<'EOF' #!/usr/bin/env bash @@ -74,6 +76,55 @@ systemd-analyze fdstore "$MYUNIT" --json=short | grep -P -q '\[{"fdname":"quux", systemctl stop "$MYUNIT" rm "$MYSCRIPT" +systemd-analyze log-level debug + +# Test fdstore pinning (this will pull in fdstore-pin.service fdstore-nopin.service) +systemctl start fdstore-pin.target + +assert_eq "$(systemctl show fdstore-pin.service -P FileDescriptorStorePreserve)" yes +assert_eq "$(systemctl show fdstore-nopin.service -P FileDescriptorStorePreserve)" restart +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" running +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" running +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 1 + +# The file descriptor store should survive service restarts +systemctl restart fdstore-pin.service fdstore-nopin.service + +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" running +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" running + +# It should not survive the service stop plus a later start (unless pinned) +systemctl stop fdstore-pin.service fdstore-nopin.service + +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 0 +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" dead-resources-pinned +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" dead + +systemctl start fdstore-pin.service fdstore-nopin.service + +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 0 +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" running +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" running + +systemctl stop fdstore-pin.service fdstore-nopin.service + +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 1 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 0 +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" dead-resources-pinned +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" dead + +systemctl clean fdstore-pin.service --what=fdstore + +assert_eq "$(systemctl show fdstore-pin.service -P NFileDescriptorStore)" 0 +assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 0 +assert_eq "$(systemctl show fdstore-pin.service -P SubState)" dead +assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" dead + touch /testok rm /failed