From: Mike Yuan Date: Wed, 27 Sep 2023 10:38:10 +0000 (+0800) Subject: core: mark units as need daemon-reload if unit file operations are X-Git-Tag: v255-rc1~399 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a82b8b3dc80619c3275ad8180069289b411206d0;p=thirdparty%2Fsystemd.git core: mark units as need daemon-reload if unit file operations are performed systemctl would issue daemon-reload after unit file operations (enable/disable/preset/...) succeed. However, such operations are not atomic, meaning that the unit file state could still change even if the operation generally fails, and the unit_file_state cached by manager becomes outdated. Fixes #29341 --- diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 3dd6e8abe3a..15ccfdc2d3d 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2543,6 +2543,7 @@ static int method_enable_unit_files_generic( return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = call(m->runtime_scope, flags, NULL, l, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); @@ -2615,6 +2616,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_preset(m->runtime_scope, flags, NULL, l, preset_mode, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); @@ -2668,6 +2670,7 @@ static int method_disable_unit_files_generic( return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = call(m->runtime_scope, flags, NULL, l, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); @@ -2710,6 +2713,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_ return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_revert(m->runtime_scope, NULL, l, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); @@ -2782,6 +2786,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata, return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ r = unit_file_preset_all(m->runtime_scope, flags, NULL, preset_mode, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); @@ -2821,6 +2826,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd return -EINVAL; r = unit_file_add_dependency(m->runtime_scope, flags, NULL, l, target, dep, &changes, &n_changes); + m->unit_file_state_outdated = m->unit_file_state_outdated || n_changes > 0; /* See comments for this variable in manager.h */ if (r < 0) return install_error(error, r, changes, n_changes); diff --git a/src/core/manager.c b/src/core/manager.c index aed2c988af3..cba3aae9ff9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3557,6 +3557,7 @@ int manager_reload(Manager *m) { /* We flushed out generated files, for which we don't watch mtime, so we should flush the old map. */ manager_free_unit_name_maps(m); + m->unit_file_state_outdated = false; /* First, enumerate what we can from kernel and suchlike */ manager_enumerate_perpetual(m); diff --git a/src/core/manager.h b/src/core/manager.h index 55543703cda..b06270fccc6 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -288,6 +288,12 @@ struct Manager { Set *unit_path_cache; uint64_t unit_cache_timestamp_hash; + /* We don't have support for atomically enabling/disabling units, and unit_file_state might become + * outdated if such operations failed half-way. Therefore, we set this flag if changes to unit files + * are made, and reset it after daemon-reload. If set, we report that daemon-reload is needed through + * unit's NeedDaemonReload property. */ + bool unit_file_state_outdated; + char **transient_environment; /* The environment, as determined from config files, kernel cmdline and environment generators */ char **client_environment; /* Environment variables created by clients through the bus API */ diff --git a/src/core/unit.c b/src/core/unit.c index ba3af56303d..2f6c1351521 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3851,9 +3851,13 @@ static bool fragment_mtime_newer(const char *path, usec_t mtime, bool path_maske } bool unit_need_daemon_reload(Unit *u) { - _cleanup_strv_free_ char **t = NULL; + _cleanup_strv_free_ char **dropins = NULL; assert(u); + assert(u->manager); + + if (u->manager->unit_file_state_outdated) + return true; /* For unit files, we allow masking… */ if (fragment_mtime_newer(u->fragment_path, u->fragment_mtime, @@ -3865,8 +3869,8 @@ bool unit_need_daemon_reload(Unit *u) { return true; if (u->load_state == UNIT_LOADED) - (void) unit_find_dropin_paths(u, &t); - if (!strv_equal(u->dropin_paths, t)) + (void) unit_find_dropin_paths(u, &dropins); + if (!strv_equal(u->dropin_paths, dropins)) return true; /* … any drop-ins that are masked are simply omitted from the list. */