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

_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Reply via email to