On Tue, 2016-03-08 at 12:02 +0100, Beniamino Galvani wrote:
During startup, when a link is detected (enp0s25 in the example below) we try to create also virtual devices (ipip1) on it through system_create_virtual_device(), however this realizes only devices for connections which can autoactivate. To support the assumption of child devices with autoconnect=no, we should take in consideration in retry_connections_for_parent_device() only connections for which the link does not exist, and let existing links be handled by platform_link_added(), which also realizes them. Reproducer: $ nmcli c add type ip-tunnel ifname ipip1 con-name ipip1+ autoconnect no \ mode ipip remote 172.25.16.1 dev enp0s25 ip4 1.2.3.4/31 $ nmcli c up ipip1+ $ systemctl restart NetworkManager Result: * before: ipip1+ is not assumed, ipip1 is not present in 'nmcli d' output * after: ipip1+ is assumed, ipip1 detected --- src/nm-manager.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index e7eb7c6..625a235 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1127,23 +1127,31 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) static void retry_connections_for_parent_device (NMManager *self, NMDevice *device) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GSList *connections, *iter; + gs_free_error GError *error = NULL; + gs_free char *ifname = NULL;
should be declared inside the loop, otherwise as you iterate they get reset and leak (or fail assertion g_return_if_fail (!error || !*error)).
g_return_if_fail (device); connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { NMConnection *candidate = iter->data; NMDevice *parent; parent = find_parent_device_for_connection (self, candidate, NULL); - if (parent == device) - connection_changed (priv->settings, candidate, self); + if (parent == device) { + /* Only try to activate devices that don't already exist */ + ifname = nm_manager_get_connection_iface (self, candidate, &parent, &error); + if (ifname) { + if (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, ifname)) + connection_changed (priv-settings, candidate, self);+ } + } } g_slist_free (connections); } static void
otherwise, lgtm. Thomas
Attachment:
signature.asc
Description: This is a digitally signed message part