From 0c8d8b4da38e2de9e5ab4b07128924872923ca8b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 5 Aug 2013 16:39:20 -0500 Subject: [PATCH] core: update UDI when it's available Software devices don't have a UDI until udev finds them, and since we need to know about the software devices before udev finds them the UDI will be missing. Instead of requiring a UDI on NMDevice creation, update the property from the NMPlatform link change signal when udev does find the device. Now that a UDI is no longer required for device creation, software devices added by NM would be created in the platform_link_added_cb() signal handler triggered by the various software device creation methods in system_create_virtual_device() (eg nm_platform_bridge_add() etc). Then the NMDevice created in system_create_virtual_device() would be a duplicate and cause problems when it was added. Since system_create_virtual_device() needs to do setup on some devices, suppress the device creation from the platform link added handler in this function. Much of this is a hack which should be cleaned up later. --- src/devices/nm-device-bond.c | 4 +- src/devices/nm-device-bond.h | 3 +- src/devices/nm-device-bridge.c | 4 +- src/devices/nm-device-bridge.h | 3 +- src/devices/nm-device-infiniband.c | 5 +-- src/devices/nm-device-infiniband.h | 3 +- src/devices/nm-device-vlan.c | 4 +- src/devices/nm-device-vlan.h | 4 +- src/devices/nm-device.c | 19 ++++++--- src/nm-manager.c | 64 ++++++++++++++---------------- 10 files changed, 52 insertions(+), 61 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index dde8089d10..fbf65b256a 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -383,13 +383,11 @@ release_slave (NMDevice *device, NMDevice *slave) /******************************************************************/ NMDevice * -nm_device_bond_new (const char *udi, const char *iface) +nm_device_bond_new (const char *iface) { - g_return_val_if_fail (udi != NULL, NULL); g_return_val_if_fail (iface != NULL, NULL); return (NMDevice *) g_object_new (NM_TYPE_DEVICE_BOND, - NM_DEVICE_UDI, udi, NM_DEVICE_IFACE, iface, NM_DEVICE_DRIVER, "bonding", NM_DEVICE_TYPE_DESC, "Bond", diff --git a/src/devices/nm-device-bond.h b/src/devices/nm-device-bond.h index d1606dd610..09c8d48b0f 100644 --- a/src/devices/nm-device-bond.h +++ b/src/devices/nm-device-bond.h @@ -54,8 +54,7 @@ typedef struct { GType nm_device_bond_get_type (void); -NMDevice *nm_device_bond_new (const char *udi, - const char *iface); +NMDevice *nm_device_bond_new (const char *iface); G_END_DECLS diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 8a4d09611a..f7e8ca448d 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -311,13 +311,11 @@ release_slave (NMDevice *device, NMDevice *slave) /******************************************************************/ NMDevice * -nm_device_bridge_new (const char *udi, const char *iface) +nm_device_bridge_new (const char *iface) { - g_return_val_if_fail (udi != NULL, NULL); g_return_val_if_fail (iface != NULL, NULL); return (NMDevice *) g_object_new (NM_TYPE_DEVICE_BRIDGE, - NM_DEVICE_UDI, udi, NM_DEVICE_IFACE, iface, NM_DEVICE_DRIVER, "bridge", NM_DEVICE_TYPE_DESC, "Bridge", diff --git a/src/devices/nm-device-bridge.h b/src/devices/nm-device-bridge.h index 7113686f34..8a910debea 100644 --- a/src/devices/nm-device-bridge.h +++ b/src/devices/nm-device-bridge.h @@ -54,8 +54,7 @@ typedef struct { GType nm_device_bridge_get_type (void); -NMDevice *nm_device_bridge_new (const char *udi, - const char *iface); +NMDevice *nm_device_bridge_new (const char *iface); G_END_DECLS diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index f41a8cac93..1aad074e35 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -105,16 +105,13 @@ nm_device_infiniband_new (NMPlatformLink *platform_device) } NMDevice * -nm_device_infiniband_new_partition (const char *udi, - const char *iface, +nm_device_infiniband_new_partition (const char *iface, const char *driver) { - g_return_val_if_fail (udi != NULL, NULL); g_return_val_if_fail (iface != NULL, NULL); g_return_val_if_fail (driver != NULL, NULL); return (NMDevice *) g_object_new (NM_TYPE_DEVICE_INFINIBAND, - NM_DEVICE_UDI, udi, NM_DEVICE_IFACE, iface, NM_DEVICE_DRIVER, driver, NM_DEVICE_TYPE_DESC, "InfiniBand", diff --git a/src/devices/nm-device-infiniband.h b/src/devices/nm-device-infiniband.h index 4a68b6c1d9..7ea1336860 100644 --- a/src/devices/nm-device-infiniband.h +++ b/src/devices/nm-device-infiniband.h @@ -53,8 +53,7 @@ typedef struct { GType nm_device_infiniband_get_type (void); NMDevice *nm_device_infiniband_new (NMPlatformLink *platform_device); -NMDevice *nm_device_infiniband_new_partition (const char *udi, - const char *iface, +NMDevice *nm_device_infiniband_new_partition (const char *iface, const char *driver); G_END_DECLS diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c index a9a031544a..4ac5be3c04 100644 --- a/src/devices/nm-device-vlan.c +++ b/src/devices/nm-device-vlan.c @@ -303,16 +303,14 @@ parent_state_changed (NMDevice *parent, /******************************************************************/ NMDevice * -nm_device_vlan_new (const char *udi, const char *iface, NMDevice *parent) +nm_device_vlan_new (const char *iface, NMDevice *parent) { NMDevice *device; - g_return_val_if_fail (udi != NULL, NULL); g_return_val_if_fail (iface != NULL, NULL); g_return_val_if_fail (parent != NULL, NULL); device = (NMDevice *) g_object_new (NM_TYPE_DEVICE_VLAN, - NM_DEVICE_UDI, udi, NM_DEVICE_IFACE, iface, NM_DEVICE_DRIVER, "8021q", NM_DEVICE_TYPE_DESC, "VLAN", diff --git a/src/devices/nm-device-vlan.h b/src/devices/nm-device-vlan.h index faa60af360..cdf073ff94 100644 --- a/src/devices/nm-device-vlan.h +++ b/src/devices/nm-device-vlan.h @@ -54,9 +54,7 @@ typedef struct { GType nm_device_vlan_get_type (void); -NMDevice *nm_device_vlan_new (const char *udi, - const char *iface, - NMDevice *parent); +NMDevice *nm_device_vlan_new (const char *iface, NMDevice *parent); G_END_DECLS diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index edaa2e7740..fc0fec6eea 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -496,6 +496,7 @@ constructor (GType type, NMDevicePrivate *priv; NMPlatform *platform; int i; + static guint32 id = 0; object = G_OBJECT_CLASS (nm_device_parent_class)->constructor (type, n_construct_params, @@ -506,16 +507,16 @@ constructor (GType type, dev = NM_DEVICE (object); priv = NM_DEVICE_GET_PRIVATE (dev); - if (!priv->udi) { - nm_log_err (LOGD_DEVICE, "No device udi provided, ignoring"); - goto error; - } - if (!priv->iface) { nm_log_err (LOGD_DEVICE, "No device interface provided, ignoring"); goto error; } + if (!priv->udi) { + /* Use a placeholder UDI until we get a real one */ + priv->udi = g_strdup_printf ("/virtual/device/placeholder/%d", id++); + } + if (NM_DEVICE_GET_CLASS (dev)->get_generic_capabilities) priv->capabilities |= NM_DEVICE_GET_CLASS (dev)->get_generic_capabilities (dev); @@ -1160,10 +1161,18 @@ static void link_changed_cb (NMPlatform *platform, int ifindex, NMPlatformLink *info, NMPlatformReason reason, NMDevice *device) { NMDeviceClass *klass = NM_DEVICE_GET_CLASS (device); + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); if (ifindex != nm_device_get_ifindex (device)) return; + if (info->udi && g_strcmp0 (info->udi, priv->udi)) { + /* Update UDI to what udev gives us */ + g_free (priv->udi); + priv->udi = g_strdup (info->udi); + g_object_notify (G_OBJECT (device), NM_DEVICE_UDI); + } + if (klass->link_changed) klass->link_changed (device, info); } diff --git a/src/nm-manager.c b/src/nm-manager.c index fced131ead..2fbc9e242f 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -171,6 +171,13 @@ static GSList * remove_one_device (NMManager *manager, static void rfkill_change_wifi (const char *desc, gboolean enabled); +static void +platform_link_added_cb (NMPlatform *platform, + int ifindex, + NMPlatformLink *link, + NMPlatformReason reason, + gpointer user_data); + #define SSD_POKE_INTERVAL 120 #define ORIGDEV_TAG "originating-device" @@ -1181,14 +1188,6 @@ connection_needs_virtual_device (NMConnection *connection) return FALSE; } -static char * -get_virtual_iface_placeholder_udi (void) -{ - static guint32 id = 0; - - return g_strdup_printf ("/virtual/device/placeholder/%d", id++); -} - /***************************/ /* FIXME: remove when we handle bridges non-destructively */ @@ -1290,7 +1289,7 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GSList *iter; - char *iface = NULL, *udi; + char *iface = NULL; NMDevice *device = NULL, *parent = NULL; iface = get_virtual_iface_name (self, connection, &parent); @@ -1313,16 +1312,20 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) g_clear_error (&error); } + /* Block notification of link added since we're creating the device + * explicitly here, otherwise adding the platform/kernel device would + * create it before this function can do the rest of the setup. + */ + g_signal_handlers_block_by_func (nm_platform_get (), G_CALLBACK (platform_link_added_cb), self); + if (nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)) { if (!nm_platform_bond_add (iface)) { nm_log_warn (LOGD_DEVICE, "(%s): failed to add bonding master interface for '%s'", iface, nm_connection_get_id (connection)); - goto out; + goto unblock; } - udi = get_virtual_iface_placeholder_udi (); - device = nm_device_bond_new (udi, iface); - g_free (udi); + device = nm_device_bond_new (iface); } else if (nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME)) { gboolean result; @@ -1330,19 +1333,17 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) if (!result && nm_platform_get_error () != NM_PLATFORM_ERROR_EXISTS) { nm_log_warn (LOGD_DEVICE, "(%s): failed to add bridging interface for '%s'", iface, nm_connection_get_id (connection)); - goto out; + goto unblock; } /* FIXME: remove when we handle bridges non-destructively */ if (!result && !bridge_created_by_nm (self, iface)) { nm_log_warn (LOGD_DEVICE, "(%s): cannot use existing bridge for '%s'", iface, nm_connection_get_id (connection)); - goto out; + goto unblock; } - udi = get_virtual_iface_placeholder_udi (); - device = nm_device_bridge_new (udi, iface); - g_free (udi); + device = nm_device_bridge_new (iface); } else if (nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)) { NMSettingVlan *s_vlan = nm_connection_get_setting_vlan (connection); int ifindex = nm_device_get_ip_ifindex (parent); @@ -1354,7 +1355,7 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) nm_setting_vlan_get_flags (s_vlan))) { nm_log_warn (LOGD_DEVICE, "(%s): failed to add VLAN interface for '%s'", iface, nm_connection_get_id (connection)); - goto out; + goto unblock; } num = nm_setting_vlan_get_num_priorities (s_vlan, NM_VLAN_INGRESS_MAP); for (i = 0; i < num; i++) { @@ -1366,9 +1367,7 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) if (nm_setting_vlan_get_priority (s_vlan, NM_VLAN_EGRESS_MAP, i, &from, &to)) nm_platform_vlan_set_egress_map (ifindex, from, to); } - udi = get_virtual_iface_placeholder_udi (); - device = nm_device_vlan_new (udi, iface, parent); - g_free (udi); + device = nm_device_vlan_new (iface, parent); } else if (nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME)) { NMSettingInfiniband *s_infiniband = nm_connection_get_setting_infiniband (connection); int p_key, parent_ifindex; @@ -1379,17 +1378,18 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) if (!nm_platform_infiniband_partition_add (parent_ifindex, p_key)) { nm_log_warn (LOGD_DEVICE, "(%s): failed to add InfiniBand P_Key interface for '%s'", iface, nm_connection_get_id (connection)); - goto out; + goto unblock; } - udi = get_virtual_iface_placeholder_udi (); - device = nm_device_infiniband_new_partition (udi, iface, nm_device_get_driver (parent)); - g_free (udi); + device = nm_device_infiniband_new_partition (iface, nm_device_get_driver (parent)); } if (device) add_device (self, device); +unblock: + g_signal_handlers_unblock_by_func (nm_platform_get (), G_CALLBACK (platform_link_added_cb), self); + out: g_free (iface); return device; @@ -2257,12 +2257,8 @@ platform_link_added_cb (NMPlatform *platform, g_return_if_fail (ifindex > 0); - device = find_device_by_ifindex (self, ifindex); - if (device) { - /* If it's a virtual device we may need to update its UDI */ - g_object_set (G_OBJECT (device), NM_DEVICE_UDI, link->udi, NULL); + if (find_device_by_ifindex (self, ifindex)) return; - } /* Try registered device factories */ for (iter = priv->factories; iter; iter = g_slist_next (iter)) { @@ -2303,12 +2299,12 @@ platform_link_added_cb (NMPlatform *platform, device = nm_device_wifi_new (link); break; case NM_LINK_TYPE_BOND: - device = nm_device_bond_new (link->udi, link->name); + device = nm_device_bond_new (link->name); break; case NM_LINK_TYPE_BRIDGE: /* FIXME: always create device when we handle bridges non-destructively */ if (bridge_created_by_nm (self, link->name)) - device = nm_device_bridge_new (link->udi, link->name); + device = nm_device_bridge_new (link->name); else nm_log_info (LOGD_BRIDGE, "(%s): ignoring bridge not created by NetworkManager", link->name); break; @@ -2317,7 +2313,7 @@ platform_link_added_cb (NMPlatform *platform, if (nm_platform_vlan_get_info (ifindex, &parent_ifindex, NULL)) { parent = find_device_by_ifindex (self, parent_ifindex); if (parent) - device = nm_device_vlan_new (link->udi, link->name, parent); + device = nm_device_vlan_new (link->name, parent); else { /* If udev signaled the VLAN interface before it signaled * the VLAN's parent at startup we may not know about the