When one of the configuration snippet is malformed, NM doesn't tell
which file caused the error:
$ NetworkManager --print-config
Failed to read configuration: Key file does not start with a group
Fix this.
$ NetworkManager --print-config
Failed to read configuration: /usr/lib/NetworkManager/conf.d/test.conf: Key file does not start with a group
After all, this state is stored persistently to /var/lib/NetworkManager,
and not to volatile storage in /var/run. Hence the name is better.
It's also shorter, so rename it.
The commit is mostly trivial, including update of code comments
and logging messages.
Fixes: 1b43c880ba
Move reading and writing of the state file to NMConfig
("/var/lib/NetworkManager/NetworkManager.state" file).
Originally, I intended to persist more state, thus it made
sense to cleanup handling of the state file and move it all
at one place. Now, it's not clear that will happen anytime soon.
Still, the change is a worthy cleanup, so do it anyway.
https://bugzilla.gnome.org/show_bug.cgi?id=764474
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
No longer support disabling the global-dns configuration via the
"enable" option.
Instead, the user can put the entire dns-configuration in one separate
snippet, and disable it altogether with ".config.enable".
Support a new configuration option
[.config]
enable=<ENABLED>
for configuration snippets.
This new [.config] section is only relevant within the snippet itself
and it is not merged into the combined configuration.
Currently only the "enable" key is supported. If the "enable" key is
missing, it obviously defaults to being enabled. It allows snippets
to be skipped from loading. The main configuration "NetworkManager.conf"
cannot be skipped.
<ENABLED> can be a boolean value (false), to skip a configuration
snippet from loading.
It can also be a string to match against the NetworkManager version,
like "enable=nm-version-min:1.1,nm-version-min:1.0.6"
There are several motivations for this:
- the user can disable an entire configuration snippet by toggeling
one entry.
This generalizes the functionality of the global-dns.enable
setting, but in a way that applies to configuration on a per-file
basis.
- for developing, we often switch between different versions of
NetworkManager. Thus, we might want to use different configuration.
E.g. before global-dns options, I want to use "dns=none" and manage
resolv.conf myself. Now, I can use global-dns setting to do that.
That can be achieved with something like the following (not exactly,
it's an example only):
[.config]
enable=nm-version-min:1.1
[main]
dns=default
[global-dns-domain-*]
nameserver=127.0.0.1
Arguably, this would be more awesome, if we would bump our micro devel
version (1.1.0) more often while developing 1.2.0 (*hint*).
- in principle, packages could drop configuration snippets and enable
them based on the NetworkManager version.
- with the "env:" spec, you can enable/disable snippets by configuring
an environment variable. Again, useful for testing and developing.
This is effectively the same as nm_config_parse_boolean(). The difference is,
that "nm-config.c" is not available to the interface-helper, thus any
code used by interface-helper (like "NetworkManager.c") cannot use this
function.
Still don't drop nm_config_parse_boolean() entirely, because it's better
to have the explicit notion of parsing a string in the config-context.
I ended up not using the function. But I'd still keep this patch.
Otherwise I get the following error (iwhile building in Jenkins):
In file included from ../include/nm-default.h:45:0,
from nm-config.c:27:
nm-config.c: In function 'nm_config_set_global_dns':
../include/gsystem-local-alloc.h:31:10: error: 'group_name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
func (*(Type*)v); \
^
nm-config.c:1483:17: note: 'group_name' was declared here
gs_free char *group_name;
^
There seems to be a bug in glib/ffi that hits on s390x/ppc64 architecture.
It causes @changes in nm-dns-manager.c:config_changed_cb() to be NONE,
although it is clearly set (see the related bug rh #1260577 for glib).
Workaround this, by making the argument type a plain guint.
Note that the ill behavior is caught by test_config_signal() in
"src/tests/config/test-config.c".
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1062301
The localization headers are now included via "nm-default.h".
Also fixes several places, where we wrongly included <glib/gi18n-lib.h>
instead of <glib/gi18n.h>. For example under "clients/" directory.
Previously, the order of destructing singleton instances
was undefined. Now, have singleton instances register their
destruction via nm_singleton_instance_register().
Objects that are registered later, will be destructed earlier. IOW,
they will be destroyed in reverse order of construction.
This is only a crude method to get the lifetime of singleton instances
right by default. Having singletons ref other singletons to keep them
alive gives more control over the lifetimes of singletons. This change
of having a defined order of destruction does not conflict with taking
references to singletons (and thus extending their lifetime).
Note that previously, NMPlatform was not registered for destruction.
We don't change that yet and intenionally leak a reference.
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
Whether NM runs in debug mode is also interesting to other
components outside of "main.c". Expose global_opt.debug
via a new nm_config_get_is_debug() function.
Actually, we should move parsing of all command line options
to NMConfig, as NMConfig is the central instance to provide
such information.
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.
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.
'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".
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
It is wrong to blindly merge keys that have an 'option+' or 'option-'.
Merging options is only possibly when we understand what the option
means and how to merge it.
No longer handle every setting but only those that are explicitly known
to be string-lists (or device-specs).
In some cases we want the returned value to be stripped. In some cases,
we want to read the raw value instead of the string parsed by GKeyFile.
Add an flags argument to nm_config_data_get_value(). It is up to the caller
to determine the exact meaning (and whether to strip).
By adding the flags argument, the caller can get the desired behavior easier
without having to workaround it afterwards. But more importantly, it becomes
apparent that there are different ways to retrieve the value and the caller
should decide on the details.
g_key_file_get_value() returns the raw value as stored in the file.
When accessing a string value, in most cases it is correct to use
g_key_file_get_string() instead.
When working with internals, such as comparing two keyfiles for
equality, g_key_file_get_value() is correct.
When parsing booleans, we parse it based on the raw value.
Fix the usages. This is a change in behavior if the config file
contained unusual strings.
Some plugins had their local defines for the name of the sections and
keys in NMConfig. Move those defines to "nm-config.h".
Usually plugins make use of code in core, but not the other
way round. Defining the names inside "nm-config.h" is no violation of
that because the config section names are anyway not local to the
plugin, but global in the shared name-space with other settings.
For example, another plugins shouldn't reuse the section "ifnet".
For that reason, it is correct and consistent to move these defines
to "nm-config.h".
We don't use those names in core, we merely signal their existance.
Add function to parse as boolean according our NMConfig convention.
Split this out from nm_config_keyfile_get_boolean() so that we can use
it independently. Also, change the return type to gint, so that one might
pass -1 to indicate an invalid/missing boolean value.
Thereby also don't log a warning in nm_config_keyfile_get_boolean()
We don't want to log a warning every time we access a keyfile value.
If we want to warn about invalid values, we should do it once after
the configuration is loaded. And then we should not only do it
for booleans, but for other types as well.
The content of the no-auto-default state file is part of NMConfig.
During a reload, also reload that.
This way, a user could edit the no-auto-default file and it would
be properly reloaded.
We used to merge the spec list for no-auto-default from keyfile with the
content of the state file. Since the addition of the "except:" spec this
is wrong.
For example, if the user configured:
no-auto-default=except:mac:11:11:11:11:11
and statefile contained "11:11:11:11:11" and "22:22:22:22:22", we would
wrongly not match "11:11:11:11:11". The two lists must be kept separate,
so that devices that are blocked by internal decision always match.
This separation is also clearer. Now the spec list is devided into a
part that comes from user configuration, and a part that comes from
internal decision.
We have a hack to extend GKeyFile to support specifying an 'option+'
key. Also add support for 'option-'.
Options that make use of these modifiers can only be string lists.
So do the concatenation not based on plain strings, but by treating
the values as string lists. Also, don't add duplicates.
We support the "NetworkManager.conf" sections '[connection]' and
'[connection.\+]' (with arbitrary suffix).
Fix the order of how we evaluate these section.
Note that the literal '[connection]' section is always evaluated lastly
after any other '[connection.\+]' section.
Within one file, we want to evaluate the sections in top-to-bottom
order. But accross multiple files, we want to order them
later-files-first. That gives a reasonable behavior if the user
looks at one file, and also if he wants to overwrite configuration
via configuration snippets like "conf.d/99-last.conf".
Note that if a later file extends/overwrites a section defined in an
earlier file, the section is still considered with lower priority
This is intentional, because the user ~extends~ a lower priority
section. If he wants to add a higher priority section, he should
choose a new suffix.
Fixes: dc0193ac02
Also react on SIGUSR1 and SIGUSR2, beside SIGHUP.
Only for SIGHUP actually reload the configuration from
disc. For the other signals only emit a config-changed
signal.
Calling read_entire_config() without passing a @cli argument would
always have caused an assert due to unset @o_config_main_file.
That is not a real problem as that situation didn't arise. Still
fix it.
Add a new 'rc-manager' configuration parameter that allows to select
the strategy used to write resolv.conf; currently supported values
are: none|resolvconf|netconfig, 'none' meaning that NM directly writes
the file.
The default value of the parameter is 'none'; however if a
RESOLVCONF_PATH (or NETCONFIG_PATH) is specified at build time, the
default value will be 'resolvconf' (or 'netconfig').
