systemd: Fix timestmap detection for configuration files
Currently, systemd only uses the timestamp of the most recent file for
tracking changes to configurations. With multiple drop-in files this
results in reloads not picking up changes to older files. This patch
fixes the reload behavior.
Issue: https://github.com/systemd/systemd/issues/21113
In-Review: https://github.com/systemd/systemd/pull/21114
Change-Id: I9b92995e0d7faa612b51bfd45dd33803cd566441
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/meta-phosphor/recipes-core/systemd/systemd/0001-conf-parse-make-config_parse_many-optionally-save-st.patch b/meta-phosphor/recipes-core/systemd/systemd/0001-conf-parse-make-config_parse_many-optionally-save-st.patch
new file mode 100644
index 0000000..fedb42d
--- /dev/null
+++ b/meta-phosphor/recipes-core/systemd/systemd/0001-conf-parse-make-config_parse_many-optionally-save-st.patch
@@ -0,0 +1,379 @@
+From 819333d81964fd110565d35a33993b831ba60725 Mon Sep 17 00:00:00 2001
+From: Yu Watanabe <watanabe.yu+github@gmail.com>
+Date: Mon, 25 Oct 2021 11:13:27 +0900
+Subject: [PATCH] conf-parse: make config_parse_many() optionally save 'struct
+ stat' for each file
+
+Fixes #21113.
+---
+ src/core/load-dropin.c | 18 +++---
+ src/network/networkd-network.c | 38 +++++++++---
+ src/network/networkd-network.h | 2 +-
+ src/shared/conf-parser.c | 103 +++++++++++++++++++++++++--------
+ src/shared/conf-parser.h | 8 ++-
+ 5 files changed, 127 insertions(+), 42 deletions(-)
+
+diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c
+index 3bb48564cc..080a63bc7e 100644
+--- a/src/core/load-dropin.c
++++ b/src/core/load-dropin.c
+@@ -113,14 +113,16 @@ int unit_load_dropin(Unit *u) {
+ }
+
+ u->dropin_mtime = 0;
+- STRV_FOREACH(f, u->dropin_paths)
+- (void) config_parse(
+- u->id, *f, NULL,
+- UNIT_VTABLE(u)->sections,
+- config_item_perf_lookup, load_fragment_gperf_lookup,
+- 0,
+- u,
+- &u->dropin_mtime);
++ STRV_FOREACH(f, u->dropin_paths) {
++ struct stat st;
++
++ r = config_parse(u->id, *f, NULL,
++ UNIT_VTABLE(u)->sections,
++ config_item_perf_lookup, load_fragment_gperf_lookup,
++ 0, u, &st);
++ if (r > 0)
++ u->dropin_mtime = MAX(u->dropin_mtime, timespec_load(&st.st_mtim));
++ }
+
+ return 0;
+ }
+diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
+index 850b4f449e..32d76e29e4 100644
+--- a/src/network/networkd-network.c
++++ b/src/network/networkd-network.c
+@@ -480,7 +480,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi
+ config_item_perf_lookup, network_network_gperf_lookup,
+ CONFIG_PARSE_WARN,
+ network,
+- &network->timestamp);
++ &network->stats_by_path);
+ if (r < 0)
+ return r;
+
+@@ -527,6 +527,28 @@ int network_load(Manager *manager, OrderedHashmap **networks) {
+ return 0;
+ }
+
++static bool stats_by_path_equal(Hashmap *a, Hashmap *b) {
++ struct stat *st_a, *st_b;
++ const char *path;
++
++ assert(a);
++ assert(b);
++
++ if (hashmap_size(a) != hashmap_size(b))
++ return false;
++
++ HASHMAP_FOREACH_KEY(st_a, path, a) {
++ st_b = hashmap_get(b, path);
++ if (!st_b)
++ return false;
++
++ if (!stat_inode_unmodified(st_a, st_b))
++ return false;
++ }
++
++ return true;
++}
++
+ int network_reload(Manager *manager) {
+ OrderedHashmap *new_networks = NULL;
+ Network *n, *old;
+@@ -540,14 +562,15 @@ int network_reload(Manager *manager) {
+
+ ORDERED_HASHMAP_FOREACH(n, new_networks) {
+ r = network_get_by_name(manager, n->name, &old);
+- if (r < 0)
+- continue; /* The .network file is new. */
+-
+- if (n->timestamp != old->timestamp)
+- continue; /* The .network file is modified. */
++ if (r < 0) {
++ log_debug("Found new .network file: %s", n->filename);
++ continue;
++ }
+
+- if (!streq(n->filename, old->filename))
++ if (!stats_by_path_equal(n->stats_by_path, old->stats_by_path)) {
++ log_debug("Found updated .network file: %s", n->filename);
+ continue;
++ }
+
+ r = ordered_hashmap_replace(new_networks, old->name, old);
+ if (r < 0)
+@@ -573,6 +596,7 @@ static Network *network_free(Network *network) {
+ return NULL;
+
+ free(network->filename);
++ hashmap_free(network->stats_by_path);
+
+ net_match_clear(&network->match);
+ condition_free_list(network->conditions);
+diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h
+index b39063fe8a..c8d24a415f 100644
+--- a/src/network/networkd-network.h
++++ b/src/network/networkd-network.h
+@@ -72,7 +72,7 @@ struct Network {
+
+ char *name;
+ char *filename;
+- usec_t timestamp;
++ Hashmap *stats_by_path;
+ char *description;
+
+ /* [Match] section */
+diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
+index d0ac1b2660..9a367d757f 100644
+--- a/src/shared/conf-parser.c
++++ b/src/shared/conf-parser.c
+@@ -264,21 +264,18 @@ int config_parse(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *latest_mtime) {
++ struct stat *ret_stat) {
+
+ _cleanup_free_ char *section = NULL, *continuation = NULL;
+ _cleanup_fclose_ FILE *ours = NULL;
+ unsigned line = 0, section_line = 0;
+ bool section_ignored = false, bom_seen = false;
++ struct stat st;
+ int r, fd;
+- usec_t mtime;
+
+ assert(filename);
+ assert(lookup);
+
+- /* latest_mtime is an input-output parameter: it will be updated if the mtime of the file we're
+- * looking at is later than the current *latest_mtime value. */
+-
+ if (!f) {
+ f = ours = fopen(filename, "re");
+ if (!f) {
+@@ -287,22 +284,28 @@ int config_parse(
+ if ((flags & CONFIG_PARSE_WARN) || errno == ENOENT)
+ log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_ERR, errno,
+ "Failed to open configuration file '%s': %m", filename);
+- return errno == ENOENT ? 0 : -errno;
++
++ if (errno == ENOENT) {
++ if (ret_stat)
++ *ret_stat = (struct stat) {};
++
++ return 0;
++ }
++
++ return -errno;
+ }
+ }
+
+ fd = fileno(f);
+ if (fd >= 0) { /* stream might not have an fd, let's be careful hence */
+- struct stat st;
+
+ if (fstat(fd, &st) < 0)
+ return log_full_errno(FLAGS_SET(flags, CONFIG_PARSE_WARN) ? LOG_ERR : LOG_DEBUG, errno,
+ "Failed to fstat(%s): %m", filename);
+
+ (void) stat_warn_permissions(filename, &st);
+- mtime = timespec_load(&st.st_mtim);
+ } else
+- mtime = 0;
++ st = (struct stat) {};
+
+ for (;;) {
+ _cleanup_free_ char *buf = NULL;
+@@ -422,12 +425,43 @@ int config_parse(
+ }
+ }
+
+- if (latest_mtime)
+- *latest_mtime = MAX(*latest_mtime, mtime);
++ if (ret_stat)
++ *ret_stat = st;
+
+ return 1;
+ }
+
++static int hashmap_put_stats_by_path(Hashmap **stats_by_path, const char *path, const struct stat *st) {
++ _cleanup_free_ struct stat *st_copy = NULL;
++ _cleanup_free_ char *path_copy = NULL;
++ int r;
++
++ assert(stats_by_path);
++ assert(path);
++ assert(st);
++
++ r = hashmap_ensure_allocated(stats_by_path, &path_hash_ops_free_free);
++ if (r < 0)
++ return r;
++
++ st_copy = newdup(struct stat, st, 1);
++ if (!st_copy)
++ return -ENOMEM;
++
++ path_copy = strdup(path);
++ if (!path)
++ return -ENOMEM;
++
++ r = hashmap_put(*stats_by_path, path_copy, st_copy);
++ if (r < 0)
++ return r;
++
++ assert(r > 0);
++ TAKE_PTR(path_copy);
++ TAKE_PTR(st_copy);
++ return 0;
++}
++
+ static int config_parse_many_files(
+ const char* const* conf_files,
+ char **files,
+@@ -436,30 +470,53 @@ static int config_parse_many_files(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *ret_mtime) {
++ Hashmap **ret_stats_by_path) {
+
+- usec_t mtime = 0;
++ _cleanup_hashmap_free_ Hashmap *stats_by_path = NULL;
++ struct stat st;
+ char **fn;
+ int r;
+
++ if (ret_stats_by_path) {
++ stats_by_path = hashmap_new(&path_hash_ops_free_free);
++ if (!stats_by_path)
++ return -ENOMEM;
++ }
++
+ /* First read the first found main config file. */
+ STRV_FOREACH(fn, (char**) conf_files) {
+- r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &mtime);
++ r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &st);
+ if (r < 0)
+ return r;
+- if (r > 0)
+- break;
++ if (r == 0)
++ continue;
++
++ if (ret_stats_by_path) {
++ r = hashmap_put_stats_by_path(&stats_by_path, *fn, &st);
++ if (r < 0)
++ return r;
++ }
++
++ break;
+ }
+
+ /* Then read all the drop-ins. */
+ STRV_FOREACH(fn, files) {
+- r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &mtime);
++ r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &st);
+ if (r < 0)
+ return r;
++ if (r == 0)
++ continue;
++
++ if (ret_stats_by_path) {
++ r = hashmap_put_stats_by_path(&stats_by_path, *fn, &st);
++ if (r < 0)
++ return r;
++ }
+ }
+
+- if (ret_mtime)
+- *ret_mtime = mtime;
++ if (ret_stats_by_path)
++ *ret_stats_by_path = TAKE_PTR(stats_by_path);
+
+ return 0;
+ }
+@@ -473,7 +530,7 @@ int config_parse_many_nulstr(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *ret_mtime) {
++ Hashmap **ret_stats_by_path) {
+
+ _cleanup_strv_free_ char **files = NULL;
+ int r;
+@@ -484,7 +541,7 @@ int config_parse_many_nulstr(
+
+ return config_parse_many_files(STRV_MAKE_CONST(conf_file),
+ files, sections, lookup, table, flags, userdata,
+- ret_mtime);
++ ret_stats_by_path);
+ }
+
+ /* Parse each config file in the directories specified as strv. */
+@@ -497,7 +554,7 @@ int config_parse_many(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *ret_mtime) {
++ Hashmap **ret_stats_by_path) {
+
+ _cleanup_strv_free_ char **dropin_dirs = NULL;
+ _cleanup_strv_free_ char **files = NULL;
+@@ -513,7 +570,7 @@ int config_parse_many(
+ if (r < 0)
+ return r;
+
+- return config_parse_many_files(conf_files, files, sections, lookup, table, flags, userdata, ret_mtime);
++ return config_parse_many_files(conf_files, files, sections, lookup, table, flags, userdata, ret_stats_by_path);
+ }
+
+ #define DEFINE_PARSER(type, vartype, conv_func) \
+diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
+index c3a138274d..f893a53aa0 100644
+--- a/src/shared/conf-parser.h
++++ b/src/shared/conf-parser.h
+@@ -6,8 +6,10 @@
+ #include <stddef.h>
+ #include <stdio.h>
+ #include <syslog.h>
++#include <sys/stat.h>
+
+ #include "alloc-util.h"
++#include "hashmap.h"
+ #include "log.h"
+ #include "macro.h"
+ #include "time-util.h"
+@@ -89,7 +91,7 @@ int config_parse(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *latest_mtime); /* input/output, possibly NULL */
++ struct stat *ret_stat); /* possibly NULL */
+
+ int config_parse_many_nulstr(
+ const char *conf_file, /* possibly NULL */
+@@ -99,7 +101,7 @@ int config_parse_many_nulstr(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *ret_mtime); /* possibly NULL */
++ Hashmap **ret_stats_by_path); /* possibly NULL */
+
+ int config_parse_many(
+ const char* const* conf_files, /* possibly empty */
+@@ -110,7 +112,7 @@ int config_parse_many(
+ const void *table,
+ ConfigParseFlags flags,
+ void *userdata,
+- usec_t *ret_mtime); /* possibly NULL */
++ Hashmap **ret_stats_by_path); /* possibly NULL */
+
+ CONFIG_PARSER_PROTOTYPE(config_parse_int);
+ CONFIG_PARSER_PROTOTYPE(config_parse_unsigned);
+--
+2.33.0.1079.g6e70778dc9-goog
+
diff --git a/meta-phosphor/recipes-core/systemd/systemd_249.3.bbappend b/meta-phosphor/recipes-core/systemd/systemd_249.3.bbappend
index 277299e..41bdcf9 100644
--- a/meta-phosphor/recipes-core/systemd/systemd_249.3.bbappend
+++ b/meta-phosphor/recipes-core/systemd/systemd_249.3.bbappend
@@ -1,2 +1,5 @@
# Pin to v249.5 to fix systemd-networkd issues.
SRCREV = "00b0393e65252bf631670604f58b844780b08c50"
+
+# Fix https://github.com/systemd/systemd/issues/21113
+SRC_URI += "file://0001-conf-parse-make-config_parse_many-optionally-save-st.patch"