From 8f43182847afa56bad6d916d9db1ffe5206fc447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 Dec 2023 10:41:13 +0100 Subject: [PATCH 01/14] various: use modern strv helpers If we're building a strv, let's just use strv_new() with the CONF_PATHS macro, which gives as an exploded string set. --- src/analyze/analyze-cat-config.c | 3 +-- src/basic/strv.c | 6 ++---- src/environment-d-generator/environment-d-generator.c | 2 +- src/shared/tpm2-util.c | 2 +- src/tmpfiles/tmpfiles.c | 3 +-- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/analyze/analyze-cat-config.c b/src/analyze/analyze-cat-config.c index 073bc94018..b480d4a009 100644 --- a/src/analyze/analyze-cat-config.c +++ b/src/analyze/analyze-cat-config.c @@ -4,7 +4,6 @@ #include "analyze-cat-config.h" #include "conf-files.h" #include "constants.h" -#include "nulstr-util.h" #include "path-util.h" #include "pretty-print.h" #include "strv.h" @@ -23,7 +22,7 @@ int verb_cat_config(int argc, char *argv[], void *userdata) { print_separator(); if (path_is_absolute(*arg)) { - NULSTR_FOREACH(dir, CONF_PATHS_NULSTR("")) { + FOREACH_STRING(dir, CONF_PATHS("")) { t = path_startswith(*arg, dir); if (t) break; diff --git a/src/basic/strv.c b/src/basic/strv.c index 72cbbfe2f4..37199ebf04 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -252,11 +252,9 @@ int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix) { if (!v) return -ENOMEM; - r = strv_push(a, v); - if (r < 0) { - free(v); + r = strv_consume(a, v); + if (r < 0) return r; - } } return 0; diff --git a/src/environment-d-generator/environment-d-generator.c b/src/environment-d-generator/environment-d-generator.c index fa2c54af31..82fc57f1b1 100644 --- a/src/environment-d-generator/environment-d-generator.c +++ b/src/environment-d-generator/environment-d-generator.c @@ -17,7 +17,7 @@ static int environment_dirs(char ***ret) { _cleanup_free_ char *c = NULL; int r; - dirs = strv_new(CONF_PATHS_USR("environment.d"), NULL); + dirs = strv_new(CONF_PATHS_USR("environment.d")); if (!dirs) return -ENOMEM; diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index f2b3bfb575..c7ea57ab09 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -7643,7 +7643,7 @@ int tpm2_load_pcr_signature(const char *path, JsonVariant **ret) { /* Tries to load a JSON PCR signature file. Takes an absolute path, a simple file name or NULL. In * the latter two cases searches in /etc/, /usr/lib/, /run/, as usual. */ - search = strv_split_nulstr(CONF_PATHS_NULSTR("systemd")); + search = strv_new(CONF_PATHS("systemd")); if (!search) return log_oom_debug(); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 75db789f50..958341bc5b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -49,7 +49,6 @@ #include "mkdir-label.h" #include "mount-util.h" #include "mountpoint-util.h" -#include "nulstr-util.h" #include "offline-passwd.h" #include "pager.h" #include "parse-argument.h" @@ -4580,7 +4579,7 @@ static int run(int argc, char *argv[]) { break; case RUNTIME_SCOPE_SYSTEM: - config_dirs = strv_split_nulstr(CONF_PATHS_NULSTR("tmpfiles.d")); + config_dirs = strv_new(CONF_PATHS("tmpfiles.d")); if (!config_dirs) return log_oom(); break; From 76d75d8b7b6b19fa1709471ff17e56daf2cb4d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 Dec 2023 10:52:34 +0100 Subject: [PATCH 02/14] constants: drop duplicated CONF_PATHS defines Follow-up for b0d3095fd6cc1791a38f57a1982116b4475244ba. --- src/basic/constants.h | 8 +------- .../environment-d-generator.c | 2 +- src/partition/repart.c | 2 +- src/shared/install.c | 4 ++-- src/shared/pretty-print.c | 19 ++++--------------- 5 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/basic/constants.h b/src/basic/constants.h index 6bb5f3c281..b86337405c 100644 --- a/src/basic/constants.h +++ b/src/basic/constants.h @@ -67,18 +67,12 @@ "/usr/local/lib/" n "\0" \ "/usr/lib/" n "\0" -#define CONF_PATHS_USR(n) \ +#define CONF_PATHS(n) \ "/etc/" n, \ "/run/" n, \ "/usr/local/lib/" n, \ "/usr/lib/" n -#define CONF_PATHS(n) \ - CONF_PATHS_USR(n) - -#define CONF_PATHS_USR_STRV(n) \ - STRV_MAKE(CONF_PATHS_USR(n)) - #define CONF_PATHS_STRV(n) \ STRV_MAKE(CONF_PATHS(n)) diff --git a/src/environment-d-generator/environment-d-generator.c b/src/environment-d-generator/environment-d-generator.c index 82fc57f1b1..236cf38597 100644 --- a/src/environment-d-generator/environment-d-generator.c +++ b/src/environment-d-generator/environment-d-generator.c @@ -17,7 +17,7 @@ static int environment_dirs(char ***ret) { _cleanup_free_ char *c = NULL; int r; - dirs = strv_new(CONF_PATHS_USR("environment.d")); + dirs = strv_new(CONF_PATHS("environment.d")); if (!dirs) return -ENOMEM; diff --git a/src/partition/repart.c b/src/partition/repart.c index 7735bd458a..44a785a7ed 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -7997,7 +7997,7 @@ static int run(int argc, char *argv[]) { if (!d) return log_oom(); - r = search_and_access(d, F_OK, NULL, CONF_PATHS_USR_STRV("systemd/repart/definitions"), &dp); + r = search_and_access(d, F_OK, NULL, CONF_PATHS_STRV("systemd/repart/definitions"), &dp); if (r < 0) return log_error_errno(r, "DDI type '%s' is not defined: %m", arg_make_ddi); diff --git a/src/shared/install.c b/src/shared/install.c index fabf5db7ed..7191f846a9 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3218,8 +3218,8 @@ static int split_pattern_into_name_and_instances(const char *pattern, char **out } static int presets_find_config(RuntimeScope scope, const char *root_dir, char ***files) { - static const char* const system_dirs[] = {CONF_PATHS("systemd/system-preset"), NULL}; - static const char* const user_dirs[] = {CONF_PATHS_USR("systemd/user-preset"), NULL}; + static const char* const system_dirs[] = { CONF_PATHS("systemd/system-preset"), NULL }; + static const char* const user_dirs[] = { CONF_PATHS("systemd/user-preset"), NULL }; const char* const* dirs; assert(scope >= 0); diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 946da5f42d..3de193e264 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -316,11 +316,10 @@ static int guess_type(const char **name, char ***prefixes, bool *is_collection, */ _cleanup_free_ char *n = NULL; - bool usr = false, run = false, coll = false; + bool run = false, coll = false; const char *ext = ".conf"; /* This is static so that the array doesn't get deallocated when we exit the function */ static const char* const std_prefixes[] = { CONF_PATHS(""), NULL }; - static const char* const usr_prefixes[] = { CONF_PATHS_USR(""), NULL }; static const char* const run_prefixes[] = { "/run/", NULL }; if (path_equal(*name, "environment.d")) @@ -332,20 +331,13 @@ static int guess_type(const char **name, char ***prefixes, bool *is_collection, if (!n) return log_oom(); - /* All systemd-style config files should support the /usr-/etc-/run split and - * dropins. Let's add a blanket rule that allows us to support them without keeping - * an explicit list. */ - if (path_startswith(n, "systemd") && endswith(n, ".conf")) - usr = true; - delete_trailing_chars(n, "/"); + /* We assume systemd-style config files support the /usr-/run-/etc split and dropins. */ + if (endswith(n, ".d")) coll = true; - if (path_equal(n, "environment")) - usr = true; - if (path_equal(n, "udev/hwdb.d")) ext = ".hwdb"; else if (path_equal(n, "udev/rules.d")) @@ -363,10 +355,7 @@ static int guess_type(const char **name, char ***prefixes, bool *is_collection, ext = ".preset"; } - if (path_equal(n, "systemd/user-preset")) - usr = true; - - *prefixes = (char**) (usr ? usr_prefixes : run ? run_prefixes : std_prefixes); + *prefixes = (char**) (run ? run_prefixes : std_prefixes); *is_collection = coll; *extension = ext; return 0; From e5abff372d028d28d50165358cd48b624cd0c2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 25 Jan 2024 18:30:43 +0100 Subject: [PATCH 03/14] shared/conf-parser: collapse pkgdir and conf_file args into one This essentially reverts 5656cdfeeabc16b5489f5ec7a0a36025a2ec1f23. I find it much easier to understand what is going on when the path-relative-to-the-search-path is passed in full, instead of being constructed from two parts, with one of the parts being implicit in some places. Also, we call 'systemd-analyze cat-config ' with with the same meaning, so this makes the internal and external APIs more consistent. --- src/core/main.c | 2 +- src/coredump/coredump.c | 2 +- src/home/homed-conf.c | 2 +- src/journal-remote/journal-remote-main.c | 2 +- src/journal-remote/journal-upload.c | 2 +- src/journal/journald-server.c | 6 ++++-- src/login/logind-core.c | 2 +- src/network/networkd-conf.c | 2 +- src/oom/oomd.c | 2 +- src/pstore/pstore.c | 2 +- src/resolve/resolved-conf.c | 2 +- src/shared/conf-parser.c | 10 ++++------ src/shared/conf-parser.h | 15 ++------------- src/shared/sleep-config.c | 2 +- src/shared/udev-util.c | 5 ++--- src/timesync/timesyncd-conf.c | 2 +- 16 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1e66770df0..c7aec5df3b 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -736,7 +736,7 @@ static int parse_config_file(void) { }; if (arg_runtime_scope == RUNTIME_SCOPE_SYSTEM) - (void) config_parse_config_file("system.conf", + (void) config_parse_config_file("systemd/system.conf", "Manager\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 51218fc4c8..4478dd1be2 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -179,7 +179,7 @@ static int parse_config(void) { int r; r = config_parse_config_file( - "coredump.conf", + "systemd/coredump.conf", "Coredump\0", config_item_table_lookup, items, diff --git a/src/home/homed-conf.c b/src/home/homed-conf.c index ffa4bb3bd7..7fec29651d 100644 --- a/src/home/homed-conf.c +++ b/src/home/homed-conf.c @@ -9,7 +9,7 @@ int manager_parse_config_file(Manager *m) { assert(m); - return config_parse_config_file("homed.conf", "Home\0", + return config_parse_config_file("systemd/homed.conf", "Home\0", config_item_perf_lookup, homed_gperf_lookup, CONFIG_PARSE_WARN, m); } diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c index 6c09c068c8..221b544fbc 100644 --- a/src/journal-remote/journal-remote-main.c +++ b/src/journal-remote/journal-remote-main.c @@ -746,7 +746,7 @@ static int parse_config(void) { {} }; - return config_parse_config_file("journal-remote.conf", "Remote\0", + return config_parse_config_file("systemd/journal-remote.conf", "Remote\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL); } diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index 97b5f929ab..6d080d7370 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -531,7 +531,7 @@ static int parse_config(void) { {} }; - return config_parse_config_file("journal-upload.conf", "Upload\0", + return config_parse_config_file("systemd/journal-upload.conf", "Upload\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL); } diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index a8c186dc20..fa340da4bb 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1884,12 +1884,14 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat } static int server_parse_config_file(Server *s) { - const char *conf_file = "journald.conf"; + const char *conf_file; assert(s); if (s->namespace) - conf_file = strjoina("journald@", s->namespace, ".conf"); + conf_file = strjoina("systemd/journald@", s->namespace, ".conf"); + else + conf_file = "systemd/journald.conf"; return config_parse_config_file(conf_file, "Journal\0", config_item_perf_lookup, journald_gperf_lookup, diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 582cbcbea8..50346089ba 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -82,7 +82,7 @@ void manager_reset_config(Manager *m) { int manager_parse_config_file(Manager *m) { assert(m); - return config_parse_config_file("logind.conf", "Login\0", + return config_parse_config_file("systemd/logind.conf", "Login\0", config_item_perf_lookup, logind_gperf_lookup, CONFIG_PARSE_WARN, m); } diff --git a/src/network/networkd-conf.c b/src/network/networkd-conf.c index 063732a3b4..af8d04b4a5 100644 --- a/src/network/networkd-conf.c +++ b/src/network/networkd-conf.c @@ -14,7 +14,7 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("networkd.conf", + r = config_parse_config_file("systemd/networkd.conf", "Network\0" "DHCPv4\0" "DHCPv6\0" diff --git a/src/oom/oomd.c b/src/oom/oomd.c index ecc2eda5dc..a88f57da0a 100644 --- a/src/oom/oomd.c +++ b/src/oom/oomd.c @@ -31,7 +31,7 @@ static int parse_config(void) { {} }; - return config_parse_config_file("oomd.conf", "OOM\0", + return config_parse_config_file("systemd/oomd.conf", "OOM\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL); } diff --git a/src/pstore/pstore.c b/src/pstore/pstore.c index 835348f90e..529193c9e9 100644 --- a/src/pstore/pstore.c +++ b/src/pstore/pstore.c @@ -77,7 +77,7 @@ static int parse_config(void) { {} }; - return config_parse_config_file("pstore.conf", "PStore\0", + return config_parse_config_file("systemd/pstore.conf", "PStore\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL); } diff --git a/src/resolve/resolved-conf.c b/src/resolve/resolved-conf.c index 182ac20c3f..f88f295437 100644 --- a/src/resolve/resolved-conf.c +++ b/src/resolve/resolved-conf.c @@ -570,7 +570,7 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("resolved.conf", "Resolve\0", + r = config_parse_config_file("systemd/resolved.conf", "Resolve\0", config_item_perf_lookup, resolved_gperf_lookup, CONFIG_PARSE_WARN, m); if (r < 0) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 40d3675b7e..fba0001242 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -600,9 +600,8 @@ static int config_parse_many_files( /* Parse one main config file located in /etc/$pkgdir and its drop-ins, which is what all systemd daemons * do. */ -int config_parse_config_file_full( +int config_parse_config_file( const char *conf_file, - const char *pkgdir, const char *sections, ConfigItemLookup lookup, const void *table, @@ -614,7 +613,6 @@ int config_parse_config_file_full( int r; assert(conf_file); - assert(pkgdir); /* build the dropin dir list */ dropin_dirs = new0(char*, strv_length(conf_paths) + 1); @@ -628,10 +626,10 @@ int config_parse_config_file_full( STRV_FOREACH(p, conf_paths) { char *d; - d = strjoin(*p, pkgdir, "/", conf_file, ".d"); + d = strjoin(*p, conf_file, ".d"); if (!d) { if (flags & CONFIG_PARSE_WARN) - return log_oom(); + log_oom(); return -ENOMEM; } @@ -642,7 +640,7 @@ int config_parse_config_file_full( if (r < 0) return r; - const char *sysconf_file = strjoina(SYSCONF_DIR, "/", pkgdir, "/", conf_file); + const char *sysconf_file = strjoina(SYSCONF_DIR, "/", conf_file); return config_parse_many_files(STRV_MAKE_CONST(sysconf_file), dropins, sections, lookup, table, flags, userdata, NULL); diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 3057356479..2c75ba5d8b 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -93,25 +93,14 @@ int config_parse( void *userdata, struct stat *ret_stat); /* possibly NULL */ -int config_parse_config_file_full( - const char *conf_file, - const char *pkgdir, +int config_parse_config_file( + const char *conf_file, /* a path like "systemd/frobnicator.conf" */ const char *sections, /* nulstr */ ConfigItemLookup lookup, const void *table, ConfigParseFlags flags, void *userdata); -static inline int config_parse_config_file( - const char *conf_file, - const char *sections, /* nulstr */ - ConfigItemLookup lookup, - const void *table, - ConfigParseFlags flags, - void *userdata) { - return config_parse_config_file_full(conf_file, "systemd", sections, lookup, table, flags, userdata); -} - int config_parse_many( const char* const* conf_files, /* possibly empty */ const char* const* conf_file_dirs, diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 7282111f49..19b75a706d 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -145,7 +145,7 @@ int parse_sleep_config(SleepConfig **ret) { {} }; - (void) config_parse_config_file("sleep.conf", "Sleep\0", + (void) config_parse_config_file("systemd/sleep.conf", "Sleep\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL); diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 9acdaeff52..205afc5723 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -28,9 +28,8 @@ int udev_parse_config_full(const ConfigTableItem config_table[]) { assert(config_table); - r = config_parse_config_file_full( - "udev.conf", - "udev", + r = config_parse_config_file( + "udev/udev.conf", /* sections = */ NULL, config_item_table_lookup, config_table, diff --git a/src/timesync/timesyncd-conf.c b/src/timesync/timesyncd-conf.c index 9c0b6f7ce1..66f3d177e9 100644 --- a/src/timesync/timesyncd-conf.c +++ b/src/timesync/timesyncd-conf.c @@ -102,7 +102,7 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("timesyncd.conf", "Time\0", + r = config_parse_config_file("systemd/timesyncd.conf", "Time\0", config_item_perf_lookup, timesyncd_gperf_lookup, CONFIG_PARSE_WARN, m); if (r < 0) From 6812498cb24eb44397a150e6c7754033495f9442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 22 Feb 2024 10:36:52 +0100 Subject: [PATCH 04/14] shared/pretty-print: rename output parameters --- src/shared/pretty-print.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 3de193e264..f3a8891a79 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -308,7 +308,7 @@ void print_separator(void) { fputs("\n\n", stdout); } -static int guess_type(const char **name, char ***prefixes, bool *is_collection, const char **extension) { +static int guess_type(const char **name, char ***ret_prefixes, bool *ret_is_collection, const char **ret_extension) { /* Try to figure out if name is like tmpfiles.d/ or systemd/system-presets/, * i.e. a collection of directories without a main config file. * Incidentally, all those formats don't use sections. So we return a single @@ -355,9 +355,9 @@ static int guess_type(const char **name, char ***prefixes, bool *is_collection, ext = ".preset"; } - *prefixes = (char**) (run ? run_prefixes : std_prefixes); - *is_collection = coll; - *extension = ext; + *ret_prefixes = (char**) (run ? run_prefixes : std_prefixes); + *ret_is_collection = coll; + *ret_extension = ext; return 0; } From 5ea4afcf006510a0efb20d52d2ff0ad29e08ea54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Feb 2024 12:38:27 +0100 Subject: [PATCH 05/14] udev,backlight,kernel-install: reword sentences starting with "Skipping to" That's not gramatically correct. In backlight, change "assocation" to "deduplication". Without the context, it's probably not clear at all that we "associate" them to ignore them. --- src/backlight/backlight.c | 6 +++--- src/kernel-install/kernel-install.c | 2 +- src/udev/net/link-config.c | 4 ++-- src/udev/udev-builtin-net_setup_link.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 6a2ad17fbf..6126405994 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -175,7 +175,7 @@ static int validate_device(sd_device *device) { /* Verify whether we should actually care for a specific backlight device. For backlight devices * there might be multiple ways to access the same control: "firmware" (i.e. ACPI), "platform" - * (i.e. via the machine's EC) and "raw" (via the graphics card). In general we should prefer + * (i.e. via the machine's EC), and "raw" (via the graphics card). In general we should prefer * "firmware" (i.e. ACPI) or "platform" access over "raw" access, in order not to confuse the * BIOS/EC, and compatibility with possible low-level hotkey handling of screen brightness. The * kernel will already make sure to expose only one of "firmware" and "platform" for the same @@ -239,8 +239,8 @@ static int validate_device(sd_device *device) { /* If the system has multiple graphics cards, then we cannot associate platform * devices on non-PCI bus (especially WMI bus) with PCI devices. Let's ignore all * backlight devices that do not have the same parent PCI device. */ - log_debug("Found multiple graphics cards on PCI bus. " - "Skipping to associate platform backlight devices on non-PCI bus."); + log_debug("Found multiple graphics cards on PCI bus; " + "skipping deduplication of platform backlight devices not on PCI bus."); r = sd_device_enumerator_add_match_parent(enumerate, parent); if (r < 0) diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index 2523d43944..aa81af70d0 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -511,7 +511,7 @@ static int context_load_machine_info(Context *c) { if (r < 0 && r != -ENXIO) log_warning_errno(r, "Failed to read $KERNEL_INSTALL_READ_MACHINE_INFO, assuming yes: %m"); if (r == 0) { - log_debug("Skipping to read /etc/machine-info."); + log_debug("Skipping reading of /etc/machine-info."); return 0; } diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index a8b2cc23a2..5320b84b1d 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -741,7 +741,7 @@ static int link_generate_new_name(Link *link) { device = link->device; if (link->action != SD_DEVICE_ADD) { - log_link_debug(link, "Skipping to apply Name= and NamePolicy= on '%s' uevent.", + log_link_debug(link, "Not applying Name= and NamePolicy= on '%s' uevent.", device_action_to_string(link->action)); goto no_rename; } @@ -821,7 +821,7 @@ static int link_generate_alternative_names(Link *link) { assert(!link->altnames); if (link->action != SD_DEVICE_ADD) { - log_link_debug(link, "Skipping to apply AlternativeNames= and AlternativeNamesPolicy= on '%s' uevent.", + log_link_debug(link, "Not applying AlternativeNames= and AlternativeNamesPolicy= on '%s' uevent.", device_action_to_string(link->action)); return 0; } diff --git a/src/udev/udev-builtin-net_setup_link.c b/src/udev/udev-builtin-net_setup_link.c index fc614443ef..df89679de3 100644 --- a/src/udev/udev-builtin-net_setup_link.c +++ b/src/udev/udev-builtin-net_setup_link.c @@ -26,7 +26,7 @@ static int builtin_net_setup_link(UdevEvent *event, int argc, char **argv, bool return log_device_error_errno(dev, r, "Failed to get action: %m"); if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_BIND, SD_DEVICE_MOVE)) { - log_device_debug(dev, "Skipping to apply .link settings on '%s' uevent.", + log_device_debug(dev, "Not applying .link settings on '%s' uevent.", device_action_to_string(action)); /* Import previously assigned .link file name. */ From 4bf32eac522bb813ff56ef0bf0565d7ffd583b69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Feb 2024 14:09:52 +0100 Subject: [PATCH 06/14] udevd: inline iterator variable --- src/udev/udevd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 684ced1315..e408768887 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -33,16 +33,15 @@ static int arg_daemonize = false; static int listen_fds(int *ret_ctrl, int *ret_netlink) { int ctrl_fd = -EBADF, netlink_fd = -EBADF; - int fd, n; assert(ret_ctrl); assert(ret_netlink); - n = sd_listen_fds(true); + int n = sd_listen_fds(true); if (n < 0) return n; - for (fd = SD_LISTEN_FDS_START; fd < n + SD_LISTEN_FDS_START; fd++) { + for (int fd = SD_LISTEN_FDS_START; fd < n + SD_LISTEN_FDS_START; fd++) { if (sd_is_socket(fd, AF_UNIX, SOCK_SEQPACKET, -1) > 0) { if (ctrl_fd >= 0) return -EINVAL; From 9bc7493098a9b2cfc044e07be6d2690a2be99e13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 22 Feb 2024 10:47:23 +0100 Subject: [PATCH 07/14] strv: add helper to extend strv from both sides Also, use the more correct type of 'const char* const*' for the input strv. This requires adding the cast in a few places, but also allows to remove some casts in others. --- src/basic/conf-files.c | 2 +- src/basic/path-lookup.c | 4 ++-- src/basic/strv.c | 4 ++-- src/basic/strv.h | 5 ++++- src/test/test-strv.c | 18 +++++++++++++++++- src/tmpfiles/tmpfiles.c | 4 ++-- .../xdg-autostart-generator.c | 2 +- 7 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 9cb66c099b..7fdcc71356 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -369,7 +369,7 @@ int conf_files_list_dropins( assert(dirs); suffix = strjoina("/", dropin_dirname); - r = strv_extend_strv_concat(&dropin_dirs, (char**) dirs, suffix); + r = strv_extend_strv_concat(&dropin_dirs, dirs, suffix); if (r < 0) return r; diff --git a/src/basic/path-lookup.c b/src/basic/path-lookup.c index 1b31a4e23a..8a03e29f65 100644 --- a/src/basic/path-lookup.c +++ b/src/basic/path-lookup.c @@ -214,7 +214,7 @@ static char** user_dirs( persistent_config) < 0) return NULL; - if (strv_extend_strv_concat(&res, config_dirs, "/systemd/user") < 0) + if (strv_extend_strv_concat(&res, (const char* const*) config_dirs, "/systemd/user") < 0) return NULL; /* global config has lower priority than the user config of the same type */ @@ -232,7 +232,7 @@ static char** user_dirs( data_home) < 0) return NULL; - if (strv_extend_strv_concat(&res, data_dirs, "/systemd/user") < 0) + if (strv_extend_strv_concat(&res, (const char* const*) data_dirs, "/systemd/user") < 0) return NULL; if (strv_extend_strv(&res, (char**) user_data_unit_paths, false) < 0) diff --git a/src/basic/strv.c b/src/basic/strv.c index 37199ebf04..b7946c8ba2 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -242,13 +242,13 @@ rollback: return -ENOMEM; } -int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix) { +int strv_extend_strv_biconcat(char ***a, const char *prefix, const char* const *b, const char *suffix) { int r; STRV_FOREACH(s, b) { char *v; - v = strjoin(*s, suffix); + v = strjoin(strempty(prefix), *s, suffix); if (!v) return -ENOMEM; diff --git a/src/basic/strv.h b/src/basic/strv.h index 91337b9287..169737d1d8 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -43,7 +43,10 @@ int strv_copy_unless_empty(char * const *l, char ***ret); size_t strv_length(char * const *l) _pure_; int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates); -int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix); +int strv_extend_strv_biconcat(char ***a, const char *prefix, const char* const *b, const char *suffix); +static inline int strv_extend_strv_concat(char ***a, const char* const *b, const char *suffix) { + return strv_extend_strv_biconcat(a, NULL, b, suffix); +} int strv_prepend(char ***l, const char *value); /* _with_size() are lower-level functions where the size can be provided externally, diff --git a/src/test/test-strv.c b/src/test/test-strv.c index da8721214a..47ad0eb639 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -527,6 +527,22 @@ TEST(strv_sort) { assert_se(streq(input_table[4], "durian")); } +TEST(strv_extend_strv_biconcat) { + _cleanup_strv_free_ char **a = NULL, **b = NULL; + + a = strv_new("without", "suffix"); + b = strv_new("with", "suffix"); + assert_se(a); + assert_se(b); + + assert_se(strv_extend_strv_biconcat(&a, "prefix_", (const char* const*) b, "_suffix") >= 0); + + assert_se(streq(a[0], "without")); + assert_se(streq(a[1], "suffix")); + assert_se(streq(a[2], "prefix_with_suffix")); + assert_se(streq(a[3], "prefix_suffix_suffix")); +} + TEST(strv_extend_strv_concat) { _cleanup_strv_free_ char **a = NULL, **b = NULL; @@ -535,7 +551,7 @@ TEST(strv_extend_strv_concat) { assert_se(a); assert_se(b); - assert_se(strv_extend_strv_concat(&a, b, "_suffix") >= 0); + assert_se(strv_extend_strv_concat(&a, (const char* const*) b, "_suffix") >= 0); assert_se(streq(a[0], "without")); assert_se(streq(a[1], "suffix")); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 958341bc5b..e4854f31b5 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -369,7 +369,7 @@ static int user_config_paths(char*** ret) { if (r < 0 && !ERRNO_IS_NOINFO(r)) return r; - r = strv_extend_strv_concat(&res, config_dirs, "/user-tmpfiles.d"); + r = strv_extend_strv_concat(&res, (const char* const*) config_dirs, "/user-tmpfiles.d"); if (r < 0) return r; @@ -381,7 +381,7 @@ static int user_config_paths(char*** ret) { if (r < 0) return r; - r = strv_extend_strv_concat(&res, data_dirs, "/user-tmpfiles.d"); + r = strv_extend_strv_concat(&res, (const char* const*) data_dirs, "/user-tmpfiles.d"); if (r < 0) return r; diff --git a/src/xdg-autostart-generator/xdg-autostart-generator.c b/src/xdg-autostart-generator/xdg-autostart-generator.c index 616c017357..71e1a66435 100644 --- a/src/xdg-autostart-generator/xdg-autostart-generator.c +++ b/src/xdg-autostart-generator/xdg-autostart-generator.c @@ -37,7 +37,7 @@ static int enumerate_xdg_autostart(Hashmap *all_services) { r = xdg_user_dirs(&config_dirs, &data_dirs); if (r < 0) return r; - r = strv_extend_strv_concat(&autostart_dirs, config_dirs, "/autostart"); + r = strv_extend_strv_concat(&autostart_dirs, (const char* const*) config_dirs, "/autostart"); if (r < 0) return r; From d8a91c6b9f64017abfefbbf7da0daed590de372a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Feb 2024 22:29:56 +0100 Subject: [PATCH 08/14] shared/conf-parser: use chase() in config_parse_many_files() The function was partially implementing chroot lookups. It would be given file names that were prefixed with the chroot, so it would mostly work. But if any of those files were symlinks, fopen() would do the wrong thing. Also we don't need locking. So give 'root' as the argument and use chase_and_fopen_unlocked() to get proper chroot-aware lookups. The only place where config_parse_many() is called with root is is repart.c. So this is a follow-up for e594a3b154bd06c535a934a1cc7231b1ef76df73 and 34f2fd5096cdb26ef57998740b1b876332d968fc. --- src/shared/conf-parser.c | 37 +++++++++++++++++-------------------- src/shared/conf-parser.h | 2 +- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index fba0001242..b1bc6c8a95 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -8,6 +8,7 @@ #include #include "alloc-util.h" +#include "chase.h" #include "conf-files.h" #include "conf-parser.h" #include "constants.h" @@ -480,6 +481,7 @@ int hashmap_put_stats_by_path(Hashmap **stats_by_path, const char *path, const s } static int config_parse_many_files( + const char *root, const char* const* conf_files, char **files, const char *sections, @@ -502,19 +504,16 @@ static int config_parse_many_files( } STRV_FOREACH(fn, files) { - _cleanup_free_ struct stat *st_dropin = NULL; _cleanup_fclose_ FILE *f = NULL; - int fd; + _cleanup_free_ char *fname = NULL; - f = fopen(*fn, "re"); - if (!f) { - if (errno == ENOENT) - continue; + r = chase_and_fopen_unlocked(*fn, root, CHASE_AT_RESOLVE_IN_ROOT, "re", &fname, &f); + if (r == -ENOENT) + continue; + if (r < 0) + return r; - return -errno; - } - - fd = fileno(f); + int fd = fileno(f); r = ordered_hashmap_ensure_put(&dropins, &config_file_hash_ops_fclose, *fn, f); if (r < 0) { @@ -527,7 +526,7 @@ static int config_parse_many_files( /* Get inodes for all drop-ins. Later we'll verify if main config is a symlink to or is * symlinked as one of them. If so, we skip reading main config file directly. */ - st_dropin = new(struct stat, 1); + _cleanup_free_ struct stat *st_dropin = new(struct stat, 1); if (!st_dropin) return -ENOMEM; @@ -543,13 +542,11 @@ static int config_parse_many_files( STRV_FOREACH(fn, conf_files) { _cleanup_fclose_ FILE *f = NULL; - f = fopen(*fn, "re"); - if (!f) { - if (errno == ENOENT) - continue; - - return -errno; - } + r = chase_and_fopen_unlocked(*fn, root, CHASE_AT_RESOLVE_IN_ROOT, "re", NULL, &f); + if (r == -ENOENT) + continue; + if (r < 0) + return r; if (inodes) { if (fstat(fileno(f), &st) < 0) @@ -642,7 +639,7 @@ int config_parse_config_file( const char *sysconf_file = strjoina(SYSCONF_DIR, "/", conf_file); - return config_parse_many_files(STRV_MAKE_CONST(sysconf_file), dropins, + return config_parse_many_files(NULL, STRV_MAKE_CONST(sysconf_file), dropins, sections, lookup, table, flags, userdata, NULL); } @@ -672,7 +669,7 @@ int config_parse_many( if (r < 0) return r; - r = config_parse_many_files(conf_files, files, sections, lookup, table, flags, userdata, ret_stats_by_path); + r = config_parse_many_files(root, conf_files, files, sections, lookup, table, flags, userdata, ret_stats_by_path); if (r < 0) return r; diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 2c75ba5d8b..9d08cbd677 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -106,7 +106,7 @@ int config_parse_many( const char* const* conf_file_dirs, const char *dropin_dirname, const char *root, - const char *sections, /* nulstr */ + const char *sections, /* nulstr */ ConfigItemLookup lookup, const void *table, ConfigParseFlags flags, From e7e52ff9b6d6bbfcdcc298ef3c156420b51d58b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Feb 2024 22:31:41 +0100 Subject: [PATCH 09/14] shared/conf-parser: add function which implements the standard config file set Also allow config_parse_many() to be called for config files without sections. The test uses such a file. --- src/shared/conf-parser.c | 45 +++++++++++++++- src/shared/conf-parser.h | 11 ++++ src/test/test-conf-parser.c | 100 ++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index b1bc6c8a95..3e3e19b67f 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -662,7 +662,6 @@ int config_parse_many( assert(conf_file_dirs); assert(dropin_dirname); - assert(sections); assert(table); r = conf_files_list_dropins(&files, dropin_dirname, root, conf_file_dirs); @@ -679,6 +678,50 @@ int config_parse_many( return 0; } +int config_parse_standard_file_with_dropins_full( + const char *root, + const char *main_file, /* A path like "systemd/frobnicator.conf" */ + const char *sections, + ConfigItemLookup lookup, + const void *table, + ConfigParseFlags flags, + void *userdata, + Hashmap **ret_stats_by_path, + char ***ret_dropin_files) { + + const char* const *conf_paths = (const char* const*) CONF_PATHS_STRV(""); + _cleanup_strv_free_ char **configs = NULL; + int r; + + /* Build the list of main config files */ + r = strv_extend_strv_biconcat(&configs, root, conf_paths, main_file); + if (r < 0) { + if (flags & CONFIG_PARSE_WARN) + log_oom(); + return r; + } + + _cleanup_free_ char *dropin_dirname = strjoin(main_file, ".d"); + if (!dropin_dirname) { + if (flags & CONFIG_PARSE_WARN) + log_oom(); + return -ENOMEM; + } + + return config_parse_many( + (const char* const*) configs, + conf_paths, + dropin_dirname, + root, + sections, + lookup, + table, + flags, + userdata, + ret_stats_by_path, + ret_dropin_files); +} + static int dropins_get_stats_by_path( const char* conf_file, const char* const* conf_file_dirs, diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 9d08cbd677..d3c65bbf92 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -114,6 +114,17 @@ int config_parse_many( Hashmap **ret_stats_by_path, /* possibly NULL */ char ***ret_drop_in_files); /* possibly NULL */ +int config_parse_standard_file_with_dropins_full( + const char *root, + const char *main_file, /* A path like "systemd/frobnicator.conf" */ + const char *sections, + ConfigItemLookup lookup, + const void *table, + ConfigParseFlags flags, + void *userdata, + Hashmap **ret_stats_by_path, /* possibly NULL */ + char ***ret_dropin_files); /* possibly NULL */ + int config_get_stats_by_path( const char *suffix, const char *root, diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index 0acb4131b5..d0d7419eaa 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -3,8 +3,10 @@ #include "conf-parser.h" #include "fd-util.h" #include "fs-util.h" +#include "fileio.h" #include "log.h" #include "macro.h" +#include "mkdir.h" #include "string-util.h" #include "strv.h" #include "tests.h" @@ -390,4 +392,102 @@ TEST(config_parse) { test_config_parse_one(i, config_file[i]); } +TEST(config_parse_standard_file_with_dropins_full) { + _cleanup_(rmdir_and_freep) char *root = NULL; + _cleanup_close_ int rfd = -EBADF; + int r; + + assert_se(mkdtemp_malloc(NULL, &root) >= 0); + assert_se(mkdir_p_root(root, "/etc/kernel/install.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/run/kernel/install.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/usr/lib/kernel/install.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/usr/local/lib/kernel/install.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + + rfd = open(root, O_CLOEXEC|O_DIRECTORY); + assert_se(rfd >= 0); + + assert_se(write_string_file_at(rfd, "usr/lib/kernel/install.conf", /* this one is ignored */ + "A=!!!", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "usr/local/lib/kernel/install.conf", + "A=aaa", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "usr/local/lib/kernel/install.conf.d/drop1.conf", + "B=bbb", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "usr/local/lib/kernel/install.conf.d/drop2.conf", + "C=c1", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "usr/lib/kernel/install.conf.d/drop2.conf", /* this one is ignored */ + "C=c2", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "run/kernel/install.conf.d/drop3.conf", + "D=ddd", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file_at(rfd, "etc/kernel/install.conf.d/drop4.conf", + "E=eee", WRITE_STRING_FILE_CREATE) == 0); + + _cleanup_free_ char *A = NULL, *B = NULL, *C = NULL, *D = NULL, *E = NULL, *F = NULL; + _cleanup_strv_free_ char **dropins = NULL; + + const ConfigTableItem items[] = { + { NULL, "A", config_parse_string, 0, &A}, + { NULL, "B", config_parse_string, 0, &B}, + { NULL, "C", config_parse_string, 0, &C}, + { NULL, "D", config_parse_string, 0, &D}, + { NULL, "E", config_parse_string, 0, &E}, + { NULL, "F", config_parse_string, 0, &F}, + {} + }; + + r = config_parse_standard_file_with_dropins_full( + root, "kernel/install.conf", + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ &dropins); + assert_se(r >= 0); + assert_se(streq_ptr(A, "aaa")); + assert_se(streq_ptr(B, "bbb")); + assert_se(streq_ptr(C, "c1")); + assert_se(streq_ptr(D, "ddd")); + assert_se(streq_ptr(E, "eee")); + assert_se(streq_ptr(F, NULL)); + + A = mfree(A); + B = mfree(B); + C = mfree(C); + D = mfree(D); + E = mfree(E); + + assert_se(strv_length(dropins) == 4); + + /* Make sure that we follow symlinks */ + assert_se(mkdir_p_root(root, "/etc/kernel/install2.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/run/kernel/install2.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/usr/lib/kernel/install2.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + assert_se(mkdir_p_root(root, "/usr/local/lib/kernel/install2.conf.d", UID_INVALID, GID_INVALID, 0755, NULL)); + + /* (Those symlinks are only useful relative to . */ + assert_se(symlinkat("/usr/lib/kernel/install.conf", rfd, "usr/lib/kernel/install2.conf") == 0); + assert_se(symlinkat("/usr/local/lib/kernel/install.conf", rfd, "usr/local/lib/kernel/install2.conf") == 0); + assert_se(symlinkat("/usr/local/lib/kernel/install.conf.d/drop1.conf", rfd, "usr/local/lib/kernel/install2.conf.d/drop1.conf") == 0); + assert_se(symlinkat("/usr/local/lib/kernel/install.conf.d/drop2.conf", rfd, "usr/local/lib/kernel/install2.conf.d/drop2.conf") == 0); + assert_se(symlinkat("/usr/lib/kernel/install.conf.d/drop2.conf", rfd, "usr/lib/kernel/install2.conf.d/drop2.conf") == 0); + assert_se(symlinkat("/run/kernel/install.conf.d/drop3.conf", rfd, "run/kernel/install2.conf.d/drop3.conf") == 0); + assert_se(symlinkat("/etc/kernel/install.conf.d/drop4.conf", rfd, "etc/kernel/install2.conf.d/drop4.conf") == 0); + + r = config_parse_standard_file_with_dropins_full( + root, "kernel/install2.conf", + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); + assert_se(r >= 0); + assert_se(streq_ptr(A, "aaa")); + assert_se(streq_ptr(B, "bbb")); + assert_se(streq_ptr(C, "c1")); + assert_se(streq_ptr(D, "ddd")); + assert_se(streq_ptr(E, "eee")); + assert_se(streq_ptr(F, NULL)); +} + DEFINE_TEST_MAIN(LOG_INFO); From 6378f257e7e7856abc32cd9b5cb33bc4c63903be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 22 Feb 2024 10:50:05 +0100 Subject: [PATCH 10/14] various: use new config loader instead of config_parse_config_file() This means the main config file is loaded also from /run and /usr. We should load the main config file from all the places where we load drop-ins. I realize I had a giant blind spot: I always assumed that we load config files from /etc, /run, /usr/local/lib, /usr/lib. But it turns out that we only used those paths for drop-ins. For the main config file, we only looked in /etc. The docs actually partially described this behaviour, i.e. most SYNOPSIS sections and some parts of the text, but not others. This is strange, because 6495361c7d5e8bf640841d1292ef6cfe1ea244cf was completely bogus with the behaviour before this patch. We had a huge discussion before it was merged, and clearly nobody noticed this. Similarly, in the previous version of the current pull request, we had a long discussion about the appropriate order of directories, and apparently nobody noticed that there was no order, because only looked in one directory. So the blind spot seems to have been shared. Also, systemd-analyze cat-config behaved incorrectly, i.e. its behaviour matches the new behaviour. Possibly, in the future it'll make it easier to add support for --root. --- src/core/main.c | 11 +++--- src/coredump/coredump.c | 2 +- src/home/homed-conf.c | 9 +++-- src/journal-remote/journal-remote-main.c | 9 +++-- src/journal-remote/journal-upload.c | 9 +++-- src/journal/journald-server.c | 9 +++-- src/login/logind-core.c | 9 +++-- src/network/networkd-conf.c | 17 +++++---- src/oom/oomd.c | 9 +++-- src/pstore/pstore.c | 9 +++-- src/resolve/resolved-conf.c | 9 +++-- src/shared/conf-parser.c | 48 ------------------------ src/shared/conf-parser.h | 27 +++++++++---- src/shared/sleep-config.c | 9 +++-- src/shared/udev-util.c | 5 +-- src/timesync/timesyncd-conf.c | 9 +++-- 16 files changed, 97 insertions(+), 103 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index c7aec5df3b..d5c3bf0e46 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -736,11 +736,12 @@ static int parse_config_file(void) { }; if (arg_runtime_scope == RUNTIME_SCOPE_SYSTEM) - (void) config_parse_config_file("systemd/system.conf", - "Manager\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, - NULL); + (void) config_parse_standard_file_with_dropins( + "systemd/system.conf", + "Manager\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); else { _cleanup_strv_free_ char **files = NULL, **dirs = NULL; int r; diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 4478dd1be2..b87bc52bde 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -178,7 +178,7 @@ static int parse_config(void) { int r; - r = config_parse_config_file( + r = config_parse_standard_file_with_dropins( "systemd/coredump.conf", "Coredump\0", config_item_table_lookup, diff --git a/src/home/homed-conf.c b/src/home/homed-conf.c index 7fec29651d..3f74096b64 100644 --- a/src/home/homed-conf.c +++ b/src/home/homed-conf.c @@ -9,9 +9,12 @@ int manager_parse_config_file(Manager *m) { assert(m); - return config_parse_config_file("systemd/homed.conf", "Home\0", - config_item_perf_lookup, homed_gperf_lookup, - CONFIG_PARSE_WARN, m); + return config_parse_standard_file_with_dropins( + "systemd/homed.conf", + "Home\0", + config_item_perf_lookup, homed_gperf_lookup, + CONFIG_PARSE_WARN, + m); } DEFINE_CONFIG_PARSE_ENUM(config_parse_default_storage, user_storage, UserStorage, "Failed to parse default storage setting"); diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c index 221b544fbc..91cb2eefaa 100644 --- a/src/journal-remote/journal-remote-main.c +++ b/src/journal-remote/journal-remote-main.c @@ -746,9 +746,12 @@ static int parse_config(void) { {} }; - return config_parse_config_file("systemd/journal-remote.conf", "Remote\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, NULL); + return config_parse_standard_file_with_dropins( + "systemd/journal-remote.conf", + "Remote\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); } static int help(void) { diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index 6d080d7370..327b7bacd7 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -531,9 +531,12 @@ static int parse_config(void) { {} }; - return config_parse_config_file("systemd/journal-upload.conf", "Upload\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, NULL); + return config_parse_standard_file_with_dropins( + "systemd/journal-upload.conf", + "Upload\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); } static int help(void) { diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index fa340da4bb..508263046c 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1893,9 +1893,12 @@ static int server_parse_config_file(Server *s) { else conf_file = "systemd/journald.conf"; - return config_parse_config_file(conf_file, "Journal\0", - config_item_perf_lookup, journald_gperf_lookup, - CONFIG_PARSE_WARN, s); + return config_parse_standard_file_with_dropins( + conf_file, + "Journal\0", + config_item_perf_lookup, journald_gperf_lookup, + CONFIG_PARSE_WARN, + /* userdata= */ s); } static int server_dispatch_sync(sd_event_source *es, usec_t t, void *userdata) { diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 50346089ba..e09bb9f2de 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -82,9 +82,12 @@ void manager_reset_config(Manager *m) { int manager_parse_config_file(Manager *m) { assert(m); - return config_parse_config_file("systemd/logind.conf", "Login\0", - config_item_perf_lookup, logind_gperf_lookup, - CONFIG_PARSE_WARN, m); + return config_parse_standard_file_with_dropins( + "systemd/logind.conf", + "Login\0", + config_item_perf_lookup, logind_gperf_lookup, + CONFIG_PARSE_WARN, + /* userdata= */ m); } int manager_add_device(Manager *m, const char *sysfs, bool master, Device **ret_device) { diff --git a/src/network/networkd-conf.c b/src/network/networkd-conf.c index af8d04b4a5..0f7015eb25 100644 --- a/src/network/networkd-conf.c +++ b/src/network/networkd-conf.c @@ -14,14 +14,15 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("systemd/networkd.conf", - "Network\0" - "DHCPv4\0" - "DHCPv6\0" - "DHCP\0", - config_item_perf_lookup, networkd_gperf_lookup, - CONFIG_PARSE_WARN, - m); + r = config_parse_standard_file_with_dropins( + "systemd/networkd.conf", + "Network\0" + "DHCPv4\0" + "DHCPv6\0" + "DHCP\0", + config_item_perf_lookup, networkd_gperf_lookup, + CONFIG_PARSE_WARN, + /* userdata= */ m); if (r < 0) return r; diff --git a/src/oom/oomd.c b/src/oom/oomd.c index a88f57da0a..4f8c12bd54 100644 --- a/src/oom/oomd.c +++ b/src/oom/oomd.c @@ -31,9 +31,12 @@ static int parse_config(void) { {} }; - return config_parse_config_file("systemd/oomd.conf", "OOM\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, NULL); + return config_parse_standard_file_with_dropins( + "systemd/oomd.conf", + "OOM\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); } static int help(void) { diff --git a/src/pstore/pstore.c b/src/pstore/pstore.c index 529193c9e9..e2dfc4d1a1 100644 --- a/src/pstore/pstore.c +++ b/src/pstore/pstore.c @@ -77,9 +77,12 @@ static int parse_config(void) { {} }; - return config_parse_config_file("systemd/pstore.conf", "PStore\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, NULL); + return config_parse_standard_file_with_dropins( + "systemd/pstore.conf", + "PStore\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); } /* File list handling - PStoreEntry is the struct and diff --git a/src/resolve/resolved-conf.c b/src/resolve/resolved-conf.c index f88f295437..b648c3e520 100644 --- a/src/resolve/resolved-conf.c +++ b/src/resolve/resolved-conf.c @@ -570,9 +570,12 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("systemd/resolved.conf", "Resolve\0", - config_item_perf_lookup, resolved_gperf_lookup, - CONFIG_PARSE_WARN, m); + r = config_parse_standard_file_with_dropins( + "systemd/resolved.conf", + "Resolve\0", + config_item_perf_lookup, resolved_gperf_lookup, + CONFIG_PARSE_WARN, + /* userdata= */ m); if (r < 0) return r; diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 3e3e19b67f..74ba6e6d4c 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -595,54 +595,6 @@ static int config_parse_many_files( return 0; } -/* Parse one main config file located in /etc/$pkgdir and its drop-ins, which is what all systemd daemons - * do. */ -int config_parse_config_file( - const char *conf_file, - const char *sections, - ConfigItemLookup lookup, - const void *table, - ConfigParseFlags flags, - void *userdata) { - - _cleanup_strv_free_ char **dropins = NULL, **dropin_dirs = NULL; - char **conf_paths = CONF_PATHS_STRV(""); - int r; - - assert(conf_file); - - /* build the dropin dir list */ - dropin_dirs = new0(char*, strv_length(conf_paths) + 1); - if (!dropin_dirs) { - if (flags & CONFIG_PARSE_WARN) - return log_oom(); - return -ENOMEM; - } - - size_t i = 0; - STRV_FOREACH(p, conf_paths) { - char *d; - - d = strjoin(*p, conf_file, ".d"); - if (!d) { - if (flags & CONFIG_PARSE_WARN) - log_oom(); - return -ENOMEM; - } - - dropin_dirs[i++] = d; - } - - r = conf_files_list_strv(&dropins, ".conf", NULL, 0, (const char**) dropin_dirs); - if (r < 0) - return r; - - const char *sysconf_file = strjoina(SYSCONF_DIR, "/", conf_file); - - return config_parse_many_files(NULL, STRV_MAKE_CONST(sysconf_file), dropins, - sections, lookup, table, flags, userdata, NULL); -} - /* Parse each config file in the directories specified as strv. */ int config_parse_many( const char* const* conf_files, diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index d3c65bbf92..254d6cb70b 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -93,14 +93,6 @@ int config_parse( void *userdata, struct stat *ret_stat); /* possibly NULL */ -int config_parse_config_file( - const char *conf_file, /* a path like "systemd/frobnicator.conf" */ - const char *sections, /* nulstr */ - ConfigItemLookup lookup, - const void *table, - ConfigParseFlags flags, - void *userdata); - int config_parse_many( const char* const* conf_files, /* possibly empty */ const char* const* conf_file_dirs, @@ -125,6 +117,25 @@ int config_parse_standard_file_with_dropins_full( Hashmap **ret_stats_by_path, /* possibly NULL */ char ***ret_dropin_files); /* possibly NULL */ +static inline int config_parse_standard_file_with_dropins( + const char *main_file, /* A path like "systemd/frobnicator.conf" */ + const char *sections, /* nulstr */ + ConfigItemLookup lookup, + const void *table, + ConfigParseFlags flags, + void *userdata) { + return config_parse_standard_file_with_dropins_full( + /* root= */ NULL, + main_file, + sections, + lookup, + table, + flags, + userdata, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); +} + int config_get_stats_by_path( const char *suffix, const char *root, diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 19b75a706d..c96f8485dd 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -145,9 +145,12 @@ int parse_sleep_config(SleepConfig **ret) { {} }; - (void) config_parse_config_file("systemd/sleep.conf", "Sleep\0", - config_item_table_lookup, items, - CONFIG_PARSE_WARN, NULL); + (void) config_parse_standard_file_with_dropins( + "systemd/sleep.conf", + "Sleep\0", + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata= */ NULL); /* use default values unless set */ sc->allow[SLEEP_SUSPEND] = allow_suspend != 0; diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 205afc5723..0014b7236f 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -28,11 +28,10 @@ int udev_parse_config_full(const ConfigTableItem config_table[]) { assert(config_table); - r = config_parse_config_file( + r = config_parse_standard_file_with_dropins( "udev/udev.conf", /* sections = */ NULL, - config_item_table_lookup, - config_table, + config_item_table_lookup, config_table, CONFIG_PARSE_WARN, /* userdata = */ NULL); if (r == -ENOENT) diff --git a/src/timesync/timesyncd-conf.c b/src/timesync/timesyncd-conf.c index 66f3d177e9..4b1d4ddbfe 100644 --- a/src/timesync/timesyncd-conf.c +++ b/src/timesync/timesyncd-conf.c @@ -102,9 +102,12 @@ int manager_parse_config_file(Manager *m) { assert(m); - r = config_parse_config_file("systemd/timesyncd.conf", "Time\0", - config_item_perf_lookup, timesyncd_gperf_lookup, - CONFIG_PARSE_WARN, m); + r = config_parse_standard_file_with_dropins( + "systemd/timesyncd.conf", + "Time\0", + config_item_perf_lookup, timesyncd_gperf_lookup, + CONFIG_PARSE_WARN, + /* userdata= */ m); if (r < 0) return r; From b83a59f8a7b621781ff5916bc067b1406374a891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 22 Feb 2024 10:58:48 +0100 Subject: [PATCH 11/14] man: document all the new paths --- man/coredump.conf.xml | 2 ++ man/homed.conf.xml | 2 ++ man/journal-remote.conf.xml | 2 ++ man/journal-upload.conf.xml | 2 ++ man/logind.conf.xml | 2 ++ man/networkd.conf.xml | 2 ++ man/oomd.conf.xml | 3 +++ man/pstore.conf.xml | 12 ++++++++---- man/resolved.conf.xml | 2 ++ man/standard-conf.xml | 19 ++++++++++++------- man/systemd-sleep.conf.xml | 2 ++ man/systemd-system.conf.xml | 11 ++++++++--- man/timesyncd.conf.xml | 2 ++ 13 files changed, 49 insertions(+), 14 deletions(-) diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml index b08d732adb..29ac35142a 100644 --- a/man/coredump.conf.xml +++ b/man/coredump.conf.xml @@ -24,6 +24,8 @@ /etc/systemd/coredump.conf + /run/systemd/coredump.conf + /usr/lib/systemd/coredump.conf /etc/systemd/coredump.conf.d/*.conf /run/systemd/coredump.conf.d/*.conf /usr/lib/systemd/coredump.conf.d/*.conf diff --git a/man/homed.conf.xml b/man/homed.conf.xml index a6c070cb53..9dfcfb6296 100644 --- a/man/homed.conf.xml +++ b/man/homed.conf.xml @@ -24,6 +24,8 @@ /etc/systemd/homed.conf + /run/systemd/homed.conf + /usr/lib/systemd/homed.conf /etc/systemd/homed.conf.d/*.conf /run/systemd/homed.conf.d/*.conf /usr/lib/systemd/homed.conf.d/*.conf diff --git a/man/journal-remote.conf.xml b/man/journal-remote.conf.xml index 1cf18c3cd8..44e056ab49 100644 --- a/man/journal-remote.conf.xml +++ b/man/journal-remote.conf.xml @@ -29,6 +29,8 @@ /etc/systemd/journal-remote.conf + /run/systemd/journal-remote.conf + /usr/lib/systemd/journal-remote.conf /etc/systemd/journal-remote.conf.d/*.conf /run/systemd/journal-remote.conf.d/*.conf /usr/lib/systemd/journal-remote.conf.d/*.conf diff --git a/man/journal-upload.conf.xml b/man/journal-upload.conf.xml index 66ea0dca6c..7d3f22f96b 100644 --- a/man/journal-upload.conf.xml +++ b/man/journal-upload.conf.xml @@ -23,6 +23,8 @@ /etc/systemd/journal-upload.conf + /run/systemd/journal-upload.conf + /usr/lib/systemd/journal-upload.conf /etc/systemd/journal-upload.conf.d/*.conf /run/systemd/journal-upload.conf.d/*.conf /usr/lib/systemd/journal-upload.conf.d/*.conf diff --git a/man/logind.conf.xml b/man/logind.conf.xml index d74c9b410f..ec16059065 100644 --- a/man/logind.conf.xml +++ b/man/logind.conf.xml @@ -27,6 +27,8 @@ /etc/systemd/logind.conf + /run/systemd/logind.conf + /usr/lib/systemd/logind.conf /etc/systemd/logind.conf.d/*.conf /run/systemd/logind.conf.d/*.conf /usr/lib/systemd/logind.conf.d/*.conf diff --git a/man/networkd.conf.xml b/man/networkd.conf.xml index 9477bfe5af..b5b825b8b8 100644 --- a/man/networkd.conf.xml +++ b/man/networkd.conf.xml @@ -29,6 +29,8 @@ /etc/systemd/networkd.conf + /run/systemd/networkd.conf + /usr/lib/systemd/networkd.conf /etc/systemd/networkd.conf.d/*.conf /usr/lib/systemd/networkd.conf.d/*.conf diff --git a/man/oomd.conf.xml b/man/oomd.conf.xml index 1c25996498..b099f8f546 100644 --- a/man/oomd.conf.xml +++ b/man/oomd.conf.xml @@ -24,7 +24,10 @@ /etc/systemd/oomd.conf + /run/systemd/oomd.conf + /usr/lib/systemd/oomd.conf /etc/systemd/oomd.conf.d/*.conf + /run/systemd/oomd.conf.d/*.conf /usr/lib/systemd/oomd.conf.d/*.conf diff --git a/man/pstore.conf.xml b/man/pstore.conf.xml index 3216a4c27b..d6b6c4b1d7 100644 --- a/man/pstore.conf.xml +++ b/man/pstore.conf.xml @@ -22,10 +22,14 @@ - - /etc/systemd/pstore.conf - /etc/systemd/pstore.conf.d/* - + + /etc/systemd/pstore.conf + /run/systemd/pstore.conf + /usr/lib/systemd/pstore.conf + /etc/systemd/pstore.conf.d/*.conf + /run/systemd/pstore.conf.d/*.conf + /usr/lib/systemd/pstore.conf.d/*.conf + diff --git a/man/resolved.conf.xml b/man/resolved.conf.xml index 24cf3e427c..084256cf05 100644 --- a/man/resolved.conf.xml +++ b/man/resolved.conf.xml @@ -27,6 +27,8 @@ /etc/systemd/resolved.conf + /run/systemd/resolved.conf + /usr/lib/systemd/resolved.conf /etc/systemd/resolved.conf.d/*.conf /run/systemd/resolved.conf.d/*.conf /usr/lib/systemd/resolved.conf.d/*.conf diff --git a/man/standard-conf.xml b/man/standard-conf.xml index c3cebdac5f..f54475f306 100644 --- a/man/standard-conf.xml +++ b/man/standard-conf.xml @@ -50,14 +50,19 @@ Configuration Directories and Precedence The default configuration is set during compilation, so configuration is only needed when it is - necessary to deviate from those defaults. The main configuration file is either in - /usr/lib/systemd/ or /etc/systemd/ and contains commented out - entries showing the defaults as a guide to the administrator. Local overrides can be created by creating - drop-ins, as described below. The main configuration file can also be edited for this purpose (or a copy - in /etc/ if it's shipped in /usr/) however using drop-ins for - local configuration is recommended over modifications to the main configuration file. + necessary to deviate from those defaults. The main configuration file is loaded from one of the + listed directories in order of priority, only the first file found is used: + /etc/systemd/, + /run/systemd/, + /usr/local/lib/systemd/, + /usr/lib/systemd/. + The vendor version of the file contains commented out entries showing the defaults as a guide to the + administrator. Local overrides can also be created by creating drop-ins, as described below. The main + configuration file can also be edited for this purpose (or a copy in /etc/ if it's + shipped under /usr/), however using drop-ins for local configuration is recommended + over modifications to the main configuration file. - In addition to the "main" configuration file, drop-in configuration snippets are read from + In addition to the main configuration file, drop-in configuration snippets are read from /usr/lib/systemd/*.conf.d/, /usr/local/lib/systemd/*.conf.d/, and /etc/systemd/*.conf.d/. Those drop-ins have higher precedence and override the main configuration file. Files in the *.conf.d/ configuration subdirectories are diff --git a/man/systemd-sleep.conf.xml b/man/systemd-sleep.conf.xml index 1abec4f34f..411577d519 100644 --- a/man/systemd-sleep.conf.xml +++ b/man/systemd-sleep.conf.xml @@ -24,6 +24,8 @@ /etc/systemd/sleep.conf + /run/systemd/sleep.conf + /usr/lib/systemd/sleep.conf /etc/systemd/sleep.conf.d/*.conf /run/systemd/sleep.conf.d/*.conf /usr/lib/systemd/sleep.conf.d/*.conf diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 31b6421399..e6611d04e7 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -28,12 +28,16 @@ /etc/systemd/system.conf, + /run/systemd/system.conf, + /usr/lib/systemd/system.conf, /etc/systemd/system.conf.d/*.conf, /run/systemd/system.conf.d/*.conf, /usr/lib/systemd/system.conf.d/*.conf ~/.config/systemd/user.conf, /etc/systemd/user.conf, + /run/systemd/user.conf, + /usr/lib/systemd/user.conf, /etc/systemd/user.conf.d/*.conf, /run/systemd/user.conf.d/*.conf, /usr/lib/systemd/user.conf.d/*.conf @@ -44,9 +48,10 @@ When run as a system instance, systemd interprets the configuration file system.conf and the files in system.conf.d directories; when - run as a user instance, it interprets the configuration file user.conf (either in - the home directory of the user, or if not found, under /etc/systemd/) and the files - in user.conf.d directories. These configuration files contain a few settings + run as a user instance, it interprets the configuration file user.conf (in order of + priority, in the home directory of the user and under /etc/systemd/, + /run/systemd/, and /usr/lib/systemd/) and the files in + user.conf.d directories. These configuration files contain a few settings controlling basic manager operations. See diff --git a/man/timesyncd.conf.xml b/man/timesyncd.conf.xml index 498bfa330b..248fd88b77 100644 --- a/man/timesyncd.conf.xml +++ b/man/timesyncd.conf.xml @@ -24,6 +24,8 @@ /etc/systemd/timesyncd.conf + /run/systemd/timesyncd.conf + /usr/lib/systemd/timesyncd.conf /etc/systemd/timesyncd.conf.d/*.conf /run/systemd/timesyncd.conf.d/*.conf /usr/lib/systemd/timesyncd.conf.d/*.conf From db26d8025e5bbc188f93b645124126bbc550caa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 21 Feb 2024 13:41:57 +0100 Subject: [PATCH 12/14] kernel-install: support full set of config files and drop-ins This brings the handling of config for kernel-install in line with most of systemd, i.e. we search the set of paths for the main config file, and the full set of drop-in paths for drop-ins. This mirrors what 07f5e35fe7967c824a87f18a3a1d3c22e5be70f5 did for udev.conf. That change worked out fine, so I hope this one will too. The update in the man page is minimal. I think we should split out a separate page for the config file later on. One motivating use case is to allow a drop-in to be created for temporary config overrides and then removed after the operation is done. --- man/kernel-install.xml | 19 ++++- src/kernel-install/kernel-install.c | 95 +++++++++++------------ src/kernel-install/test-kernel-install.sh | 5 +- 3 files changed, 65 insertions(+), 54 deletions(-) diff --git a/man/kernel-install.xml b/man/kernel-install.xml index 519829579f..168776b4d3 100644 --- a/man/kernel-install.xml +++ b/man/kernel-install.xml @@ -510,8 +510,12 @@ $KERNEL_INSTALL_CONF_ROOT can be set to override the location of the configuration files read by kernel-install. When set, - install.conf, entry-token, and other files will be - read from this directory. + install.conf, entry-token, and other files will be read from + this directory only. Note that this path is relative to the host, and in particular symlinks + in this directory are resolved relative to the host, even if + is used. This means that it is generally + not correct to use this variable to specify a directory underneath + root if symlinks are used there. $KERNEL_INSTALL_PLUGINS can be set to override the list of plugins executed by kernel-install. The argument is a whitespace-separated list of paths. @@ -639,14 +643,23 @@ /etc/kernel/install.conf + /run/kernel/install.conf + /usr/local/lib/kernel/install.conf /usr/lib/kernel/install.conf + /etc/kernel/install.conf.d/*.conf + /run/kernel/install.conf.d/*.conf + /usr/local/lib/kernel/install.conf.d/*.conf + /usr/lib/kernel/install.conf.d/*.conf Configuration file with options for kernel-install, as a series of KEY=VALUE assignments, compatible with shell syntax, following the same rules as described in os-release5. The first of the files that is found will be used. $KERNEL_INSTALL_CONF_ROOT may be - used to override the search path; see below for details. + used to override the search path; see below for details. Drop-in files may also be used + to extend the configuration with overrides, see + systemd.unit5. + Currently, the following keys are supported: MACHINE_ID=, diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index aa81af70d0..af44ab911b 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -431,64 +431,59 @@ static int context_load_environment(Context *c) { return 0; } -static int context_load_install_conf_one(Context *c, const char *path) { - _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char - *conf = NULL, *machine_id = NULL, *boot_root = NULL, *layout = NULL, - *initrd_generator = NULL, *uki_generator = NULL; - int r; - - assert(c); - assert(path); - - conf = path_join(path, "install.conf"); - if (!conf) - return log_oom(); - - r = chase_and_fopenat_unlocked(c->rfd, conf, CHASE_AT_RESOLVE_IN_ROOT, "re", NULL, &f); - if (r == -ENOENT) - return 0; - if (r < 0) - return log_error_errno(r, "Failed to chase %s: %m", conf); - - log_debug("Loading %s…", conf); - - r = parse_env_file(f, conf, - "MACHINE_ID", &machine_id, - "BOOT_ROOT", &boot_root, - "layout", &layout, - "initrd_generator", &initrd_generator, - "uki_generator", &uki_generator); - if (r < 0) - return log_error_errno(r, "Failed to parse '%s': %m", conf); - - (void) context_set_machine_id(c, machine_id, conf); - (void) context_set_boot_root(c, boot_root, conf); - (void) context_set_layout(c, layout, conf); - (void) context_set_initrd_generator(c, initrd_generator, conf); - (void) context_set_uki_generator(c, uki_generator, conf); - - log_debug("Loaded %s.", conf); - return 1; -} - static int context_load_install_conf(Context *c) { + _cleanup_free_ char *machine_id = NULL, *boot_root = NULL, *layout = NULL, + *initrd_generator = NULL, *uki_generator = NULL; + const ConfigTableItem items[] = { + { NULL, "MACHINE_ID", config_parse_string, 0, &machine_id }, + { NULL, "BOOT_ROOT", config_parse_string, 0, &boot_root }, + { NULL, "layout", config_parse_string, 0, &layout }, + { NULL, "initrd_generator", config_parse_string, 0, &initrd_generator }, + { NULL, "uki_generator", config_parse_string, 0, &uki_generator }, + {} + }; int r; assert(c); if (c->conf_root) { - r = context_load_install_conf_one(c, c->conf_root); - if (r != 0) - return r; - } + _cleanup_free_ char *conf = NULL; - FOREACH_STRING(p, "/etc/kernel", "/usr/lib/kernel") { - r = context_load_install_conf_one(c, p); - if (r != 0) - return r; - } + conf = path_join(c->conf_root, "install.conf"); + if (!conf) + return log_oom(); + r = config_parse_many( + STRV_MAKE_CONST(conf), + STRV_MAKE_CONST(c->conf_root), + "install.conf.d", + /* root= */ NULL, /* $KERNEL_INSTALL_CONF_ROOT and --root are independent */ + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata = */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); + } else + r = config_parse_standard_file_with_dropins_full( + arg_root, + "kernel/install.conf", + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata = */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); + if (r < 0) + return r == -ENOENT ? 0 : r; + + (void) context_set_machine_id(c, machine_id, "config"); + (void) context_set_boot_root(c, boot_root, "config"); + (void) context_set_layout(c, layout, "config"); + (void) context_set_initrd_generator(c, initrd_generator, "config"); + (void) context_set_uki_generator(c, uki_generator, "config"); + + log_debug("Loaded config."); return 0; } diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh index 338d811957..0e41979878 100755 --- a/src/kernel-install/test-kernel-install.sh +++ b/src/kernel-install/test-kernel-install.sh @@ -125,7 +125,8 @@ grep -qE 'initrd' "$BOOT_ROOT/the-token/1.1.1/initrd" # Install UKI if [ -f "$ukify" ]; then - cat >>"$D/sources/install.conf" <>"$D/sources/install.conf.d/override.conf" < Date: Wed, 21 Feb 2024 23:44:09 +0100 Subject: [PATCH 13/14] bootctl: use the full parser too --- src/boot/bootctl-install.c | 64 +++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/src/boot/bootctl-install.c b/src/boot/bootctl-install.c index a0a474537e..b805fa8f7a 100644 --- a/src/boot/bootctl-install.c +++ b/src/boot/bootctl-install.c @@ -81,44 +81,52 @@ static int load_etc_machine_info(void) { return 0; } -static int load_kernel_install_conf_one(const char *dir) { - _cleanup_free_ char *layout = NULL, *p = NULL; +static int load_kernel_install_conf(void) { + _cleanup_free_ char *layout = NULL; + const ConfigTableItem items[] = { + { NULL, "layout", config_parse_string, 0, &layout }, + {} + }; int r; - assert(dir); + const char *conf_root = getenv("KERNEL_INSTALL_CONF_ROOT"); - p = path_join(arg_root, dir, "install.conf"); - if (!p) - return log_oom(); + if (conf_root) { + _cleanup_free_ char *conf = NULL; - r = parse_env_file(NULL, p, "layout", &layout); - if (r == -ENOENT) - return 0; + conf = path_join(conf_root, "install.conf"); + if (!conf) + return log_oom(); + + r = config_parse_many( + STRV_MAKE_CONST(conf), + STRV_MAKE_CONST(conf_root), + "install.conf.d", + /* root= */ NULL, /* $KERNEL_INSTALL_CONF_ROOT and --root are independent */ + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata = */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); + } else + r = config_parse_standard_file_with_dropins_full( + arg_root, + "kernel/install.conf", + /* sections= */ NULL, + config_item_table_lookup, items, + CONFIG_PARSE_WARN, + /* userdata = */ NULL, + /* ret_stats_by_path= */ NULL, + /* ret_dropin_files= */ NULL); if (r < 0) - return log_error_errno(r, "Failed to parse %s: %m", p); + return r == -ENOENT ? 0 : r; if (!isempty(layout)) { - log_debug("layout=%s is specified in %s.", layout, p); + log_debug("layout=%s is specified in config.", layout); free_and_replace(arg_install_layout, layout); } - return 1; -} - -static int load_kernel_install_conf(void) { - const char *conf_root; - int r; - - conf_root = getenv("KERNEL_INSTALL_CONF_ROOT"); - if (conf_root) - return load_kernel_install_conf_one(conf_root); - - FOREACH_STRING(p, "/etc/kernel", "/usr/lib/kernel") { - r = load_kernel_install_conf_one(p); - if (r != 0) - return r; - } - return 0; } From b7d62bdbd08174cf4e0fd5a947379c7a7de32ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 7 Mar 2024 18:47:31 +0100 Subject: [PATCH 14/14] shared/conf-parser: add two more annotations --- src/shared/conf-parser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 74ba6e6d4c..e2d3b65f88 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -558,7 +558,7 @@ static int config_parse_many_files( } } - r = config_parse(NULL, *fn, f, sections, lookup, table, flags, userdata, &st); + r = config_parse(/* unit= */ NULL, *fn, f, sections, lookup, table, flags, userdata, &st); if (r < 0) return r; assert(r > 0); @@ -577,7 +577,7 @@ static int config_parse_many_files( const char *path_dropin; FILE *f_dropin; ORDERED_HASHMAP_FOREACH_KEY(f_dropin, path_dropin, dropins) { - r = config_parse(NULL, path_dropin, f_dropin, sections, lookup, table, flags, userdata, &st); + r = config_parse(/* unit= */ NULL, path_dropin, f_dropin, sections, lookup, table, flags, userdata, &st); if (r < 0) return r; assert(r > 0);