From 32dbc51dbd2842a6196bc84f9c303bee9c16a2de Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 17:40:54 +0200 Subject: [PATCH 1/8] build: add nmlibdir define --- configure.ac | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure.ac b/configure.ac index e1a1917db6..b197ef7926 100644 --- a/configure.ac +++ b/configure.ac @@ -78,6 +78,7 @@ AC_SUBST(runstatedir) # NetworkManager paths AC_SUBST(nmbinary, "$sbindir/$PACKAGE", [NetworkManager binary executable]) AC_SUBST(nmconfdir, "$sysconfdir/$PACKAGE", [NetworkManager configuration directory]) +AC_SUBST(nmlibdir, "$libdir/$PACKAGE", [NetworkManager library directory]) AC_SUBST(nmdatadir, "$datadir/$PACKAGE", [NetworkManager shared data directory]) AC_SUBST(nmstatedir, "$localstatedir/lib/$PACKAGE", [NetworkManager persistent state directory]) AC_SUBST(nmrundir, "$runstatedir/$PACKAGE", [NetworkManager runtime state directory]) @@ -1065,6 +1066,7 @@ echo " exec_prefix: $exec_prefix" echo " systemdunitdir: $with_systemdsystemunitdir" echo " nmbinary: $nmbinary" echo " nmconfdir: $nmconfdir" +echo " nmlibdir: $nmlibdir" echo " nmdatadir: $nmdatadir" echo " nmstatedir: $nmstatedir" echo " nmrundir: $nmrundir" From d783742b2207afed9e12293d06ce7f9c491fced8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2015 17:51:04 +0200 Subject: [PATCH 2/8] config: read configuration directory "/usr/lib/NetworkManager/conf.d" This allows packages to install their configuration snippets to "/usr/", which is a better place for system-provided configuration files then "/etc". "/usr/lib/NetworkManager/conf.d/" is read first, so that the values in /etc have higher priority. In general, we want to move system-provided configuration away from /etc, so that a user can do a "factory-reset" by purging /etc. https://bugzilla.gnome.org/show_bug.cgi?id=738853 --- contrib/fedora/rpm/NetworkManager.conf | 18 +++- contrib/fedora/rpm/NetworkManager.spec | 4 + man/NetworkManager.conf.xml.in | 13 ++- src/Makefile.am | 1 + src/nm-config.c | 124 +++++++++++++++++-------- src/tests/config/test-config.c | 24 +++-- 6 files changed, 130 insertions(+), 54 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.conf b/contrib/fedora/rpm/NetworkManager.conf index 900ca9b23b..9901b03935 100644 --- a/contrib/fedora/rpm/NetworkManager.conf +++ b/contrib/fedora/rpm/NetworkManager.conf @@ -2,11 +2,21 @@ # # See "man 5 NetworkManager.conf" for details. # +# The directory /usr/lib/NetworkManager/conf.d/ can contain additional configuration +# snippets installed by packages. These files are read before NetworkManager.conf +# and have thus lowest priority. # The directory /etc/NetworkManager/conf.d/ can contain additional configuration -# snippets that are installed by some packages. Those snippets override the -# settings from this main file. -# To override a configuration from a conf.d/ snippet, add another configuration -# with a name sorted lastly (such as 99-my.conf). +# snippets. Those snippets override the settings from this main file. +# +# The files within one conf.d/ directory are read in asciibetical order. +# +# If /etc/NetworkManager/conf.d/ contains a file with the same name as +# /usr/lib/NetworkManager/conf.d/, the latter file is shadowed and thus ignored. +# Hence, to disable loading a file from /usr/lib/NetworkManager/conf.d/ you can +# put an empty file with the same name. +# +# If two files define the same key, the one that is read afterwards will overwrite +# the previous one. [main] plugins=ifcfg-rh,ibft diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 0bdabe8dec..c026cef08f 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -34,6 +34,7 @@ %define systemd_dir %{_prefix}/lib/systemd/system %define udev_dir %{_prefix}/lib/udev +%define nmlibdir %{_prefix}/lib/%{name} %global with_adsl 1 %global with_bluetooth 1 @@ -443,6 +444,7 @@ make install DESTDIR=$RPM_BUILD_ROOT %{__cp} %{SOURCE1} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/ mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d +mkdir -p $RPM_BUILD_ROOT%{nmlibdir}/conf.d %{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d %{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d %{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d @@ -543,6 +545,8 @@ fi %endif %dir %{_sysconfdir}/%{name} %dir %{_sysconfdir}/%{name}/conf.d +%dir %{nmlibdir} +%dir %{nmlibdir}/conf.d %config %{_sysconfdir}/%{name}/conf.d/10-ibft-plugin.conf %{_mandir}/man1/* %{_mandir}/man5/* diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index e2501c8464..0156a655fb 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -27,7 +27,8 @@ Copyright 2010 - 2014 Red Hat, Inc. /etc/NetworkManager/NetworkManager.conf, - /etc/NetworkManager/conf.d/name.conf + /etc/NetworkManager/conf.d/name.conf, + /usr/lib/NetworkManager/conf.d/name.conf @@ -42,8 +43,14 @@ Copyright 2010 - 2014 Red Hat, Inc. provided by your distribution's packages, you should not modify it, since your changes may get overwritten by package updates. Instead, you can add additional .conf - files to the conf.d directory. These will be read in order, - with later files overriding earlier ones. + files to the /etc/NetworkManager/conf.d directory. + These will be read in order, with later files overriding earlier ones. + Packages might install further configuration snippets to /usr/lib/NetworkManager/conf.d. + This directory is parsed first, even before NetworkManager.conf. + The loading of a file /usr/lib/NetworkManager/conf.d/name.conf + can be prevented by adding a file /etc/NetworkManager/conf.d/name.conf. + In this case, the file from the etc configuration shadows the file from the + system configuration directory. diff --git a/src/Makefile.am b/src/Makefile.am index 3aa8b5f4f9..e994b19cad 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -428,6 +428,7 @@ AM_CPPFLAGS += \ -DNMPLUGINDIR=\"$(pkglibdir)\" \ -DNMRUNDIR=\"$(nmrundir)\" \ -DNMSTATEDIR=\"$(nmstatedir)\" \ + -DNMLIBDIR=\"$(nmlibdir)\" \ \ -DDHCLIENT_PATH=\"$(DHCLIENT_PATH)\" \ -DDHCPCD_PATH=\"$(DHCPCD_PATH)\" \ diff --git a/src/nm-config.c b/src/nm-config.c index add9b4d696..f98b906fb8 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -40,11 +40,13 @@ #define DEFAULT_CONFIG_MAIN_FILE NMCONFDIR "/NetworkManager.conf" #define DEFAULT_CONFIG_DIR NMCONFDIR "/conf.d" #define DEFAULT_CONFIG_MAIN_FILE_OLD NMCONFDIR "/nm-system-settings.conf" +#define DEFAULT_SYSTEM_CONFIG_DIR NMLIBDIR "/conf.d" #define DEFAULT_NO_AUTO_DEFAULT_FILE NMSTATEDIR "/no-auto-default.state" struct NMConfigCmdLineOptions { char *config_main_file; char *config_dir; + char *system_config_dir; char *no_auto_default_file; char *plugins; gboolean configure_and_quit; @@ -64,6 +66,7 @@ typedef struct { NMConfigData *config_data_orig; char *config_dir; + char *system_config_dir; char *no_auto_default_file; char **plugins; @@ -407,6 +410,7 @@ _nm_config_cmd_line_options_clear (NMConfigCmdLineOptions *cli) { g_clear_pointer (&cli->config_main_file, g_free); g_clear_pointer (&cli->config_dir, g_free); + g_clear_pointer (&cli->system_config_dir, g_free); g_clear_pointer (&cli->no_auto_default_file, g_free); g_clear_pointer (&cli->plugins, g_free); cli->configure_and_quit = FALSE; @@ -424,6 +428,7 @@ _nm_config_cmd_line_options_copy (const NMConfigCmdLineOptions *cli, NMConfigCmd _nm_config_cmd_line_options_clear (dst); dst->config_dir = g_strdup (cli->config_dir); + dst->system_config_dir = g_strdup (cli->system_config_dir); dst->config_main_file = g_strdup (cli->config_main_file); dst->no_auto_default_file = g_strdup (cli->no_auto_default_file); dst->plugins = g_strdup (cli->plugins); @@ -462,6 +467,7 @@ nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli, GOptionEntry config_options[] = { { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(DEFAULT_CONFIG_MAIN_FILE) }, { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(DEFAULT_CONFIG_DIR) }, + { "system-config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->system_config_dir, N_("System config directory location"), N_(DEFAULT_SYSTEM_CONFIG_DIR) }, { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(DEFAULT_NO_AUTO_DEFAULT_FILE) }, { "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_(CONFIG_PLUGINS_DEFAULT) }, { "configure-and-quit", 0, 0, G_OPTION_ARG_NONE, &cli->configure_and_quit, N_("Quit after initial configuration"), NULL }, @@ -550,17 +556,23 @@ _setting_is_string_list (const char *group, const char *key) } static gboolean -read_config (GKeyFile *keyfile, const char *path, GError **error) +read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError **error) { GKeyFile *kf; char **groups, **keys; gsize ngroups, nkeys; int g, k; + gs_free char *path_free = NULL; g_return_val_if_fail (keyfile, FALSE); g_return_val_if_fail (path, FALSE); g_return_val_if_fail (!error || !*error, FALSE); + if (dirname) { + path_free = g_build_filename (dirname, path, NULL); + path = path_free; + } + if (g_file_test (path, G_FILE_TEST_EXISTS) == FALSE) { g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND, "file %s not found", path); return FALSE; @@ -710,7 +722,7 @@ read_base_config (GKeyFile *keyfile, /* Try a user-specified config file first */ if (cli_config_main_file) { /* Bad user-specific config file path is a hard error */ - if (read_config (keyfile, cli_config_main_file, error)) { + if (read_config (keyfile, NULL, cli_config_main_file, error)) { *out_config_main_file = g_strdup (cli_config_main_file); return TRUE; } else @@ -725,7 +737,7 @@ read_base_config (GKeyFile *keyfile, */ /* Try deprecated nm-system-settings.conf first */ - if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) { + if (read_config (keyfile, NULL, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) { *out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE_OLD); return TRUE; } @@ -738,7 +750,7 @@ read_base_config (GKeyFile *keyfile, g_clear_error (&my_error); /* Try the standard config file location next */ - if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE, &my_error)) { + if (read_config (keyfile, NULL, DEFAULT_CONFIG_MAIN_FILE, &my_error)) { *out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE); return TRUE; } @@ -771,24 +783,20 @@ sort_asciibetically (gconstpointer a, gconstpointer b) } static GPtrArray * -_get_config_dir_files (const char *config_main_file, - const char *config_dir, - char **out_config_description) +_get_config_dir_files (const char *config_dir) { GFile *dir; GFileEnumerator *direnum; GFileInfo *info; GPtrArray *confs; - GString *config_description; const char *name; - guint i; - g_return_val_if_fail (config_main_file, NULL); g_return_val_if_fail (config_dir, NULL); - g_return_val_if_fail (out_config_description && !*out_config_description, NULL); confs = g_ptr_array_new_with_free_func (g_free); - config_description = g_string_new (config_main_file); + if (!*config_dir) + return confs; + dir = g_file_new_for_path (config_dir); direnum = g_file_enumerate_children (dir, G_FILE_ATTRIBUTE_STANDARD_NAME, 0, NULL, NULL); if (direnum) { @@ -802,43 +810,51 @@ _get_config_dir_files (const char *config_main_file, } g_object_unref (dir); - if (confs->len > 0) { - g_ptr_array_sort (confs, sort_asciibetically); - g_string_append (config_description, " and conf.d: "); - for (i = 0; i < confs->len; i++) { - char *n = confs->pdata[i]; - - if (i > 0) - g_string_append (config_description, ", "); - g_string_append (config_description, n); - confs->pdata[i] = g_build_filename (config_dir, n, NULL); - g_free (n); - } - } - - *out_config_description = g_string_free (config_description, FALSE); + g_ptr_array_sort (confs, sort_asciibetically); return confs; } static GKeyFile * read_entire_config (const NMConfigCmdLineOptions *cli, const char *config_dir, + const char *system_config_dir, char **out_config_main_file, char **out_config_description, GError **error) { GKeyFile *keyfile = nm_config_create_keyfile (); - GPtrArray *confs; + gs_unref_ptrarray GPtrArray *system_confs = NULL; + gs_unref_ptrarray GPtrArray *confs = NULL; guint i; - char *o_config_main_file = NULL; - char *o_config_description = NULL; + gs_free char *o_config_main_file = NULL; char **plugins_tmp; + GString *str; g_return_val_if_fail (config_dir, NULL); + g_return_val_if_fail (system_config_dir, NULL); g_return_val_if_fail (out_config_main_file && !*out_config_main_file, FALSE); g_return_val_if_fail (out_config_description && !*out_config_description, NULL); g_return_val_if_fail (!error || !*error, FALSE); + system_confs = _get_config_dir_files (system_config_dir); + confs = _get_config_dir_files (config_dir); + + for (i = 0; i < system_confs->len; ) { + const char *filename = system_confs->pdata[i]; + + /* if a same named file exists in config_dir, skip it. */ + if (_nm_utils_strv_find_first ((char **) confs->pdata, confs->len, filename) >= 0) { + g_ptr_array_remove_index (system_confs, i); + continue; + } + + if (!read_config (keyfile, system_config_dir, filename, error)) { + g_key_file_free (keyfile); + return NULL; + } + i++; + } + /* First read the base config file */ if (!read_base_config (keyfile, cli ? cli->config_main_file : NULL, &o_config_main_file, error)) { g_key_file_free (keyfile); @@ -847,17 +863,12 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_assert (o_config_main_file); - confs = _get_config_dir_files (o_config_main_file, config_dir, &o_config_description); for (i = 0; i < confs->len; i++) { - if (!read_config (keyfile, confs->pdata[i], error)) { + if (!read_config (keyfile, config_dir, confs->pdata[i], error)) { g_key_file_free (keyfile); - g_free (o_config_main_file); - g_free (o_config_description); - g_ptr_array_unref (confs); return NULL; } } - g_ptr_array_unref (confs); /* Merge settings from command line. They overwrite everything read from * config files. */ @@ -881,8 +892,32 @@ read_entire_config (const NMConfigCmdLineOptions *cli, if (cli && cli->connectivity_response && cli->connectivity_response[0]) g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); + str = g_string_new (o_config_main_file); + if (system_confs->len > 0) { + for (i = 0; i < system_confs->len; i++) { + if (i == 0) + g_string_append (str, " (lib: "); + else + g_string_append (str, ", "); + g_string_append (str, system_confs->pdata[i]); + } + g_string_append (str, ")"); + } + if (confs->len > 0) { + for (i = 0; i < confs->len; i++) { + if (i == 0) + g_string_append (str, " (etc: "); + else + g_string_append (str, ", "); + g_string_append (str, confs->pdata[i]); + } + g_string_append (str, ")"); + } + *out_config_main_file = o_config_main_file; - *out_config_description = o_config_description; + *out_config_description = g_string_free (str, FALSE); + + o_config_main_file = NULL; return keyfile; } @@ -928,6 +963,7 @@ nm_config_reload (NMConfig *self, int signal) */ keyfile = read_entire_config (&priv->cli, priv->config_dir, + priv->system_config_dir, &config_main_file, &config_description, &error); @@ -1087,8 +1123,21 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) else priv->config_dir = g_strdup (DEFAULT_CONFIG_DIR); + if (priv->cli.system_config_dir) + priv->system_config_dir = g_strdup (priv->cli.system_config_dir); + else + priv->system_config_dir = g_strdup (DEFAULT_SYSTEM_CONFIG_DIR); + + if (strcmp (priv->config_dir, priv->system_config_dir) == 0) { + /* having the same directory twice makes no sense. In that case, clear + * @system_config_dir. */ + g_free (priv->system_config_dir); + priv->system_config_dir = g_strdup (""); + } + keyfile = read_entire_config (&priv->cli, priv->config_dir, + priv->system_config_dir, &config_main_file, &config_description, error); @@ -1156,6 +1205,7 @@ finalize (GObject *gobject) NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (gobject); g_free (priv->config_dir); + g_free (priv->system_config_dir); g_free (priv->no_auto_default_file); g_strfreev (priv->plugins); g_free (priv->dhcp_client); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index ca115fd1ab..bc4944e021 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -33,7 +33,7 @@ #include "nm-test-utils.h" static NMConfig * -setup_config (GError **error, const char *config_file, const char *config_dir, ...) +setup_config (GError **error, const char *config_file, const char *config_dir, const char *system_config_dir, ...) { va_list ap; GPtrArray *args; @@ -53,8 +53,12 @@ setup_config (GError **error, const char *config_file, const char *config_dir, . g_ptr_array_add (args, (char *)config_file); g_ptr_array_add (args, "--config-dir"); g_ptr_array_add (args, (char *)config_dir); + if (system_config_dir) { + g_ptr_array_add (args, "--system-config-dir"); + g_ptr_array_add (args, (char *) system_config_dir); + } - va_start (ap, config_dir); + va_start (ap, system_config_dir); while ((arg = va_arg (ap, char *))) g_ptr_array_add (args, arg); va_end (ap); @@ -97,7 +101,7 @@ test_config_simple (void) gs_unref_object NMDevice *dev51 = nm_test_device_new ("00:00:00:00:00:51"); gs_unref_object NMDevice *dev52 = nm_test_device_new ("00:00:00:00:00:52"); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient"); @@ -176,7 +180,7 @@ test_config_non_existent (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/no-such-file", "/no/such/dir", NULL); + setup_config (&error, SRCDIR "/no-such-file", "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND); g_clear_error (&error); } @@ -186,7 +190,7 @@ test_config_parse_error (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/bad.conf", "/no/such/dir", NULL); + setup_config (&error, SRCDIR "/bad.conf", "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } @@ -197,7 +201,7 @@ test_config_override (void) NMConfig *config; const char **plugins; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", "--plugins", "alpha,beta,gamma,delta", "--connectivity-interval", "12", NULL); @@ -235,7 +239,7 @@ test_config_no_auto_default (void) g_assert_cmpint (nwrote, ==, 18); close (fd); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -257,7 +261,7 @@ test_config_no_auto_default (void) g_object_unref (config); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -285,7 +289,7 @@ test_config_confdir (void) char *value; GSList *specs; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhcpcd"); @@ -391,7 +395,7 @@ test_config_confdir_parse_error (void) GError *error = NULL; /* Using SRCDIR as the conf dir will pick up bad.conf */ - setup_config (&error, SRCDIR "/NetworkManager.conf", SRCDIR, NULL); + setup_config (&error, SRCDIR "/NetworkManager.conf", SRCDIR, "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } From ffa70203fdd6434712ccf7ad42ea76a6abc46a87 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 15:58:11 +0200 Subject: [PATCH 3/8] contrib/rpm: update spec file to install configuration snippets in "/usr/lib/NetworkManager/conf.d" Don't move "10-ibft-plugin.conf", because we need it to be read *after* "NetworkManager.conf". Many users might have an old "NetworkManager.conf" file that contains "plugin=ifcfg-rh". --- contrib/fedora/rpm/NetworkManager.spec | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index c026cef08f..aa091e04f1 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -445,9 +445,9 @@ make install DESTDIR=$RPM_BUILD_ROOT mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d mkdir -p $RPM_BUILD_ROOT%{nmlibdir}/conf.d -%{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d +%{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/ %{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d -%{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d +%{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/ # create a VPN directory %{__mkdir_p} $RPM_BUILD_ROOT%{_sysconfdir}/NetworkManager/VPN @@ -664,15 +664,15 @@ fi %files config-connectivity-fedora %defattr(-,root,root,0755) -%dir %{_sysconfdir}/%{name} -%dir %{_sysconfdir}/%{name}/conf.d -%config(noreplace) %{_sysconfdir}/%{name}/conf.d/20-connectivity-fedora.conf +%dir %{nmlibdir} +%dir %{nmlibdir}/conf.d +%{nmlibdir}/conf.d/20-connectivity-fedora.conf %files config-server %defattr(-,root,root,0755) -%dir %{_sysconfdir}/%{name} -%dir %{_sysconfdir}/%{name}/conf.d -%config(noreplace) %{_sysconfdir}/%{name}/conf.d/00-server.conf +%dir %{nmlibdir} +%dir %{nmlibdir}/conf.d +%{nmlibdir}/conf.d/00-server.conf %if 0%{?with_nmtui} %files tui From 98dd29e4ae79e08e2f6b0e607f3cdb8af564daa1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 00:02:20 +0200 Subject: [PATCH 4/8] config: fix setting default configuration for 'main.plugins' 'main.plugins' is the only configuration options for which we have a default value and which we always want to set. This property has a compile time default and can be set via command line, fix the logic to set the value. The proper way is: - first set it (always) to the compile time default - then read the configuration files, which potentially modify the value. - finally, if set via command line, overwrite it because command line always wins. Also comment-out the setting from our default file in "contrib/fedora/rpm/NetworkManager.conf". We don't really need it to be configured there, as we have a compile time default. Commenting it out makes this clearer to the user. Note that we cannot drop "10-ibft-plugin.conf" snippet from NetworkManager package, because many users might have an old "NetworkManager.conf" file with "plugin=ifcfg-rh". This is a change in behavior if the user has no explicit "plugins=ifcfg-rh" setting but followed by "plugins+=ibft". --- contrib/fedora/rpm/NetworkManager.conf | 2 +- src/nm-config.c | 26 ++++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.conf b/contrib/fedora/rpm/NetworkManager.conf index 9901b03935..0352aa1087 100644 --- a/contrib/fedora/rpm/NetworkManager.conf +++ b/contrib/fedora/rpm/NetworkManager.conf @@ -19,7 +19,7 @@ # the previous one. [main] -plugins=ifcfg-rh,ibft +#plugins=ifcfg-rh,ibft [logging] #level=DEBUG diff --git a/src/nm-config.c b/src/nm-config.c index f98b906fb8..9e49905954 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -822,13 +822,13 @@ read_entire_config (const NMConfigCmdLineOptions *cli, char **out_config_description, GError **error) { - GKeyFile *keyfile = nm_config_create_keyfile (); + GKeyFile *keyfile; gs_unref_ptrarray GPtrArray *system_confs = NULL; gs_unref_ptrarray GPtrArray *confs = NULL; guint i; gs_free char *o_config_main_file = NULL; - char **plugins_tmp; GString *str; + char **plugins_default; g_return_val_if_fail (config_dir, NULL); g_return_val_if_fail (system_config_dir, NULL); @@ -836,6 +836,14 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_return_val_if_fail (out_config_description && !*out_config_description, NULL); g_return_val_if_fail (!error || !*error, FALSE); + /* create a default configuration file. */ + keyfile = nm_config_create_keyfile (); + + plugins_default = g_strsplit (CONFIG_PLUGINS_DEFAULT, ",", -1); + if (plugins_default && plugins_default[0]) + nm_config_keyfile_set_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", (const char *const*) plugins_default, -1); + g_strfreev (plugins_default); + system_confs = _get_config_dir_files (system_config_dir); confs = _get_config_dir_files (config_dir); @@ -872,19 +880,13 @@ read_entire_config (const NMConfigCmdLineOptions *cli, /* Merge settings from command line. They overwrite everything read from * config files. */ - - if (cli && cli->plugins && cli->plugins[0]) + if (cli && cli->plugins) { + /* plugins is a string list. Set the value directly, so the user has to do proper escaping + * on the command line. */ g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", cli->plugins); - plugins_tmp = g_key_file_get_string_list (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NULL, NULL); - if (!plugins_tmp) { - if (STRLEN (CONFIG_PLUGINS_DEFAULT) > 0) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", CONFIG_PLUGINS_DEFAULT); - } else - g_strfreev (plugins_tmp); - + } if (cli && cli->configure_and_quit) g_key_file_set_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", TRUE); - if (cli && cli->connectivity_uri && cli->connectivity_uri[0]) g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); if (cli && cli->connectivity_interval >= 0) From 6f0036151f464921a62d15c03cd9aa0f213e6200 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 19:27:55 +0200 Subject: [PATCH 5/8] settings: enable "ibft" plugin by default together with "ifcfg-rh" Originally, ibft settings were handled by "ifcfg-rh" plugin. Later, we added a separate "ibft" plugin and moved the functionality there. The problem was that users quite possibly had a configuration like [main] plugins=ifcfg-rh in their "NetworkManager.conf". That meant, after upgrade users would no longer have ibft support. We fixed that by installing "/etc/NetworkManager/conf.d/10-ibft-plugin.conf" which was read after the main file and contained: [main] plugins+=ibft We no longer want to install configuration snippets with our core packages to /etc. Avoid the regression by changing the meaning of "ifcfg-rh". By enabling "ifcfg-rh" you now implicitly enable "ibft" plugin as well. This can be turned off via "no-ibft". And you can continue to enable "ibft" plugin alone. --- configure.ac | 5 + contrib/fedora/rpm/NetworkManager.spec | 7 +- contrib/fedora/rpm/build.sh | 3 - man/NetworkManager.conf.xml.in | 9 +- src/settings/nm-settings.c | 145 +++++++++++++++---------- 5 files changed, 100 insertions(+), 69 deletions(-) diff --git a/configure.ac b/configure.ac index b197ef7926..e9b39d01f7 100644 --- a/configure.ac +++ b/configure.ac @@ -126,6 +126,11 @@ test "$enable_ifnet" = "yes" && distro_plugins="$distro_plugins,ifn distro_plugins="${distro_plugins#,}" AC_DEFINE_UNQUOTED(CONFIG_PLUGINS_DEFAULT, "$config_plugins_default", [Default configuration option for main.plugins setting]) +if test "${enable_config_plugin_ibft}" = yes; then + AC_DEFINE(WITH_SETTINGS_PLUGIN_IBFT, 1, [Whether compilation of ibft setting plugin is enabled]) +else + AC_DEFINE(WITH_SETTINGS_PLUGIN_IBFT, 0, [Whether compilation of ibft setting plugin is enabled]) +fi if test "$enable_ifcfg_rh" = "yes"; then DISTRO_NETWORK_SERVICE=network.service diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index aa091e04f1..0786b305a8 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -84,8 +84,7 @@ URL: http://www.gnome.org/projects/NetworkManager/ Source: __SOURCE1__ Source1: NetworkManager.conf Source2: 00-server.conf -Source3: 10-ibft-plugin.conf -Source4: 20-connectivity-fedora.conf +Source3: 20-connectivity-fedora.conf #Patch1: 0001-some.patch @@ -446,8 +445,7 @@ make install DESTDIR=$RPM_BUILD_ROOT mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d mkdir -p $RPM_BUILD_ROOT%{nmlibdir}/conf.d %{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/ -%{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d -%{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/ +%{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{nmlibdir}/conf.d/ # create a VPN directory %{__mkdir_p} $RPM_BUILD_ROOT%{_sysconfdir}/NetworkManager/VPN @@ -547,7 +545,6 @@ fi %dir %{_sysconfdir}/%{name}/conf.d %dir %{nmlibdir} %dir %{nmlibdir}/conf.d -%config %{_sysconfdir}/%{name}/conf.d/10-ibft-plugin.conf %{_mandir}/man1/* %{_mandir}/man5/* %{_mandir}/man8/* diff --git a/contrib/fedora/rpm/build.sh b/contrib/fedora/rpm/build.sh index c1ededb554..8e6539dcb1 100755 --- a/contrib/fedora/rpm/build.sh +++ b/contrib/fedora/rpm/build.sh @@ -73,7 +73,6 @@ SOURCE="$(abs_path "$SOURCE" "$(ls -1 "$GITDIR/NetworkManager-$VERSION"*.tar* 2> [[ -f "$SOURCE" ]] || die "could not find source ${_SOURCE:-$GITDIR/NetworkManager-$VERSION*.tar*} . Did you execute \`make dist\`? Otherwise set \$SOURCE variable" SOURCE_NETWORKMANAGER_CONF="$(abs_path "$SOURCE_NETWORKMANAGER_CONF" "$SCRIPTDIR/NetworkManager.conf")" SOURCE_CONFIG_SERVER="$(abs_path "$SOURCE_CONFIG_SERVER" "$SCRIPTDIR/00-server.conf")" -SOURCE_CONFIG_IBFT_PLUGIN="$(abs_path "$SOURCE_CONFIG_IBFT_PLUGIN" "$SCRIPTDIR/10-ibft-plugin.conf")" SOURCE_CONFIG_CONNECTIVITY_FEDORA="$(abs_path "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$SCRIPTDIR/20-connectivity-fedora.conf")" TEMP="$(mktemp -d "$SCRIPTDIR/NetworkManager.$DATE.XXXXXX")" @@ -88,7 +87,6 @@ LOG "SPECFILE=$SPECFILE" LOG "SOURCE=$SOURCE" LOG "SOURCE_NETWORKMANAGER_CONF=$SOURCE_NETWORKMANAGER_CONF" LOG "SOURCE_CONFIG_SERVER=$SOURCE_CONFIG_SERVER" -LOG "SOURCE_CONFIG_IBFT_PLUGIN=$SOURCE_CONFIG_IBFT_PLUGIN" LOG "SOURCE_CONFIG_CONNECTIVITY_FEDORA=$SOURCE_CONFIG_CONNECTIVITY_FEDORA" LOG "BASEDIR=$TEMP" @@ -102,7 +100,6 @@ mkdir -p "$TEMP/SOURCES/" "$TEMP/SPECS/" || die "error creating SPECS directoy" cp "$SOURCE" "$TEMP/SOURCES/" || die "Could not copy source $SOURCE to $TEMP/SOURCES" cp "$SOURCE_NETWORKMANAGER_CONF" "$TEMP/SOURCES/NetworkManager.conf" || die "Could not copy source $SOURCE_NETWORKMANAGER_CONF to $TEMP/SOURCES" cp "$SOURCE_CONFIG_SERVER" "$TEMP/SOURCES/00-server.conf" || die "Could not copy source $SOURCE_CONFIG_SERVER to $TEMP/SOURCES" -cp "$SOURCE_CONFIG_IBFT_PLUGIN" "$TEMP/SOURCES/10-ibft-plugin.conf" || die "Could not copy source $SOURCE_CONFIG_IBFT_PLUGIN to $TEMP/SOURCES" cp "$SOURCE_CONFIG_CONNECTIVITY_FEDORA" "$TEMP/SOURCES/20-connectivity-fedora.conf" || die "Could not copy source $SOURCE_CONFIG_CONNECTIVITY_FEDORA to $TEMP/SOURCES" write_changelog diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index 0156a655fb..9557d7a9be 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -632,6 +632,9 @@ ipv6.ip6-privacy=1 /etc/sysconfig/network-scripts/ifcfg-* files. It currently supports reading Ethernet, Wi-Fi, InfiniBand, VLAN, Bond, Bridge, and Team connections. + Enabling ifcfg-rh implicitly enables + ibft plugin, if it is available. + This can be disabled by adding no-ibft. @@ -665,12 +668,16 @@ ipv6.ip6-privacy=1 - ibft + ibft, no-ibft This plugin allows to read iBFT configuration (iSCSI Boot Firmware Table). The configuration is read using /sbin/iscsiadm. Users are expected to configure iBFT connections via the firmware interfaces. + If ibft support is available, it is automatically enabled after + ifcfg-rh. This can be disabled by no-ibft. + You can also explicitly specify ibft to load the + plugin without ifcfg-rh or to change the plugin order. diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 9f7b47d9bc..befebc1591 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -764,18 +764,20 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) const char **iter; gboolean keyfile_added = FALSE; gboolean success = TRUE; + gboolean add_ibft = FALSE; + gboolean has_no_ibft; + gssize idx_no_ibft, idx_ibft; + + idx_ibft = _nm_utils_strv_find_first ((char **) plugins, -1, "ibft"); + idx_no_ibft = _nm_utils_strv_find_first ((char **) plugins, -1, "no-ibft"); + has_no_ibft = idx_no_ibft >= 0 && idx_no_ibft > idx_ibft; +#if WITH_SETTINGS_PLUGIN_IBFT + add_ibft = idx_no_ibft < 0 && idx_ibft < 0; +#endif for (iter = plugins; iter && *iter; iter++) { - GModule *plugin; - gs_free char *full_name = NULL; - gs_free char *path = NULL; - const char *pname; + const char *pname = *iter; GObject *obj; - GObject * (*factory_func) (void); - struct stat st; - int errsv; - - pname = *iter; if (!*pname || strchr (pname, '/')) { LOG (LOGL_WARN, "ignore invalid plugin \"%s\"", pname); @@ -787,6 +789,11 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) continue; } + if (!strcmp (pname, "no-ibft")) + continue; + if (has_no_ibft && !strcmp (pname, "ibft")) + continue; + obj = find_plugin (list, pname); if (obj) continue; @@ -800,61 +807,79 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) continue; } - full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); - path = g_module_build_path (NMPLUGINDIR, full_name); +load_plugin: + { + GModule *plugin; + gs_free char *full_name = NULL; + gs_free char *path = NULL; + GObject * (*factory_func) (void); + struct stat st; + int errsv; - if (stat (path, &st) != 0) { - errsv = errno; - LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", pname, path, strerror (errsv)); - continue; - } - if (!S_ISREG (st.st_mode)) { - LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': not a file", pname, path); - continue; - } - if (st.st_uid != 0) { - LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': file must be owned by root", pname, path); - continue; - } - if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) { - LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': invalid file permissions", pname, path); - continue; - } + full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); + path = g_module_build_path (NMPLUGINDIR, full_name); - plugin = g_module_open (path, G_MODULE_BIND_LOCAL); - if (!plugin) { - LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", - pname, path, g_module_error ()); - continue; + if (stat (path, &st) != 0) { + errsv = errno; + LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", pname, path, strerror (errsv)); + goto next; + } + if (!S_ISREG (st.st_mode)) { + LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': not a file", pname, path); + goto next; + } + if (st.st_uid != 0) { + LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': file must be owned by root", pname, path); + goto next; + } + if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) { + LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': invalid file permissions", pname, path); + goto next; + } + + plugin = g_module_open (path, G_MODULE_BIND_LOCAL); + if (!plugin) { + LOG (LOGL_WARN, "Could not load plugin '%s' from file '%s': %s", + pname, path, g_module_error ()); + goto next; + } + + /* errors after this point are fatal, because we loaded the shared library already. */ + + if (!g_module_symbol (plugin, "nm_system_config_factory", (gpointer) (&factory_func))) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Could not find plugin '%s' factory function.", + pname); + success = FALSE; + g_module_close (plugin); + break; + } + + obj = (*factory_func) (); + if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Plugin '%s' returned invalid system config object.", + pname); + success = FALSE; + g_module_close (plugin); + break; + } + + g_module_make_resident (plugin); + g_object_weak_ref (obj, (GWeakNotify) g_module_close, plugin); + g_object_set_data_full (obj, PLUGIN_MODULE_PATH, path, g_free); + path = NULL; + add_plugin (self, NM_SYSTEM_CONFIG_INTERFACE (obj)); + list = g_slist_append (list, obj); } - - /* errors after this point are fatal, because we loaded the shared library already. */ - - if (!g_module_symbol (plugin, "nm_system_config_factory", (gpointer) (&factory_func))) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "Could not find plugin '%s' factory function.", - pname); - success = FALSE; - g_module_close (plugin); - break; +next: + if (add_ibft && !strcmp (pname, "ifcfg-rh")) { + /* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh". + * Enable "ibft" by default after "ifcfg-rh". */ + pname = "ibft"; + add_ibft = FALSE; + goto load_plugin; } - - obj = (*factory_func) (); - if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "Plugin '%s' returned invalid system config object.", - pname); - success = FALSE; - g_module_close (plugin); - break; - } - - g_module_make_resident (plugin); - g_object_weak_ref (obj, (GWeakNotify) g_module_close, plugin); - g_object_set_data_full (obj, PLUGIN_MODULE_PATH, path, g_free); - path = NULL; - add_plugin (self, NM_SYSTEM_CONFIG_INTERFACE (obj)); - list = g_slist_append (list, obj); } /* If keyfile plugin was not among configured plugins, add it as the last one */ From 947fc9a27885c268ea742e33d18f27666fa9a043 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 24 Jun 2015 20:11:42 +0200 Subject: [PATCH 6/8] config: add write support for NMConfig Internal configuration is written as keyfile to NMSTATEDIR"/NetworkManager-intern.conf" Basically, the content of this file is merged with user configuration from "NetworkManager.conf" files. After loading the configuration, NMConfig exposes a merged view of user-provided settings and internal overwrites. All sections/groups named [.intern*] are reserved for internal configuration values. They can be written by API, but are ignored when the user sets them via "NetworkManager.conf". For these internal sections, no conflicts can arise. We can also overwrite individual properties from user configuration. In this case, we store the value we want to set, but also remember the value that the user configuration had, at the time of setting. If on a later reload the user configuration changed, we ignore our internal value -- as we assume that the user modified the value afterwards. We can also hide/delete value from user configuration. This works on a per-setting basis. --- man/NetworkManager.conf.xml.in | 17 +- src/devices/nm-device.c | 2 +- src/nm-config-data.c | 181 ++++++++++++- src/nm-config-data.h | 24 +- src/nm-config.c | 468 +++++++++++++++++++++++++++++++-- src/nm-config.h | 11 + src/tests/config/test-config.c | 22 +- 7 files changed, 677 insertions(+), 48 deletions(-) diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index 9557d7a9be..bf15c9e8b5 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -28,16 +28,19 @@ Copyright 2010 - 2014 Red Hat, Inc. /etc/NetworkManager/NetworkManager.conf, /etc/NetworkManager/conf.d/name.conf, - /usr/lib/NetworkManager/conf.d/name.conf + /usr/lib/NetworkManager/conf.d/name.conf, + /var/lib/NetworkManager/NetworkManager-intern.conf Description - This is a configuration file for NetworkManager. It is used + NetworkManager.conf is the configuration file for NetworkManager. It is used to set up various aspects of NetworkManager's behavior. The - location of the file may be changed through use of the - argument for NetworkManager. + location of the main file and configuration directories may be changed + through use of the , , + , and + argument for NetworkManager, respectively. If a default NetworkManager.conf is provided by your distribution's packages, you should not modify @@ -52,6 +55,12 @@ Copyright 2010 - 2014 Red Hat, Inc. In this case, the file from the etc configuration shadows the file from the system configuration directory. + + NetworkManager can overwrite certain user configuration options via D-Bus or other internal + operations. In this case it writes those changes to /var/lib/NetworkManager/NetworkManager-intern.conf. + This file is not intended to be modified by the user, but it is read last and can shadow + user configuration from NetworkManager.conf. + diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index dcdd29e440..16d4c48c69 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3323,7 +3323,7 @@ ip4_config_merge_and_apply (NMDevice *self, if ( !priv->default_route.v4_configure_first_time && !nm_device_uses_assumed_connection (self) && connection_is_never_default) { - /* If the connection is explicitly configured as never-default, we enforce the (absense of the) + /* If the connection is explicitly configured as never-default, we enforce the (absence of the) * default-route only once. That allows the user to configure a connection as never-default, * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ goto END_ADD_DEFAULT_ROUTE; diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 82c1deab11..c3f279a521 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -48,6 +48,8 @@ typedef struct { char *config_description; GKeyFile *keyfile; + GKeyFile *keyfile_user; + GKeyFile *keyfile_intern; /* A zero-terminated list of pre-processed information from the * [connection] sections. This is to speed up lookup. */ @@ -77,7 +79,8 @@ enum { PROP_0, PROP_CONFIG_MAIN_FILE, PROP_CONFIG_DESCRIPTION, - PROP_KEYFILE, + PROP_KEYFILE_USER, + PROP_KEYFILE_INTERN, PROP_CONNECTIVITY_URI, PROP_CONNECTIVITY_INTERVAL, PROP_CONNECTIVITY_RESPONSE, @@ -92,6 +95,14 @@ G_DEFINE_TYPE (NMConfigData, nm_config_data, G_TYPE_OBJECT) /************************************************************************/ +#define _HAS_PREFIX(str, prefix) \ + ({ \ + const char *_str = (str); \ + g_str_has_prefix ( _str, ""prefix"") && _str[STRLEN(prefix)] != '\0'; \ + }) + +/************************************************************************/ + const char * nm_config_data_get_config_main_file (const NMConfigData *self) { @@ -238,6 +249,40 @@ nm_config_data_get_assume_ipv6ll_only (const NMConfigData *self, NMDevice *devic return nm_device_spec_match_list (device, NM_CONFIG_DATA_GET_PRIVATE (self)->assume_ipv6ll_only); } +GKeyFile * +nm_config_data_clone_keyfile_intern (const NMConfigData *self) +{ + NMConfigDataPrivate *priv; + GKeyFile *keyfile; + + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), FALSE); + + priv = NM_CONFIG_DATA_GET_PRIVATE (self); + + keyfile = nm_config_create_keyfile (); + if (priv->keyfile_intern) + _nm_keyfile_copy (keyfile, priv->keyfile_intern); + return keyfile; +} + +GKeyFile * +_nm_config_data_get_keyfile (const NMConfigData *self) +{ + return NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile; +} + +GKeyFile * +_nm_config_data_get_keyfile_intern (const NMConfigData *self) +{ + return NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile_intern; +} + +GKeyFile * +_nm_config_data_get_keyfile_user (const NMConfigData *self) +{ + return NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile_user; +} + /************************************************************************/ /** @@ -267,13 +312,80 @@ nm_config_data_get_keys (const NMConfigData *self, const char *group) /************************************************************************/ +static GKeyFile * +_merge_keyfiles (GKeyFile *keyfile_user, GKeyFile *keyfile_intern) +{ + gs_strfreev char **groups = NULL; + guint g, k; + GKeyFile *keyfile; + gsize ngroups; + + keyfile = nm_config_create_keyfile (); + if (keyfile_user) + _nm_keyfile_copy (keyfile, keyfile_user); + if (!keyfile_intern) + return keyfile; + + groups = g_key_file_get_groups (keyfile_intern, &ngroups); + if (!groups) + return keyfile; + + /* we must reverse the order of the connection settings so that we + * have lowest priority last. */ + _nm_config_sort_groups (groups, ngroups); + for (g = 0; groups[g]; g++) { + const char *group = groups[g]; + gs_strfreev char **keys = NULL; + gboolean is_intern; + + keys = g_key_file_get_keys (keyfile_intern, group, NULL, NULL); + if (!keys) + continue; + + is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + + for (k = 0; keys[k]; k++) { + const char *key = keys[k]; + gs_free char *value = NULL; + + if (!is_intern && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { + const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_WAS)]; + + if (!g_key_file_has_key (keyfile_intern, group, key_base, NULL)) + g_key_file_remove_key (keyfile, group, key_base, NULL); + continue; + } + if (!is_intern && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) + continue; + + value = g_key_file_get_value (keyfile_intern, group, key, NULL); + g_key_file_set_value (keyfile, group, key, value); + } + } + return keyfile; +} + +/************************************************************************/ + static int _nm_config_data_log_sort (const char **pa, const char **pb, gpointer dummy) { gboolean a_is_connection, b_is_connection; + gboolean a_is_intern, b_is_intern; const char *a = *pa; const char *b = *pb; + /* we sort intern groups to the end. */ + a_is_intern = g_str_has_prefix (a, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + b_is_intern = g_str_has_prefix (b, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + + if (a_is_intern && b_is_intern) + return 0; + if (a_is_intern) + return 1; + if (b_is_intern) + return -1; + /* we sort connection groups before intern groups (to the end). */ a_is_connection = a && g_str_has_prefix (a, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); b_is_connection = b && g_str_has_prefix (b, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION); @@ -479,8 +591,11 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data) priv_old = NM_CONFIG_DATA_GET_PRIVATE (old_data); priv_new = NM_CONFIG_DATA_GET_PRIVATE (new_data); - if (!_nm_keyfile_equals (priv_old->keyfile, priv_new->keyfile, TRUE)) - changes |= NM_CONFIG_CHANGE_VALUES; + if (!_nm_keyfile_equals (priv_old->keyfile_user, priv_new->keyfile_user, TRUE)) + changes |= NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER; + + if (!_nm_keyfile_equals (priv_old->keyfile_intern, priv_new->keyfile_intern, TRUE)) + changes |= NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN; if ( g_strcmp0 (nm_config_data_get_config_main_file (old_data), nm_config_data_get_config_main_file (new_data)) != 0 || g_strcmp0 (nm_config_data_get_config_description (old_data), nm_config_data_get_config_description (new_data)) != 0) @@ -553,10 +668,21 @@ set_property (GObject *object, case PROP_CONFIG_DESCRIPTION: priv->config_description = g_value_dup_string (value); break; - case PROP_KEYFILE: - priv->keyfile = g_value_dup_boxed (value); - if (!priv->keyfile) - priv->keyfile = nm_config_create_keyfile (); + case PROP_KEYFILE_USER: + priv->keyfile_user = g_value_dup_boxed (value); + if ( priv->keyfile_user + && !_nm_keyfile_has_values (priv->keyfile_user)) { + g_key_file_unref (priv->keyfile_user); + priv->keyfile_user = NULL; + } + break; + case PROP_KEYFILE_INTERN: + priv->keyfile_intern = g_value_dup_boxed (value); + if ( priv->keyfile_intern + && !_nm_keyfile_has_values (priv->keyfile_intern)) { + g_key_file_unref (priv->keyfile_intern); + priv->keyfile_intern = NULL; + } break; case PROP_NO_AUTO_DEFAULT: { @@ -620,6 +746,10 @@ finalize (GObject *gobject) } g_key_file_unref (priv->keyfile); + if (priv->keyfile_user) + g_key_file_unref (priv->keyfile_user); + if (priv->keyfile_intern) + g_key_file_unref (priv->keyfile_intern); G_OBJECT_CLASS (nm_config_data_parent_class)->finalize (gobject); } @@ -636,6 +766,8 @@ constructed (GObject *object) NMConfigDataPrivate *priv = NM_CONFIG_DATA_GET_PRIVATE (self); char *interval; + priv->keyfile = _merge_keyfiles (priv->keyfile_user, priv->keyfile_intern); + priv->connection_infos = _get_connection_infos (priv->keyfile); priv->connectivity.uri = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL)); @@ -664,16 +796,32 @@ NMConfigData * nm_config_data_new (const char *config_main_file, const char *config_description, const char *const*no_auto_default, - GKeyFile *keyfile) + GKeyFile *keyfile_user, + GKeyFile *keyfile_intern) { return g_object_new (NM_TYPE_CONFIG_DATA, NM_CONFIG_DATA_CONFIG_MAIN_FILE, config_main_file, NM_CONFIG_DATA_CONFIG_DESCRIPTION, config_description, - NM_CONFIG_DATA_KEYFILE, keyfile, + NM_CONFIG_DATA_KEYFILE_USER, keyfile_user, + NM_CONFIG_DATA_KEYFILE_INTERN, keyfile_intern, NM_CONFIG_DATA_NO_AUTO_DEFAULT, no_auto_default, NULL); } +NMConfigData * +nm_config_data_new_update_keyfile_intern (const NMConfigData *base, GKeyFile *keyfile_intern) +{ + NMConfigDataPrivate *priv = NM_CONFIG_DATA_GET_PRIVATE (base); + + return g_object_new (NM_TYPE_CONFIG_DATA, + NM_CONFIG_DATA_CONFIG_MAIN_FILE, priv->config_main_file, + NM_CONFIG_DATA_CONFIG_DESCRIPTION, priv->config_description, + NM_CONFIG_DATA_KEYFILE_USER, priv->keyfile_user, /* the keyfile is unchanged. It's safe to share it. */ + NM_CONFIG_DATA_KEYFILE_INTERN, keyfile_intern, + NM_CONFIG_DATA_NO_AUTO_DEFAULT, priv->no_auto_default.arr, + NULL); +} + NMConfigData * nm_config_data_new_update_no_auto_default (const NMConfigData *base, const char *const*no_auto_default) @@ -683,7 +831,8 @@ nm_config_data_new_update_no_auto_default (const NMConfigData *base, return g_object_new (NM_TYPE_CONFIG_DATA, NM_CONFIG_DATA_CONFIG_MAIN_FILE, priv->config_main_file, NM_CONFIG_DATA_CONFIG_DESCRIPTION, priv->config_description, - NM_CONFIG_DATA_KEYFILE, priv->keyfile, /* the keyfile is unchanged. It's safe to share it. */ + NM_CONFIG_DATA_KEYFILE_USER, priv->keyfile_user, /* the keyfile is unchanged. It's safe to share it. */ + NM_CONFIG_DATA_KEYFILE_INTERN, priv->keyfile_intern, NM_CONFIG_DATA_NO_AUTO_DEFAULT, no_auto_default, NULL); } @@ -718,8 +867,16 @@ nm_config_data_class_init (NMConfigDataClass *config_class) G_PARAM_STATIC_STRINGS)); g_object_class_install_property - (object_class, PROP_KEYFILE, - g_param_spec_boxed (NM_CONFIG_DATA_KEYFILE, "", "", + (object_class, PROP_KEYFILE_USER, + g_param_spec_boxed (NM_CONFIG_DATA_KEYFILE_USER, "", "", + G_TYPE_KEY_FILE, + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); + + g_object_class_install_property + (object_class, PROP_KEYFILE_INTERN, + g_param_spec_boxed (NM_CONFIG_DATA_KEYFILE_INTERN, "", "", G_TYPE_KEY_FILE, G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | diff --git a/src/nm-config-data.h b/src/nm-config-data.h index bd26b9dfbb..d061583540 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -38,7 +38,8 @@ G_BEGIN_DECLS #define NM_CONFIG_DATA_CONFIG_MAIN_FILE "config-main-file" #define NM_CONFIG_DATA_CONFIG_DESCRIPTION "config-description" -#define NM_CONFIG_DATA_KEYFILE "keyfile" +#define NM_CONFIG_DATA_KEYFILE_USER "keyfile-user" +#define NM_CONFIG_DATA_KEYFILE_INTERN "keyfile-intern" #define NM_CONFIG_DATA_CONNECTIVITY_URI "connectivity-uri" #define NM_CONFIG_DATA_CONNECTIVITY_INTERVAL "connectivity-interval" #define NM_CONFIG_DATA_CONNECTIVITY_RESPONSE "connectivity-response" @@ -71,10 +72,12 @@ typedef enum { /*< flags >*/ NM_CONFIG_CHANGE_CONFIG_FILES = (1L << 3), NM_CONFIG_CHANGE_VALUES = (1L << 4), - NM_CONFIG_CHANGE_CONNECTIVITY = (1L << 5), - NM_CONFIG_CHANGE_NO_AUTO_DEFAULT = (1L << 6), - NM_CONFIG_CHANGE_DNS_MODE = (1L << 7), - NM_CONFIG_CHANGE_RC_MANAGER = (1L << 8), + NM_CONFIG_CHANGE_VALUES_USER = (1L << 5), + NM_CONFIG_CHANGE_VALUES_INTERN = (1L << 6), + NM_CONFIG_CHANGE_CONNECTIVITY = (1L << 7), + NM_CONFIG_CHANGE_NO_AUTO_DEFAULT = (1L << 8), + NM_CONFIG_CHANGE_DNS_MODE = (1L << 9), + NM_CONFIG_CHANGE_RC_MANAGER = (1L << 10), _NM_CONFIG_CHANGE_LAST, NM_CONFIG_CHANGE_ALL = ((_NM_CONFIG_CHANGE_LAST - 1) << 1) - 1, @@ -93,7 +96,9 @@ GType nm_config_data_get_type (void); NMConfigData *nm_config_data_new (const char *config_main_file, const char *config_description, const char *const*no_auto_default, - GKeyFile *keyfile); + GKeyFile *keyfile_user, + GKeyFile *keyfile_intern); +NMConfigData *nm_config_data_new_update_keyfile_intern (const NMConfigData *base, GKeyFile *keyfile_intern); NMConfigData *nm_config_data_new_update_no_auto_default (const NMConfigData *base, const char *const*no_auto_default); NMConfigChangeFlags nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data); @@ -128,6 +133,13 @@ char *nm_config_data_get_connection_default (const NMConfigData *self, char **nm_config_data_get_groups (const NMConfigData *self); char **nm_config_data_get_keys (const NMConfigData *self, const char *group); +GKeyFile *nm_config_data_clone_keyfile_intern (const NMConfigData *self); + +/* private accessors */ +GKeyFile *_nm_config_data_get_keyfile (const NMConfigData *self); +GKeyFile *_nm_config_data_get_keyfile_user (const NMConfigData *self); +GKeyFile *_nm_config_data_get_keyfile_intern (const NMConfigData *self); + G_END_DECLS #endif /* NM_CONFIG_DATA_H */ diff --git a/src/nm-config.c b/src/nm-config.c index 9e49905954..0c36f7a773 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -33,6 +33,7 @@ #include "gsystem-local-alloc.h" #include "nm-enum-types.h" #include "nm-core-internal.h" +#include "nm-keyfile-internal.h" #include #include @@ -42,9 +43,11 @@ #define DEFAULT_CONFIG_MAIN_FILE_OLD NMCONFDIR "/nm-system-settings.conf" #define DEFAULT_SYSTEM_CONFIG_DIR NMLIBDIR "/conf.d" #define DEFAULT_NO_AUTO_DEFAULT_FILE NMSTATEDIR "/no-auto-default.state" +#define DEFAULT_INTERN_CONFIG_FILE NMSTATEDIR "/NetworkManager-intern.conf" struct NMConfigCmdLineOptions { char *config_main_file; + char *intern_config_file; char *config_dir; char *system_config_dir; char *no_auto_default_file; @@ -68,6 +71,7 @@ typedef struct { char *config_dir; char *system_config_dir; char *no_auto_default_file; + char *intern_config_file; char **plugins; gboolean monitor_connection_files; @@ -111,6 +115,14 @@ static void _set_config_data (NMConfig *self, NMConfigData *new_data, int signal /************************************************************************/ +#define _HAS_PREFIX(str, prefix) \ + ({ \ + const char *_str = (str); \ + g_str_has_prefix ( _str, ""prefix"") && _str[STRLEN(prefix)] != '\0'; \ + }) + +/************************************************************************/ + gint nm_config_parse_boolean (const char *str, gint default_value) @@ -362,7 +374,7 @@ nm_config_get_no_auto_default_for_device (NMConfig *self, NMDevice *device) void nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device) { - NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (self); + NMConfigPrivate *priv; GError *error = NULL; NMConfigData *new_data = NULL; const char *hw_address; @@ -373,6 +385,8 @@ nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device) g_return_if_fail (NM_IS_CONFIG (self)); g_return_if_fail (NM_IS_DEVICE (device)); + priv = NM_CONFIG_GET_PRIVATE (self); + hw_address = nm_device_get_hw_address (device); no_auto_default_current = nm_config_data_get_no_auto_default (priv->config_data); @@ -412,6 +426,7 @@ _nm_config_cmd_line_options_clear (NMConfigCmdLineOptions *cli) g_clear_pointer (&cli->config_dir, g_free); g_clear_pointer (&cli->system_config_dir, g_free); g_clear_pointer (&cli->no_auto_default_file, g_free); + g_clear_pointer (&cli->intern_config_file, g_free); g_clear_pointer (&cli->plugins, g_free); cli->configure_and_quit = FALSE; g_clear_pointer (&cli->connectivity_uri, g_free); @@ -431,6 +446,7 @@ _nm_config_cmd_line_options_copy (const NMConfigCmdLineOptions *cli, NMConfigCmd dst->system_config_dir = g_strdup (cli->system_config_dir); dst->config_main_file = g_strdup (cli->config_main_file); dst->no_auto_default_file = g_strdup (cli->no_auto_default_file); + dst->intern_config_file = g_strdup (cli->intern_config_file); dst->plugins = g_strdup (cli->plugins); dst->configure_and_quit = cli->configure_and_quit; dst->connectivity_uri = g_strdup (cli->connectivity_uri); @@ -468,6 +484,7 @@ nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli, { "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(DEFAULT_CONFIG_MAIN_FILE) }, { "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(DEFAULT_CONFIG_DIR) }, { "system-config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->system_config_dir, N_("System config directory location"), N_(DEFAULT_SYSTEM_CONFIG_DIR) }, + { "intern-config", 0, 0, G_OPTION_ARG_FILENAME, &cli->intern_config_file, N_("Internal config file location"), N_(DEFAULT_INTERN_CONFIG_FILE) }, { "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(DEFAULT_NO_AUTO_DEFAULT_FILE) }, { "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_(CONFIG_PLUGINS_DEFAULT) }, { "configure-and-quit", 0, 0, G_OPTION_ARG_NONE, &cli->configure_and_quit, N_("Quit after initial configuration"), NULL }, @@ -534,6 +551,18 @@ _sort_groups_cmp (const char **pa, const char **pb, gpointer dummy) return pa > pb ? -1 : 1; } +void +_nm_config_sort_groups (char **groups, gsize ngroups) +{ + if (ngroups > 1) { + g_qsort_with_data (groups, + ngroups, + sizeof (char *), + (GCompareDataFunc) _sort_groups_cmp, + NULL); + } +} + static gboolean _setting_is_device_spec (const char *group, const char *key) { @@ -599,17 +628,15 @@ read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError ** * At the very end, we will revert the order of all sections again and * get thus the right behavior. This final reversing is done in * NMConfigData:_get_connection_infos(). */ - if (ngroups > 1) { - g_qsort_with_data (groups, - ngroups, - sizeof (char *), - (GCompareDataFunc) _sort_groups_cmp, - NULL); - } + _nm_config_sort_groups (groups, ngroups); for (g = 0; groups[g]; g++) { const char *group = groups[g]; + if (g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN)) { + /* internal groups cannot be set by user configuration. */ + continue; + } keys = g_key_file_get_keys (kf, group, &nkeys, NULL); if (!keys) continue; @@ -621,6 +648,13 @@ read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError ** key = keys[k]; g_assert (key && *key); + + if ( _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS) + || _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) { + /* these keys are protected. We ignore them if the user sets them. */ + continue; + } + key_len = strlen (key); last_char = key[key_len - 1]; if ( key_len > 1 @@ -832,8 +866,8 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_return_val_if_fail (config_dir, NULL); g_return_val_if_fail (system_config_dir, NULL); - g_return_val_if_fail (out_config_main_file && !*out_config_main_file, FALSE); - g_return_val_if_fail (out_config_description && !*out_config_description, NULL); + g_return_val_if_fail (!out_config_main_file || !*out_config_main_file, FALSE); + g_return_val_if_fail (!out_config_description || !*out_config_description, NULL); g_return_val_if_fail (!error || !*error, FALSE); /* create a default configuration file. */ @@ -916,13 +950,310 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_string_append (str, ")"); } - *out_config_main_file = o_config_main_file; - *out_config_description = g_string_free (str, FALSE); + if (out_config_main_file) + *out_config_main_file = o_config_main_file; + else + g_free (o_config_main_file); + if (out_config_description) + *out_config_description = g_string_free (str, FALSE); + else + g_string_free (str, TRUE); o_config_main_file = NULL; return keyfile; } +/** + * intern_config_read: + * @filename: the filename where to store the internal config + * @keyfile_conf: the merged configuration from user (/etc/NM/NetworkManager.conf). + * @out_needs_rewrite: (allow-none): whether the read keyfile contains inconsistent + * data (compared to @keyfile_conf). If %TRUE, you might want to rewrite + * the file. + * + * Does the opposite of intern_config_write(). It reads the internal configuration. + * Note that the actual format of how the configuration is saved in @filename + * is different then what we return here. NMConfig manages what is written internally + * by having it inside a keyfile_intern. But we don't write that to disk as is. + * Especially, we also store parts of @keyfile_conf as ".was" and on read we compare + * what we have, with what ".was". + * + * Returns: a #GKeyFile instance with the internal configuration. + */ +static GKeyFile * +intern_config_read (const char *filename, + GKeyFile *keyfile_conf, + gboolean *out_needs_rewrite) +{ + GKeyFile *keyfile_intern; + GKeyFile *keyfile; + gboolean needs_rewrite = FALSE; + gs_strfreev char **groups = NULL; + guint g, k; + gboolean has_intern = FALSE; + + g_return_val_if_fail (filename, NULL); + + if (!*filename) { + if (out_needs_rewrite) + *out_needs_rewrite = FALSE; + return NULL; + } + + keyfile_intern = nm_config_create_keyfile (); + + keyfile = nm_config_create_keyfile (); + if (!g_key_file_load_from_file (keyfile, filename, G_KEY_FILE_NONE, NULL)) { + needs_rewrite = TRUE; + goto out; + } + + groups = g_key_file_get_groups (keyfile, NULL); + for (g = 0; groups && groups[g]; g++) { + gs_strfreev char **keys = NULL; + const char *group = groups[g]; + gboolean is_intern; + + keys = g_key_file_get_keys (keyfile, group, NULL, NULL); + if (!keys) + continue; + + is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + + for (k = 0; keys[k]; k++) { + gs_free char *value_set = NULL; + const char *key = keys[k]; + + value_set = g_key_file_get_value (keyfile, group, key, NULL); + + if (is_intern) { + has_intern = TRUE; + g_key_file_set_value (keyfile_intern, group, key, value_set); + } else if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) { + const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_SET)]; + gs_free char *value_was = NULL; + gs_free char *value_conf = NULL; + gs_free char *key_was = g_strdup_printf (NM_CONFIG_KEYFILE_KEYPREFIX_WAS"%s", key_base); + + if (keyfile_conf) + value_conf = g_key_file_get_value (keyfile_conf, group, key_base, NULL); + value_was = g_key_file_get_value (keyfile, group, key_was, NULL); + + if (g_strcmp0 (value_conf, value_was) != 0) { + /* if value_was is no longer the same as @value_conf, it means the user + * changed the configuration since the last write. In this case, we + * drop the value. It also means our file is out-of-date, and we should + * rewrite it. */ + needs_rewrite = TRUE; + continue; + } + has_intern = TRUE; + g_key_file_set_value (keyfile_intern, group, key_base, value_set); + } else if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { + const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_WAS)]; + gs_free char *key_set = g_strdup_printf (NM_CONFIG_KEYFILE_KEYPREFIX_SET"%s", key_base); + gs_free char *value_was = NULL; + gs_free char *value_conf = NULL; + + if (g_key_file_has_key (keyfile, group, key_set, NULL)) { + /* we have a matching "set" key too. Handle the "was" key there. */ + continue; + } + + if (keyfile_conf) + value_conf = g_key_file_get_value (keyfile_conf, group, key_base, NULL); + value_was = g_key_file_get_value (keyfile, group, key, NULL); + + if (g_strcmp0 (value_conf, value_was) != 0) { + /* if value_was is no longer the same as @value_conf, it means the user + * changed the configuration since the last write. In this case, we + * don't overwrite the user-provided value. It also means our file is + * out-of-date, and we should rewrite it. */ + needs_rewrite = TRUE; + continue; + } + has_intern = TRUE; + /* signal the absence of the value. That means, we must propagate the + * "was" key to NMConfigData, so that it knows to hide the corresponding + * user key. */ + g_key_file_set_value (keyfile_intern, group, key, ""); + } else + needs_rewrite = TRUE; + } + } + +out: + g_key_file_unref (keyfile); + + if (out_needs_rewrite) + *out_needs_rewrite = needs_rewrite; + + nm_log_dbg (LOGD_CORE, "intern config file \"%s\"", filename); + + if (!has_intern) { + g_key_file_unref (keyfile_intern); + return NULL; + } + return keyfile_intern; +} + +static int +_intern_config_write_sort_fcn (const char **a, const char **b, gpointer dummy) +{ + const char *g_a = (a ? *a : NULL); + const char *g_b = (b ? *b : NULL); + gboolean a_is, b_is; + + a_is = g_str_has_prefix (g_a, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + b_is = g_str_has_prefix (g_b, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + + if (a_is != b_is) { + if (a_is) + return 1; + return -1; + } + return g_strcmp0 (g_a, g_b); +} + +static gboolean +intern_config_write (const char *filename, + GKeyFile *keyfile_intern, + GKeyFile *keyfile_conf, + GError **error) +{ + GKeyFile *keyfile; + gs_strfreev char **groups = NULL; + guint g, k; + gboolean has_intern = FALSE; + gboolean success = FALSE; + GError *local = NULL; + + g_return_val_if_fail (filename, FALSE); + + if (!*filename) { + g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND, "no filename to write (use --intern-config?)"); + return FALSE; + } + + keyfile = nm_config_create_keyfile (); + + if (keyfile_intern) { + groups = g_key_file_get_groups (keyfile_intern, NULL); + if (groups && groups[0]) { + g_qsort_with_data (groups, + g_strv_length (groups), + sizeof (char *), + (GCompareDataFunc) _intern_config_write_sort_fcn, + NULL); + } + } + for (g = 0; groups && groups[g]; g++) { + gs_strfreev char **keys = NULL; + const char *group = groups[g]; + gboolean is_intern; + + keys = g_key_file_get_keys (keyfile_intern, group, NULL, NULL); + if (!keys) + continue; + + is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + + for (k = 0; keys[k]; k++) { + const char *key = keys[k]; + gs_free char *value_set = NULL; + gs_free char *key_set = NULL; + + value_set = g_key_file_get_value (keyfile_intern, group, key, NULL); + + if (is_intern) { + has_intern = TRUE; + g_key_file_set_value (keyfile, group, key, value_set); + } else { + gs_free char *value_was = NULL; + + if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) { + /* Setting a key with .set prefix has no meaning, as these keys + * are protected. Just set the value you want to set instead. + * Why did this happen?? */ + g_warn_if_reached (); + } else if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { + const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_WAS)]; + + if ( _HAS_PREFIX (key_base, NM_CONFIG_KEYFILE_KEYPREFIX_SET) + || _HAS_PREFIX (key_base, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { + g_warn_if_reached (); + continue; + } + + if (g_key_file_has_key (keyfile_intern, group, key_base, NULL)) { + /* There is also a matching key_base entry. Skip processing + * the .was. key ad handle the key_base in the other else branch. */ + continue; + } + + if (keyfile_conf) { + value_was = g_key_file_get_value (keyfile_conf, group, key_base, NULL); + if (value_was) + g_key_file_set_value (keyfile, group, key, value_was); + } + } else { + if (keyfile_conf) { + value_was = g_key_file_get_value (keyfile_conf, group, key, NULL); + if (g_strcmp0 (value_set, value_was) == 0) { + /* there is no point in storing the identical value as we have via + * user configuration. Skip it. */ + continue; + } + if (value_was) { + gs_free char *key_was = NULL; + + key_was = g_strdup_printf (NM_CONFIG_KEYFILE_KEYPREFIX_WAS"%s", key); + g_key_file_set_value (keyfile, group, key_was, value_was); + } + } + key = key_set = g_strdup_printf (NM_CONFIG_KEYFILE_KEYPREFIX_SET"%s", key); + g_key_file_set_value (keyfile, group, key, value_set); + } + } + } + if ( is_intern + && g_key_file_has_group (keyfile, group)) { + g_key_file_set_comment (keyfile, group, NULL, + " Internal section. Not overwritable via user configuration in 'NetworkManager.conf'", + NULL); + } + } + + g_key_file_set_comment (keyfile, NULL, NULL, + " Internal configuration file. This file is written and read\n" + " by NetworkManager and its configuration values are merged\n" + " with the configuration from 'NetworkManager.conf'.\n" + "\n" + " Keys with a \""NM_CONFIG_KEYFILE_KEYPREFIX_SET"\" prefix specify the value to set.\n" + " A corresponding key with a \""NM_CONFIG_KEYFILE_KEYPREFIX_WAS"\" prefix records the value\n" + " of the user configuration at the time of storing the file.\n" + " The value from internal configuration is rejected if the corresponding\n" + " \""NM_CONFIG_KEYFILE_KEYPREFIX_WAS"\" key no longer matches the configuration from 'NetworkManager.conf'.\n" + " That means, if you modify a value in 'NetworkManager.conf', the internal\n" + " overwrite no longer matches and is ignored.\n" + "\n" + " Internal sections of the form [" NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN "*] cannot\n" + " be set by user configuration.\n" + "\n" + " CHANGES TO THIS FILE WILL BE OVERWRITTEN", + NULL); + + success = g_key_file_save_to_file (keyfile, filename, &local); + + nm_log_dbg (LOGD_CORE, "write intern config file \"%s\"%s%s", filename, success ? "" : ": ", success ? "" : local->message); + g_key_file_unref (keyfile); + if (!success) + g_propagate_error (error, local); + return success; +} + +/************************************************************************/ + GSList * nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key) { @@ -939,16 +1270,97 @@ nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, con /************************************************************************/ +/** + * nm_config_set_values: + * @self: the NMConfig instance + * @keyfile_intern_new: (allow-none): the new internal settings to set. + * If %NULL, it is equal to an empty keyfile. + * @allow_write: only if %TRUE, allow writing the changes to file. Otherwise, + * do the changes in-memory only. + * @force_rewrite: if @allow_write is %FALSE, this has no effect. If %FALSE, + * only write the configuration to file, if there are any actual changes. + * If %TRUE, always write the configuration to file, even if tere are seemingly + * no changes. + * + * This is the most flexible function to set values. It all depends on the + * keys and values you set in @keyfile_intern_new. You basically reset all + * internal configuration values to what is in @keyfile_intern_new. + * + * There are 2 types of settings: + * - all groups/sections with a prefix [.intern.*] are taken as is. As these + * groups are separate from user configuration, there is no conflict. You set + * them, that's it. + * - otherwise you can overwrite individual values from user-configuration. + * Just set the value. Keys with a prefix NM_CONFIG_KEYFILE_KEYPREFIX_* + * are protected -- as they are not value user keys. + * You can also hide a certain user setting by putting only a key + * NM_CONFIG_KEYFILE_KEYPREFIX_WAS"keyname" into the keyfile. + */ +void +nm_config_set_values (NMConfig *self, + GKeyFile *keyfile_intern_new, + gboolean allow_write, + gboolean force_rewrite) +{ + NMConfigPrivate *priv; + GKeyFile *keyfile_intern_current; + GKeyFile *keyfile_user; + GKeyFile *keyfile_new; + GError *local = NULL; + NMConfigData *new_data = NULL; + + g_return_if_fail (NM_IS_CONFIG (self)); + + priv = NM_CONFIG_GET_PRIVATE (self); + + keyfile_intern_current = _nm_config_data_get_keyfile_intern (priv->config_data); + + keyfile_new = nm_config_create_keyfile (); + if (keyfile_intern_new) + _nm_keyfile_copy (keyfile_new, keyfile_intern_new); + + if (!_nm_keyfile_equals (keyfile_intern_current, keyfile_new, TRUE)) + new_data = nm_config_data_new_update_keyfile_intern (priv->config_data, keyfile_new); + + nm_log_dbg (LOGD_CORE, "set values(): %s", new_data ? "has changes" : "no changes"); + + if (allow_write + && (new_data || force_rewrite)) { + /* We write the internal config file based on the user configuration from + * the last load/reload. That is correct, because the intern properties might + * be in accordance to what NM thinks is currently configured. Even if the files + * on disk changed in the meantime. + * But if they changed, on the next reload with might throw away our just + * written data. That is correct, because from NM's point of view, those + * changes on disk happened in any case *after* now. */ + if (*priv->intern_config_file) { + keyfile_user = _nm_config_data_get_keyfile_user (priv->config_data); + if (!intern_config_write (priv->intern_config_file, keyfile_new, keyfile_user, &local)) { + nm_log_warn (LOGD_CORE, "error saving internal configuration \"%s\": %s", priv->intern_config_file, local->message); + g_clear_error (&local); + } + } else + nm_log_dbg (LOGD_CORE, "don't persistate internal configuration (no file set, use --intern-config?)"); + } + if (new_data) + _set_config_data (self, new_data, 0); + + g_key_file_unref (keyfile_new); +} + +/************************************************************************/ + void nm_config_reload (NMConfig *self, int signal) { NMConfigPrivate *priv; GError *error = NULL; - GKeyFile *keyfile; + 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; g_return_if_fail (NM_IS_CONFIG (self)); @@ -975,12 +1387,19 @@ nm_config_reload (NMConfig *self, int signal) _set_config_data (self, NULL, signal); return; } + no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); - new_data = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile); + keyfile_intern = intern_config_read (priv->intern_config_file, keyfile, &intern_config_needs_rewrite); + if (intern_config_needs_rewrite) + intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, NULL); + + new_data = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile, keyfile_intern); g_free (config_main_file); g_free (config_description); g_key_file_unref (keyfile); + if (keyfile_intern) + g_key_file_unref (keyfile_intern); _set_config_data (self, new_data, signal); } @@ -999,6 +1418,10 @@ _change_flags_one_to_string (NMConfigChangeFlags flag) return "config-files"; case NM_CONFIG_CHANGE_VALUES: return "values"; + case NM_CONFIG_CHANGE_VALUES_USER: + return "values-user"; + case NM_CONFIG_CHANGE_VALUES_INTERN: + return "values-intern"; case NM_CONFIG_CHANGE_CONNECTIVITY: return "connectivity"; case NM_CONFIG_CHANGE_NO_AUTO_DEFAULT: @@ -1107,10 +1530,11 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) { NMConfig *self = NM_CONFIG (initable); NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (self); - GKeyFile *keyfile; + GKeyFile *keyfile, *keyfile_intern; char *config_main_file = NULL; char *config_description = NULL; gs_strfreev char **no_auto_default = NULL; + gboolean intern_config_needs_rewrite; if (priv->config_dir) { /* Object is already initialized. */ @@ -1137,6 +1561,11 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->system_config_dir = g_strdup (""); } + if (priv->cli.intern_config_file) + priv->intern_config_file = g_strdup (priv->cli.intern_config_file); + else + priv->intern_config_file = g_strdup (DEFAULT_INTERN_CONFIG_FILE); + keyfile = read_entire_config (&priv->cli, priv->config_dir, priv->system_config_dir, @@ -1173,13 +1602,19 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); - priv->config_data_orig = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile); + keyfile_intern = intern_config_read (priv->intern_config_file, keyfile, &intern_config_needs_rewrite); + if (intern_config_needs_rewrite) + intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, NULL); + + priv->config_data_orig = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile, keyfile_intern); priv->config_data = g_object_ref (priv->config_data_orig); g_free (config_main_file); g_free (config_description); g_key_file_unref (keyfile); + if (keyfile_intern) + g_key_file_unref (keyfile_intern); return TRUE; } @@ -1209,6 +1644,7 @@ finalize (GObject *gobject) g_free (priv->config_dir); g_free (priv->system_config_dir); g_free (priv->no_auto_default_file); + g_free (priv->intern_config_file); g_strfreev (priv->plugins); g_free (priv->dhcp_client); g_free (priv->log_level); diff --git a/src/nm-config.h b/src/nm-config.h index 5b096f165d..d4ff887ba3 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -48,6 +48,7 @@ G_BEGIN_DECLS #define NM_CONFIG_KEYFILE_LIST_SEPARATOR ',' +#define NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN ".intern." #define NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION "connection" #define NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST ".test-append-stringlist" @@ -63,6 +64,9 @@ G_BEGIN_DECLS #define NM_CONFIG_KEYFILE_KEY_IFNET_MANAGED "managed" #define NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED "managed" +#define NM_CONFIG_KEYFILE_KEYPREFIX_WAS ".was." +#define NM_CONFIG_KEYFILE_KEYPREFIX_SET ".set." + typedef struct NMConfigCmdLineOptions NMConfigCmdLineOptions; struct _NMConfig { @@ -97,6 +101,11 @@ const char *nm_config_get_log_domains (NMConfig *config); const char *nm_config_get_debug (NMConfig *config); gboolean nm_config_get_configure_and_quit (NMConfig *config); +void nm_config_set_values (NMConfig *self, + GKeyFile *keyfile_intern_new, + gboolean allow_write, + gboolean force_rewrite); + /* for main.c only */ NMConfigCmdLineOptions *nm_config_cmd_line_options_new (void); void nm_config_cmd_line_options_free (NMConfigCmdLineOptions *cli); @@ -128,6 +137,8 @@ void nm_config_keyfile_set_string_list (GKeyFile *keyfile, gssize len); GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key); +void _nm_config_sort_groups (char **groups, gsize ngroups); + G_END_DECLS #endif /* __NETWORKMANAGER_CONFIG_H__ */ diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index bc4944e021..82fb73dad2 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -33,7 +33,7 @@ #include "nm-test-utils.h" static NMConfig * -setup_config (GError **error, const char *config_file, const char *config_dir, const char *system_config_dir, ...) +setup_config (GError **error, const char *config_file, const char *intern_config, const char *config_dir, const char *system_config_dir, ...) { va_list ap; GPtrArray *args; @@ -51,6 +51,10 @@ setup_config (GError **error, const char *config_file, const char *config_dir, c g_ptr_array_add (args, "test-config"); g_ptr_array_add (args, "--config"); g_ptr_array_add (args, (char *)config_file); + if (intern_config) { + g_ptr_array_add (args, "--intern-config"); + g_ptr_array_add (args, (char *)intern_config); + } g_ptr_array_add (args, "--config-dir"); g_ptr_array_add (args, (char *)config_dir); if (system_config_dir) { @@ -101,7 +105,7 @@ test_config_simple (void) gs_unref_object NMDevice *dev51 = nm_test_device_new ("00:00:00:00:00:51"); gs_unref_object NMDevice *dev52 = nm_test_device_new ("00:00:00:00:00:52"); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient"); @@ -180,7 +184,7 @@ test_config_non_existent (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/no-such-file", "/no/such/dir", "", NULL); + setup_config (&error, SRCDIR "/no-such-file", "", "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND); g_clear_error (&error); } @@ -190,7 +194,7 @@ test_config_parse_error (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/bad.conf", "/no/such/dir", "", NULL); + setup_config (&error, SRCDIR "/bad.conf", "", "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } @@ -201,7 +205,7 @@ test_config_override (void) NMConfig *config; const char **plugins; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", "--plugins", "alpha,beta,gamma,delta", "--connectivity-interval", "12", NULL); @@ -239,7 +243,7 @@ test_config_no_auto_default (void) g_assert_cmpint (nwrote, ==, 18); close (fd); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -261,7 +265,7 @@ test_config_no_auto_default (void) g_object_unref (config); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -289,7 +293,7 @@ test_config_confdir (void) char *value; GSList *specs; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", "", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", SRCDIR "/conf.d", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhcpcd"); @@ -395,7 +399,7 @@ test_config_confdir_parse_error (void) GError *error = NULL; /* Using SRCDIR as the conf dir will pick up bad.conf */ - setup_config (&error, SRCDIR "/NetworkManager.conf", SRCDIR, "", NULL); + setup_config (&error, SRCDIR "/NetworkManager.conf", "", SRCDIR, "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } From 40c57fa7f197ad9456b1359862d6bf1a7c3fd352 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Jun 2015 11:59:46 +0200 Subject: [PATCH 7/8] config: add write support to atomic-sections We already support setting configuration values, either: (1) set any internal section, i.e. groups starting with [.intern*]. Those values don't ever interfere with that the user can configure. (2) set individual properties that overwrite user configuration. When doing that, we record the value from user configuration and on load, we reject our internal overwrite if the user configuration changed in the meantime. This is done by storing the values with ".set." and ".was." prefixes. Now add support for "atomic sections". In this case, certain groups can be marked as "atomic". When writing to such sections, we overwrite the entire user-provided setting. We also record the values from user configuration, and reject our internal value if we notice modifications. This basically extends (2) from individual properties to the entire section. --- src/main.c | 2 +- src/nm-config-data.c | 57 ++++++++- src/nm-config-data.h | 1 + src/nm-config.c | 209 ++++++++++++++++++++++++++++++--- src/nm-config.h | 6 +- src/tests/config/test-config.c | 2 +- 6 files changed, 253 insertions(+), 24 deletions(-) diff --git a/src/main.c b/src/main.c index dc6973446f..4f906f5574 100644 --- a/src/main.c +++ b/src/main.c @@ -343,7 +343,7 @@ main (int argc, char *argv[]) } /* Read the config file and CLI overrides */ - config = nm_config_setup (config_cli, &error); + config = nm_config_setup (config_cli, NULL, &error); nm_config_cmd_line_options_free (config_cli); config_cli = NULL; if (config == NULL) { diff --git a/src/nm-config-data.c b/src/nm-config-data.c index c3f279a521..03cae638c7 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -310,6 +310,42 @@ nm_config_data_get_keys (const NMConfigData *self, const char *group) return g_key_file_get_keys (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, NULL, NULL); } +/** + * nm_config_data_is_intern_atomic_group: + * @self: + * @group: name of the group to check. + * + * whether a configuration group @group exists and is entirely overwritten + * by internal configuration, i.e. whether it is an atomic group that is + * overwritten. + * + * It doesn't say, that there actually is a user setting that was overwritten. That + * means there could be no corresponding section defined in user configuration + * that required overwriting. + * + * Returns: %TRUE if @group exists and is an atomic group set via internal configuration. + */ +gboolean +nm_config_data_is_intern_atomic_group (const NMConfigData *self, const char *group) +{ + NMConfigDataPrivate *priv; + + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), FALSE); + g_return_val_if_fail (group && *group, FALSE); + + priv = NM_CONFIG_DATA_GET_PRIVATE (self); + + if ( !priv->keyfile_intern + || !g_key_file_has_key (priv->keyfile_intern, group, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, NULL)) + return FALSE; + + /* we have a .was entry for the section. That means that the section would be overwritten + * from user configuration. But it doesn't mean that the merged configuration contains this + * groups, because the internal setting could hide the user section. + * Only return TRUE, if we actually have such a group in the merged configuration.*/ + return g_key_file_has_group (priv->keyfile, group); +} + /************************************************************************/ static GKeyFile * @@ -336,26 +372,36 @@ _merge_keyfiles (GKeyFile *keyfile_user, GKeyFile *keyfile_intern) for (g = 0; groups[g]; g++) { const char *group = groups[g]; gs_strfreev char **keys = NULL; - gboolean is_intern; + gboolean is_intern, is_atomic = FALSE; keys = g_key_file_get_keys (keyfile_intern, group, NULL, NULL); if (!keys) continue; is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + if ( !is_intern + && g_key_file_has_key (keyfile_intern, group, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, NULL)) { + /* the entire section is atomically overwritten by @keyfile_intern. */ + g_key_file_remove_group (keyfile, group, NULL); + is_atomic = TRUE; + } for (k = 0; keys[k]; k++) { const char *key = keys[k]; gs_free char *value = NULL; - if (!is_intern && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { + if (is_atomic && strcmp (key, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS) == 0) + continue; + + if ( !is_intern && !is_atomic + && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_WAS)) { const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_WAS)]; if (!g_key_file_has_key (keyfile_intern, group, key_base, NULL)) g_key_file_remove_key (keyfile, group, key_base, NULL); continue; } - if (!is_intern && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) + if (!is_intern && !is_atomic && _HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) continue; value = g_key_file_get_value (keyfile_intern, group, key, NULL); @@ -448,9 +494,12 @@ nm_config_data_log (const NMConfigData *self, const char *prefix) for (g = 0; g < ngroups; g++) { const char *group = groups[g]; gs_strfreev char **keys = NULL; + gboolean is_atomic; + + is_atomic = nm_config_data_is_intern_atomic_group (self, group); _LOG (""); - _LOG ("[%s]", group); + _LOG ("[%s]%s", group, is_atomic ? "*" : ""); keys = g_key_file_get_keys (priv->keyfile, group, NULL, NULL); for (k = 0; keys && keys[k]; k++) { diff --git a/src/nm-config-data.h b/src/nm-config-data.h index d061583540..0dfdf531ec 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -132,6 +132,7 @@ char *nm_config_data_get_connection_default (const NMConfigData *self, char **nm_config_data_get_groups (const NMConfigData *self); char **nm_config_data_get_keys (const NMConfigData *self, const char *group); +gboolean nm_config_data_is_intern_atomic_group (const NMConfigData *self, const char *group); GKeyFile *nm_config_data_clone_keyfile_intern (const NMConfigData *self); diff --git a/src/nm-config.c b/src/nm-config.c index 0c36f7a773..215595c208 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -84,11 +84,14 @@ typedef struct { char *debug; gboolean configure_and_quit; + + char **atomic_section_prefixes; } NMConfigPrivate; enum { PROP_0, PROP_CMD_LINE_OPTIONS, + PROP_ATOMIC_SECTION_PREFIXES, LAST_PROP, }; @@ -655,6 +658,11 @@ read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError ** continue; } + if (!strcmp (key, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS)) { + /* the "was" key is protected and it cannot be set by user configuration. */ + continue; + } + key_len = strlen (key); last_char = key[key_len - 1]; if ( key_len > 1 @@ -963,6 +971,70 @@ read_entire_config (const NMConfigCmdLineOptions *cli, return keyfile; } +static gboolean +_is_atomic_section (const char *const*atomic_section_prefixes, const char *group) +{ + if (atomic_section_prefixes) { + for (; *atomic_section_prefixes; atomic_section_prefixes++) { + if ( **atomic_section_prefixes + && g_str_has_prefix (group, *atomic_section_prefixes)) + return TRUE; + } + } + return FALSE; +} + +static void +_string_append_val (GString *str, const char *value) +{ + if (!value) + return; + g_string_append_c (str, '+'); + while (TRUE) { + switch (*value) { + case '\0': + return; + case '\\': + case '+': + case '#': + case ':': + g_string_append_c (str, '+'); + default: + g_string_append_c (str, *value); + } + value++; + } +} + +static char * +_keyfile_serialize_section (GKeyFile *keyfile, const char *group) +{ + gs_strfreev char **keys = NULL; + GString *str; + guint k; + + if (keyfile) + keys = g_key_file_get_keys (keyfile, group, NULL, NULL); + if (!keys) + return g_strdup ("0#"); + + /* prepend a version. */ + str = g_string_new ("1#"); + + for (k = 0; keys[k]; k++) { + const char *key = keys[k]; + gs_free char *value = NULL; + + _string_append_val (str, key); + g_string_append_c (str, ':'); + + value = g_key_file_get_value (keyfile, group, key, NULL); + _string_append_val (str, value); + g_string_append_c (str, '#'); + } + return g_string_free (str, FALSE); +} + /** * intern_config_read: * @filename: the filename where to store the internal config @@ -983,6 +1055,7 @@ read_entire_config (const NMConfigCmdLineOptions *cli, static GKeyFile * intern_config_read (const char *filename, GKeyFile *keyfile_conf, + const char *const*atomic_section_prefixes, gboolean *out_needs_rewrite) { GKeyFile *keyfile_intern; @@ -1012,13 +1085,32 @@ intern_config_read (const char *filename, for (g = 0; groups && groups[g]; g++) { gs_strfreev char **keys = NULL; const char *group = groups[g]; - gboolean is_intern; + gboolean is_intern, is_atomic; keys = g_key_file_get_keys (keyfile, group, NULL, NULL); if (!keys) continue; is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + is_atomic = !is_intern && _is_atomic_section (atomic_section_prefixes, group); + + if (is_atomic) { + gs_free char *conf_section_was = NULL; + gs_free char *conf_section_is = NULL; + + conf_section_is = _keyfile_serialize_section (keyfile_conf, group); + conf_section_was = g_key_file_get_string (keyfile, group, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, NULL); + + if (g_strcmp0 (conf_section_was, conf_section_is) != 0) { + /* the section no longer matches. Skip it entirely. */ + needs_rewrite = TRUE; + continue; + } + /* we must set the "was" marker in our keyfile, so that we know that the section + * from user config is overwritten. The value doesn't matter, it's just a marker + * that this section is present. */ + g_key_file_set_value (keyfile_intern, group, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, ""); + } for (k = 0; keys[k]; k++) { gs_free char *value_set = NULL; @@ -1029,6 +1121,10 @@ intern_config_read (const char *filename, if (is_intern) { has_intern = TRUE; g_key_file_set_value (keyfile_intern, group, key, value_set); + } else if (is_atomic) { + if (strcmp (key, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS) == 0) + continue; + g_key_file_set_value (keyfile_intern, group, key, value_set); } else if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) { const char *key_base = &key[STRLEN (NM_CONFIG_KEYFILE_KEYPREFIX_SET)]; gs_free char *value_was = NULL; @@ -1098,7 +1194,7 @@ out: } static int -_intern_config_write_sort_fcn (const char **a, const char **b, gpointer dummy) +_intern_config_write_sort_fcn (const char **a, const char **b, const char *const*atomic_section_prefixes) { const char *g_a = (a ? *a : NULL); const char *g_b = (b ? *b : NULL); @@ -1112,6 +1208,16 @@ _intern_config_write_sort_fcn (const char **a, const char **b, gpointer dummy) return 1; return -1; } + if (!a_is) { + a_is = _is_atomic_section (atomic_section_prefixes, g_a); + b_is = _is_atomic_section (atomic_section_prefixes, g_b); + + if (a_is != b_is) { + if (a_is) + return 1; + return -1; + } + } return g_strcmp0 (g_a, g_b); } @@ -1119,6 +1225,7 @@ static gboolean intern_config_write (const char *filename, GKeyFile *keyfile_intern, GKeyFile *keyfile_conf, + const char *const*atomic_section_prefixes, GError **error) { GKeyFile *keyfile; @@ -1144,31 +1251,58 @@ intern_config_write (const char *filename, g_strv_length (groups), sizeof (char *), (GCompareDataFunc) _intern_config_write_sort_fcn, - NULL); + (gpointer) atomic_section_prefixes); } } for (g = 0; groups && groups[g]; g++) { gs_strfreev char **keys = NULL; const char *group = groups[g]; - gboolean is_intern; + gboolean is_intern, is_atomic; keys = g_key_file_get_keys (keyfile_intern, group, NULL, NULL); if (!keys) continue; is_intern = g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN); + is_atomic = !is_intern && _is_atomic_section (atomic_section_prefixes, group); + + if (is_atomic) { + if ( (!keys[0] || (!keys[1] && strcmp (keys[0], NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS) == 0)) + && !g_key_file_has_group (keyfile_conf, group)) { + /* we are about to save an atomic section. However, we don't have any additional + * keys on our own and there is no user-provided (overlapping) section either. + * We don't have to write an empty section (i.e. skip the useless ".was=0#"). */ + continue; + } else { + gs_free char *conf_section_is = NULL; + + conf_section_is = _keyfile_serialize_section (keyfile_conf, group); + g_key_file_set_string (keyfile, group, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, conf_section_is); + g_key_file_set_comment (keyfile, group, NULL, + " Overwrites entire section from 'NetworkManager.conf'", + NULL); + } + } for (k = 0; keys[k]; k++) { const char *key = keys[k]; gs_free char *value_set = NULL; gs_free char *key_set = NULL; + if ( !is_intern + && strcmp (key, NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS) == 0) { + g_warn_if_fail (is_atomic); + continue; + } + value_set = g_key_file_get_value (keyfile_intern, group, key, NULL); if (is_intern) { has_intern = TRUE; g_key_file_set_value (keyfile, group, key, value_set); - } else { + } else if (is_atomic) + g_key_file_set_value (keyfile, group, key, value_set); + else { gs_free char *value_was = NULL; if (_HAS_PREFIX (key, NM_CONFIG_KEYFILE_KEYPREFIX_SET)) { @@ -1237,6 +1371,10 @@ intern_config_write (const char *filename, " That means, if you modify a value in 'NetworkManager.conf', the internal\n" " overwrite no longer matches and is ignored.\n" "\n" + " Certain sections can only be overwritten whole, not on a per key basis.\n" + " Such sections are marked with a \""NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS"\" key that records the user configuration\n" + " at the time of writing.\n" + "\n" " Internal sections of the form [" NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN "*] cannot\n" " be set by user configuration.\n" "\n" @@ -1286,10 +1424,15 @@ nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, con * keys and values you set in @keyfile_intern_new. You basically reset all * internal configuration values to what is in @keyfile_intern_new. * - * There are 2 types of settings: + * There are 3 types of settings: * - all groups/sections with a prefix [.intern.*] are taken as is. As these * groups are separate from user configuration, there is no conflict. You set * them, that's it. + * - there are atomic sections, i.e. sections whose name start with one of + * NM_CONFIG_ATOMIC_SECTION_PREFIXES. If you put values in these sections, + * it means you completely replace the section from user configuration. + * You can also hide a user provided section by only putting the special + * key NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS into that section. * - otherwise you can overwrite individual values from user-configuration. * Just set the value. Keys with a prefix NM_CONFIG_KEYFILE_KEYPREFIX_* * are protected -- as they are not value user keys. @@ -1308,6 +1451,8 @@ nm_config_set_values (NMConfig *self, GKeyFile *keyfile_new; GError *local = NULL; NMConfigData *new_data = NULL; + gs_strfreev char **groups = NULL; + gint g; g_return_if_fail (NM_IS_CONFIG (self)); @@ -1319,6 +1464,13 @@ nm_config_set_values (NMConfig *self, if (keyfile_intern_new) _nm_keyfile_copy (keyfile_new, keyfile_intern_new); + /* ensure that every atomic section has a .was entry. */ + groups = g_key_file_get_groups (keyfile_new, NULL); + for (g = 0; groups && groups[g]; g++) { + if (_is_atomic_section ((const char *const*) priv->atomic_section_prefixes, groups[g])) + g_key_file_set_value (keyfile_new, groups[g], NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, ""); + } + if (!_nm_keyfile_equals (keyfile_intern_current, keyfile_new, TRUE)) new_data = nm_config_data_new_update_keyfile_intern (priv->config_data, keyfile_new); @@ -1335,7 +1487,8 @@ nm_config_set_values (NMConfig *self, * changes on disk happened in any case *after* now. */ if (*priv->intern_config_file) { keyfile_user = _nm_config_data_get_keyfile_user (priv->config_data); - if (!intern_config_write (priv->intern_config_file, keyfile_new, keyfile_user, &local)) { + if (!intern_config_write (priv->intern_config_file, keyfile_new, keyfile_user, + (const char *const*) priv->atomic_section_prefixes, &local)) { nm_log_warn (LOGD_CORE, "error saving internal configuration \"%s\": %s", priv->intern_config_file, local->message); g_clear_error (&local); } @@ -1390,9 +1543,14 @@ nm_config_reload (NMConfig *self, int signal) no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); - keyfile_intern = intern_config_read (priv->intern_config_file, keyfile, &intern_config_needs_rewrite); - if (intern_config_needs_rewrite) - intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, NULL); + keyfile_intern = intern_config_read (priv->intern_config_file, + keyfile, + (const char *const*) priv->atomic_section_prefixes, + &intern_config_needs_rewrite); + if (intern_config_needs_rewrite) { + intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, + (const char *const*) priv->atomic_section_prefixes, NULL); + } new_data = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile, keyfile_intern); g_free (config_main_file); @@ -1515,11 +1673,11 @@ nm_config_get (void) } NMConfig * -nm_config_setup (const NMConfigCmdLineOptions *cli, GError **error) +nm_config_setup (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error) { g_assert (!singleton_instance); - singleton_instance = nm_config_new (cli, error); + singleton_instance = nm_config_new (cli, atomic_section_prefixes, error); if (singleton_instance) nm_singleton_instance_weak_ref_register (); return singleton_instance; @@ -1602,9 +1760,14 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) no_auto_default = no_auto_default_from_file (priv->no_auto_default_file); - keyfile_intern = intern_config_read (priv->intern_config_file, keyfile, &intern_config_needs_rewrite); - if (intern_config_needs_rewrite) - intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, NULL); + keyfile_intern = intern_config_read (priv->intern_config_file, + keyfile, + (const char *const*) priv->atomic_section_prefixes, + &intern_config_needs_rewrite); + if (intern_config_needs_rewrite) { + intern_config_write (priv->intern_config_file, keyfile_intern, keyfile, + (const char *const*) priv->atomic_section_prefixes, NULL); + } priv->config_data_orig = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile, keyfile_intern); @@ -1619,12 +1782,13 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) } NMConfig * -nm_config_new (const NMConfigCmdLineOptions *cli, GError **error) +nm_config_new (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error) { return NM_CONFIG (g_initable_new (NM_TYPE_CONFIG, NULL, error, NM_CONFIG_CMD_LINE_OPTIONS, cli, + NM_CONFIG_ATOMIC_SECTION_PREFIXES, atomic_section_prefixes, NULL)); } @@ -1650,6 +1814,7 @@ finalize (GObject *gobject) g_free (priv->log_level); g_free (priv->log_domains); g_free (priv->debug); + g_strfreev (priv->atomic_section_prefixes); _nm_config_cmd_line_options_clear (&priv->cli); @@ -1676,6 +1841,10 @@ set_property (GObject *object, guint prop_id, else _nm_config_cmd_line_options_copy (cli, &priv->cli); break; + case PROP_ATOMIC_SECTION_PREFIXES: + /* construct only */ + priv->atomic_section_prefixes = g_strdupv (g_value_get_boxed (value)); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1698,6 +1867,14 @@ nm_config_class_init (NMConfigClass *config_class) G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property + (object_class, PROP_ATOMIC_SECTION_PREFIXES, + g_param_spec_boxed (NM_CONFIG_ATOMIC_SECTION_PREFIXES, "", "", + G_TYPE_STRV, + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); + signals[SIGNAL_CONFIG_CHANGED] = g_signal_new (NM_CONFIG_SIGNAL_CONFIG_CHANGED, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/nm-config.h b/src/nm-config.h index d4ff887ba3..3caf0c5c31 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -39,6 +39,7 @@ G_BEGIN_DECLS /* Properties */ #define NM_CONFIG_CMD_LINE_OPTIONS "cmd-line-options" +#define NM_CONFIG_ATOMIC_SECTION_PREFIXES "atomic-section-prefixes" /* Signals */ #define NM_CONFIG_SIGNAL_CONFIG_CHANGED "config-changed" @@ -60,6 +61,7 @@ G_BEGIN_DECLS #define NM_CONFIG_KEYFILE_GROUP_IFUPDOWN "ifupdown" #define NM_CONFIG_KEYFILE_GROUP_IFNET "ifnet" +#define NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS ".was" #define NM_CONFIG_KEYFILE_KEY_IFNET_AUTO_REFRESH "auto_refresh" #define NM_CONFIG_KEYFILE_KEY_IFNET_MANAGED "managed" #define NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED "managed" @@ -115,8 +117,8 @@ void nm_config_cmd_line_options_add_to_entries (NMConfigCmdLi gboolean nm_config_get_no_auto_default_for_device (NMConfig *config, NMDevice *device); void nm_config_set_no_auto_default_for_device (NMConfig *config, NMDevice *device); -NMConfig *nm_config_new (const NMConfigCmdLineOptions *cli, GError **error); -NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, GError **error); +NMConfig *nm_config_new (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error); +NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error); void nm_config_reload (NMConfig *config, int signal); gint nm_config_parse_boolean (const char *str, gint default_value); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index 82fb73dad2..d259b620bb 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -82,7 +82,7 @@ setup_config (GError **error, const char *config_file, const char *intern_config g_ptr_array_free (args, TRUE); - config = nm_config_setup (cli, &local_error); + config = nm_config_setup (cli, NULL, &local_error); if (error) { g_assert (!config); g_assert (local_error); From 25b23f931ec0d26b3f68720e8d9ab299d4c43cdb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Jun 2015 11:37:21 +0200 Subject: [PATCH 8/8] config/test: add test for set_values() --- src/tests/config/test-config.c | 366 ++++++++++++++++++++++++++++++++- 1 file changed, 356 insertions(+), 10 deletions(-) diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index d259b620bb..33b66d614f 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -32,8 +32,27 @@ #include "nm-test-utils.h" +/********************************************************************************/ + +static void +_assert_config_value (const NMConfigData *config_data, const char *group, const char *key, const char *expected_value, const char *file, int line) +{ + gs_free char *value = NULL; + + value = nm_config_data_get_value (config_data, group, key, NM_CONFIG_GET_VALUE_NONE); + if (g_strcmp0 (value, expected_value)) { + g_error ("(%s:%d) invalid value in config-data %s.%s = %s%s%s (instead of %s%s%s)", + file, line, group, key, + NM_PRINT_FMT_QUOTED (value, "\"", value, "\"", "(null)"), + NM_PRINT_FMT_QUOTED (expected_value, "\"", expected_value, "\"", "(null)")); + } +} +#define assert_config_value(config_data, group, key, expected_value) _assert_config_value (config_data, group, key, expected_value, __FILE__, __LINE__) + +/********************************************************************************/ + static NMConfig * -setup_config (GError **error, const char *config_file, const char *intern_config, const char *config_dir, const char *system_config_dir, ...) +setup_config (GError **error, const char *config_file, const char *intern_config, const char *const* atomic_section_prefixes, const char *config_dir, const char *system_config_dir, ...) { va_list ap; GPtrArray *args; @@ -82,7 +101,7 @@ setup_config (GError **error, const char *config_file, const char *intern_config g_ptr_array_free (args, TRUE); - config = nm_config_setup (cli, NULL, &local_error); + config = nm_config_setup (cli, (char **) atomic_section_prefixes, &local_error); if (error) { g_assert (!config); g_assert (local_error); @@ -105,7 +124,7 @@ test_config_simple (void) gs_unref_object NMDevice *dev51 = nm_test_device_new ("00:00:00:00:00:51"); gs_unref_object NMDevice *dev52 = nm_test_device_new ("00:00:00:00:00:52"); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient"); @@ -184,7 +203,7 @@ test_config_non_existent (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/no-such-file", "", "/no/such/dir", "", NULL); + setup_config (&error, SRCDIR "/no-such-file", "", NULL, "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND); g_clear_error (&error); } @@ -194,7 +213,7 @@ test_config_parse_error (void) { GError *error = NULL; - setup_config (&error, SRCDIR "/bad.conf", "", "/no/such/dir", "", NULL); + setup_config (&error, SRCDIR "/bad.conf", "", NULL, "/no/such/dir", "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } @@ -205,7 +224,7 @@ test_config_override (void) NMConfig *config; const char **plugins; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", "--plugins", "alpha,beta,gamma,delta", "--connectivity-interval", "12", NULL); @@ -243,7 +262,7 @@ test_config_no_auto_default (void) g_assert_cmpint (nwrote, ==, 18); close (fd); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -265,7 +284,7 @@ test_config_no_auto_default (void) g_object_unref (config); - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", "/no/such/dir", "", + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", "--no-auto-default", state_file, NULL); @@ -293,7 +312,7 @@ test_config_confdir (void) char *value; GSList *specs; - config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", SRCDIR "/conf.d", "", NULL); + config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, SRCDIR "/conf.d", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhcpcd"); @@ -399,11 +418,336 @@ test_config_confdir_parse_error (void) GError *error = NULL; /* Using SRCDIR as the conf dir will pick up bad.conf */ - setup_config (&error, SRCDIR "/NetworkManager.conf", "", SRCDIR, "", NULL); + setup_config (&error, SRCDIR "/NetworkManager.conf", "", NULL, SRCDIR, "", NULL); g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE); g_clear_error (&error); } +/*****************************************************************************/ + +typedef void (*TestSetValuesUserSetFcn) (NMConfig *config, gboolean is_user, GKeyFile *keyfile_user, NMConfigChangeFlags *out_expected_changes); +typedef void (*TestSetValuesCheckStateFcn) (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data); + +typedef struct { + NMConfigChangeFlags changes; + TestSetValuesCheckStateFcn check_state_fcn; +} TestSetValuesConfigChangedData; + +static void +_set_values_config_changed_cb (NMConfig *config, + NMConfigData *config_data, + NMConfigChangeFlags changes, + NMConfigData *old_data, + TestSetValuesConfigChangedData *config_changed_data) +{ + g_assert (changes != NM_CONFIG_CHANGE_NONE); + g_assert (config_changed_data); + g_assert (config_changed_data->changes == NM_CONFIG_CHANGE_NONE); + + if (changes == NM_CONFIG_CHANGE_SIGHUP) + return; + changes &= ~NM_CONFIG_CHANGE_SIGHUP; + + config_changed_data->changes = changes; + + if (config_changed_data->check_state_fcn) + config_changed_data->check_state_fcn (config, config_data, TRUE, changes, old_data); +} + +static void +_set_values_user (NMConfig *config, + const char *CONFIG_USER, + TestSetValuesUserSetFcn set_fcn, + TestSetValuesCheckStateFcn check_state_fcn) +{ + GKeyFile *keyfile_user; + gboolean success; + gs_free_error GError *error = NULL; + TestSetValuesConfigChangedData config_changed_data = { + .changes = NM_CONFIG_CHANGE_NONE, + .check_state_fcn = check_state_fcn, + }; + NMConfigChangeFlags expected_changes = NM_CONFIG_CHANGE_NONE; + gs_unref_object NMConfigData *config_data_before = NULL; + + keyfile_user = nm_config_create_keyfile (); + + success = g_key_file_load_from_file (keyfile_user, CONFIG_USER, G_KEY_FILE_NONE, &error); + nmtst_assert_success (success, error); + + if (set_fcn) + set_fcn (config, TRUE, keyfile_user, &expected_changes); + + success = g_key_file_save_to_file (keyfile_user, CONFIG_USER, &error); + nmtst_assert_success (success, error); + + g_signal_connect (G_OBJECT (config), + NM_CONFIG_SIGNAL_CONFIG_CHANGED, + G_CALLBACK (_set_values_config_changed_cb), + &config_changed_data); + + config_data_before = g_object_ref (nm_config_get_data (config)); + + if (expected_changes != NM_CONFIG_CHANGE_NONE) + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: update *"); + else + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: signal SIGHUP (no changes from disk)*"); + + nm_config_reload (config, SIGHUP); + + g_test_assert_expected_messages (); + + g_assert (expected_changes == config_changed_data.changes); + + if (check_state_fcn) + check_state_fcn (config, nm_config_get_data (config), FALSE, NM_CONFIG_CHANGE_NONE, config_data_before); + + g_signal_handlers_disconnect_by_func (config, _set_values_config_changed_cb, &config_changed_data); + + g_key_file_unref (keyfile_user); +} + +static void +_set_values_intern (NMConfig *config, + TestSetValuesUserSetFcn set_fcn, + TestSetValuesCheckStateFcn check_state_fcn) +{ + GKeyFile *keyfile_intern; + TestSetValuesConfigChangedData config_changed_data = { + .changes = NM_CONFIG_CHANGE_NONE, + .check_state_fcn = check_state_fcn, + }; + NMConfigChangeFlags expected_changes = NM_CONFIG_CHANGE_NONE; + gs_unref_object NMConfigData *config_data_before = NULL; + + config_data_before = g_object_ref (nm_config_get_data (config)); + + keyfile_intern = nm_config_data_clone_keyfile_intern (config_data_before); + + if (set_fcn) + set_fcn (config, FALSE, keyfile_intern, &expected_changes); + + g_signal_connect (G_OBJECT (config), + NM_CONFIG_SIGNAL_CONFIG_CHANGED, + G_CALLBACK (_set_values_config_changed_cb), + &config_changed_data); + + if (expected_changes != NM_CONFIG_CHANGE_NONE) + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: update *"); + + nm_config_set_values (config, keyfile_intern, TRUE, FALSE); + + g_test_assert_expected_messages (); + + g_assert (expected_changes == config_changed_data.changes); + + if (check_state_fcn) + check_state_fcn (config, nm_config_get_data (config), FALSE, NM_CONFIG_CHANGE_NONE, config_data_before); + + g_signal_handlers_disconnect_by_func (config, _set_values_config_changed_cb, &config_changed_data); + + g_key_file_unref (keyfile_intern); +} + +static void +_set_values_user_intern_section_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", "key", "this-should-be-ignored"); +} + +static void +_set_values_user_intern_section_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + g_assert (changes == NM_CONFIG_CHANGE_NONE); + g_assert (!nm_config_data_has_group (config_data, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1")); +} + +static void +_set_values_user_initial_values_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_remove_group (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", NULL); + g_key_file_set_string (keyfile, "section1", "key1", "value1"); + *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER; +} + +static void +_set_values_user_initial_values_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER)); + assert_config_value (config_data, "section1", "key1", "value1"); +} + +static void +_set_values_intern_internal_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", "key", "internal-section"); + *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN; +} + +static void +_set_values_intern_internal_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN)); + assert_config_value (config_data, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", "key", "internal-section"); +} + +static void +_set_values_user_atomic_section_1_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key1", "user-value1"); + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key2", "user-value2"); + 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; +} + +static void +_set_values_user_atomic_section_1_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER)); + 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"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key1", "user-value1"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2"); +} + +static void +_set_values_intern_atomic_section_1_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key1", "intern-value1"); + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key3", "intern-value3"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key1", "intern-value1"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3"); + *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN; +} + +static void +_set_values_intern_atomic_section_1_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN)); + assert_config_value (config_data, "atomic-prefix-1.section-a", "key1", "intern-value1"); + assert_config_value (config_data, "atomic-prefix-1.section-a", "key2", NULL); + assert_config_value (config_data, "atomic-prefix-1.section-a", "key3", "intern-value3"); + assert_config_value (config_data, "atomic-prefix-1.section-b", "key1", "user-value1"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key1", "intern-value1"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3"); + g_assert ( nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-a")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-b")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "non-atomic-prefix-1.section-a")); +} + +static void +_set_values_user_atomic_section_2_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key1", "user-value1-x"); + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key2", "user-value2"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key1", "user-value1-x"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2-x"); + *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER | NM_CONFIG_CHANGE_VALUES_INTERN; +} + +static void +_set_values_user_atomic_section_2_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_USER | NM_CONFIG_CHANGE_VALUES_INTERN)); + assert_config_value (config_data, "atomic-prefix-1.section-a", "key1", "user-value1-x"); + assert_config_value (config_data, "atomic-prefix-1.section-a", "key2", "user-value2"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key1", "user-value1-x"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2-x"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3"); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-a")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-b")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "non-atomic-prefix-1.section-a")); +} + +static void +_set_values_intern_atomic_section_2_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes) +{ + /* let's hide an atomic section and one key. */ + g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS, "any-value"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", NM_CONFIG_KEYFILE_KEYPREFIX_WAS"nap1-key1", "any-value"); + g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3"); + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key1", " b c\\, d "); + g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key2", " b c\\, d "); + *out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN; +} + +static void +_set_values_intern_atomic_section_2_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data) +{ + if (is_change_event) + g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN)); + g_assert (!nm_config_data_has_group (config_data, "atomic-prefix-1.section-a")); + assert_config_value (config_data, "atomic-prefix-1.section-b", "key1", "user-value1"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key1", NULL); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key2", "user-value2-x"); + assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3"); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-a")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "atomic-prefix-1.section-b")); + g_assert (!nm_config_data_is_intern_atomic_group (config_data, "non-atomic-prefix-1.section-a")); + assert_config_value (config_data, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key1", " b c\\, d "); + assert_config_value (config_data, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key2", " b c\\, d "); +} + +static void +test_config_set_values (void) +{ + gs_unref_object NMConfig *config = NULL; + const char *CONFIG_USER = SRCDIR"/test-set-values-user.conf"; + const char *CONFIG_INTERN = SRCDIR"/test-set-values-intern.conf"; + const char *atomic_section_prefixes[] = { + "atomic-prefix-1.", + "atomic-prefix-2.", + NULL, + }; + + g_assert (g_file_set_contents (CONFIG_USER, "", 0, NULL)); + g_assert (g_file_set_contents (CONFIG_INTERN, "", 0, NULL)); + + config = setup_config (NULL, CONFIG_USER, CONFIG_INTERN, atomic_section_prefixes, "", "", NULL); + + _set_values_user (config, CONFIG_USER, + _set_values_user_intern_section_set, + _set_values_user_intern_section_check); + + _set_values_user (config, CONFIG_USER, + _set_values_user_initial_values_set, + _set_values_user_initial_values_check); + + _set_values_intern (config, + _set_values_intern_internal_set, + _set_values_intern_internal_check); + + _set_values_user (config, CONFIG_USER, + _set_values_user_atomic_section_1_set, + _set_values_user_atomic_section_1_check); + + _set_values_intern (config, + _set_values_intern_atomic_section_1_set, + _set_values_intern_atomic_section_1_check); + + _set_values_user (config, CONFIG_USER, + _set_values_user_atomic_section_2_set, + _set_values_user_atomic_section_2_check); + + _set_values_intern (config, + _set_values_intern_atomic_section_2_set, + _set_values_intern_atomic_section_2_check); + + g_assert (remove (CONFIG_USER) == 0); + g_assert (remove (CONFIG_INTERN) == 0); +} + +/*****************************************************************************/ + NMTST_DEFINE (); int @@ -427,6 +771,8 @@ main (int argc, char **argv) g_test_add_func ("/config/confdir", test_config_confdir); g_test_add_func ("/config/confdir-parse-error", test_config_confdir_parse_error); + g_test_add_func ("/config/set-values", test_config_set_values); + /* This one has to come last, because it leaves its values in * nm-config.c's global variables, and there's no way to reset * those to NULL.