We must check if a conflicting route/address is configured on
*any* interface, not only on the target ifindex.
This at least restores the previous behavior. Note that this whole
check_reinstall_device_route() still has problems and it might
be better to move it all to NMRouteManager.
Fixes: 470bcefa5f
After doing all the refactoring, rename the ObjectType enum to NMPObjectType.
I didn't do this earlier to avoid problems with rebasing. But since I will
backport the platform changes to nm-1-0, this is the right time to get
the name right.
warning: function declaration isn’t a prototype [-Wstrict-prototypes]
In C function() and function(void) are two different prototypes (as opposed to
C++).
function() accepts an arbitrary number of arguments
function(void) accepts zero arguments
AUTOCONNECT_SLAVES is an NetworkManager extension. initscripts always activate
slaves with the master connection for bond and team, and doesn't activate
automatically slaves for bridge.
NetworkManager behaviour is controlled by this variable. If the variable is
missing the default value from configuration file is used.
The property is used for controlling whether slaves should be brought up with
a master connection. If 0, activating the master will not activate slaves.
But if set to 1, activating the master will bring up slaves as well.
The property can have the third state (-1), meaning that the value is default.
That is either a value set in the configuration file for the property, or 0.
We already protected route-metrics that are configured as default-routes
in platform. For most cases, that list is identical to our internal list
of non-synced routes.
But if for some reason that is not the case, we must also protect the
metric of routs that we currently track as "non-synced".
After refactoring platform, nm_platform_ipx_route_get_all() and
nm_platform_ipx_address_get_all() was broken for calling with a
non-posititive ifindex (which has the meaning: ignore ifindex).
While fixing that, also refactor the NMPCacheIdType so that it matches
better the supported id-types.
Fixes: 470bcefa5f
When inside a state-change, we set for example the device up.
This triggers a link-changed event, which then causes further
state-changes of the devices.
A state-change in process of a device is not reentrant, so we must
delay the handling of the link-changed event.
nm_utils_get_shared_wifi_permission() that is called for each AddAndActivate
uses this and triggers an assertion failure for InifnibandConnections which
don't have IPv4 configuration:
NetworkManager[54006]: nm_utils_get_ip_config_method: assertion 's_ip4 != NULL' failed
#2 0x000055555562b833 in nm_utils_get_ip_config_method (connection=connection@entry=0x5555559c3b60, ip_setting_type=93824996202304) at NetworkManagerUtils.c:1329
#3 0x000055555562b914 in nm_utils_get_shared_wifi_permission (connection=0x5555559c3b60) at NetworkManagerUtils.c:1105
#4 0x00005555555fc012 in nm_active_connection_authorize (self=0x5555559b30a0, result_func=0x55555561b980 <_add_and_activate_auth_done>, user_data1=0x555555957210, user_data2=0x555555a055f0)
at nm-active-connection.c:683
#5 0x0000555555621b21 in impl_manager_add_and_activate_connection (self=0x555555957210, settings=<optimized out>, device_path=<optimized out>, specific_object_path=0x0, context=0x555555a055f0)
at nm-manager.c:3495
For NMPlatform instances we had an error reporting mechanism
which stores the last error reason in a private field. Later we
would check it via nm_platform_get_error().
Remove this. It was not used much, and it is not a great way
to report errors.
One problem is that at the point where the error happens, you don't
know whether anybody cares about an error code. So, you add code to set
the error reason because somebody *might* need it (but in realitiy, almost
no caller cares).
Also, we tested this functionality which is hardly used in non-testing code.
While this was a burden to maintain in the tests, it was likely still buggy
because there were no real use-cases, beside the tests.
Then, sometimes platform functions call each other which might overwrite the
error reason. So, every function must be cautious to preserve/set
the error reason according to it's own meaning. This can involve storing
the error code, calling another function, and restoring it afterwards.
This is harder to get right compared to a "return-error-code" pattern, where
every function manages its error code independently.
It is better to return the error reason whenever due. For that we already
have our common glib patterns
(1) gboolean fcn (...);
(2) gboolean fcn (..., GError **error);
In few cases, we need more details then a #gboolean, but don't want
to bother constructing a #GError. Then we should do instead:
(3) NMPlatformError fcn (...);
Unconditionally log a warning if the function fails.
We are about to drop nm_platform_get_error(), it's anyway unclear
why we don't want to log a warning about non-existing interface.
Later remove nm_platform_get_error() and signal errors via return
error codes.
Also, fix nm_platform_infiniband_partition_add() and
nm_platform_vlan_add() to check the type of the existing link
and fail with WRONG_TYPE otherwise.
- rename "NONE" to "SUCCESS", what it really is.
- change the to-string result not to contain spaces
and being closer the name of the enum value.
- add new error reasons "UNSPECIFIED" and "BUG".
- remove the code comments around the enum definition.
They add no further description about why this error
happens and only paraphrase the name of the enum.
- reserve negative integers for 'errno'. This is neat
because if we get a system error we can pass on the
underlying errno as cause.
After hiding the udi field, there are no more users of NMRefString.
Remove the code by explitly reverting the patch so that in case of a future
need, we can find and resurrect NMRefString.
This reverts commit 430658b17a.
The @udi field is not a static string, so any user of a NMPlatformLink
instance must make sure not to use the field beyond the lifetime of the
NMPlatformLink instance.
As we pass on the platform link instance during platform changed events,
this is hard to ensure for the subscriber of the signal -- because a
call back into platform could invalidate/modify the object.
Just not expose this field as part of the link instance. The few callers
who actually needed it should instead call nm_platform_get_uid(). With
that, the lifetime of the returned 'const char *' pointer is clearly
defined.
Use the event socket to request object via NLM_F_DUMP.
No longer use 'priv->nlh' socket to fetch objects.
Instead fetch them via the priv->nlh_event socket that also
provides asynchronous events when objects change.
That way, the events are in sync with our explicit requests
and we can directly use the events. Previously, the events were
only used to indicate that a refetch must happen, so that every
event triggered a complete dump of all addresses/routes.
We still use 'priv->nlh' to make synchronous requests such as
adding/changing/deleting objects. That means, after we send a
request, we must make sure that the result manifested itself
at 'nlh_event' socket and the platform cache.
That's why we sometimes still must force a dump to sync changes.
That could be improved by using only one netlink socket so that
we would wait for the ACK of our request.
While not yet perfect, this already significantly reduces the number of
fetches. Additionally, before, whenever requesting a dump of addresses
or routes (which we did much more often, search for "get_kernel_object for type"
log lines), we always dumped IPv4 and IPv6 together. Now only request
the addr-family in question.
https://bugzilla.gnome.org/show_bug.cgi?id=747985https://bugzilla.redhat.com/show_bug.cgi?id=1211133
Add a construct-only property NM_PLATFORM_REGISTER_SINGLETON to NMPlatform.
When set to TRUE, the constructor will self-register to nm_platform_setup().
The reason for this is that the _LOG() macro in NMLinuxPlatform logs the
self pointer if the instance is not the singleton instance.
During construction, we already have many log lines due to initialization
of the instance. These lines all end up qualified with the self pointer.
By earlier self-registering, printing the pointer value is omitted.
Yes, this patch is really just to prettify logging.
