From fd314ed7f779eed4c56efd2daa71bbf9af1cf005 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 17 Feb 2016 15:11:02 +0100 Subject: [PATCH 1/8] device-factory/trivial: rename get_virtual_iface_name() to get_connection_iface() --- src/devices/nm-device-factory.c | 20 ++++++++++---------- src/devices/nm-device-factory.h | 16 ++++++++-------- src/devices/nm-device-infiniband.c | 8 ++++---- src/devices/nm-device-ip-tunnel.c | 8 ++++---- src/devices/nm-device-macvlan.c | 8 ++++---- src/devices/nm-device-vlan.c | 8 ++++---- src/devices/nm-device-vxlan.c | 8 ++++---- src/nm-manager.c | 8 ++++---- 8 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/devices/nm-device-factory.c b/src/devices/nm-device-factory.c index 2c2aae9d27..b2c9050a7a 100644 --- a/src/devices/nm-device-factory.c +++ b/src/devices/nm-device-factory.c @@ -158,9 +158,9 @@ nm_device_factory_get_connection_parent (NMDeviceFactory *factory, } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { const char *iface; @@ -173,10 +173,10 @@ get_virtual_iface_name (NMDeviceFactory *factory, } char * -nm_device_factory_get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface, - GError **error) +nm_device_factory_get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface, + GError **error) { char *ifname; @@ -193,7 +193,7 @@ nm_device_factory_get_virtual_iface_name (NMDeviceFactory *factory, return NULL; } - if (!NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_virtual_iface_name) { + if (!NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, @@ -202,7 +202,7 @@ nm_device_factory_get_virtual_iface_name (NMDeviceFactory *factory, return NULL; } - ifname = NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_virtual_iface_name (factory, connection, parent_iface); + ifname = NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface (factory, connection, parent_iface); if (!ifname) { g_set_error (error, NM_MANAGER_ERROR, @@ -230,7 +230,7 @@ nm_device_factory_get_virtual_iface_name (NMDeviceFactory *factory, static void nm_device_factory_default_init (NMDeviceFactoryInterface *factory_iface) { - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; /* Signals */ signals[DEVICE_ADDED] = g_signal_new (NM_DEVICE_FACTORY_DEVICE_ADDED, diff --git a/src/devices/nm-device-factory.h b/src/devices/nm-device-factory.h index 943fc5cd13..4cd12e12ad 100644 --- a/src/devices/nm-device-factory.h +++ b/src/devices/nm-device-factory.h @@ -101,7 +101,7 @@ typedef struct { NMConnection *connection); /** - * get_virtual_iface_name: + * get_connection_iface: * @factory: the #NMDeviceFactory * @connection: the #NMConnection to return the virtual interface name for * @parent_iface: parent interface name @@ -111,9 +111,9 @@ typedef struct { * * Returns: the interface name, or %NULL */ - char * (*get_virtual_iface_name) (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface); + char * (*get_connection_iface) (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface); /** * create_device: @@ -175,10 +175,10 @@ void nm_device_factory_get_supported_types (NMDeviceFactory *factory, const char *nm_device_factory_get_connection_parent (NMDeviceFactory *factory, NMConnection *connection); -char * nm_device_factory_get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface, - GError **error); +char * nm_device_factory_get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface, + GError **error); void nm_device_factory_start (NMDeviceFactory *factory); diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index c5e73c9920..a2a686e334 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -399,9 +399,9 @@ get_connection_parent (NMDeviceFactory *factory, NMConnection *connection) } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { NMSettingInfiniband *s_infiniband; @@ -423,6 +423,6 @@ NM_DEVICE_FACTORY_DEFINE_INTERNAL (INFINIBAND, Infiniband, infiniband, NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_INFINIBAND_SETTING_NAME), factory_iface->create_device = create_device; factory_iface->get_connection_parent = get_connection_parent; - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; ) diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index 7923d545ba..47e5ab6151 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -1023,9 +1023,9 @@ get_connection_parent (NMDeviceFactory *factory, NMConnection *connection) } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { const char *ifname; NMSettingIPTunnel *s_ip_tunnel; @@ -1048,5 +1048,5 @@ NM_DEVICE_FACTORY_DEFINE_INTERNAL (IP_TUNNEL, IPTunnel, ip_tunnel, NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_IP_TUNNEL_SETTING_NAME), factory_iface->create_device = create_device; factory_iface->get_connection_parent = get_connection_parent; - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; ) diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index 912449e8fc..17ee6e82fa 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -744,9 +744,9 @@ get_connection_parent (NMDeviceFactory *factory, NMConnection *connection) } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { NMSettingMacvlan *s_macvlan; const char *ifname; @@ -768,6 +768,6 @@ NM_DEVICE_FACTORY_DEFINE_INTERNAL (MACVLAN, Macvlan, macvlan, NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_MACVLAN_SETTING_NAME), factory_iface->create_device = create_device; factory_iface->get_connection_parent = get_connection_parent; - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; ) diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index e2b0e50605..a9b3c8bc69 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -758,9 +758,9 @@ get_connection_parent (NMDeviceFactory *factory, NMConnection *connection) } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { const char *ifname; NMSettingVlan *s_vlan; @@ -789,6 +789,6 @@ NM_DEVICE_FACTORY_DEFINE_INTERNAL (VLAN, Vlan, vlan, NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_VLAN_SETTING_NAME), factory_iface->create_device = create_device; factory_iface->get_connection_parent = get_connection_parent; - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; ) diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index 08f0f69881..d7ded881dd 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -810,9 +810,9 @@ get_connection_parent (NMDeviceFactory *factory, NMConnection *connection) } static char * -get_virtual_iface_name (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) +get_connection_iface (NMDeviceFactory *factory, + NMConnection *connection, + const char *parent_iface) { const char *ifname; NMSettingVxlan *s_vxlan; @@ -834,6 +834,6 @@ NM_DEVICE_FACTORY_DEFINE_INTERNAL (VXLAN, Vxlan, vxlan, NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES (NM_SETTING_VXLAN_SETTING_NAME), factory_iface->create_device = create_device; factory_iface->get_connection_parent = get_connection_parent; - factory_iface->get_virtual_iface_name = get_virtual_iface_name; + factory_iface->get_connection_iface = get_connection_iface; ) diff --git a/src/nm-manager.c b/src/nm-manager.c index c5d47eab8a..9738d49e08 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -985,10 +985,10 @@ get_virtual_iface_name (NMManager *self, } parent = find_parent_device_for_connection (self, connection); - iface = nm_device_factory_get_virtual_iface_name (factory, - connection, - parent ? nm_device_get_ip_iface (parent) : NULL, - error); + iface = nm_device_factory_get_connection_iface (factory, + connection, + parent ? nm_device_get_ip_iface (parent) : NULL, + error); if (!iface) return NULL; From cd81e0bd9ff1fb575e5c98724a3b41d7edd119d2 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 17 Feb 2016 15:11:02 +0100 Subject: [PATCH 2/8] device-factory: always use the factory to determine the connection's interface name This makes things a bit simpler when determining if any connection is activatable on an existing device. --- src/devices/nm-device-factory.c | 25 +++++-------------------- src/devices/nm-device-factory.h | 4 ++-- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/devices/nm-device-factory.c b/src/devices/nm-device-factory.c index b2c9050a7a..ddf798dfc1 100644 --- a/src/devices/nm-device-factory.c +++ b/src/devices/nm-device-factory.c @@ -162,14 +162,8 @@ get_connection_iface (NMDeviceFactory *factory, NMConnection *connection, const char *parent_iface) { - const char *iface; - - /* For any other virtual connection, NMSettingConnection:interface-name is - * the virtual device name. - */ - iface = nm_connection_get_interface_name (connection); - g_return_val_if_fail (iface != NULL, NULL); - return g_strdup (iface); + /* Default to just using NMSettingConnection:interface-name */ + return g_strdup (nm_connection_get_interface_name (connection)); } char * @@ -184,20 +178,11 @@ nm_device_factory_get_connection_iface (NMDeviceFactory *factory, g_return_val_if_fail (connection != NULL, NULL); g_return_val_if_fail (!error || !*error, NULL); - if (!nm_connection_is_virtual (connection)) { - g_set_error (error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_FAILED, - "failed to determine virtual interface name: connection type '%s' is not a software device", - nm_connection_get_connection_type (connection)); - return NULL; - } - if (!NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, - "failed to determine virtual interface name: cannot generate name for %s", + "failed to determine interface name: cannot determine name for %s", nm_connection_get_connection_type (connection)); return NULL; } @@ -207,7 +192,7 @@ nm_device_factory_get_connection_iface (NMDeviceFactory *factory, g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, - "failed to determine virtual interface name: error generating name for %s", + "failed to determine interface name: error determine name for %s", nm_connection_get_connection_type (connection)); return NULL; } @@ -216,7 +201,7 @@ nm_device_factory_get_connection_iface (NMDeviceFactory *factory, g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, - "failed to determine virtual interface name: invalid name \"%s\" generated", + "failed to determine interface name: name \"%s\" is invalid", ifname); g_free (ifname); return NULL; diff --git a/src/devices/nm-device-factory.h b/src/devices/nm-device-factory.h index 4cd12e12ad..ae78968a42 100644 --- a/src/devices/nm-device-factory.h +++ b/src/devices/nm-device-factory.h @@ -103,8 +103,8 @@ typedef struct { /** * get_connection_iface: * @factory: the #NMDeviceFactory - * @connection: the #NMConnection to return the virtual interface name for - * @parent_iface: parent interface name + * @connection: the #NMConnection to return the interface name for + * @parent_iface: optional parent interface name for virtual devices * * Given a connection, returns the interface name that a device activating * that connection would have. From adc9895ebe33389a4ef89a319e9480a510bb160d Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 16 Feb 2016 15:07:49 +0100 Subject: [PATCH 3/8] manager: export nm_manager_get_connection_iface() We'll need the actual device name that should be used for a connection activated on a given device when checking the connection availability. --- src/nm-manager.c | 24 ++++++++++++------------ src/nm-manager.h | 6 ++++++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 9738d49e08..eb9576eeeb 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -948,24 +948,24 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection) } /** - * get_virtual_iface_name: + * nm_manager_get_connection_iface: * @self: the #NMManager - * @connection: the #NMConnection representing a virtual interface + * @connection: the #NMConnection to get the interface for * @out_parent: on success, the parent device if any * @error: an error if determining the virtual interface name failed * * Given @connection, returns the interface name that the connection - * would represent if it is a virtual connection. %NULL is returned and - * @error is set if the connection is not virtual, or if the name could - * not be determined. + * would need to use when activated. %NULL is returned if the name + * is not specified in connection or a the name for a virtual device + * could not be generated. * * Returns: the expected interface name (caller takes ownership), or %NULL */ -static char * -get_virtual_iface_name (NMManager *self, - NMConnection *connection, - NMDevice **out_parent, - GError **error) +char * +nm_manager_get_connection_iface (NMManager *self, + NMConnection *connection, + NMDevice **out_parent, + GError **error) { NMDeviceFactory *factory; char *iface = NULL; @@ -1021,7 +1021,7 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) g_return_val_if_fail (NM_IS_MANAGER (self), NULL); g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); - iface = get_virtual_iface_name (self, connection, &parent, &error); + iface = nm_manager_get_connection_iface (self, connection, &parent, &error); if (!iface) { nm_log_warn (LOGD_DEVICE, "(%s) can't get a name of a virtual device: %s", nm_connection_get_id (connection), error->message); @@ -3219,7 +3219,7 @@ validate_activation_request (NMManager *self, char *iface; /* Look for an existing device with the connection's interface name */ - iface = get_virtual_iface_name (self, connection, NULL, error); + iface = nm_manager_get_connection_iface (self, connection, NULL, error); if (!iface) goto error; diff --git a/src/nm-manager.h b/src/nm-manager.h index 2935d2a273..9783020102 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -101,6 +101,11 @@ const GSList * nm_manager_get_devices (NMManager *manager); NMDevice * nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex); +char * nm_manager_get_connection_iface (NMManager *self, + NMConnection *connection, + NMDevice **out_parent, + GError **error); + NMActiveConnection *nm_manager_activate_connection (NMManager *manager, NMSettingsConnection *connection, const char *specific_object, @@ -113,4 +118,5 @@ gboolean nm_manager_deactivate_connection (NMManager *manager, NMDeviceStateReason reason, GError **error); + #endif /* __NETWORKMANAGER_MANAGER_H__ */ From f9ec7136413d941d7dcafc82bcc0f89e1fd2261f Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 16 Feb 2016 15:07:57 +0100 Subject: [PATCH 4/8] device: move the interface name check from manager We not only want to check the device name when creating a virtual device, but also when determining if the connection can actually be activated there. Otherwise the device names will mix up if there's more connections that use virtual devices of the same type. --- src/devices/nm-device.c | 17 ++++++++++------- src/nm-manager.c | 4 +--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 711ac2acb0..21e653df43 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -2958,15 +2958,18 @@ nm_device_complete_connection (NMDevice *self, static gboolean check_connection_compatible (NMDevice *self, NMConnection *connection) { - NMSettingConnection *s_con; - const char *config_iface, *device_iface; + const char *device_iface = nm_device_get_iface (self); + gs_free char *conn_iface = nm_manager_get_connection_iface (nm_manager_get (), + connection, + NULL, NULL); - s_con = nm_connection_get_setting_connection (connection); - g_assert (s_con); + /* We always need a interface name for virtual devices, but for + * physical ones a connection without interface name is fine for + * any device. */ + if (!conn_iface) + return !nm_connection_is_virtual (connection); - config_iface = nm_setting_connection_get_interface_name (s_con); - device_iface = nm_device_get_iface (self); - if (config_iface && strcmp (config_iface, device_iface) != 0) + if (strcmp (conn_iface, device_iface) != 0) return FALSE; return TRUE; diff --git a/src/nm-manager.c b/src/nm-manager.c index eb9576eeeb..dce5d3c80c 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1033,9 +1033,7 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) for (iter = priv->devices; iter; iter = g_slist_next (iter)) { NMDevice *candidate = iter->data; - if ( g_strcmp0 (nm_device_get_iface (candidate), iface) == 0 - && nm_device_check_connection_compatible (candidate, connection)) { - + if (nm_device_check_connection_compatible (candidate, connection)) { if (nm_device_is_real (candidate)) { nm_log_dbg (LOGD_DEVICE, "(%s) already created virtual interface name %s", nm_connection_get_id (connection), iface); From 8b016cd9a70914713f85f65fbec25b4fb10d6954 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 16 Feb 2016 16:50:00 +0100 Subject: [PATCH 5/8] device: remove interface name checks from all classes Generic check_connection_compatible() already does the check. --- src/devices/nm-device-bond.c | 6 ------ src/devices/nm-device-bridge.c | 6 ------ src/devices/nm-device-macvlan.c | 9 +-------- src/devices/nm-device-vlan.c | 12 +----------- src/devices/nm-device-vxlan.c | 8 +------- src/devices/team/nm-device-team.c | 6 ------ 6 files changed, 3 insertions(+), 44 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index fda1085063..cef3f9b3b0 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -75,7 +75,6 @@ check_connection_available (NMDevice *device, static gboolean check_connection_compatible (NMDevice *device, NMConnection *connection) { - const char *iface; NMSettingBond *s_bond; if (!NM_DEVICE_CLASS (nm_device_bond_parent_class)->check_connection_compatible (device, connection)) @@ -85,11 +84,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) if (!s_bond || !nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)) return FALSE; - /* Bond connections must specify the virtual interface name */ - iface = nm_connection_get_interface_name (connection); - if (!iface || strcmp (nm_device_get_iface (device), iface)) - return FALSE; - /* FIXME: match bond properties like mode, etc? */ return TRUE; diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 4e23de2bb4..e008213906 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -73,7 +73,6 @@ check_connection_available (NMDevice *device, static gboolean check_connection_compatible (NMDevice *device, NMConnection *connection) { - const char *iface; NMSettingBridge *s_bridge; const char *mac_address; @@ -84,11 +83,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) if (!s_bridge || !nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME)) return FALSE; - /* Bridge connections must specify the virtual interface name */ - iface = nm_connection_get_interface_name (connection); - if (!iface || strcmp (nm_device_get_iface (device), iface)) - return FALSE; - mac_address = nm_setting_bridge_get_mac_address (s_bridge); if (mac_address && nm_device_is_real (device)) { const char *hw_addr; diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c index 17ee6e82fa..4560dd9259 100644 --- a/src/devices/nm-device-macvlan.c +++ b/src/devices/nm-device-macvlan.c @@ -376,7 +376,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) { NMDeviceMacvlanPrivate *priv = NM_DEVICE_MACVLAN_GET_PRIVATE (device); NMSettingMacvlan *s_macvlan; - const char *parent, *iface = NULL; + const char *parent = NULL; if (!NM_DEVICE_CLASS (nm_device_macvlan_parent_class)->check_connection_compatible (device, connection)) return FALSE; @@ -409,13 +409,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) } } - /* Ensure the interface name matches */ - iface = nm_connection_get_interface_name (connection); - if (iface) { - if (g_strcmp0 (nm_device_get_ip_iface (device), iface) != 0) - return FALSE; - } - return TRUE; } diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index a9b3c8bc69..fab48d7367 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -390,7 +390,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) { NMDeviceVlanPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (device); NMSettingVlan *s_vlan; - const char *parent, *iface = NULL; + const char *parent = NULL; if (!NM_DEVICE_CLASS (nm_device_vlan_parent_class)->check_connection_compatible (device, connection)) return FALSE; @@ -416,16 +416,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) } } - /* Ensure the interface name matches. If not specified we assume a match - * since both the parent interface and the VLAN ID matched by the time we - * get here. - */ - iface = nm_connection_get_interface_name (connection); - if (iface) { - if (g_strcmp0 (nm_device_get_ip_iface (device), iface) != 0) - return FALSE; - } - return TRUE; } diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index d7ded881dd..9317ef5be0 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -296,7 +296,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) { NMDeviceVxlanPrivate *priv = NM_DEVICE_VXLAN_GET_PRIVATE (device); NMSettingVxlan *s_vxlan; - const char *iface, *parent; + const char *parent; if (!NM_DEVICE_CLASS (nm_device_vxlan_parent_class)->check_connection_compatible (device, connection)) return FALSE; @@ -305,12 +305,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) if (!s_vxlan) return FALSE; - iface = nm_connection_get_interface_name (connection); - if (iface) { - if (g_strcmp0 (nm_device_get_ip_iface (device), iface) != 0) - return FALSE; - } - if (nm_device_is_real (device)) { parent = nm_setting_vxlan_get_parent (s_vxlan); if ( parent diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 0fc3164313..2258d99d32 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -85,7 +85,6 @@ check_connection_available (NMDevice *device, static gboolean check_connection_compatible (NMDevice *device, NMConnection *connection) { - const char *iface; NMSettingTeam *s_team; if (!NM_DEVICE_CLASS (nm_device_team_parent_class)->check_connection_compatible (device, connection)) @@ -95,11 +94,6 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) if (!s_team || !nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME)) return FALSE; - /* Team connections must specify the virtual interface name */ - iface = nm_connection_get_interface_name (connection); - if (!iface || strcmp (nm_device_get_iface (device), iface)) - return FALSE; - /* FIXME: match team properties like mode, etc? */ return TRUE; From ca0dbefb0247caa8ead1f04639958c743dfb0800 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Feb 2016 15:18:37 +0100 Subject: [PATCH 6/8] manager: reused the factory looked up in nm_manager_get_connection_iface() Only lookup the factory once and pass it down to find_parent_device_for_connection(). --- src/nm-manager.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index dce5d3c80c..4eb5b81eab 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -896,7 +896,7 @@ nm_manager_get_state (NMManager *manager) /***************************/ static NMDevice * -find_parent_device_for_connection (NMManager *self, NMConnection *connection) +find_parent_device_for_connection (NMManager *self, NMConnection *connection, NMDeviceFactory *cached_factory) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMDeviceFactory *factory; @@ -907,9 +907,12 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection) g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); - factory = nm_device_factory_manager_find_factory_for_connection (connection); - if (!factory) - return NULL; + if (!cached_factory) { + factory = nm_device_factory_manager_find_factory_for_connection (connection); + if (!factory) + return NULL; + } else + factory = cached_factory; parent_name = nm_device_factory_get_connection_parent (factory, connection); if (!parent_name) @@ -984,7 +987,7 @@ nm_manager_get_connection_iface (NMManager *self, return NULL; } - parent = find_parent_device_for_connection (self, connection); + parent = find_parent_device_for_connection (self, connection, factory); iface = nm_device_factory_get_connection_iface (factory, connection, parent ? nm_device_get_ip_iface (parent) : NULL, @@ -1119,7 +1122,7 @@ retry_connections_for_parent_device (NMManager *self, NMDevice *device) NMConnection *candidate = iter->data; NMDevice *parent; - parent = find_parent_device_for_connection (self, candidate); + parent = find_parent_device_for_connection (self, candidate, NULL); if (parent == device) connection_changed (priv->settings, candidate, self); } @@ -2770,7 +2773,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * if (!nm_device_is_real (device)) { NMDevice *parent; - parent = find_parent_device_for_connection (self, (NMConnection *) connection); + parent = find_parent_device_for_connection (self, (NMConnection *) connection, NULL); if (!nm_device_create_and_realize (device, (NMConnection *) connection, parent, error)) { g_prefix_error (error, "%s failed to create resources: ", nm_device_get_iface (device)); return FALSE; From f31954c07132092e30981b5fff557000a312e0a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Feb 2016 16:09:42 +0100 Subject: [PATCH 7/8] device: don't overwrite get_connection_iface() by default Factories that overwrite this function are not supposed to chain up the parent implementation. Thus there is no reason to have a default implementation and it's clearer to inline it. --- src/devices/nm-device-factory.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device-factory.c b/src/devices/nm-device-factory.c index ddf798dfc1..1af07d406d 100644 --- a/src/devices/nm-device-factory.c +++ b/src/devices/nm-device-factory.c @@ -157,37 +157,26 @@ nm_device_factory_get_connection_parent (NMDeviceFactory *factory, return NULL; } -static char * -get_connection_iface (NMDeviceFactory *factory, - NMConnection *connection, - const char *parent_iface) -{ - /* Default to just using NMSettingConnection:interface-name */ - return g_strdup (nm_connection_get_interface_name (connection)); -} - char * nm_device_factory_get_connection_iface (NMDeviceFactory *factory, NMConnection *connection, const char *parent_iface, GError **error) { + NMDeviceFactoryInterface *klass; char *ifname; g_return_val_if_fail (factory != NULL, NULL); g_return_val_if_fail (connection != NULL, NULL); g_return_val_if_fail (!error || !*error, NULL); - if (!NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface) { - g_set_error (error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_FAILED, - "failed to determine interface name: cannot determine name for %s", - nm_connection_get_connection_type (connection)); - return NULL; - } + klass = NM_DEVICE_FACTORY_GET_INTERFACE (factory); + + if (klass->get_connection_iface) + ifname = klass->get_connection_iface (factory, connection, parent_iface); + else + ifname = g_strdup (nm_connection_get_interface_name (connection)); - ifname = NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface (factory, connection, parent_iface); if (!ifname) { g_set_error (error, NM_MANAGER_ERROR, @@ -215,8 +204,6 @@ nm_device_factory_get_connection_iface (NMDeviceFactory *factory, static void nm_device_factory_default_init (NMDeviceFactoryInterface *factory_iface) { - factory_iface->get_connection_iface = get_connection_iface; - /* Signals */ signals[DEVICE_ADDED] = g_signal_new (NM_DEVICE_FACTORY_DEVICE_ADDED, NM_TYPE_DEVICE_FACTORY, From e93abf0552cf67c7fac5af9d365a833e747192f5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Feb 2016 16:12:46 +0100 Subject: [PATCH 8/8] device: optimize nm_manager_get_connection_iface() --- src/nm-manager.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/nm-manager.c b/src/nm-manager.c index 4eb5b81eab..589fc10f68 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -987,6 +987,20 @@ nm_manager_get_connection_iface (NMManager *self, return NULL; } + if ( !out_parent + && !NM_DEVICE_FACTORY_GET_INTERFACE (factory)->get_connection_iface) { + /* optimization. Shortcut lookup of the partent device. */ + iface = g_strdup (nm_connection_get_interface_name (connection)); + if (!iface) { + g_set_error (error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_FAILED, + "failed to determine interface name: error determine name for %s", + nm_connection_get_connection_type (connection)); + } + return iface; + } + parent = find_parent_device_for_connection (self, connection, factory); iface = nm_device_factory_get_connection_iface (factory, connection,