From cc1a0f2337c5ffded4c3cef318b6e00afe424df9 Mon Sep 17 00:00:00 2001 From: "William A. Kennington III" Date: Sun, 24 Oct 2021 21:09:33 -0700 Subject: 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 --- ...make-config_parse_many-optionally-save-st.patch | 379 +++++++++++++++++++++ .../recipes-core/systemd/systemd_249.3.bbappend | 3 + 2 files changed, 382 insertions(+) create mode 100644 meta-phosphor/recipes-core/systemd/systemd/0001-conf-parse-make-config_parse_many-optionally-save-st.patch 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 000000000..fedb42d9b --- /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 +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 + #include + #include ++#include + + #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 277299eb0..41bdcf908 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" -- cgit v1.2.3