From ae7e46863062fdafb3392ce13f2f1bd558157df2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 12 Jan 2016 09:46:28 +0100 Subject: [PATCH 1/6] trivial: fix whitespace --- src/nm-manager.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 9c67d3ef64..e2adbe463e 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -3257,9 +3257,9 @@ _activation_auth_done (NMActiveConnection *active, if (success) { if (_internal_activate_generic (self, active, &error)) { - g_dbus_method_invocation_return_value ( - context, - g_variant_new ("(o)", nm_exported_object_get_path (NM_EXPORTED_OBJECT (active)))); + g_dbus_method_invocation_return_value (context, + g_variant_new ("(o)", + nm_exported_object_get_path (NM_EXPORTED_OBJECT (active)))); nm_audit_log_connection_op (NM_AUDIT_OP_CONN_ACTIVATE, connection, TRUE, subject, NULL); g_object_unref (active); @@ -3267,8 +3267,8 @@ _activation_auth_done (NMActiveConnection *active, } } else { error = g_error_new_literal (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_PERMISSION_DENIED, - error_desc); + NM_MANAGER_ERROR_PERMISSION_DENIED, + error_desc); } g_assert (error); From 7830b79d50fb0902a9d877cc74ecb27043665730 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 12 Jan 2016 17:07:04 +0100 Subject: [PATCH 2/6] device: minor logging changes to device creation --- src/devices/nm-device.c | 11 ++++++++--- src/nm-manager.c | 4 +--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 231ab00672..804cf1d86c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1762,6 +1762,8 @@ nm_device_create_and_realize (NMDevice *self, /* Must be set before device is realized */ priv->is_nm_owned = !nm_platform_link_get_by_ifname (NM_PLATFORM_GET, priv->iface); + _LOGD (LOGD_DEVICE, "create (is %snm-owned)", priv->is_nm_owned ? "" : "not "); + /* Create any resources the device needs */ if (NM_DEVICE_GET_CLASS (self)->create_and_realize) { if (!NM_DEVICE_GET_CLASS (self)->create_and_realize (self, connection, parent, &plink, error)) @@ -1867,6 +1869,8 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) g_return_if_fail (priv->ip_ifindex <= 0); g_return_if_fail (priv->ip_iface == NULL); + _LOGD (LOGD_DEVICE, "start setup of %s, kernel ifindex %d", G_OBJECT_TYPE_NAME (self), plink ? plink->ifindex : 0); + klass = NM_DEVICE_GET_CLASS (self); /* Balanced by a thaw in nm_device_realize_finish() */ @@ -1878,8 +1882,6 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) } if (priv->ifindex > 0) { - _LOGD (LOGD_DEVICE, "start setup of %s, kernel ifindex %d", G_OBJECT_TYPE_NAME (self), priv->ifindex); - priv->physical_port_id = nm_platform_link_get_physical_port_id (NM_PLATFORM_GET, priv->ifindex); g_object_notify (G_OBJECT (self), NM_DEVICE_PHYSICAL_PORT_ID); @@ -2017,8 +2019,11 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) g_object_freeze_notify (G_OBJECT (self)); + ifindex = nm_device_get_ifindex (self); + + _LOGD (LOGD_DEVICE, "unrealize (ifindex %d)", ifindex > 0 ? ifindex : 0); + if (remove_resources) { - ifindex = nm_device_get_ifindex (self); if (ifindex > 0) nm_platform_link_delete (NM_PLATFORM_GET, ifindex); } diff --git a/src/nm-manager.c b/src/nm-manager.c index e2adbe463e..de0a5afeb9 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1886,8 +1886,6 @@ add_device (NMManager *self, NMDevice *device, GError **error) type_desc = nm_device_get_type_desc (device); g_assert (type_desc); - nm_log_info (LOGD_HW, "(%s): new %s device", iface, type_desc); - unmanaged_specs = nm_settings_get_unmanaged_specs (priv->settings); nm_device_set_unmanaged_flags_initial (device, NM_UNMANAGED_USER, @@ -1897,7 +1895,7 @@ add_device (NMManager *self, NMDevice *device, GError **error) manager_sleeping (self)); dbus_path = nm_exported_object_export (NM_EXPORTED_OBJECT (device)); - nm_log_dbg (LOGD_DEVICE, "(%s): exported as %s", nm_device_get_iface (device), dbus_path); + nm_log_info (LOGD_DEVICE, "(%s): new %s device (%s)", iface, type_desc, dbus_path); nm_device_finish_init (device); From 2292cb63bf6213f85b30071a2929e853e6640da6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 Jan 2016 17:24:25 +0100 Subject: [PATCH 3/6] core: always unrealize software devices when platform link disappears We possibly need the unrealized device for connections that need this. nm_device_unrealize() will check if there are still any available-connections and possibly emit DEVICE_REMOVED signal. Fixes: 7e5f27a21ca22d986755e4cc480e85d8a51ff17d --- src/nm-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index de0a5afeb9..455691d50c 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2088,7 +2088,7 @@ _platform_link_cb_idle (PlatformLinkCbData *data) device = nm_manager_get_device_by_ifindex (self, data->ifindex); if (device) { - if (nm_device_is_software (device) && nm_device_get_is_nm_owned (device)) { + if (nm_device_is_software (device)) { /* Our software devices stick around until their connection is removed */ if (!nm_device_unrealize (device, FALSE, &error)) { nm_log_warn (LOGD_DEVICE, "(%s): failed to unrealize: %s", From eab9cca87c6484a039db9c42464f5be8d5454769 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 Jan 2016 18:18:25 +0100 Subject: [PATCH 4/6] device: refactor cp_connection* functions to use @self pointer --- src/devices/nm-device.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 804cf1d86c..2c55aff53f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9173,29 +9173,40 @@ nm_device_get_available_connections (NMDevice *self, const char *specific_object static void cp_connection_added (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data) { - if (_try_add_available_connection (NM_DEVICE (user_data), connection)) - _signal_available_connections_changed (NM_DEVICE (user_data)); + NMDevice *self = user_data; + + g_return_if_fail (NM_IS_DEVICE (self)); + + if (_try_add_available_connection (self, connection)) + _signal_available_connections_changed (self); } static void cp_connection_removed (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data) { - if (_del_available_connection (NM_DEVICE (user_data), connection)) - _signal_available_connections_changed (NM_DEVICE (user_data)); + NMDevice *self = user_data; + + g_return_if_fail (NM_IS_DEVICE (self)); + + if (_del_available_connection (self, connection)) + _signal_available_connections_changed (self); } static void cp_connection_updated (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data) { + NMDevice *self = user_data; gboolean added, deleted; + g_return_if_fail (NM_IS_DEVICE (self)); + /* FIXME: don't remove it from the hash if it's just going to get re-added */ - deleted = _del_available_connection (NM_DEVICE (user_data), connection); - added = _try_add_available_connection (NM_DEVICE (user_data), connection); + deleted = _del_available_connection (self, connection); + added = _try_add_available_connection (self, connection); /* Only signal if the connection was removed OR added, but not both */ if (added != deleted) - _signal_available_connections_changed (NM_DEVICE (user_data)); + _signal_available_connections_changed (self); } gboolean From 9c2266741b29efa5d44a79b4fcabf4854403b6dc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 Jan 2016 18:04:42 +0100 Subject: [PATCH 5/6] device: cleanup unrealized device without connection During nm_device_unrealize(), the connection might still have some connections that only get removed later. We must garbage collect unrealized devices when they loose their available-connections, not during unrealize. Fixes: 436ec5b8e37ca2e5fc0f8d09a4c5f09cf406b4a5 --- src/devices/nm-device.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2c55aff53f..4914d2f812 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -2082,8 +2082,6 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) /* Garbage-collect unneeded unrealized devices. */ nm_device_recheck_available_connections (self); - if (g_hash_table_size (priv->available_connections) == 0) - g_signal_emit_by_name (self, NM_DEVICE_REMOVED); return TRUE; } @@ -9086,6 +9084,16 @@ _del_available_connection (NMDevice *self, NMConnection *connection) return g_hash_table_remove (NM_DEVICE_GET_PRIVATE (self)->available_connections, connection); } +static void +available_connection_check_delete_unrealized (NMDevice *self) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + + if ( g_hash_table_size (priv->available_connections) == 0 + && !nm_device_is_real (self)) + g_signal_emit_by_name (self, NM_DEVICE_REMOVED); +} + static gboolean check_connection_available (NMDevice *self, NMConnection *connection, @@ -9132,6 +9140,8 @@ nm_device_recheck_available_connections (NMDevice *self) _signal_available_connections_changed (self); } + + available_connection_check_delete_unrealized (self); } /** @@ -9188,8 +9198,10 @@ cp_connection_removed (NMConnectionProvider *cp, NMConnection *connection, gpoin g_return_if_fail (NM_IS_DEVICE (self)); - if (_del_available_connection (self, connection)) + if (_del_available_connection (self, connection)) { _signal_available_connections_changed (self); + available_connection_check_delete_unrealized (self); + } } static void @@ -9205,8 +9217,10 @@ cp_connection_updated (NMConnectionProvider *cp, NMConnection *connection, gpoin added = _try_add_available_connection (self, connection); /* Only signal if the connection was removed OR added, but not both */ - if (added != deleted) + if (added != deleted) { _signal_available_connections_changed (self); + available_connection_check_delete_unrealized (self); + } } gboolean From 3d463e2f697af59a66e0d28251c12f3e4612e70a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 12 Jan 2016 10:38:16 +0100 Subject: [PATCH 6/6] device: only set NMDevice as "real" at the end of nm_device_realize_finish() Only set the device as "real" at the end of nm_device_realize_finish(), not already during nm_device_realize_start()/realize_start_setup(). --- src/devices/nm-device.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 4914d2f812..43d7b9ead5 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -583,6 +583,8 @@ nm_device_is_software (NMDevice *self) gboolean nm_device_is_real (NMDevice *self) { + g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); + return NM_DEVICE_GET_PRIVATE (self)->real; } @@ -1710,6 +1712,9 @@ link_type_compatible (NMDevice *self, * the device is ready for use nm_device_realize_finish() must be called. * @out_compatible will only be set if @plink is not %NULL, and * + * Important: if nm_device_realize_start() returns %TRUE, the caller MUST + * also call nm_device_realize_finish() to balance g_object_freeze_notify(). + * * Returns: %TRUE on success, %FALSE on error */ gboolean @@ -1866,6 +1871,7 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) priv = NM_DEVICE_GET_PRIVATE (self); /* The device should not be realized */ + g_return_if_fail (!priv->real); g_return_if_fail (priv->ip_ifindex <= 0); g_return_if_fail (priv->ip_iface == NULL); @@ -1945,8 +1951,6 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) g_object_notify (G_OBJECT (self), NM_DEVICE_CAPABILITIES); - priv->real = TRUE; - klass->realize_start_notify (self, plink); } @@ -1962,14 +1966,21 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) void nm_device_realize_finish (NMDevice *self, const NMPlatformLink *plink) { + NMDevicePrivate *priv; + + g_return_if_fail (NM_IS_DEVICE (self)); g_return_if_fail (!plink || link_type_compatible (self, plink->type, NULL, NULL)); + priv = NM_DEVICE_GET_PRIVATE (self); + + g_return_if_fail (!priv->real); + if (plink) { update_device_from_platform_link (self, plink); device_recheck_slave_status (self, plink); } - NM_DEVICE_GET_PRIVATE (self)->real = TRUE; + priv->real = TRUE; g_object_notify (G_OBJECT (self), NM_DEVICE_REAL); nm_device_recheck_available_connections (self); @@ -2016,6 +2027,7 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) priv = NM_DEVICE_GET_PRIVATE (self); g_return_val_if_fail (priv->iface != NULL, FALSE); + g_return_val_if_fail (priv->real, FALSE); g_object_freeze_notify (G_OBJECT (self)); @@ -10885,7 +10897,7 @@ get_property (GObject *object, guint prop_id, } break; case PROP_REAL: - g_value_set_boolean (value, priv->real); + g_value_set_boolean (value, nm_device_is_real (self)); break; case PROP_SLAVES: { GSList *slave_iter;