Rename nm_connection_to_hash() to nm_connection_to_dbus(), and
nm_connection_new_from_hash() to nm_connection_new_from_dbus(). In
addition to clarifying that this is specifically the D-Bus
serialization format, these names will also work better in the
GDBus-based future where the serialization format is GVariant, not
GHashTable.
Also, move NMSettingHashFlags to nm-connection.h, and rename it
NMConnectionSerializationFlags.
The fact that NMRemoteConnection has to be an NMConnection and
therefore can't be an NMObject means that it needs to reimplement bits
of NMObject functionality (and likewise NMObject needs some special
magic to deal with it). Likewise, we will need a daemon-side
equivalent of NMObject as part of the gdbus port, and we would want
NMSettingsConnection to be able to inherit from this as well.
Solve this problem by making NMConnection into an interface, and
having NMRemoteConnection and NMSettingsConnection implement it. (We
use some hacks to keep the GHashTable of NMSettings objects inside
nm-connection.c rather than having to be implemented by the
implementations.)
Since NMConnection is no longer an instantiable type, this adds
NMSimpleConnection to replace the various non-D-Bus-based uses of
NMConnection throughout the code. nm_connection_new() becomes
nm_simple_connection_new(), nm_connection_new_from_hash() becomes
nm_simple_connection_new_from_hash(), and nm_connection_duplicate()
becomes nm_simple_connection_new_clone().
Previously, src/nm-ip4-config.h, libnm/nm-ip4-config.h, and
libnm-glib/nm-ip4-config.h all used "NM_IP4_CONFIG_H" as an include
guard, which meant that nm-test-utils.h could not tell which of them
was being included (and so, eg, if you tried to include
nm-ip4-config.h in a libnm test, it would fail to compile because
nm-test-utils.h was referring to symbols in src/nm-ip4-config.h).
Fix this by changing the include guards in the non-API-stable parts of
the tree:
- libnm-glib/nm-ip4-config.h remains NM_IP4_CONFIG_H
- libnm/nm-ip4-config.h now uses __NM_IP4_CONFIG_H__
- src/nm-ip4-config.h now uses __NETWORKMANAGER_IP4_CONFIG_H__
And likewise for all other headers.
The two non-"nm"-prefixed headers, libnm/NetworkManager.h and
src/NetworkManagerUtils.h are now __NETWORKMANAGER_H__ and
__NETWORKMANAGER_UTILS_H__ respectively, which, while not entirely
consistent with the general scheme, do still mostly make sense in
isolation.
"NetworkManager.h"'s name (and non-standard capitalization) suggest
that it's some sort of high-level super-important header, but it's
really just low-level D-Bus stuff. Rename it to "nm-dbus-interface.h"
and likewise "NetworkManagerVPN.h" to "nm-vpn-dbus-interface.h"
For some reason, the flags used by o.fd.NM.SecretAgent.GetSecrets were
defined as both NMSecretAgentGetSecretsFlags in
libnm{,-glib}/nm-secret-agent.h, and then separately as
NMSettingsGetSecretsFlags in include/nm-settings-flags.h.
(NMSettingsGetSecretsFlags also had an additional internal-use-only
value, but that was added later after the duplication already
existed.)
Fix this by moving NMSecretAgentGetSecretsFlags from libnm to
nm-dbus-interface.h, adding the internal-use-only value to it as well,
updating the core code to use that, and then removing
nm-settings-flags.h.
GLib/Gtk have mostly settled on the convention that two-letter
acronyms in type names remain all-caps (eg, "IO"), but longer acronyms
become initial-caps-only (eg, "Tcp").
NM was inconsistent, with most long acronyms using initial caps only
(Adsl, Cdma, Dcb, Gsm, Olpc, Vlan), but others using all caps (DHCP,
PPP, PPPOE, VPN). Fix libnm and src/ to use initial-caps only for all
three-or-more-letter-long acronyms (and update nmcli and nmtui for the
libnm changes).
This bug has no real consequense, because the numerical values of the
enum values are identical.
clang warns:
make[4]: Entering directory `./NetworkManager/src'
CC nm-vpn-connection.lo
vpn-manager/nm-vpn-connection.c:179:10: error: implicit conversion from enumeration type 'VpnState' to different enumeration type 'NMVPNConnectionState' (aka 'enum NMVPNConnectionState') [-Werror,-Wenum-conversion]
return STATE_UNKNOWN;
~~~~~~ ^~~~~~~~~~~~~
Signed-off-by: Thomas Haller <thaller@redhat.com>
Clean up some of the cross-includes between headers (which made it so
that, eg, if you included NetworkManagerUtils.h in a test program, you
would need to build the test with -I$(top_srcdir)/src/platform, and if
you included nm-device.h you'd need $(POLKIT_CFLAGS)) by moving all
GObject struct definitions for src/ and src/settings/ into nm-types.h
(which already existed to solve the NMDevice/NMActRequest circular
references).
Update various .c files to explicitly include the headers they used to
get implicitly, and remove some now-unnecessary -I options from
Makefiles.
Remove all remaining GParamSpec name and blurb strings (and fix
indentation while we're there), and add G_PARAM_STATIC_STRINGS to all
paramspecs that were lacking it.
Since the event loop isn't running on quit, but we want to ensure that
scripts can fully process the DOWN event, block on scripts completing
when disconnecting the VPN when quitting.
On shutdown we can't defer the response to a callback, so we need to
use synchronous D-Bus calls. Second, sometimes we want to block on
the dispatcher response, like for pre-down.
Future patches will allow blocking dispatcher calls, which we
don't want to happen when deactivating a VPN connection during
normal operation. So move code that stops VPN connections outside
of the VPNService object's dispose() function and require the object
that owns the VPNService (the VPNManager) to stop connections at the
right times.
When quitting, blocking calls are acceptable (because NetworkManager's
D-Bus interface is no longer useful, plus we can't easily schedule
callbacks because no mainloop is running), so it's ok to stop
connections from NMVPNManager:dispose.
D-Bus already watches the life-cycle, and we'll get a NameOwnerChanged
signal when the VPN service daemon quit. So the GLib child watch is
just duplicated code that we don't need. Remove it.
Since NM_VPN_CONNECTION_STATE_* are linear in number and defined API,
we can't add a DEACTIVATING state in the right place. So create a
NMVPNConnection internal VPN state and convert that to the old
NM_VPN_CONNECTION_STATE_* numbers at exit points from the object.
(This will also allow us to get rid of priv->secrets_req in the future
and fold that into the VPN states)
Instead of telling a VPN service to quit, leave that up to the service
itself. Services based on libnm-glib-vpn already have a quit timeout
of 20 seconds. We also eventually want to D-Bus activate the VPN
services, and at that point we won't have a PID we can send signals
to.
Instead of creating most routes with metric 0 and then fixing them
just before applying them, create the routes with the correct metric
in the first place (so that NMIP4Config and NMIP6Config don't have to
try to guess whether "metric 0" means "unset" or "actually metric 0").
f3c2851c (port NMAgentManager, etc, to use NMAuthSubject) made it
unnecessary for callers to nm_settings_connection_get_secrets() to
decide whether to filter agents by UID or not, but NMVPNConnection was
still doing it itself anyway.
Tag addresses and routes with their source. We'll use this later to do
(or not do) operations based on where the item came from.
One thing to note is that when synchronizing items with the kernel, all
items are read as source=KERNEL even when they originally came from
NetworkManager, since the kernel has no way of providing this source
information. This requires the source 'priority', which
nm_ip*_config_add_address() and nm_ip*_config_add_route() must respect
to ensure that NM-owned routes don't have their source overwritten
when merging various IP configs in ip*_config_merge_and_apply().
Also of note is that memcmp() can no longer be used to compare
addresses/routes in nm-platform.c, but this had problems before
anyway with ifindex, so that workaround from nm_platform_ip4_route_sync()
can be removed.
https://bugzilla.gnome.org/show_bug.cgi?id=722843https://bugzilla.redhat.com/show_bug.cgi?id=1005416
NMActiveConnection was categorizing all deactivation of master
connections as "failure", and NMActRequest was deactivating all of the
master's slaves with REASON_DEPENDENCY_FAILED no matter what the real
reason was.
In fact, NMActiveConnection only needs to handle the cases where the
master fails before enslaving the device; any failure after that point
will be caught by existing master/slave checks in NMDevice. So update
the code accordingly (and remove the master_failed code from
NMVpnConnection entirely, since no master supports having VPN slaves).
Add IP and DHCP config properties to the D-Bus ActiveConnection
objects.
For device connections, this is redundant with the properties already
on the Device object, but for VPN connections, this information was
not previously available.
Rather than explicitly passing around a UID and a flag saying whether
or not it's relevant.
(This also fixes a bug where the wrong UID was being recorded in
nm-settings-connection.c::auth_start(), which caused problems such as
agent-owned secrets not getting saved because of a perceived UID
mismatch.)
In the migration to NMPlatform, support for ptp/peer addresses was
accidentally dropped. This broke OpenVPN configurations using 'p2p'
topology, which send a different peer address than the local address
for tunX, plus the server may also push routes that use the peer
address as the next hop. NetworkManager was unable to add these
routes, because the kernel had no idea how to talk to the peer,
because the peer's address was not assigned to any interface or
reachable over any routes.
Partly based on a patch from Dan Williams.
These are (most likely) only warnings and not severe bugs.
Some of these changes are mostly made to get a clean run of
Coverity without any warnings.
Error found by running Coverity scan
https://bugzilla.redhat.com/show_bug.cgi?id=1025894
Co-Authored-By: Jiří Klimeš <jklimes@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Both NMActRequest and NMVPNConnection need to track their device's state,
so instead of both subclasses having to do so, consolidate that code into
the superclass.
