From 40ac7b1406ca8c0e53f00ca608a0b85249bfa984 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 27 Aug 2024 10:49:01 +0200 Subject: [PATCH 1/3] core: print full configuration paths with "--print-config" In the output of "NetworkManager --print-config" we currently print the list of configuration snippets in an abbreviated form: ... (lib: 00-server.conf, 22-wifi-mac-addr.conf) (etc: 08-unmanaged.conf) While it is concise and unambiguous, it can be cryptic for users. Instead, print the full paths: ... /usr/lib/NetworkManager/conf.d/{00-server.conf,22-wifi-mac-addr.conf}, /etc/NetworkManager/conf.d/{08-unmanaged.conf} --- src/core/nm-config.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/core/nm-config.c b/src/core/nm-config.c index 878f343a54..bafb434649 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -1284,19 +1284,23 @@ _get_config_dir_files(const char *config_dir) static void _confs_to_description(GString *str, const GPtrArray *confs, const char *name) { - guint i; + guint i; + gboolean need_braces; if (!confs->len) return; + need_braces = confs->len > 1; + for (i = 0; i < confs->len; i++) { if (i == 0) - g_string_append_printf(str, " (%s: ", name); + g_string_append_printf(str, ", %s/%s", name, need_braces ? "{" : ""); else - g_string_append(str, ", "); + g_string_append(str, ","); g_string_append(str, confs->pdata[i]); } - g_string_append(str, ")"); + if (need_braces) + g_string_append(str, "}"); } static GKeyFile * @@ -1427,9 +1431,9 @@ read_entire_config(const NMConfigCmdLineOptions *cli, GString *str; str = g_string_new(o_config_main_file); - _confs_to_description(str, system_confs, "lib"); - _confs_to_description(str, run_confs, "run"); - _confs_to_description(str, confs, "etc"); + _confs_to_description(str, system_confs, system_config_dir); + _confs_to_description(str, run_confs, run_config_dir); + _confs_to_description(str, confs, config_dir); *out_config_description = g_string_free(str, FALSE); } NM_SET_OUT(out_config_main_file, g_steal_pointer(&o_config_main_file)); From 07113dde30df4f52383161a60ba2baf220305758 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 29 Aug 2024 10:58:01 +0200 Subject: [PATCH 2/3] core: fix deleting internal global DNS configuration The tracking of variable "has_intern" in intern_config_read() is wrong: we set it when adding any entry to the keyfile, but then we remove the global DNS section without updating the variable. The effect is that the function might return an empty keyfile instead of NULL. Fix this by moving the check on global DNS above. Fixes: 55c204b9a357 ('core: add support for reading global DNS configuration from keyfile') --- src/core/nm-config.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/core/nm-config.c b/src/core/nm-config.c index bafb434649..20a362a5e6 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -1562,6 +1562,7 @@ intern_config_read(const char *filename, gs_strfreev char **groups = NULL; guint g, k; gboolean has_intern = FALSE; + gboolean has_global_dns; g_return_val_if_fail(filename, NULL); @@ -1579,6 +1580,8 @@ intern_config_read(const char *filename, goto out; } + has_global_dns = nm_config_keyfile_has_global_dns_config(keyfile_conf, FALSE); + groups = g_key_file_get_groups(keyfile, NULL); for (g = 0; groups && groups[g]; g++) { gs_strfreev char **keys = NULL; @@ -1595,6 +1598,21 @@ intern_config_read(const char *filename, is_intern = NM_STR_HAS_PREFIX(group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); is_atomic = !is_intern && _is_atomic_section(atomic_section_prefixes, group); + if (has_global_dns + && (nm_streq0(group, NM_CONFIG_KEYFILE_GROUP_INTERN_GLOBAL_DNS) + || NM_STR_HAS_PREFIX_WITH_MORE( + group, + NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN_GLOBAL_DNS_DOMAIN))) { + /* + * If user configuration specifies global DNS options, the DNS + * options in internal configuration must be deleted. Otherwise, a + * deletion of options from user configuration may cause the + * internal options to appear again. + */ + needs_rewrite = TRUE; + continue; + } + if (is_atomic) { gs_free char *conf_section_was = NULL; gs_free char *conf_section_is = NULL; @@ -1688,26 +1706,6 @@ intern_config_read(const char *filename, } out: - /* - * If user configuration specifies global DNS options, the DNS - * options in internal configuration must be deleted. Otherwise, a - * deletion of options from user configuration may cause the - * internal options to appear again. - */ - if (nm_config_keyfile_has_global_dns_config(keyfile_conf, FALSE)) { - if (g_key_file_remove_group(keyfile_intern, - NM_CONFIG_KEYFILE_GROUP_INTERN_GLOBAL_DNS, - NULL)) - needs_rewrite = TRUE; - for (g = 0; groups && groups[g]; g++) { - if (NM_STR_HAS_PREFIX(groups[g], NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN_GLOBAL_DNS_DOMAIN) - && groups[g][NM_STRLEN(NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN_GLOBAL_DNS_DOMAIN)]) { - g_key_file_remove_group(keyfile_intern, groups[g], NULL); - needs_rewrite = TRUE; - } - } - } - g_key_file_unref(keyfile); if (out_needs_rewrite) From 2844b205ab55c353787431278b6fef6d61afc27c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 29 Aug 2024 11:16:27 +0200 Subject: [PATCH 3/3] core: print the internal configuration file with "--print-config" When there is a non-empty internal configuration file, print it in the output of "NetworkManager --print-config". Before: NetworkManager --print-config: # NetworkManager configuration: /etc/NetworkManager/NetworkManager.conf, /usr/lib/NetworkManager/conf.d/{00-server.conf,22-eth-mac-addr.conf} ... After: NetworkManager --print-config: # NetworkManager configuration: /etc/NetworkManager/NetworkManager.conf, /usr/lib/NetworkManager/conf.d/{00-server.conf,22-eth-mac-addr.conf}, /var/lib/NetworkManager/NetworkManager-intern.conf ... Tests needs to be changed because now writing to the internal file causes a change of the description of the NMConfigData and therefore the NM_CONFIG_CHANGE_CONFIG_FILES flag is set. --- src/core/nm-config.c | 46 +++++++++++++++++++---------- src/core/tests/config/test-config.c | 7 +++-- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/core/nm-config.c b/src/core/nm-config.c index 20a362a5e6..c6dc78d887 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -2819,12 +2819,12 @@ nm_config_reload(NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emit NMConfigPrivate *priv; GError *error = NULL; GKeyFile *keyfile, *keyfile_intern; - NMConfigData *new_data = NULL; - char *config_main_file = NULL; - char *config_description = NULL; - gs_strfreev char **no_auto_default = NULL; - gboolean intern_config_needs_rewrite; - gs_unref_ptrarray GPtrArray *warnings = NULL; + NMConfigData *new_data = NULL; + char *config_main_file = NULL; + char *config_description = NULL; + gs_strfreev char **no_auto_default = NULL; + gboolean intern_config_needs_rewrite = FALSE; + gs_unref_ptrarray GPtrArray *warnings = NULL; guint i; g_return_if_fail(NM_IS_CONFIG(self)); @@ -2875,6 +2875,13 @@ nm_config_reload(NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emit NULL); } + if (keyfile_intern) { + gs_free char *desc = config_description; + + config_description = + g_strdup_printf("%s, %s", config_description, priv->intern_config_file); + } + new_data = nm_config_data_new(config_main_file, config_description, (const char *const *) no_auto_default, @@ -3046,16 +3053,16 @@ nm_config_kernel_command_line_nm_debug(void) static gboolean init_sync(GInitable *initable, GCancellable *cancellable, GError **error) { - NMConfig *self = NM_CONFIG(initable); - NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE(self); - nm_auto_unref_keyfile GKeyFile *keyfile = NULL; - nm_auto_unref_keyfile GKeyFile *keyfile_intern = NULL; - gs_free char *config_main_file = NULL; - gs_free char *config_description = NULL; - gs_strfreev char **no_auto_default = NULL; - gs_unref_ptrarray GPtrArray *warnings = NULL; - gs_free char *configure_and_quit = NULL; - gboolean intern_config_needs_rewrite; + NMConfig *self = NM_CONFIG(initable); + NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE(self); + nm_auto_unref_keyfile GKeyFile *keyfile = NULL; + nm_auto_unref_keyfile GKeyFile *keyfile_intern = NULL; + gs_free char *config_main_file = NULL; + gs_free char *config_description = NULL; + gs_strfreev char **no_auto_default = NULL; + gs_unref_ptrarray GPtrArray *warnings = NULL; + gs_free char *configure_and_quit = NULL; + gboolean intern_config_needs_rewrite = FALSE; const char *s; if (priv->config_dir) { @@ -3130,6 +3137,13 @@ init_sync(GInitable *initable, GCancellable *cancellable, GError **error) NULL); } + if (keyfile_intern) { + gs_free char *desc = config_description; + + config_description = + g_strdup_printf("%s, %s", config_description, priv->intern_config_file); + } + priv->config_data_orig = nm_config_data_new(config_main_file, config_description, (const char *const *) no_auto_default, diff --git a/src/core/tests/config/test-config.c b/src/core/tests/config/test-config.c index 8365fc8232..2980cda7fb 100644 --- a/src/core/tests/config/test-config.c +++ b/src/core/tests/config/test-config.c @@ -965,7 +965,8 @@ _set_values_user_atomic_section_1_set(NMConfig *config, g_key_file_set_string(keyfile, "atomic-prefix-1.section-b", "key1", "user-value1"); g_key_file_set_string(keyfile, "non-atomic-prefix-1.section-a", "nap1-key1", "user-value1"); g_key_file_set_string(keyfile, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2"); - *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER; + *out_expected_changes = + NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER | NM_CONFIG_CHANGE_CONFIG_FILES; } static void @@ -976,7 +977,9 @@ _set_values_user_atomic_section_1_check(NMConfig *config, NMConfigData *old_data) { if (is_change_event) - g_assert(changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER)); + g_assert(changes + == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER + | NM_CONFIG_CHANGE_CONFIG_FILES)); assert_config_value(config_data, "atomic-prefix-1.section-a", "key1", "user-value1"); assert_config_value(config_data, "atomic-prefix-1.section-a", "key2", "user-value2"); assert_config_value(config_data, "atomic-prefix-1.section-b", "key1", "user-value1");