From f8da87af32a6d5fb5a75d0b04866e3ca88b3b8b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Tue, 3 Dec 2013 16:19:10 +0100 Subject: [PATCH 1/3] policy: remove schedule_activate_check() from FAILED handler The call is redundant, because the device will transition to DISCONNECTED and schedule_activate_check() will be called of this state. --- src/nm-policy.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 739ac8193..f9dd0474d 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1454,7 +1454,6 @@ device_state_changed (NMDevice *device, } nm_connection_clear_secrets (connection); } - schedule_activate_check (policy, device, 3); break; case NM_DEVICE_STATE_ACTIVATED: if (connection) { From b0fb239df3c207cc7ea2b13fbc278a7e1400c7a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Tue, 3 Dec 2013 16:49:21 +0100 Subject: [PATCH 2/3] core: clear device on NMActiveConnection when the connection is DEACTIVATED --- src/nm-active-connection.c | 26 +++++++++++++++++--------- src/vpn-manager/nm-vpn-connection.c | 8 ++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 34f438c0f..cb4241735 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -86,6 +86,7 @@ enum { }; static void check_master_ready (NMActiveConnection *self); +static void _device_cleanup (NMActiveConnectionPrivate *priv); /****************************************************************/ @@ -146,10 +147,11 @@ nm_active_connection_set_state (NMActiveConnection *self, } if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { - /* Device is no longer relevant when deactivated; emit property change - * notification so clients re-read the value, which will be NULL due to - * conditions in get_property(). + /* Device is no longer relevant when deactivated. So remove it and + * emit property change notification so clients re-read the value, + * which will be NULL due to conditions in get_property(). */ + _device_cleanup (priv); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES); } } @@ -726,6 +728,17 @@ get_property (GObject *object, guint prop_id, } } +static void +_device_cleanup (NMActiveConnectionPrivate *priv) +{ + if (priv->device_state_id) { + g_assert (priv->device); + g_signal_handler_disconnect (priv->device, priv->device_state_id); + priv->device_state_id = 0; + } + g_clear_object (&priv->device); +} + static void dispose (GObject *object) { @@ -743,12 +756,7 @@ dispose (GObject *object) g_clear_object (&priv->connection); - if (priv->device_state_id) { - g_assert (priv->device); - g_signal_handler_disconnect (priv->device, priv->device_state_id); - priv->device_state_id = 0; - } - g_clear_object (&priv->device); + _device_cleanup (priv); if (priv->master) { g_signal_handlers_disconnect_by_func (priv->master, diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index d6f0c8d32..e7c3e073e 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -206,6 +206,12 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, old_vpn_state = priv->vpn_state; priv->vpn_state = vpn_state; + /* The device gets destroyed by active connection when it enters + * the deactivated state, so we need to ref it for usage below. + */ + if (parent_dev) + g_object_ref (parent_dev); + /* Update active connection base class state */ nm_active_connection_set_state (NM_ACTIVE_CONNECTION (connection), ac_state_from_vpn_state (vpn_state)); @@ -271,6 +277,8 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, } g_object_unref (connection); + if (parent_dev) + g_object_unref (parent_dev); } static void From 0234bd4acc3be1948b24a2779212170441651a4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Thu, 12 Dec 2013 10:26:32 +0100 Subject: [PATCH 3/3] policy: invoke NMPolicy::device_state_changed() after other handlers (rh #1033187) This fixes automatic activation after changes in commit ff7e47a41858881e102ce7c3686962f43d08cce4. When a connection is deactivated impl_manager_deactivate_connection() is called and the device goes to NM_DEVICE_STATE_DISCONNECTED. nm_device_state_changed() then issues "state-changed" signal. The signal is connected to by various listeners. The most interesting ones for this case are NMPolicy and NMActiveConnection. The problem is that NMPolicy's device_state_changed() is processed first and thus in schedule_activate_check() we still have the old active connection present (in ACTIVATED state). This commit fixes the issue by connecting to "state-changed" signal using g_signal_connect_after() in NMPolicy. This ensures NMPolicy's state-changed handler is called after active connections are processed. https://bugzilla.redhat.com/show_bug.cgi?id=1033187 --- src/nm-policy.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index f9dd0474d..37b82340e 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1627,14 +1627,21 @@ typedef struct { } DeviceSignalId; static void -_connect_device_signal (NMPolicy *policy, NMDevice *device, const char *name, gpointer callback) +_connect_device_signal (NMPolicy *policy, + NMDevice *device, + const char *name, + gpointer callback, + gboolean after) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); DeviceSignalId *data; data = g_slice_new0 (DeviceSignalId); g_assert (data); - data->id = g_signal_connect (device, name, callback, policy); + if (after) + data->id = g_signal_connect_after (device, name, callback, policy); + else + data->id = g_signal_connect (device, name, callback, policy); data->device = device; priv->dev_ids = g_slist_prepend (priv->dev_ids, data); } @@ -1644,22 +1651,23 @@ device_added (NMManager *manager, NMDevice *device, gpointer user_data) { NMPolicy *policy = (NMPolicy *) user_data; - _connect_device_signal (policy, device, "state-changed", device_state_changed); - _connect_device_signal (policy, device, NM_DEVICE_IP4_CONFIG_CHANGED, device_ip4_config_changed); - _connect_device_signal (policy, device, NM_DEVICE_IP6_CONFIG_CHANGED, device_ip6_config_changed); - _connect_device_signal (policy, device, "notify::" NM_DEVICE_AUTOCONNECT, device_autoconnect_changed); + /* Connect state-changed with _after, so that the handler is invoked after other handlers. */ + _connect_device_signal (policy, device, "state-changed", device_state_changed, TRUE); + _connect_device_signal (policy, device, NM_DEVICE_IP4_CONFIG_CHANGED, device_ip4_config_changed, FALSE); + _connect_device_signal (policy, device, NM_DEVICE_IP6_CONFIG_CHANGED, device_ip6_config_changed, FALSE); + _connect_device_signal (policy, device, "notify::" NM_DEVICE_AUTOCONNECT, device_autoconnect_changed, FALSE); switch (nm_device_get_device_type (device)) { case NM_DEVICE_TYPE_WIFI: - _connect_device_signal (policy, device, "access-point-added", wireless_networks_changed); - _connect_device_signal (policy, device, "access-point-removed", wireless_networks_changed); + _connect_device_signal (policy, device, "access-point-added", wireless_networks_changed, FALSE); + _connect_device_signal (policy, device, "access-point-removed", wireless_networks_changed, FALSE); break; case NM_DEVICE_TYPE_WIMAX: - _connect_device_signal (policy, device, "nsp-added", nsps_changed); - _connect_device_signal (policy, device, "nsp-removed", nsps_changed); + _connect_device_signal (policy, device, "nsp-added", nsps_changed, FALSE); + _connect_device_signal (policy, device, "nsp-removed", nsps_changed, FALSE); break; case NM_DEVICE_TYPE_MODEM: - _connect_device_signal (policy, device, "enable-changed", modem_enabled_changed); + _connect_device_signal (policy, device, "enable-changed", modem_enabled_changed, FALSE); break; default: break;