From 72de0afa35c88d40eda87855aee4a80034bef765 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 17 Aug 2018 22:18:17 +0200 Subject: [PATCH] device: refactor setting parent in device's update_connection() Add a helper function nm_device_parent_find_for_connection() to unify implementations of setting the parent in update_connection(). There is some change in behavior, in particular for nm-device-vlan.c, which no longer compares the link information from platform. But update_connection() is anyway a questionable concept, only used for external assumed connection (which itself, is questionable). Meaning, update_connection() is a hack not science, and it's not at all clear what the correct behavior is. Also, note how vlan's implementation differs from all others. Why? Should we always resort to also check the information from platform? Either way, one of the two approaches should be used consistently and nm_device_parent_find_for_connection() opts to not consult platform cache. --- src/devices/nm-device-6lowpan.c | 26 +++++------------------ src/devices/nm-device-ip-tunnel.c | 26 +++++------------------ src/devices/nm-device-macvlan.c | 25 +++++----------------- src/devices/nm-device-vlan.c | 27 +++++------------------- src/devices/nm-device-vxlan.c | 25 +++++----------------- src/devices/nm-device.c | 35 +++++++++++++++++++++++++++++++ src/devices/nm-device.h | 3 +++ 7 files changed, 63 insertions(+), 104 deletions(-) diff --git a/src/devices/nm-device-6lowpan.c b/src/devices/nm-device-6lowpan.c index 52dda3bf74..600d1b80b1 100644 --- a/src/devices/nm-device-6lowpan.c +++ b/src/devices/nm-device-6lowpan.c @@ -216,33 +216,17 @@ static void update_connection (NMDevice *device, NMConnection *connection) { NMSetting6Lowpan *s_6lowpan = nm_connection_get_setting_6lowpan (connection); - NMDevice *parent_device; - const char *setting_parent, *new_parent; if (!s_6lowpan) { s_6lowpan = (NMSetting6Lowpan *) nm_setting_6lowpan_new (); nm_connection_add_setting (connection, (NMSetting *) s_6lowpan); } - /* Update parent in the connection; default to parent's interface name */ - parent_device = nm_device_parent_get_device (device); - if (parent_device) { - new_parent = nm_device_get_iface (parent_device); - setting_parent = nm_setting_6lowpan_get_parent (s_6lowpan); - if (setting_parent && nm_utils_is_uuid (setting_parent)) { - NMConnection *parent_connection; - - /* Don't change a parent specified by UUID if it's still valid */ - parent_connection = (NMConnection *) nm_settings_get_connection_by_uuid (nm_device_get_settings (device), setting_parent); - if ( parent_connection - && nm_device_check_connection_compatible (parent_device, parent_connection, NULL)) - new_parent = NULL; - } - if (new_parent) - g_object_set (s_6lowpan, NM_SETTING_6LOWPAN_PARENT, new_parent, NULL); - } else - g_object_set (s_6lowpan, NM_SETTING_6LOWPAN_PARENT, NULL, NULL); - + g_object_set (s_6lowpan, + NM_SETTING_6LOWPAN_PARENT, + nm_device_parent_find_for_connection (device, + nm_setting_6lowpan_get_parent (s_6lowpan)), + NULL); } static NMActStageReturn diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index 642bb25447..568403c626 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -435,8 +435,6 @@ update_connection (NMDevice *device, NMConnection *connection) NMDeviceIPTunnel *self = NM_DEVICE_IP_TUNNEL (device); NMDeviceIPTunnelPrivate *priv = NM_DEVICE_IP_TUNNEL_GET_PRIVATE (self); NMSettingIPTunnel *s_ip_tunnel = nm_connection_get_setting_ip_tunnel (connection); - NMDevice *parent = NULL; - const char *setting_parent, *new_parent; if (!s_ip_tunnel) { s_ip_tunnel = (NMSettingIPTunnel *) nm_setting_ip_tunnel_new (); @@ -446,25 +444,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (nm_setting_ip_tunnel_get_mode (s_ip_tunnel) != priv->mode) g_object_set (G_OBJECT (s_ip_tunnel), NM_SETTING_IP_TUNNEL_MODE, priv->mode, NULL); - parent = nm_device_parent_get_device (device); - - /* Update parent in the connection; default to parent's interface name */ - if (parent) { - new_parent = nm_device_get_iface (parent); - setting_parent = nm_setting_ip_tunnel_get_parent (s_ip_tunnel); - if (setting_parent && nm_utils_is_uuid (setting_parent)) { - NMConnection *parent_connection; - - /* Don't change a parent specified by UUID if it's still valid */ - parent_connection = (NMConnection *) nm_settings_get_connection_by_uuid (nm_device_get_settings (device), - setting_parent); - if (parent_connection && nm_device_check_connection_compatible (parent, parent_connection, NULL)) - new_parent = NULL; - } - if (new_parent) - g_object_set (s_ip_tunnel, NM_SETTING_IP_TUNNEL_PARENT, new_parent, NULL); - } else - g_object_set (s_ip_tunnel, NM_SETTING_IP_TUNNEL_PARENT, NULL, NULL); + g_object_set (s_ip_tunnel, + NM_SETTING_IP_TUNNEL_PARENT, + nm_device_parent_find_for_connection (device, + nm_setting_ip_tunnel_get_parent (s_ip_tunnel)), + NULL); if (!address_equal_pp (priv->addr_family, nm_setting_ip_tunnel_get_local (s_ip_tunnel), diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index fa9a7b0d69..ff386c82ed 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -391,8 +391,6 @@ update_connection (NMDevice *device, NMConnection *connection) { NMDeviceMacvlanPrivate *priv = NM_DEVICE_MACVLAN_GET_PRIVATE ((NMDeviceMacvlan *) device); NMSettingMacvlan *s_macvlan = nm_connection_get_setting_macvlan (connection); - NMDevice *parent_device; - const char *setting_parent, *new_parent; int new_mode; if (!s_macvlan) { @@ -410,24 +408,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (priv->props.tap != nm_setting_macvlan_get_tap (s_macvlan)) g_object_set (s_macvlan, NM_SETTING_MACVLAN_TAP, !!priv->props.tap, NULL); - /* Update parent in the connection; default to parent's interface name */ - parent_device = nm_device_parent_get_device (device); - if (parent_device) { - new_parent = nm_device_get_iface (parent_device); - setting_parent = nm_setting_macvlan_get_parent (s_macvlan); - if (setting_parent && nm_utils_is_uuid (setting_parent)) { - NMConnection *parent_connection; - - /* Don't change a parent specified by UUID if it's still valid */ - parent_connection = (NMConnection *) nm_settings_get_connection_by_uuid (nm_device_get_settings (device), setting_parent); - if (parent_connection && nm_device_check_connection_compatible (parent_device, parent_connection, NULL)) - new_parent = NULL; - } - if (new_parent) - g_object_set (s_macvlan, NM_SETTING_MACVLAN_PARENT, new_parent, NULL); - } else - g_object_set (s_macvlan, NM_SETTING_MACVLAN_PARENT, NULL, NULL); - + g_object_set (s_macvlan, + NM_SETTING_MACVLAN_PARENT, + nm_device_parent_find_for_connection (device, + nm_setting_macvlan_get_parent (s_macvlan)), + NULL); } static NMActStageReturn diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index d6376489f4..b7f0c4e756 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -427,10 +427,8 @@ update_connection (NMDevice *device, NMConnection *connection) NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (device); NMSettingVlan *s_vlan = nm_connection_get_setting_vlan (connection); int ifindex = nm_device_get_ifindex (device); - const char *setting_parent, *new_parent; const NMPlatformLink *plink; const NMPObject *polnk; - NMDevice *parent_device; guint vlan_id; guint vlan_flags; @@ -448,26 +446,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (vlan_id != nm_setting_vlan_get_id (s_vlan)) g_object_set (s_vlan, NM_SETTING_VLAN_ID, vlan_id, NULL); - /* Update parent in the connection; default to parent's interface name */ - parent_device = nm_device_parent_get_device (device); - if ( parent_device - && polnk - && plink->parent > 0 - && nm_device_get_ifindex (parent_device) == plink->parent) { - new_parent = nm_device_get_iface (parent_device); - setting_parent = nm_setting_vlan_get_parent (s_vlan); - if (setting_parent && nm_utils_is_uuid (setting_parent)) { - NMConnection *parent_connection; - - /* Don't change a parent specified by UUID if it's still valid */ - parent_connection = (NMConnection *) nm_settings_get_connection_by_uuid (nm_device_get_settings (device), setting_parent); - if (parent_connection && nm_device_check_connection_compatible (parent_device, parent_connection, NULL)) - new_parent = NULL; - } - if (new_parent) - g_object_set (s_vlan, NM_SETTING_VLAN_PARENT, new_parent, NULL); - } else - g_object_set (s_vlan, NM_SETTING_VLAN_PARENT, NULL, NULL); + g_object_set (s_vlan, + NM_SETTING_VLAN_PARENT, + nm_device_parent_find_for_connection (device, + nm_setting_vlan_get_parent (s_vlan)), + NULL); if (polnk) vlan_flags = polnk->lnk_vlan.flags; diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index f6f15729b2..c34f4142b2 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -386,9 +386,6 @@ update_connection (NMDevice *device, NMConnection *connection) { NMDeviceVxlanPrivate *priv = NM_DEVICE_VXLAN_GET_PRIVATE ((NMDeviceVxlan *) device); NMSettingVxlan *s_vxlan = nm_connection_get_setting_vxlan (connection); - NMDevice *parent_device; - const char *setting_parent; - const char *new_parent = NULL; if (!s_vxlan) { s_vxlan = (NMSettingVxlan *) nm_setting_vxlan_new (); @@ -398,23 +395,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (priv->props.id != nm_setting_vxlan_get_id (s_vxlan)) g_object_set (G_OBJECT (s_vxlan), NM_SETTING_VXLAN_ID, priv->props.id, NULL); - parent_device = nm_device_parent_get_device (device); - - /* Update parent in the connection; default to parent's interface name */ - if (parent_device) { - new_parent = nm_device_get_iface (parent_device); - setting_parent = nm_setting_vxlan_get_parent (s_vxlan); - if (setting_parent && nm_utils_is_uuid (setting_parent)) { - NMConnection *parent_connection; - - /* Don't change a parent specified by UUID if it's still valid */ - parent_connection = (NMConnection *) nm_settings_get_connection_by_uuid (nm_device_get_settings (device), - setting_parent); - if (parent_connection && nm_device_check_connection_compatible (parent_device, parent_connection, NULL)) - new_parent = NULL; - } - } - g_object_set (s_vxlan, NM_SETTING_VXLAN_PARENT, new_parent, NULL); + g_object_set (s_vxlan, + NM_SETTING_VXLAN_PARENT, + nm_device_parent_find_for_connection (device, + nm_setting_vxlan_get_parent (s_vxlan)), + NULL); if (!address_matches (nm_setting_vxlan_get_remote (s_vxlan), priv->props.group, &priv->props.group6)) { if (priv->props.group) { diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9f425b5370..b1faf13e15 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1711,6 +1711,41 @@ nm_device_parent_notify_changed (NMDevice *self, /*****************************************************************************/ +const char * +nm_device_parent_find_for_connection (NMDevice *self, + const char *current_setting_parent) +{ + const char *new_parent; + NMDevice *parent_device; + + parent_device = nm_device_parent_get_device (self); + if (!parent_device) + return NULL; + + new_parent = nm_device_get_iface (parent_device); + if (!new_parent) + return NULL; + + if ( current_setting_parent + && !nm_streq (current_setting_parent, new_parent) + && nm_utils_is_uuid (current_setting_parent)) { + NMSettingsConnection *parent_connection; + + /* Don't change a parent specified by UUID if it's still valid */ + parent_connection = nm_settings_get_connection_by_uuid (nm_device_get_settings (self), + current_setting_parent); + if ( parent_connection + && nm_device_check_connection_compatible (parent_device, + NM_CONNECTION (parent_connection), + NULL)) + return current_setting_parent; + } + + return new_parent; +} + +/*****************************************************************************/ + static void _stats_update_counters (NMDevice *self, guint64 tx_bytes, diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 41156c1008..9660ee3eb2 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -501,6 +501,9 @@ gboolean nm_device_parent_notify_changed (NMDevice *self, NMDevice *change_candidate, gboolean device_removed); +const char *nm_device_parent_find_for_connection (NMDevice *self, + const char *current_setting_parent); + /* Master */ gboolean nm_device_is_master (NMDevice *dev);