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 3dd6e8abe3a9387d30bccde3fc520cabd1e95a61..15ccfdc2d3d0e0925ec916439c4e8c4eaef317e4 100644 (file)
--- 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 aed2c988af3bd29a8f58dfa12407d2601246c7c9..cba3aae9ff900e6df89a3bb8943e8316730d1af5 100644 (file)
--- 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 55543703cda778d71ee0acf25654c3ce30c2ad81..b06270fccc6b31b28a8859aa486caaf94814461a 100644 (file)
--- 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 ba3af56303d6563f24c06ee8948253ed746389f5..2f6c13515219c6d9a6b8580c2d0a955e4d0fae2a 100644 (file)
--- 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. */