Introduce logging helpers where possible, to uniform the format of
messages and prepend a tag specifying the module that generated the
message, along with other useful information (interface name, ...).
https://bugzilla.gnome.org/show_bug.cgi?id=763040
Originally m4/introspection.m4 was copied from gobject-introspection
repository. We however modified it in commit f6272144e9.
Reimport the file with latest changes, but still preserving our
workaround.
GError codes are only unique per domain, so logging the code without
also indicating the domain is not helpful. And anyway, if the error
messages are not distinctive enough to tell the whole story then we
should fix the error messages.
Based-on-patch-by: Dan Winship <danw@gnome.org>
Use g_error_matches() where we're testing error codes. In particular,
use it rather than looking at only ->code and not also ->domain, which
is just wrong.
[thaller@redhat.com: rebase and modify original patch]
Functions that take a GError** MUST fill it in on error. There is no
need to check whether error is NULL if the function it was passed to
had a failing return value.
Likewise, a proper GError must have a non-NULL message, so there's no
need to double-check that either.
Based-on-patch-by: Dan Winship <danw@gnome.org>
We construct new IP6Config on each rdisc_config_changed(). That's not a smart
thing to do, since that makes us throw away the previous configuration.
In case the two routers on the same network, the first RA triggers
rdisc_config_changed() for changed gateway and addresses. On handling the
second RA rdisc_config_changed() doesn't add the address, resulting in the
address being removed on ip6 config sync.
A side effect of this is that the address is still tentative, resulting in DAD
retry and an new address being added. So the networking still works, but at the
expense of a single DAD failure and an address that's perhaps different from what
was expected.
Until now the internal DHCP client could start a DHCPv6 transaction
but was not able to parse the lease and pass the information back to
the core. Add the missing glue code to make this work.
https://bugzilla.gnome.org/show_bug.cgi?id=762432
If the plugin supports interactive mode, but the VPN binary (like vpnc
or openvpn) doesn't support it, then the plugin should return
NM_VPN_PLUGIN_ERROR_INTERACTIVE_NOT_SUPPORTED from its connect_interactive()
hook. This lets NetworkManager know to fall back to plain Connect().
Since this notification is done through an error return, the VPN service
plugin code sees the failure and moves the plugin state back to
STOPPED. NetworkManager sees that state change, and terminates the
connection attempt while waiting for a reply to the Connect() method.
(VPN service plugins that don't support interactive mode at all don't
have this problem because that error is returned before the plugin's
state is moved to STARTING.)
To fix this, do two things:
1) if the connect_interactive() hook fails and returns the error
NM_VPN_PLUGIN_ERROR_INTERACTIVE_NOT_SUPPORTED, postpone the STOPPED
state change for a few seconds to allow NM time to fall back to
plain Connect(). We still want to move the plugin state back to
STOPPED eventually, because otherwise it could stay in STARTING
forever.
2) change state to STARTING only if the connect/connect_interactive
plugin hooks were successful. Otherwise the plugin would still be
in STARTING state, and it's not valid to call Connect()/ConnectInteractive()
during the STARTING state.
https://mail.gnome.org/archives/networkmanager-list/2016-February/msg00091.htmlhttps://bugzilla.redhat.com/show_bug.cgi?id=1298732
Choose a new logging format.
- the logging format must not be configurable and it must be the
same for all backends. It is neat that journal supports additional
fields, but an average user still posts the output of plain
journalctl, without "--output verbose" (which would also be hard
to read).
Also, we get used to a certain logging format, so having different
formats is confusing. If one format is better then another, it should
be used for all backends: syslog, journal and debug.
The only question is, what is the best format.
- the timestamp: I find it useful to see how much time between two
events passed. The timestamp printed by syslog doesn't have sufficient
granularity, and the internal journal fields are not readily available.
We used to print the timestamps for <error>, <debug> and <trace>,
but ommited them for <info> and <warn> levels. We now print them for
all levels, which has a uniform alignment.
- the location: the "[file:line] func():" part is mostly redundant
and results in wide lines. It also causes a misalignment of the
logging lines, or -- as I recently added alignment of the location --
it results in awkward whitespace and truncation.
But the location is really just necessary because our logging messages
are bad:
"<debug> [1456397604.038226] (9) 11-dhclient succeeded"
The solution to this is not
"<debug> [1456397604.038226] [nm-dispatcher.c:358] dispatcher_results_process(): (9) 11-dhclient succeeded"
but a properly worded message:
"<debug> [1456397604.038226] dispatcher: request #9, script 11-dhclient succeeded"
- logging-message: we need to write better logging messages.
I like some form of "tags" that are easy to grep:
"platform: signal: link changed: 4: ..."
Downside is, that this is not nice to read as a full sentence.
So, especially for <info> and <warn> logging, more human readable
messages are better.
We should find a compromise, where the log message explains what
happens, but is still concise and contains patterns that are easy
to grep and identify visually.
https://mail.gnome.org/archives/networkmanager-list/2016-February/msg00077.html
It's useful to track the flags for IPv4 addresses too.
- we might want to sort IPv4 addresses according to whether they
are permanent or dynamic.
- later we want to set IFA_F_NOPREFIXROUTE also for IPv4 addresses.
While the ability to "set" a flag doesn't necessarily require that we
also keep the flags present in NMPlatformIP4Address, it is more consistent.
This test makes sense because "test-systemd" is not
linked against any other systemd library. So this test
verifies that our libsystemd clone is self contained.
Most interestingly is also, whether we can link libsystemd.a without
having undefined references (which might be wrongly satisfied by an
externally loaded libsystem shared library.
"NetworkManagerUtils.h" contains a bunch of helper tools for core
daemon ("src/").
Unfortunately, it has dependencies to other parts of core,
such as "nm-device.h" and "nm-platform.h". Split out a part
of tools that are independent so that they can be used without
dragging in other dependencies.
"nm-core-utils.h" should only use libnm-core, "nm-logging.h"
and shared.
"NetworkManagerUtils.h" should provide all "nm-core-utils.h" and
possibly other utilities that have larger dependencies.
"nm-logging.h" is a basic core module that should have few other
dependencies. Instead of letting nm-logging.c directly call a function
from "nm-linux-platform.c", let platform register a handler as needed.
This way, you can build a core library containing nm-logging but no
nm-platform.
On older NM versions the default value for vlan.flags was 0, but then
the actual value set on interfaces was REORDER_HDR. In order to
maintain backwards compatibility in behavior, remove the special
handling of vlan.flags so that a missing key is treated as the default
value REORDER_HDR.
https://bugzilla.gnome.org/show_bug.cgi?id=762626
On NM 1.0 connections were created by default without the REORDER_HDR
flag, but then due to a bug in platform code (fixed in [1]), the
kernel interface always had the flag set.
Now that the setting is honored, users upgrading to the new version of
NM will see a change from the previous behavior, since interfaces will
not have REORDER_HDR and this will certainly break functionality.
The only solution here seems to be to ignore the REORDER_HDR variable
in ifcfg files (since it never had any effect) and introduce a new
NO_REORDER_HDR option for the VLAN_FLAGS variable which allows to turn
the flag off. The consequence is that the flag will be set for all old
connections.
This change introduces an incompatibility with initscripts, however is
necessary to avoid breaking user functionality upon upgrade.
Connections created through NetworkManager will still be parsed
correctly by initscripts (since we always write the REORDER_HDR
variable).
[1] db62fc9d72 ("platform: fix adding VLAN flags")
https://bugzilla.gnome.org/show_bug.cgi?id=762626
There are far too many "flags". Rename the "flags" to "n_ifa_flags"
which reminds to "ifa_flags" in 'struct ifaddrmsg', but with a
distinctive "n_" prefix.
