Add nm_utils_invoke_on_timeout() beside nm_utils_invoke_on_idle().
They are fundamentally similar, except one schedules an idle handler
and the other a timeout.
Also, use the current g_main_context_get_thread_default() as context
instead of the singleton instance. That is a change in behavior, but
the only caller of nm_utils_invoke_on_idle() is the daemon, which
doesn't use different main contexts. Anyway, to avoid anybody being
tripped up by this also change the order of arguments. It anyway
seems nicer to first pass the cancellable, and the callback and user
data as last arguments. It's more in line with glib's asynchronous
methods.
Also, in the unlikely case that the cancellable is already cancelled
from the start, always schedule an idle action to complete fast.
There was only API to schedule the stage on an idle handler.
Sometimes, we are just in the right situation to schedule the stage
right away. It should be possibly to avoid going through the extra hop.
For now, none of the caller makes use of this. So, there isn't any
actual change in behavior. But by adding this possibility, we may do
use in the future.
The _GET_PRIVATE() macros are all implemented based on
_NM_GET_PRIVATE(). That macro tries to be more type safe and uses
_Generic() to do the right thing. Explicitly casting is not only
unnecessary, it defeats these (static) type checks.
Don't do that.
Most callers would pass FALSE to nm_utils_error_is_cancelled(). That's
not very useful. Split the two functions and have nm_utils_error_is_cancelled()
and nm_utils_error_is_cancelled_is_disposing().
It seems to complicate things more than helping. Drop it. What we still have
is a wrapper around plain g_dbus_connection_signal_subscribe(). That one is
trivial and helpful. The previous wrapper seems to add more complexity.
nm_dbus_connection_signal_subscribe_object_manager() wraps the subscription. The problem
is that this requires to pass a destroy notify function for cleaning up. Such a destroy
notify function will result in an idle source when unsubscribing, which keeps the associated
GMainContext alive (until it gets iterated some more). That seems error prone and outright
unsuitable for NMClient.
While the helper may be useful, it cannot be used by NMClient. So, there is only one
user of this function and we don't expect a second one. It seems better to get rid of
this wrapper and implement it directly.
The abbreviations "ns" and "ms" seem not very clear to me. Spell them
out to nsec/msec. Also, in parts we already used the longer abbreviations,
so it wasn't consistent.
The Bluetooth DUN device's NMModem would signal the reset of ifindex to zero
when it's disconnected and the NMDeviceBt would accordingly update the
bluetooth device's ip ifindex. This is not okay since commit ab4578302d
('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()') which,
although claiming to be a refactoring, made such use of
nm_device_set_ip_ifindex() illegal. Resetting the ifindex is anyway not
necessary, since it's taken care of _cleanup_generic_post().
Let's leave the ifindex alone once the device is activated, in a manner
analogous to what NMDeviceModem.
Fixes: ab4578302d ('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()')
Fixes: 78ca2a70c7 ('device: don't set invalid ip-iface'):
The devices tests' meson build files include only the build of a
single executable file and its execution as a test unit.
This has been moved to the devices' main meson build files so this
files can be removed.
In 878d4963e a new `nm-bt-test` helper program was added. However,
although `autotools` build steps were included, meson build steps
were not.
This add meson's build steps.
The `libnm-core` build file has been improved by applying a set of
changes:
- Indentation has been fixed to be consistent.
- Library variable names have been changed to `lib{name}` pattern
following their filename pattern.
- `shared` prefix has been removed from all variables using it.
- Dependencies have been reviewed to store the necessary data.
- The use of the libraries and dependencies created in this file
has been reviewed through the entire source code. This has
required the addition or the removal of different libraries and
dependencies in different targets.
- Some files used directly with the `files` function have been moved
to their nearest path build file because meson stores their full
path seamessly and they can be used anywhere later.
The `nm-default.h` header is used widely in the code by many
targets. This header includes different headers and needs different
libraries depending the compilation flags.
A new set of `*nm_default_dep` dependencies have been created to
ease the inclusion of different directorires and libraries.
This allows cleaner build files and avoiding linking unnecessary
libraries so this has been applied allowing the removal of some
dependencies involving the linking of unnecessary libraries.
This is a complete refactoring of the bluetooth code.
Now that BlueZ 4 support was dropped, the separation of NMBluezManager
and NMBluez5Manager makes no sense. They should be merged.
At that point, notice that BlueZ 5's D-Bus API is fully centered around
D-Bus's ObjectManager interface. Using that interface, we basically only
call GetManagedObjects() once and register to InterfacesAdded,
InterfacesRemoved and PropertiesChanged signals. There is no need to
fetch individual properties ever.
Note how NMBluezDevice used to query the D-Bus properties itself by
creating a GDBusProxy. This is redundant, because when using the ObjectManager
interfaces, we have all information already.
Instead, let NMBluezManager basically become the client-side cache of
all of BlueZ's ObjectManager interface. NMBluezDevice was mostly concerned
about caching the D-Bus interface's state, tracking suitable profiles
(pan_connection), and moderate between bluez and NMDeviceBt.
These tasks don't get simpler by moving them to a seprate file. Let them
also be handled by NMBluezManager.
I mean, just look how it was previously: NMBluez5Manager registers to
ObjectManager interface and sees a device appearing. It creates a
NMBluezDevice object and registers to its "initialized" and
"notify:usable" signal. In the meantime, NMBluezDevice fetches the
relevant information from D-Bus (although it was already present in the
data provided by the ObjectManager) and eventually emits these usable
and initialized signals.
Then, NMBlue5Manager emits a "bdaddr-added" signal, for which NMBluezManager
creates the NMDeviceBt instance. NMBluezManager, NMBluez5Manager and
NMBluezDevice are strongly cooperating to the point that it is simpler
to merge them.
This is not mere refactoring. This patch aims to make everything
asynchronously and always cancellable. Also, it aims to fix races
and inconsistencies of the state.
- Registering to a NAP server now waits for the response and delays
activation of the NMDeviceBridge accordingly.
- For NAP connections we now watch the bnep0 interface in platform, and tear
down the device when it goes away. Bluez doesn't send us a notification
on D-Bus in that case.
- Rework establishing a DUN connection. It no longer uses blocking
connect() and does not block until rfcomm device appears. It's
all async now. It also watches the rfcomm file descriptor for
POLLERR/POLLHUP to notice disconnect.
- drop nm_device_factory_emit_component_added() and instead let
NMDeviceBt directly register to the WWan factory's "added" signal.
The previous function arguments of nm_modem_act_stage2_config() act as if the
function could fail or even postpone the action. It never did.
We cannot treat this generic. A caller needs to know whether nm_modem_act_stage2_config()
can postpone the action, and when it does, which signal is emitted upon completion. That
is, the caller needs to know how to proceed after postponing.
In other words, since this function currently cannot fail or postpone
the stage, so must all callers already rely on that. At this point it makes
no sense to pretend that the function could be any different, if all callers
assume it is not. Simplify the API.
NMModem-s are either used by NMDeviceModem or by NMDeviceBt.
The mechanism how that is coordinated it odd:
- the factory emits component-added, and then NMDeviceBt
might take the device (and claim it). In that case, component-added
would return TRUE to indicate that the modem should not be also
used by NMDeviceModem.
- next, if the modem has a driver that looks like bluetooth, NMDeviceModem
ignores it too.
- finally, NMDeviceModem claims the modem (which is now considered to
be non-bluetooth).
I think the first problem is that the device factory tries to have this
generic mechanism of "component-added". It's literally only used to
cover this special case. Note that NMDeviceBt is aware of modems. So,
abstracting this just adds lots of code that could be solved better
by handling the case (of giving the modem to either NMDeviceBt or
NMDeviceModem) specifically.
NMWWanFactory itself registers to the NM_MODEM_MANAGER_MODEM_ADDED
signal and emits nm_device_factory_emit_component_added().
We could just have NMWWanFactory and NMDeviceBt both register to
that signal. Signals even support priorities, so we could have
NMDeviceBt be called first to claim the device.
Anyway, as the modem can only have one owner, the modem should have
a flag that indicates whether it's claimed or not. That will allow
multiple components all look at the same modem and moderate who is
going to take ownership.
BlueZ 5.0 was released in December 2012 and broke API with
BlueZ 4. NetworkManager supports Bluez 5 for years already.
Of course, version 4 is long gone by now, so remove it.
It should be possible to add a profile with autoconnect blocked form the
start. Update2() has a %NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT flag to
block autoconnect, and so we need something similar when adding a connection.
As the existing AddConnection() and AddConnectionUnsaved() API is not
extensible, add AddConnection2() that has flags and room for additional
arguments.
Then add and implement the new flag %NM_SETTINGS_ADD_CONNECTION2_FLAG_BLOCK_AUTOCONNECT
for AddConnection2().
Note that libnm's nm_client_add_connection2() API can completely replace
the existing nm_client_add_connection_async() call. In particular, it
will automatically prefer to call the D-Bus methods AddConnection() and
AddConnectionUnsaved(), in order to work with server versions older than
1.20. The purpose of this is that when upgrading the package, the
running NetworkManager might still be older than the installed libnm.
Anyway, so since nm_client_add_connection2_finish() also has a result
output, the caller needs to decide whether he cares about that result.
Hence it has an argument ignore_out_result, which allows to fallback to
the old API. One might argue that a caller who doesn't care about the
output results while still wanting to be backward compatible, should
itself choose to call nm_client_add_connection_async() or
nm_client_add_connection2(). But instead, it's more convenient if the
new function can fully replace the old one, so that the caller does not
need to switch which start/finish method to call.
https://bugzilla.redhat.com/show_bug.cgi?id=1677068
Completely rework how settings plugin handle connections and how
NMSettings tracks the list of connections.
Previously, settings plugins would return objects of (a subtype of) type
NMSettingsConnection. The NMSettingsConnection was tightly coupled with
the settings plugin. That has a lot of downsides.
Change that. When changing this basic relation how settings connections
are tracked, everything falls appart. That's why this is a huge change.
Also, since I have to largely rewrite the settings plugins, I also
added support for multiple keyfile directories, handle in-memory
connections only by keyfile plugin and (partly) use copy-on-write NMConnection
instances. I don't want to spend effort rewriting large parts while
preserving the old way, that anyway should change. E.g. while rewriting ifcfg-rh,
I don't want to let it handle in-memory connections because that's not right
long-term.
--
If the settings plugins themself create subtypes of NMSettingsConnection
instances, then a lot of knowledge about tracking connections moves
to the plugins.
Just try to follow the code what happend during nm_settings_add_connection().
Note how the logic is spread out:
- nm_settings_add_connection() calls plugin's add_connection()
- add_connection() creates a NMSettingsConnection subtype
- the plugin has to know that it's called during add-connection and
not emit NM_SETTINGS_PLUGIN_CONNECTION_ADDED signal
- NMSettings calls claim_connection() which hocks up the new
NMSettingsConnection instance and configures the instance
(like calling nm_settings_connection_added()).
This summary does not sound like a lot, but try to follow that code. The logic
is all over the place.
Instead, settings plugins should have a very simple API for adding, modifying,
deleting, loading and reloading connections. All the plugin does is to return a
NMSettingsStorage handle. The storage instance is a handle to identify a profile
in storage (e.g. a particular file). The settings plugin is free to subtype
NMSettingsStorage, but it's not necessary.
There are no more events raised, and the settings plugin implements the small
API in a straightforward manner.
NMSettings now drives all of this. Even NMSettingsConnection has now
very little concern about how it's tracked and delegates only to NMSettings.
This should make settings plugins simpler. Currently settings plugins
are so cumbersome to implement, that we avoid having them. It should not be
like that and it should be easy, beneficial and lightweight to create a new
settings plugin.
Note also how the settings plugins no longer care about duplicate UUIDs.
Duplicated UUIDs are a fact of life and NMSettings must handle them. No
need to overly concern settings plugins with that.
--
NMSettingsConnection is exposed directly on D-Bus (being a subtype of
NMDBusObject) but it was also a GObject type provided by the settings
plugin. Hence, it was not possible to migrate a profile from one plugin to
another.
However that would be useful when one profile does not support a
connection type (like ifcfg-rh not supporting VPN). Currently such
migration is not implemented except for migrating them to/from keyfile's
run directory. The problem is that migrating profiles in general is
complicated but in some cases it is important to do.
For example checkpoint rollback should recreate the profile in the right
settings plugin, not just add it to persistent storage. This is not yet
properly implemented.
--
Previously, both keyfile and ifcfg-rh plugin implemented in-memory (unsaved)
profiles, while ifupdown plugin cannot handle them. That meant duplication of code
and a ifupdown profile could not be modified or made unsaved.
This is now unified and only keyfile plugin handles in-memory profiles (bgo #744711).
Also, NMSettings is aware of such profiles and treats them specially.
In particular, NMSettings drives the migration between persistent and non-persistent
storage.
Note that a settings plugins may create truly generated, in-memory profiles.
The settings plugin is free to generate and persist the profiles in any way it
wishes. But the concept of "unsaved" profiles is now something explicitly handled
by keyfile plugin. Also, these "unsaved" keyfile profiles are persisted to file system
too, to the /run directory. This is great for two reasons: first of all, all
profiles from keyfile storage in fact have a backing file -- even the
unsaved ones. It also means you can create "unsaved" profiles in /run
and load them with `nmcli connection load`, meaning there is a file
based API for creating unsaved profiles.
The other advantage is that these profiles now survive restarting
NetworkManager. It's paramount that restarting the daemon is as
non-disruptive as possible. Persisting unsaved files to /run improves
here significantly.
--
In the past, NMSettingsConnection also implemented NMConnection interface.
That was already changed a while ago and instead users call now
nm_settings_connection_get_connection() to delegate to a
NMSimpleConnection. What however still happened was that the NMConnection
instance gets never swapped but instead the instance was modified with
nm_connection_replace_settings_from_connection(), clear-secrets, etc.
Change that and treat the NMConnection instance immutable. Instead of modifying
it, reference/clone a new instance. This changes that previously when somebody
wanted to keep a reference to an NMConnection, then the profile would be cloned.
Now, it is supposed to be safe to reference the instance directly and everybody
must ensure not to modify the instance. nmtst_connection_assert_unchanging()
should help with that.
The point is that the settings plugins may keep references to the
NMConnection instance, and so does the NMSettingsConnection. We want
to avoid cloning the instances as long as they are the same.
Likewise, the device's applied connection can now also be referenced
instead of cloning it. This is not yet done, and possibly there are
further improvements possible.
--
Also implement multiple keyfile directores /usr/lib, /etc, /run (rh #1674545,
bgo #772414).
It was always the case that multiple files could provide the same UUID
(both in case of keyfile and ifcfg-rh). For keyfile plugin, if a profile in
read-only storage in /usr/lib gets modified, then it gets actually stored in
/etc (or /run, if the profile is unsaved).
--
While at it, make /etc/network/interfaces profiles for ifupdown plugin reloadable.
--
https://bugzilla.gnome.org/show_bug.cgi?id=772414https://bugzilla.gnome.org/show_bug.cgi?id=744711https://bugzilla.redhat.com/show_bug.cgi?id=1674545
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
It is preferable to treat IPv4 and IPv6 in a similar manner.
This moves the places where we differ down the call-stack.
It also make it clearer how IPv6 behaves differently. I think this
is a bug, but leave it for now.
+ /* If IP had previously failed, move it back to IP_CONF since we
+ * clearly now have configuration.
+ */
+ if (priv->ip6_state == IP_FAIL)
+ _set_ip_state (self, AF_INET6, IP_CONF);
(cherry picked from commit 1585eaf473)
The 'number' property in GSM settings is a legacy thing that comes
from when ModemManager used user-provided numbers, if any, to connect
3GPP modems.
Since ModemManager 1.0, this property is completely unused for 3GPP
modems, and so it doesn't make sense to use it in the NetworkManager
settings. Ofono does not use it either.
For AT+PPP-based 3GPP modems, the 'number' to call to establish the
data connection is decided by ModemManager itself, e.g. for standard
GSM/UMTS/LTE modems it will connect a given predefined PDP context,
and for other modems like Iridium it will have the number to call
hardcoded in the plugin itself.
https://github.com/NetworkManager/NetworkManager/pull/261
