Reduce the use of NM_PLATFORM_GET / nm_platform_get() to get
the platform singleton instance.
For one, this is a step towards supporting namespaces, where we need
to use different NMNetns/NMPlatform instances depending on in which
namespace the device lives.
Also, we should reduce our use of singletons. They are difficult to
coordinate on shutdown. Instead there should be a clear order of
dependencies, expressed by owning a reference to those singelton
instances. We already own a reference to the platform singelton,
so use it and avoid NM_PLATFORM_GET.
(cherry picked from commit 94d9ee129d)
This argument is only relevant when the NMActStageReturn argument
indicates NM_ACT_STAGE_RETURN_FAILURE. In all other cases it is ignored.
Rename the argument to make the meaning clearer. The argument is passed
through several layers of code, it isn't obvious that this argument only
matters for the failure case. Also, the distinct name makes it easier
to distinguish from other uses of the "reason" name.
While at it, do some drive-by cleanup:
- use g_return_*() instead of g_assert() to have a more graceful
assertion.
- functions like dhcp4_start() don't need to return a failure reason.
Most callers don't care, and the caller who does can determine the
proper reason.
- allow omitting the out-argument via NM_SET_OUT().
Instead of overwriting ip4_config_pre_commit(), add a new function
get_mtu().
This also adds a default value in case there is no user-configuration.
This will allow us later to reset a default MTU based on the device
type.
Most implementations of realize_start_notify() do the same
for link_changed().
Let NMDevice's base implementation of realize_start_notify() call
link_changed() -- which by default does notthing. This allows subclasses
to only overwrite link_changed().
This makes it easier to install the files with proper names.
Also, it makes the makefile rules slightly simpler.
Lastly, the documentation is now generated into docs/api, which makes it
possible to get rid of the awkward relative file names in docbook.
Keep the include paths clean and separate. We use directories to group source
files together. That makes sense (I guess), but then we should use this
grouping also when including files. Thus require to #include files with their
path relative to "src/".
Also, we build various artifacts from the "src/" tree. Instead of having
individual CFLAGS for each artifact in Makefile.am, the CFLAGS should be
unified. Previously, the CFLAGS for each artifact differ and are inconsistent
in which paths they add to the search path. Fix the inconsistency by just
don't add the paths at all.
An interface would make sense to allow the actual device-factory to inherit
from another type.
However, glib interfaces make code much harder to follow and less
efficient. The device factory shall be a very simple type with meta data
about supported device types and the ability to create device instances.
There is no need to make this an interface implementation, instead just
let the factories inherit from NM_TYPE_DEVICE_FACTORY directly.
- use _NM_GET_PRIVATE() and _NM_GET_PRIVATE_PTR() everywhere.
- reorder statements, to have GObject related functions (init, dispose,
constructed) at the bottom of each file and in a consistent order w.r.t.
each other.
- unify whitespaces in signal and properties declarations.
- use NM_GOBJECT_PROPERTIES_DEFINE() and _notify()
- drop unused signal slots in class structures
- drop unused header files for device factories
When a user want to explicitly spoof the MAC address, a failure
to do so should fail activation. For one, failing to do so may
be a security problem. In any case, if user asks to configure the
interface in a certain way and we fail to do so that shall result
in a failure to activate.
Extend the "ethernet.cloned-mac-address" and "wifi.cloned-mac-address"
settings. Instead of specifying an explicit MAC address, the additional
special values "permanent", "preserve", "random", "random-bia", "stable" and
"stable-bia" are supported.
"permanent" means to use the permanent hardware address. Previously that
was the default if no explict cloned-mac-address was set. The default is
thus still "permanent", but it can be overwritten by global
configuration.
"preserve" means not to configure the MAC address when activating the
device. That was actually the default behavior before introducing MAC
address handling with commit 1b49f941a6.
"random" and "random-bia" use a randomized MAC address for each
connection. "stable" and "stable-bia" use a generated, stable
address based on some token. The "bia" suffix says to generate a
burned-in address. The stable method by default uses as token the
connection UUID, but the token can be explicitly choosen via
"stable:<TOKEN>" and "stable-bia:<TOKEN>".
On a D-Bus level, the "cloned-mac-address" is a bytestring and thus
cannot express the new forms. It is replaced by the new
"assigned-mac-address" field. For the GObject property, libnm's API,
nmcli, keyfile, etc. the old name "cloned-mac-address" is still used.
Deprecating the old field seems more complicated then just extending
the use of the existing "cloned-mac-address" field, although the name
doesn't match well with the extended meaning.
There is some overlap with the "wifi.mac-address-randomization" setting.
https://bugzilla.gnome.org/show_bug.cgi?id=705545https://bugzilla.gnome.org/show_bug.cgi?id=708820https://bugzilla.gnome.org/show_bug.cgi?id=758301
When the entire NMSettingWired setting is missing, it should be treated
exactly the same as each property having the default/unset value.
Otherwise, adding a NMSettingWired setting only to set (say) MTU,
would result in different behavior. Although effectively the
"cloned-mac-address" shall be in both cases the same.
Software devices must report the NM_DEVICE_CAP_IS_SOFTWARE capability
in order to be properly activated. Add the flag to NMDeviceTun and
NMDeviceVxlan.
https://bugzilla.gnome.org/show_bug.cgi?id=767846
Instead of accessing the singleton getter nm_settings_get(), obtain
the settings instance from the device instance itself via
nm_device_get_settings().
Don't rely on what's already on the device. It could be that the MAC address
set on the device is not meaningful -- the NM crashed while two devices were
teamed together and now they have the same hardware address and now it's
impossible to bond them with mode=5.
- 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
When we decide to add a new link, we alredy checked that no such link exists
(ignoring race conditions).
It is wrong to accept a EXITS failure when adding the link. There is no guarantee
that the existing link has all the same properties as the one we intend to add.
More importantly, this link was added externally outside of NetworkManager and it
should not be taken over.
Just treat EXISTS as a failure as any other.
The virtual function NMDevice:realize() is only called by
nm_device_realize() and immediately followed by nm_device_setup_start().
Devices already overwrite setup_start_notify() to update their properties.
No need to duplicate that in realize().
All implementations of NMDevice:setup_start() in derived classes
invoke the parent implementation first. Enforce that by moving
NMDevice:setup_start() to realize_start_setup() and only notify
derived classes afterwards via NMDevice:realize_start_notify().
Let the link-add functions return the internal pointer to the platform
link object. Similar to link-get, which doesn't copy the link either.
Also adjust the sole users of the add-functions (create-and-realize)
to take the pointer.
Eventually we still copy the returned data, because accessing platform can
invalidate the returned pointer. Thus we don't actually safe any copying
by this (at least every use of the function currently leads to the data
being copied).
Still change it, because I think the API of NMPlatform should look like that.
NMPlatformLink is a plain struct (not a GObject, for which we usually
don't use const). We certainly don't want the functions to modify the
passed-in data.
Ensure the platform link with the same interface name as the
NMDevice is actually compatible with it before using the link
for initialization of device properties. If not, remove the
NMDevice and create a new one since there are kernel resources
with a different type.
Unrealized devices aren't backed by kernel resources and so won't know
all of their attributes. That means three things:
1) they must update their attributes when they become realized
2) they must clear those attributes when unrealized
3) they must be looser in checking compatible connections until
they are realized
This requires that the setup() function be split into two parts, start & finish,
because finish must be run after add_device()
Also, we can simplify whether to pay attention to 'recheck-assume', which
is now dependent on priv->is_nm_owned, because the only case where NM should
*not* listen for the 'recheck-assume' signal is when the device is a
software device created by NM itself. That logic was previously spread
across the callers of add_device() but is now consolidated into
nm-manager.c::device_realized() and nm-device.c::nm_device_create_and_realize().
Future patches will create devices long before they are backed by
kernel resources, so we need to split NMDevice object creation from
actual setup based on the backing resources.
This patch combines the NMDeviceFactory's new_link() and
create_virtual_device_for_connection() class methods into a single
create_device() method that simply creates an unrealized NMDevice
object; this method is not expected to fail unless the device is
supposed to be ignored. This also means that the NMDevice
'platform-device' property is removed, because a platform link
object may not be available at NMDevice object creation time.
After the device is created, it is then "realized" at some later
time from a platform link (for existing/hardware devices via the
realize() method) or from an NMConnection (for newly created software
devices via the create_and_realize() NMDeviceClass methods).
https://bugzilla.gnome.org/show_bug.cgi?id=737458
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
Add NMExportedObject, make it the base class of all D-Bus-exported
types, and move the nm-properties-changed-signal logic into it. (Also,
make NMSettings use the same properties-changed code as everything
else, which it was not previously doing, presumably for historical
reasons).
(This is mostly just shuffling code around at this point, but
NMExportedObject will be more important in the gdbus port, since
gdbus-codegen doesn't do a very good job of supporting objects that
export multiple interfaces [as each NMDevice subclass does, for
example], so we will need more glue/helper code in NMExportedObject
then.)
Instead of hacky stuff in the Manager, let plugins themselves indicate
which links should be ignored (because they are really child links that
are controlled by a different device that the plugin handles).
Instead of looping over all plugins and asking each plugin whether it
can handle a link or a connection, have them advertise the link and
connection types they support, and use that when creating new devices.
Most nm_platform_*() functions operate on the platform
singleton nm_platform_get(). That made sense because the
NMPlatform instance was mainly to hook fake platform for
testing.
While the implicit argument saved some typing, I think explicit is
better. Especially, because NMPlatform could become a more usable
object then just a hook for testing.
With this change, NMPlatform instances can be used individually, not
only as a singleton instance.
Before this change, the constructor of NMLinuxPlatform could not
call any nm_platform_*() functions because the singleton was not
yet initialized. We could only instantiate an incomplete instance,
register it via nm_platform_setup(), and then complete initialization
via singleton->setup().
With this change, we can create and fully initialize NMPlatform instances
before/without setting them up them as singleton.
Also, currently there is no clear distinction between functions
that operate on the NMPlatform instance, and functions that can
be used stand-alone (e.g. nm_platform_ip4_address_to_string()).
The latter can not be mocked for testing. With this change, the
distinction becomes obvious. That is also useful because it becomes
clearer which functions make use of the platform cache and which not.
Inside nm-linux-platform.c, continue the pattern that the
self instance is named @platform. That makes sense because
its type is NMPlatform, and not NMLinuxPlatform what we
would expect from a paramter named @self.
This is a major diff that causes some pain when rebasing. Try
to rebase to the parent commit of this commit as a first step.
Then rebase on top of this commit using merge-strategy "ours".
