From 3d30ff01ef1d78cee8169f5bb464b988c8ac817b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 5 Dec 2013 08:50:10 -0500 Subject: [PATCH 01/12] core: remove useless NMSettings::connections-loaded signal NMSettings (and NMConnectionProvider) had a signal to indicate when it had loaded the connections, but in reality this always happened before nm_settings_new() returned (as a side effect of calling unmanaged_specs_changed()) and so no one else would ever actually see the signal. So just kill it. --- src/bluez-manager/nm-bluez-device.c | 22 ++++--------- src/devices/nm-device.c | 20 ----------- src/nm-connection-provider.c | 17 ---------- src/nm-connection-provider.h | 17 ---------- src/nm-policy.c | 14 -------- src/settings/nm-settings.c | 51 +++-------------------------- src/settings/nm-settings.h | 3 -- 7 files changed, 11 insertions(+), 133 deletions(-) diff --git a/src/bluez-manager/nm-bluez-device.c b/src/bluez-manager/nm-bluez-device.c index 88e0ad70ae..8e8f39d066 100644 --- a/src/bluez-manager/nm-bluez-device.c +++ b/src/bluez-manager/nm-bluez-device.c @@ -180,11 +180,6 @@ pan_connection_check_create (NMBluezDevice *self) return; } - if (!nm_connection_provider_has_connections_loaded (priv->provider)) { - /* do not try to create any connections until the connection provider is ready. */ - return; - } - /* Only try once to create a connection. If it does not succeed, we do not try again. Also, * if the connection gets deleted later, do not create another one for this device. */ priv->pan_connection_no_autocreate = TRUE; @@ -387,13 +382,14 @@ cp_connection_updated (NMConnectionProvider *provider, } static void -cp_connections_loaded (NMConnectionProvider *provider, NMBluezDevice *self) +load_connections (NMBluezDevice *self) { + NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self); const GSList *connections, *iter; - connections = nm_connection_provider_get_connections (provider); + connections = nm_connection_provider_get_connections (priv->provider); for (iter = connections; iter; iter = g_slist_next (iter)) - cp_connection_added (provider, NM_CONNECTION (iter->data), self); + cp_connection_added (priv->provider, NM_CONNECTION (iter->data), self); } /***********************************************************/ @@ -832,7 +828,7 @@ get_properties_cb_4 (GObject *source_object, GAsyncResult *res, gpointer user_da g_variant_unref (v_properties); /* Check if any connections match this device */ - cp_connections_loaded (priv->provider, self); + load_connections (self); priv->initialized = TRUE; g_signal_emit (self, signals[INITIALIZED], 0, TRUE); @@ -883,7 +879,7 @@ query_properties (NMBluezDevice *self) } /* Check if any connections match this device */ - cp_connections_loaded (priv->provider, self); + load_connections (self); break; } @@ -970,11 +966,6 @@ nm_bluez_device_new (const char *path, NMConnectionProvider *provider, int bluez G_CALLBACK (cp_connection_updated), self); - g_signal_connect (priv->provider, - NM_CP_SIGNAL_CONNECTIONS_LOADED, - G_CALLBACK (cp_connections_loaded), - self); - g_bus_get (G_BUS_TYPE_SYSTEM, NULL, (GAsyncReadyCallback) on_bus_acquired, @@ -1027,7 +1018,6 @@ dispose (GObject *object) g_signal_handlers_disconnect_by_func (priv->provider, cp_connection_added, self); g_signal_handlers_disconnect_by_func (priv->provider, cp_connection_removed, self); g_signal_handlers_disconnect_by_func (priv->provider, cp_connection_updated, self); - g_signal_handlers_disconnect_by_func (priv->provider, cp_connections_loaded, self); g_slist_free_full (priv->connections, g_object_unref); priv->connections = NULL; diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b3b324b3e6..4157598055 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -351,7 +351,6 @@ static const char *reason_to_string (NMDeviceStateReason reason); static void ip_check_gw_ping_cleanup (NMDevice *self); static void cp_connection_added (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data); -static void cp_connections_loaded (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data); static void cp_connection_removed (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data); static void cp_connection_updated (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data); @@ -907,11 +906,6 @@ nm_device_set_connection_provider (NMDevice *device, G_CALLBACK (cp_connection_added), device); - priv->cp_loaded_id = g_signal_connect (priv->con_provider, - NM_CP_SIGNAL_CONNECTIONS_LOADED, - G_CALLBACK (cp_connections_loaded), - device); - priv->cp_removed_id = g_signal_connect (priv->con_provider, NM_CP_SIGNAL_CONNECTION_REMOVED, G_CALLBACK (cp_connection_removed), @@ -7042,20 +7036,6 @@ cp_connection_added (NMConnectionProvider *cp, NMConnection *connection, gpointe _signal_available_connections_changed (NM_DEVICE (user_data)); } -static void -cp_connections_loaded (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data) -{ - const GSList *connections, *iter; - gboolean added = FALSE; - - connections = nm_connection_provider_get_connections (cp); - for (iter = connections; iter; iter = g_slist_next (iter)) - added |= _try_add_available_connection (NM_DEVICE (user_data), NM_CONNECTION (iter->data)); - - if (added) - _signal_available_connections_changed (NM_DEVICE (user_data)); -} - static void cp_connection_removed (NMConnectionProvider *cp, NMConnection *connection, gpointer user_data) { diff --git a/src/nm-connection-provider.c b/src/nm-connection-provider.c index 39ede1d1e8..f6ee7454d9 100644 --- a/src/nm-connection-provider.c +++ b/src/nm-connection-provider.c @@ -41,15 +41,6 @@ nm_connection_provider_get_connections (NMConnectionProvider *self) return NULL; } -gboolean -nm_connection_provider_has_connections_loaded (NMConnectionProvider *self) -{ - g_return_val_if_fail (NM_IS_CONNECTION_PROVIDER (self), FALSE); - - g_assert (NM_CONNECTION_PROVIDER_GET_INTERFACE (self)->has_connections_loaded); - return NM_CONNECTION_PROVIDER_GET_INTERFACE (self)->has_connections_loaded (self); -} - /** * nm_connection_provider_add_connection: * @self: the #NMConnectionProvider @@ -131,14 +122,6 @@ nm_connection_provider_init (gpointer g_iface) NULL, NULL, g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, 1, G_TYPE_OBJECT); - - g_signal_new (NM_CP_SIGNAL_CONNECTIONS_LOADED, - iface_type, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMConnectionProvider, connections_loaded), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); } GType diff --git a/src/nm-connection-provider.h b/src/nm-connection-provider.h index 05c49a73e6..96db76adf7 100644 --- a/src/nm-connection-provider.h +++ b/src/nm-connection-provider.h @@ -29,7 +29,6 @@ typedef struct _NMConnectionProvider NMConnectionProvider; #define NM_CP_SIGNAL_CONNECTION_ADDED "cp-connection-added" #define NM_CP_SIGNAL_CONNECTION_UPDATED "cp-connection-updated" #define NM_CP_SIGNAL_CONNECTION_REMOVED "cp-connection-removed" -#define NM_CP_SIGNAL_CONNECTIONS_LOADED "cp-connections-loaded" /** @@ -58,8 +57,6 @@ struct _NMConnectionProvider { const GSList * (*get_connections) (NMConnectionProvider *self); - gboolean (*has_connections_loaded) (NMConnectionProvider *self); - NMConnection * (*add_connection) (NMConnectionProvider *self, NMConnection *connection, gboolean save_to_disk, @@ -75,7 +72,6 @@ struct _NMConnectionProvider { void (*connection_removed) (NMConnectionProvider *self, NMConnection *connection); - void (*connections_loaded) (NMConnectionProvider *self); }; GType nm_connection_provider_get_type (void); @@ -113,19 +109,6 @@ GSList *nm_connection_provider_get_best_connections (NMConnectionProvider *self, */ const GSList *nm_connection_provider_get_connections (NMConnectionProvider *self); -/** - * nm_connection_provider_has_connections_loaded: - * @self: the #NMConnectionProvider - * - * Returns: TRUE or FALSE indicating whether the connections of the provider are already - * loaded. If they are not yet loaded, the provider will not emit the signals - * NM_CP_SIGNAL_CONNECTION_ADDED, NM_CP_SIGNAL_CONNECTION_UPDATED and - * NM_CP_SIGNAL_CONNECTION_REMOVED until NM_CP_SIGNAL_CONNECTIONS_LOADED gets - * emited. - */ -gboolean nm_connection_provider_has_connections_loaded (NMConnectionProvider *self); - - /** * nm_connection_provider_add_connection: * @self: the #NMConnectionProvider diff --git a/src/nm-policy.c b/src/nm-policy.c index 0a0fe3f672..d8f64475a9 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1834,19 +1834,6 @@ connection_added (NMSettings *settings, schedule_activate_all ((NMPolicy *) user_data); } -static void -connections_loaded (NMSettings *settings, gpointer user_data) -{ - // FIXME: "connections-loaded" signal is emmitted *before* we connect to it - // in nm_policy_new(). So this function is never called. Currently we work around - // that by calling reset_retries_all() in nm_policy_new() - - /* Initialize connections' auto-retries */ - reset_retries_all (settings, NULL); - - schedule_activate_all ((NMPolicy *) user_data); -} - static void add_or_change_zone_cb (GError *error, gpointer user_data) { @@ -2099,7 +2086,6 @@ nm_policy_new (NMManager *manager, NMSettings *settings) _connect_manager_signal (policy, NM_MANAGER_ACTIVE_CONNECTION_ADDED, active_connection_added); _connect_manager_signal (policy, NM_MANAGER_ACTIVE_CONNECTION_REMOVED, active_connection_removed); - _connect_settings_signal (policy, NM_SETTINGS_SIGNAL_CONNECTIONS_LOADED, connections_loaded); _connect_settings_signal (policy, NM_SETTINGS_SIGNAL_CONNECTION_ADDED, connection_added); _connect_settings_signal (policy, NM_SETTINGS_SIGNAL_CONNECTION_UPDATED, connection_updated); _connect_settings_signal (policy, NM_SETTINGS_SIGNAL_CONNECTION_UPDATED_BY_USER, connection_updated_by_user); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index a114f524e3..42c8b95f68 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -152,7 +152,6 @@ enum { CONNECTION_UPDATED_BY_USER, CONNECTION_REMOVED, CONNECTION_VISIBILITY_CHANGED, - CONNECTIONS_LOADED, AGENT_REGISTERED, NEW_CONNECTION, /* exported, not used internally */ @@ -183,9 +182,6 @@ load_connections (NMSettings *self) NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); GSList *iter; - if (priv->connections_loaded) - return; - for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { NMSystemConfigInterface *plugin = NM_SYSTEM_CONFIG_INTERFACE (iter->data); GSList *plugin_connections; @@ -214,9 +210,6 @@ load_connections (NMSettings *self) unmanaged_specs_changed (NULL, self); unrecognized_specs_changed (NULL, self); - - g_signal_emit (self, signals[CONNECTIONS_LOADED], 0); - g_signal_emit_by_name (self, NM_CP_SIGNAL_CONNECTIONS_LOADED); } void @@ -233,8 +226,6 @@ nm_settings_for_each_connection (NMSettings *self, priv = NM_SETTINGS_GET_PRIVATE (self); - load_connections (self); - g_hash_table_iter_init (&iter, priv->connections); while (g_hash_table_iter_next (&iter, NULL, &data)) for_each_func (self, NM_SETTINGS_CONNECTION (data), user_data); @@ -249,8 +240,6 @@ impl_settings_list_connections (NMSettings *self, GHashTableIter iter; gpointer key; - load_connections (self); - *connections = g_ptr_array_sized_new (g_hash_table_size (priv->connections) + 1); g_hash_table_iter_init (&iter, priv->connections); while (g_hash_table_iter_next (&iter, &key, NULL)) @@ -270,8 +259,6 @@ nm_settings_get_connection_by_uuid (NMSettings *self, const char *uuid) priv = NM_SETTINGS_GET_PRIVATE (self); - load_connections (self); - g_hash_table_iter_init (&iter, priv->connections); while (g_hash_table_iter_next (&iter, NULL, (gpointer) &candidate)) { if (g_strcmp0 (uuid, nm_connection_get_uuid (NM_CONNECTION (candidate))) == 0) @@ -361,8 +348,6 @@ nm_settings_get_connection_by_path (NMSettings *self, const char *path) priv = NM_SETTINGS_GET_PRIVATE (self); - load_connections (self); - return (NMSettingsConnection *) g_hash_table_lookup (priv->connections, path); } @@ -416,7 +401,6 @@ nm_settings_get_unmanaged_specs (NMSettings *self) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - load_connections (self); return priv->unmanaged_specs; } @@ -861,9 +845,7 @@ claim_connection (NMSettings *self, g_object_ref (connection)); /* Only emit the individual connection-added signal after connections - * have been initially loaded. While getting the first list of connections - * we suppress it, then send the connections-loaded signal after we're all - * done to minimize processing. + * have been initially loaded. */ if (priv->connections_loaded) { /* Internal added signal */ @@ -1314,14 +1296,10 @@ impl_settings_reload_connections (NMSettings *self, if (!ensure_root (priv->dbus_mgr, context)) return; - if (!priv->connections_loaded) { - load_connections (self); - } else { - for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { - NMSystemConfigInterface *plugin = NM_SYSTEM_CONFIG_INTERFACE (iter->data); + for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { + NMSystemConfigInterface *plugin = NM_SYSTEM_CONFIG_INTERFACE (iter->data); - nm_system_config_interface_reload_connections (plugin); - } + nm_system_config_interface_reload_connections (plugin); } dbus_g_method_return (context, TRUE); @@ -1770,14 +1748,6 @@ get_connections (NMConnectionProvider *provider) return list; } -static gboolean -has_connections_loaded (NMConnectionProvider *provider) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (provider); - - return priv->connections_loaded; -} - static NMConnection * cp_get_connection_by_uuid (NMConnectionProvider *provider, const char *uuid) { @@ -1805,8 +1775,7 @@ nm_settings_new (GError **error) return NULL; } - unmanaged_specs_changed (NULL, self); - unrecognized_specs_changed (NULL, self); + load_connections (self); nm_dbus_manager_register_object (priv->dbus_mgr, NM_DBUS_PATH_SETTINGS, self); return self; @@ -1817,7 +1786,6 @@ connection_provider_init (NMConnectionProvider *cp_class) { cp_class->get_best_connections = get_best_connections; cp_class->get_connections = get_connections; - cp_class->has_connections_loaded = has_connections_loaded; cp_class->add_connection = _nm_connection_provider_add_connection; cp_class->get_connection_by_uuid = cp_get_connection_by_uuid; } @@ -1996,15 +1964,6 @@ nm_settings_class_init (NMSettingsClass *class) g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, 1, G_TYPE_OBJECT); - signals[CONNECTIONS_LOADED] = - g_signal_new (NM_SETTINGS_SIGNAL_CONNECTIONS_LOADED, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMSettingsClass, connections_loaded), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); - signals[AGENT_REGISTERED] = g_signal_new (NM_SETTINGS_SIGNAL_AGENT_REGISTERED, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h index d9a5211a31..5b316a4d56 100644 --- a/src/settings/nm-settings.h +++ b/src/settings/nm-settings.h @@ -49,7 +49,6 @@ #define NM_SETTINGS_SIGNAL_CONNECTION_UPDATED_BY_USER "connection-updated-by-user" #define NM_SETTINGS_SIGNAL_CONNECTION_REMOVED "connection-removed" #define NM_SETTINGS_SIGNAL_CONNECTION_VISIBILITY_CHANGED "connection-visibility-changed" -#define NM_SETTINGS_SIGNAL_CONNECTIONS_LOADED "connections-loaded" #define NM_SETTINGS_SIGNAL_AGENT_REGISTERED "agent-registered" typedef struct { @@ -70,8 +69,6 @@ typedef struct { void (*connection_visibility_changed) (NMSettings *self, NMSettingsConnection *connection); - void (*connections_loaded) (NMSettings *self); - void (*agent_registered) (NMSettings *self, NMSecretAgent *agent); } NMSettingsClass; From 828c316080ce58c43a521eca856efa95d008fd87 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 5 Dec 2013 04:57:01 -0500 Subject: [PATCH 02/12] core: simplify autoconnect retry handling Move some of the can-autoconnect tracking into NMSettingsConnection rather than having NMPolicy track it using object data. --- src/nm-policy.c | 164 +++++++++++--------------- src/settings/nm-settings-connection.c | 75 ++++++++++++ src/settings/nm-settings-connection.h | 13 ++ 3 files changed, 155 insertions(+), 97 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index d8f64475a9..55effd5c52 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -89,12 +89,6 @@ enum { PROP_ACTIVATING_IP6_DEVICE }; -#define RETRIES_TAG "autoconnect-retries" -#define RETRIES_DEFAULT 4 -#define RESET_RETRIES_TIMESTAMP_TAG "reset-retries-timestamp-tag" -#define RESET_RETRIES_TIMER 300 -#define FAILURE_REASON_TAG "failure-reason" - static void schedule_activate_all (NMPolicy *policy); @@ -927,20 +921,6 @@ check_activating_devices (NMPolicy *policy) g_object_thaw_notify (object); } -static void -set_connection_auto_retries (NMConnection *connection, guint retries) -{ - /* add +1 so that the tag still exists if the # retries is 0 */ - g_object_set_data (G_OBJECT (connection), RETRIES_TAG, GUINT_TO_POINTER (retries + 1)); -} - -static guint32 -get_connection_auto_retries (NMConnection *connection) -{ - /* subtract 1 to handle the +1 from set_connection_auto_retries() */ - return GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (connection), RETRIES_TAG)) - 1; -} - typedef struct { NMPolicy *policy; NMDevice *device; @@ -989,28 +969,11 @@ auto_activate_device (gpointer user_data) /* Remove connections that shouldn't be auto-activated */ while (iter) { NMSettingsConnection *candidate = NM_SETTINGS_CONNECTION (iter->data); - gboolean remove_it = FALSE; - const char *permission; /* Grab next item before we possibly delete the current item */ iter = g_slist_next (iter); - /* Ignore connections that were tried too many times or are not visible - * to any logged-in users. Also ignore shared wifi connections for - * which no user has the shared wifi permission. - */ - if ( get_connection_auto_retries (NM_CONNECTION (candidate)) == 0 - || nm_settings_connection_is_visible (candidate) == FALSE) - remove_it = TRUE; - else { - permission = nm_utils_get_shared_wifi_permission (NM_CONNECTION (candidate)); - if (permission) { - if (nm_settings_connection_check_permission (candidate, permission) == FALSE) - remove_it = TRUE; - } - } - - if (remove_it) + if (!nm_settings_connection_can_autoconnect (candidate)) connections = g_slist_remove (connections, candidate); } @@ -1164,32 +1127,34 @@ hostname_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) } static void -reset_retries_all (NMSettings *settings, NMDevice *device) +reset_autoconnect_all (NMPolicy *policy, NMDevice *device) { + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); GSList *connections, *iter; - GError *error = NULL; - connections = nm_settings_get_connections (settings); + connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { - if (!device || nm_device_check_connection_compatible (device, iter->data, &error)) - set_connection_auto_retries (NM_CONNECTION (iter->data), RETRIES_DEFAULT); - g_clear_error (&error); + if (!device || nm_device_check_connection_compatible (device, iter->data, NULL)) { + nm_settings_connection_reset_autoconnect_retries (iter->data); + nm_settings_connection_set_autoconnect_blocked_reason (iter->data, NM_DEVICE_STATE_REASON_NONE); + } } g_slist_free (connections); } static void -reset_retries_for_failed_secrets (NMSettings *settings) +reset_autoconnect_for_failed_secrets (NMPolicy *policy) { + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); GSList *connections, *iter; - connections = nm_settings_get_connections (settings); + connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { - NMDeviceStateReason reason = GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (iter->data), FAILURE_REASON_TAG)); + NMSettingsConnection *connection = NM_SETTINGS_CONNECTION (iter->data); - if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) { - set_connection_auto_retries (NM_CONNECTION (iter->data), RETRIES_DEFAULT); - g_object_set_data (G_OBJECT (iter->data), FAILURE_REASON_TAG, GUINT_TO_POINTER (0)); + if (nm_settings_connection_get_autoconnect_blocked_reason (connection) == NM_DEVICE_STATE_REASON_NO_SECRETS) { + nm_settings_connection_reset_autoconnect_retries (connection); + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); } } g_slist_free (connections); @@ -1199,7 +1164,6 @@ static void sleeping_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) { NMPolicy *policy = user_data; - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); gboolean sleeping = FALSE, enabled = FALSE; g_object_get (G_OBJECT (manager), NM_MANAGER_SLEEPING, &sleeping, NULL); @@ -1207,7 +1171,7 @@ sleeping_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) /* Reset retries on all connections so they'll checked on wakeup */ if (sleeping || !enabled) - reset_retries_all (priv->settings, NULL); + reset_autoconnect_all (policy, NULL); } static void @@ -1253,26 +1217,28 @@ reset_connections_retries (gpointer user_data) priv->reset_retries_id = 0; - min_stamp = now = time (NULL); + min_stamp = 0; + now = time (NULL); connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { - con_stamp = GPOINTER_TO_SIZE (g_object_get_data (G_OBJECT (iter->data), RESET_RETRIES_TIMESTAMP_TAG)); + NMSettingsConnection *connection = NM_SETTINGS_CONNECTION (iter->data); + + con_stamp = nm_settings_connection_get_autoconnect_retry_time (connection); if (con_stamp == 0) continue; - if (con_stamp + RESET_RETRIES_TIMER <= now) { - set_connection_auto_retries (NM_CONNECTION (iter->data), RETRIES_DEFAULT); - g_object_set_data (G_OBJECT (iter->data), RESET_RETRIES_TIMESTAMP_TAG, GSIZE_TO_POINTER (0)); + + if (con_stamp < now) { + nm_settings_connection_reset_autoconnect_retries (connection); + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); changed = TRUE; - continue; - } - if (con_stamp < min_stamp) + } else if (min_stamp == 0 || min_stamp > con_stamp) min_stamp = con_stamp; } g_slist_free (connections); /* Schedule the handler again if there are some stamps left */ - if (min_stamp != now) - priv->reset_retries_id = g_timeout_add_seconds (RESET_RETRIES_TIMER - (now - min_stamp), reset_connections_retries, policy); + if (min_stamp != 0) + priv->reset_retries_id = g_timeout_add_seconds (min_stamp - now, reset_connections_retries, policy); /* If anything changed, try to activate the newly re-enabled connections */ if (changed) @@ -1284,8 +1250,7 @@ reset_connections_retries (gpointer user_data) static void schedule_activate_all (NMPolicy *policy); static void -activate_slave_connections (NMPolicy *policy, NMConnection *connection, - NMDevice *device) +activate_slave_connections (NMPolicy *policy, NMDevice *device) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); const char *master_device; @@ -1306,7 +1271,7 @@ activate_slave_connections (NMPolicy *policy, NMConnection *connection, g_assert (s_slave_con); if (!g_strcmp0 (nm_setting_connection_get_master (s_slave_con), master_device)) - set_connection_auto_retries (slave, RETRIES_DEFAULT); + nm_settings_connection_reset_autoconnect_retries (NM_SETTINGS_CONNECTION (slave)); } g_slist_free (connections); @@ -1393,14 +1358,14 @@ device_state_changed (NMDevice *device, { NMPolicy *policy = (NMPolicy *) user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); - NMConnection *connection = nm_device_get_connection (device); + NMSettingsConnection *connection = NM_SETTINGS_CONNECTION (nm_device_get_connection (device)); const char *ip_iface = nm_device_get_ip_iface (device); NMIP4Config *ip4_config; NMIP6Config *ip6_config; NMSettingConnection *s_con; if (connection) - g_object_set_data (G_OBJECT (connection), FAILURE_REASON_TAG, GUINT_TO_POINTER (0)); + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); switch (new_state) { case NM_DEVICE_STATE_FAILED: @@ -1410,46 +1375,50 @@ device_state_changed (NMDevice *device, if ( connection && old_state >= NM_DEVICE_STATE_PREPARE && old_state <= NM_DEVICE_STATE_ACTIVATED) { - guint32 tries = get_connection_auto_retries (connection); + guint32 tries = nm_settings_connection_get_autoconnect_retries (connection); if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) { /* If the connection couldn't get the secrets it needed (ex because * the user canceled, or no secrets exist), there's no point in * automatically retrying because it's just going to fail anyway. */ - set_connection_auto_retries (connection, 0); + nm_settings_connection_set_autoconnect_retries (connection, 0); - /* Mark the connection as failed due to missing secrets so that we can reset - * RETRIES_TAG and automatically re-try when an secret agent registers. + /* Mark the connection as failed due to missing secrets so that we can + * automatically re-try when an secret agent registers. */ - g_object_set_data (G_OBJECT (connection), FAILURE_REASON_TAG, GUINT_TO_POINTER (NM_DEVICE_STATE_REASON_NO_SECRETS)); + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NO_SECRETS); } else if (tries > 0) { /* Otherwise if it's a random failure, just decrease the number * of automatic retries so that the connection gets tried again * if it still has a retry count. */ - set_connection_auto_retries (connection, tries - 1); + nm_settings_connection_set_autoconnect_retries (connection, tries - 1); } - if (get_connection_auto_retries (connection) == 0) { - nm_log_info (LOGD_DEVICE, "Marking connection '%s' invalid.", nm_connection_get_id (connection)); + if (nm_settings_connection_get_autoconnect_retries (connection) == 0) { + nm_log_info (LOGD_DEVICE, "Marking connection '%s' invalid.", + nm_connection_get_id (NM_CONNECTION (connection))); /* Schedule a handler to reset retries count */ - g_object_set_data (G_OBJECT (connection), RESET_RETRIES_TIMESTAMP_TAG, GSIZE_TO_POINTER ((gsize) time (NULL))); - if (!priv->reset_retries_id) - priv->reset_retries_id = g_timeout_add_seconds (RESET_RETRIES_TIMER, reset_connections_retries, policy); + if (!priv->reset_retries_id) { + time_t retry_time = nm_settings_connection_get_autoconnect_retry_time (connection); + + g_warn_if_fail (retry_time != 0); + priv->reset_retries_id = g_timeout_add_seconds (MAX (0, retry_time - time (NULL)), reset_connections_retries, policy); + } } - nm_connection_clear_secrets (connection); + nm_connection_clear_secrets (NM_CONNECTION (connection)); } break; case NM_DEVICE_STATE_ACTIVATED: if (connection) { /* Reset auto retries back to default since connection was successful */ - set_connection_auto_retries (connection, RETRIES_DEFAULT); + nm_settings_connection_reset_autoconnect_retries (connection); /* And clear secrets so they will always be requested from the * settings service when the next connection is made. */ - nm_connection_clear_secrets (connection); + nm_connection_clear_secrets (NM_CONNECTION (connection)); } /* Add device's new IPv4 and IPv6 configs to DNS */ @@ -1473,10 +1442,11 @@ device_state_changed (NMDevice *device, update_routing_and_dns (policy, FALSE); break; case NM_DEVICE_STATE_DISCONNECTED: - /* Reset RETRIES_TAG when carrier on. If cable was unplugged - * and plugged again, we should try to reconnect */ + /* Reset retry counts for a device's connections when carrier on; if cable + * was unplugged and plugged in again, we should try to reconnect. + */ if (reason == NM_DEVICE_STATE_REASON_CARRIER && old_state == NM_DEVICE_STATE_UNAVAILABLE) - reset_retries_all (priv->settings, device); + reset_autoconnect_all (policy, device); if (old_state > NM_DEVICE_STATE_DISCONNECTED) update_routing_and_dns (policy, FALSE); @@ -1488,16 +1458,16 @@ device_state_changed (NMDevice *device, case NM_DEVICE_STATE_PREPARE: /* Reset auto-connect retries of all slaves and schedule them for * activation. */ - activate_slave_connections (policy, connection, device); + activate_slave_connections (policy, device); break; case NM_DEVICE_STATE_SECONDARIES: - s_con = nm_connection_get_setting_connection (connection); + s_con = nm_connection_get_setting_connection (NM_CONNECTION (connection)); if (s_con && nm_setting_connection_get_num_secondaries (s_con) > 0) { /* Make routes and DNS up-to-date before activating dependent connections */ update_routing_and_dns (policy, FALSE); /* Activate secondary (VPN) connections */ - if (!activate_secondary_connections (policy, connection, device)) + if (!activate_secondary_connections (policy, NM_CONNECTION (connection), device)) nm_device_queue_state (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SECONDARY_CONNECTION_FAILED); } else @@ -1827,11 +1797,12 @@ schedule_activate_all (NMPolicy *policy) static void connection_added (NMSettings *settings, - NMConnection *connection, + NMSettingsConnection *connection, gpointer user_data) { - set_connection_auto_retries (connection, RETRIES_DEFAULT); - schedule_activate_all ((NMPolicy *) user_data); + NMPolicy *policy = NM_POLICY (user_data); + + schedule_activate_all (policy); } static void @@ -1944,11 +1915,11 @@ connection_updated (NMSettings *settings, static void connection_updated_by_user (NMSettings *settings, - NMConnection *connection, + NMSettingsConnection *connection, gpointer user_data) { /* Reset auto retries back to default since connection was updated */ - set_connection_auto_retries (connection, RETRIES_DEFAULT); + nm_settings_connection_reset_autoconnect_retries (connection); } static void @@ -2008,12 +1979,14 @@ secret_agent_registered (NMSettings *settings, NMSecretAgent *agent, gpointer user_data) { + NMPolicy *policy = NM_POLICY (user_data); + /* The registered secret agent may provide some missing secrets. Thus we * reset retries count here and schedule activation, so that the * connections failed due to missing secrets may re-try auto-connection. */ - reset_retries_for_failed_secrets (settings); - schedule_activate_all ((NMPolicy *) user_data); + reset_autoconnect_for_failed_secrets (policy); + schedule_activate_all (policy); } static void @@ -2094,9 +2067,6 @@ nm_policy_new (NMManager *manager, NMSettings *settings) connection_visibility_changed); _connect_settings_signal (policy, NM_SETTINGS_SIGNAL_AGENT_REGISTERED, secret_agent_registered); - /* Initialize connections' auto-retries */ - reset_retries_all (priv->settings, NULL); - initialized = TRUE; return policy; } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 55cc157180..63bc8aa083 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -124,6 +124,11 @@ typedef struct { guint64 timestamp; /* Up-to-date timestamp of connection use */ gboolean timestamp_set; GHashTable *seen_bssids; /* Up-to-date BSSIDs that's been seen for the connection */ + + int autoconnect_retries; + time_t autoconnect_retry_time; + NMDeviceStateReason autoconnect_blocked_reason; + } NMSettingsConnectionPrivate; /**************************************************************/ @@ -1899,6 +1904,73 @@ nm_settings_connection_read_and_fill_seen_bssids (NMSettingsConnection *connecti } } +#define AUTOCONNECT_RETRIES_DEFAULT 4 +#define AUTOCONNECT_RESET_RETRIES_TIMER 300 + +int +nm_settings_connection_get_autoconnect_retries (NMSettingsConnection *connection) +{ + return NM_SETTINGS_CONNECTION_GET_PRIVATE (connection)->autoconnect_retries; +} + +void +nm_settings_connection_set_autoconnect_retries (NMSettingsConnection *connection, + int retries) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (connection); + + priv->autoconnect_retries = retries; + if (retries) + priv->autoconnect_retry_time = 0; + else + priv->autoconnect_retry_time = time (NULL) + AUTOCONNECT_RESET_RETRIES_TIMER; +} + +void +nm_settings_connection_reset_autoconnect_retries (NMSettingsConnection *connection) +{ + nm_settings_connection_set_autoconnect_retries (connection, AUTOCONNECT_RETRIES_DEFAULT); +} + +time_t +nm_settings_connection_get_autoconnect_retry_time (NMSettingsConnection *connection) +{ + return NM_SETTINGS_CONNECTION_GET_PRIVATE (connection)->autoconnect_retry_time; +} + +NMDeviceStateReason +nm_settings_connection_get_autoconnect_blocked_reason (NMSettingsConnection *connection) +{ + return NM_SETTINGS_CONNECTION_GET_PRIVATE (connection)->autoconnect_blocked_reason; +} + +void +nm_settings_connection_set_autoconnect_blocked_reason (NMSettingsConnection *connection, + NMDeviceStateReason reason) +{ + NM_SETTINGS_CONNECTION_GET_PRIVATE (connection)->autoconnect_blocked_reason = reason; +} + +gboolean +nm_settings_connection_can_autoconnect (NMSettingsConnection *connection) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (connection); + const char *permission; + + if ( !priv->visible + || priv->autoconnect_retries == 0 + || priv->autoconnect_blocked_reason != NM_DEVICE_STATE_REASON_NONE) + return FALSE; + + permission = nm_utils_get_shared_wifi_permission (NM_CONNECTION (connection)); + if (permission) { + if (nm_settings_connection_check_permission (connection, permission) == FALSE) + return FALSE; + } + + return TRUE; +} + /**************************************************************/ static void @@ -1918,6 +1990,9 @@ nm_settings_connection_init (NMSettingsConnection *self) priv->seen_bssids = g_hash_table_new_full (mac_hash, mac_equal, g_free, g_free); + priv->autoconnect_retries = AUTOCONNECT_RETRIES_DEFAULT; + priv->autoconnect_blocked_reason = NM_DEVICE_STATE_REASON_NONE; + g_signal_connect (self, NM_CONNECTION_SECRETS_CLEARED, G_CALLBACK (secrets_cleared_cb), NULL); g_signal_connect (self, NM_CONNECTION_CHANGED, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE)); } diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index a16acc9f5b..d79c62d912 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -149,6 +149,19 @@ void nm_settings_connection_add_seen_bssid (NMSettingsConnection *connection, void nm_settings_connection_read_and_fill_seen_bssids (NMSettingsConnection *connection); +int nm_settings_connection_get_autoconnect_retries (NMSettingsConnection *connection); +void nm_settings_connection_set_autoconnect_retries (NMSettingsConnection *connection, + int retries); +void nm_settings_connection_reset_autoconnect_retries (NMSettingsConnection *connection); + +time_t nm_settings_connection_get_autoconnect_retry_time (NMSettingsConnection *connection); + +NMDeviceStateReason nm_settings_connection_get_autoconnect_blocked_reason (NMSettingsConnection *connection); +void nm_settings_connection_set_autoconnect_blocked_reason (NMSettingsConnection *connection, + NMDeviceStateReason reason); + +gboolean nm_settings_connection_can_autoconnect (NMSettingsConnection *connection); + G_END_DECLS #endif /* NM_SETTINGS_CONNECTION_H */ From 4e74670b472e34bfa6c40534714cd86f5dfa7b4a Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 23 Jan 2014 10:30:50 -0500 Subject: [PATCH 03/12] core: clarify clearing of autoconnect-blocked state NMPolicy was clearing the autoconnect-blocked state on a connection any time a device with that connection changed state. This happened to basically do the right thing, but it would be clearer if we only reset the state after successfully getting past the NEED_AUTH stage. --- src/nm-policy.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 55effd5c52..b47343cc20 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1364,9 +1364,6 @@ device_state_changed (NMDevice *device, NMIP6Config *ip6_config; NMSettingConnection *s_con; - if (connection) - nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); - switch (new_state) { case NM_DEVICE_STATE_FAILED: /* Mark the connection invalid if it failed during activation so that @@ -1460,6 +1457,10 @@ device_state_changed (NMDevice *device, * activation. */ activate_slave_connections (policy, device); break; + case NM_DEVICE_STATE_IP_CONFIG: + /* We must have secrets if we got here. */ + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); + break; case NM_DEVICE_STATE_SECONDARIES: s_con = nm_connection_get_setting_connection (NM_CONNECTION (connection)); if (s_con && nm_setting_connection_get_num_secondaries (s_con) > 0) { From eceb613f4c62eb9bb2d29d1e8ae720db8311606d Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 15 Jan 2014 12:47:13 -0500 Subject: [PATCH 04/12] core: don't retry connection with no secrets after timeout NMPolicy was resetting the "don't autoconnect because we don't have secrets" state on a connection when the autoconnect-retries timer timed out, but this doesn't make sense, since the timeout doesn't change the fact that there are no secrets. https://bugzilla.gnome.org/show_bug.cgi?id=670631 --- src/nm-policy.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index b47343cc20..bcf36ce762 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1229,7 +1229,6 @@ reset_connections_retries (gpointer user_data) if (con_stamp < now) { nm_settings_connection_reset_autoconnect_retries (connection); - nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NONE); changed = TRUE; } else if (min_stamp == 0 || min_stamp > con_stamp) min_stamp = con_stamp; @@ -1375,12 +1374,6 @@ device_state_changed (NMDevice *device, guint32 tries = nm_settings_connection_get_autoconnect_retries (connection); if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) { - /* If the connection couldn't get the secrets it needed (ex because - * the user canceled, or no secrets exist), there's no point in - * automatically retrying because it's just going to fail anyway. - */ - nm_settings_connection_set_autoconnect_retries (connection, 0); - /* Mark the connection as failed due to missing secrets so that we can * automatically re-try when an secret agent registers. */ From c4fc72c795c144fb540b5fd056b4cbbaae60b6e1 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 3 Jan 2014 13:58:05 -0500 Subject: [PATCH 05/12] devices: start using the DEACTIVATING state When a device is disconnected by the user (as opposed to due to network or hardware error, etc), set it first to DEACTIVATING, which does nothing but queue a transition to disconnected. This lets other parts of NM observe the device when it is about-to-disconnect, but still has an associated connection. --- src/devices/nm-device.c | 5 ++++- src/nm-manager.c | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 4157598055..2a459bb64f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4618,7 +4618,7 @@ disconnect_cb (NMDevice *device, TRUE); nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, + NM_DEVICE_STATE_DEACTIVATING, NM_DEVICE_STATE_REASON_USER_REQUESTED); dbus_g_method_return (context); } @@ -6304,6 +6304,9 @@ nm_device_state_changed (NMDevice *device, nm_device_queue_state (device, NM_DEVICE_STATE_UNMANAGED, NM_DEVICE_STATE_REASON_NONE); } break; + case NM_DEVICE_STATE_DEACTIVATING: + nm_device_queue_state (device, NM_DEVICE_STATE_DISCONNECTED, reason); + break; case NM_DEVICE_STATE_DISCONNECTED: if (old_state > NM_DEVICE_STATE_DISCONNECTED && priv->default_unmanaged) nm_device_queue_state (device, NM_DEVICE_STATE_UNMANAGED, NM_DEVICE_STATE_REASON_NONE); diff --git a/src/nm-manager.c b/src/nm-manager.c index 2f4785257d..d3fa52436e 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -3489,9 +3489,8 @@ nm_manager_deactivate_connection (NMManager *manager, "The VPN connection was not active."); } else { g_assert (NM_IS_ACT_REQUEST (active)); - /* FIXME: use DEACTIVATING state */ nm_device_state_changed (nm_active_connection_get_device (active), - NM_DEVICE_STATE_DISCONNECTED, + NM_DEVICE_STATE_DEACTIVATING, reason); success = TRUE; } From 971167e2a8693c75e84854a030ba491932b7d105 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 3 Jan 2014 14:04:54 -0500 Subject: [PATCH 06/12] core: disable auto-re-connect of intentionally-disconnected connection If a connection is disconnected by the user, don't allow it to autoconnect again immediately after. --- src/nm-policy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/nm-policy.c b/src/nm-policy.c index bcf36ce762..01ddc15436 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1431,6 +1431,10 @@ device_state_changed (NMDevice *device, if (old_state > NM_DEVICE_STATE_DISCONNECTED) update_routing_and_dns (policy, FALSE); break; + case NM_DEVICE_STATE_DEACTIVATING: + if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_USER_REQUESTED); + break; case NM_DEVICE_STATE_DISCONNECTED: /* Reset retry counts for a device's connections when carrier on; if cable * was unplugged and plugged in again, we should try to reconnect. From 979b8920b465867ea248dee23a8a290da28f75e5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 17 Dec 2013 10:09:47 -0500 Subject: [PATCH 07/12] core: move virtual device autoconnect tracking bits out of NMManager Virtual devices may be created and destroyed, but we need to keep their autoconnect state across that. Previously this was handled by NMManager, but it really belongs with the other autoconnect tracking in NMPolicy and NMSettingsConnection. This also fixes a bug where NMPolicy would sometimes decide to autoactivate a virtual device connection which NMManager would then have to cancel. --- src/devices/nm-device.c | 7 --- src/nm-manager.c | 76 ++------------------------- src/nm-manager.h | 6 --- src/nm-policy.c | 33 +++++++++++- src/settings/nm-settings-connection.c | 5 ++ 5 files changed, 40 insertions(+), 87 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2a459bb64f..6d1d5cd7ee 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4610,13 +4610,6 @@ disconnect_cb (NMDevice *device, } else { priv->autoconnect = FALSE; - /* Software devices are removed when manually disconnected and thus - * we need to track the autoconnect flag outside the device. - */ - nm_manager_prevent_device_auto_connect (nm_manager_get (), - nm_device_get_ip_iface (device), - TRUE); - nm_device_state_changed (device, NM_DEVICE_STATE_DEACTIVATING, NM_DEVICE_STATE_REASON_USER_REQUESTED); diff --git a/src/nm-manager.c b/src/nm-manager.c index d3fa52436e..1edcda1622 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -249,9 +249,6 @@ typedef struct { guint timestamp_update_id; - /* Track auto-activation for software devices */ - GHashTable *noauto_sw_devices; - gboolean startup; gboolean disposed; } NMManagerPrivate; @@ -1214,21 +1211,11 @@ system_create_virtual_devices (NMManager *self) connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { NMConnection *connection = iter->data; - NMSettingConnection *s_con = nm_connection_get_setting_connection (connection); - g_assert (s_con); - if (connection_needs_virtual_device (connection)) { - char *iface = get_virtual_iface_name (self, connection, NULL); - - /* We only create a virtual interface if the connection can autoconnect - * and the interface was not manually disconnected before. - */ - if ( nm_setting_connection_get_autoconnect (s_con) - && iface - && nm_manager_can_device_auto_connect (self, iface)) - system_create_virtual_device (self, connection); - g_free (iface); - } + /* We only create a virtual interface if the connection can autoconnect */ + if ( connection_needs_virtual_device (connection) + && nm_settings_connection_can_autoconnect (NM_SETTINGS_CONNECTION (connection))) + system_create_virtual_device (self, connection); } g_slist_free (connections); } @@ -2720,31 +2707,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * device = nm_active_connection_get_device (active); if (!device) { - char *iface; - g_assert (connection_needs_virtual_device (connection)); - iface = get_virtual_iface_name (self, connection, NULL); - g_assert (iface); - - /* Create the software device. Only exception is when: - * - this is an auto-activation *and* the device denies auto-activation - * at this time (the device was manually disconnected/deleted before) - */ - if (!nm_manager_can_device_auto_connect (self, iface)) { - if (nm_active_connection_get_user_requested (active)) { - /* Manual activation - allow device auto-activation again */ - nm_manager_prevent_device_auto_connect (self, iface, FALSE); - } else { - g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_AUTOCONNECT_NOT_ALLOWED, - "Automatic activation of '%s' not allowed for connection '%s'", - iface, nm_connection_get_id (connection)); - g_free (iface); - return FALSE; - } - } - g_free (iface); - device = system_create_virtual_device (self, connection); if (!device) { g_set_error_literal (error, @@ -3617,33 +3581,6 @@ done: g_clear_error (&error); } -/* - * Track (software) devices that cannot auto activate. - * It is needed especially for software devices, that can be removed and added - * again. So we can't simply use a flag inside the device. - */ -void -nm_manager_prevent_device_auto_connect (NMManager *manager, const char *ifname, gboolean prevent) -{ - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); - - if (prevent) - g_hash_table_add (priv->noauto_sw_devices, g_strdup (ifname)); - else - g_hash_table_remove (priv->noauto_sw_devices, ifname); -} - -gboolean -nm_manager_can_device_auto_connect (NMManager *manager, const char *ifname) -{ - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); - - if (!ifname) - return FALSE; - - return !g_hash_table_contains (priv->noauto_sw_devices, ifname); -} - static void do_sleep_wake (NMManager *self) { @@ -4719,8 +4656,6 @@ dispose (GObject *object) g_object_unref (priv->fw_monitor); } - g_hash_table_unref (priv->noauto_sw_devices); - g_slist_free (priv->factories); if (priv->timestamp_update_id) { @@ -5085,9 +5020,6 @@ nm_manager_init (NMManager *manager) KERNEL_FIRMWARE_DIR); } - /* Hash table storing software devices that should not auto activate */ - priv->noauto_sw_devices = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); - load_device_factories (manager); /* Update timestamps in active connections */ diff --git a/src/nm-manager.h b/src/nm-manager.h index 4d84db5d26..25e30f3352 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -128,12 +128,6 @@ gboolean nm_manager_deactivate_connection (NMManager *manager, NMDeviceStateReason reason, GError **error); -void nm_manager_prevent_device_auto_connect (NMManager *manager, - const char *ifname, - gboolean prevent); -gboolean nm_manager_can_device_auto_connect (NMManager *manager, - const char *ifname); - /* State handling */ NMState nm_manager_get_state (NMManager *manager); diff --git a/src/nm-policy.c b/src/nm-policy.c index 01ddc15436..dee85d678b 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1160,6 +1160,28 @@ reset_autoconnect_for_failed_secrets (NMPolicy *policy) g_slist_free (connections); } +static void +block_autoconnect_for_device (NMPolicy *policy, NMDevice *device) +{ + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); + GSList *connections, *iter; + + /* NMDevice keeps its own autoconnect-able-ness state; we only need to + * explicitly block connections for software devices, where the NMDevice + * might be destroyed and recreated later. + */ + if (!nm_device_is_software (device)) + return; + + connections = nm_settings_get_connections (priv->settings); + for (iter = connections; iter; iter = g_slist_next (iter)) { + if (nm_device_check_connection_compatible (device, iter->data, NULL)) { + nm_settings_connection_set_autoconnect_blocked_reason (NM_SETTINGS_CONNECTION (iter->data), + NM_DEVICE_STATE_REASON_USER_REQUESTED); + } + } +} + static void sleeping_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) { @@ -1432,8 +1454,15 @@ device_state_changed (NMDevice *device, update_routing_and_dns (policy, FALSE); break; case NM_DEVICE_STATE_DEACTIVATING: - if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) - nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_USER_REQUESTED); + if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) { + if (!nm_device_get_autoconnect (device)) { + /* The device was disconnected; block all connections on it */ + block_autoconnect_for_device (policy, device); + } else { + /* The connection was deactivated, so block just this connection */ + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_USER_REQUESTED); + } + } break; case NM_DEVICE_STATE_DISCONNECTED: /* Reset retry counts for a device's connections when carrier on; if cable diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 63bc8aa083..2a23396759 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1955,6 +1955,7 @@ gboolean nm_settings_connection_can_autoconnect (NMSettingsConnection *connection) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (connection); + NMSettingConnection *s_con; const char *permission; if ( !priv->visible @@ -1962,6 +1963,10 @@ nm_settings_connection_can_autoconnect (NMSettingsConnection *connection) || priv->autoconnect_blocked_reason != NM_DEVICE_STATE_REASON_NONE) return FALSE; + s_con = nm_connection_get_setting_connection (NM_CONNECTION (connection)); + if (!nm_setting_connection_get_autoconnect (s_con)) + return FALSE; + permission = nm_utils_get_shared_wifi_permission (NM_CONNECTION (connection)); if (permission) { if (nm_settings_connection_check_permission (connection, permission) == FALSE) From 12ee696d8375279d14733cb98dec4ce2405bc0b4 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 17 Dec 2013 10:30:04 -0500 Subject: [PATCH 08/12] core: add some autoconnect debugging messages --- src/nm-policy.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index dee85d678b..92ec1abe96 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1132,6 +1132,12 @@ reset_autoconnect_all (NMPolicy *policy, NMDevice *device) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); GSList *connections, *iter; + if (device) { + nm_log_dbg (LOGD_DEVICE, "Re-enabling autoconnect for all connections on %s", + nm_device_get_iface (device)); + } else + nm_log_dbg (LOGD_DEVICE, "Re-enabling autoconnect for all connections"); + connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { if (!device || nm_device_check_connection_compatible (device, iter->data, NULL)) { @@ -1148,6 +1154,8 @@ reset_autoconnect_for_failed_secrets (NMPolicy *policy) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); GSList *connections, *iter; + nm_log_dbg (LOGD_DEVICE, "Re-enabling autoconnect for all connections with failed secrets"); + connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { NMSettingsConnection *connection = NM_SETTINGS_CONNECTION (iter->data); @@ -1166,6 +1174,9 @@ block_autoconnect_for_device (NMPolicy *policy, NMDevice *device) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); GSList *connections, *iter; + nm_log_dbg (LOGD_DEVICE, "Blocking autoconnect for all connections on %s", + nm_device_get_iface (device)); + /* NMDevice keeps its own autoconnect-able-ness state; we only need to * explicitly block connections for software devices, where the NMDevice * might be destroyed and recreated later. @@ -1396,20 +1407,18 @@ device_state_changed (NMDevice *device, guint32 tries = nm_settings_connection_get_autoconnect_retries (connection); if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) { - /* Mark the connection as failed due to missing secrets so that we can - * automatically re-try when an secret agent registers. - */ + nm_log_dbg (LOGD_DEVICE, "Connection '%s' now blocked from autoconnect due to no secrets", + nm_connection_get_id (NM_CONNECTION (connection))); + nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_NO_SECRETS); } else if (tries > 0) { - /* Otherwise if it's a random failure, just decrease the number - * of automatic retries so that the connection gets tried again - * if it still has a retry count. - */ + nm_log_dbg (LOGD_DEVICE, "Connection '%s' failed to autoconnect; %d tries left", + nm_connection_get_id (NM_CONNECTION (connection)), tries); nm_settings_connection_set_autoconnect_retries (connection, tries - 1); } if (nm_settings_connection_get_autoconnect_retries (connection) == 0) { - nm_log_info (LOGD_DEVICE, "Marking connection '%s' invalid.", + nm_log_info (LOGD_DEVICE, "Disabling autoconnect for connection '%s'.", nm_connection_get_id (NM_CONNECTION (connection))); /* Schedule a handler to reset retries count */ if (!priv->reset_retries_id) { @@ -1460,6 +1469,8 @@ device_state_changed (NMDevice *device, block_autoconnect_for_device (policy, device); } else { /* The connection was deactivated, so block just this connection */ + nm_log_dbg (LOGD_DEVICE, "Blocking autoconnect of connection '%s' by user request", + nm_connection_get_id (NM_CONNECTION (connection))); nm_settings_connection_set_autoconnect_blocked_reason (connection, NM_DEVICE_STATE_REASON_USER_REQUESTED); } } From 5cac8dad79ed9b418d128314954373080c324347 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 2 Jan 2014 14:46:02 -0500 Subject: [PATCH 09/12] devices: add :master property Add a property to NMDevice that can be used to tell whether the device is enslaved, and if so what its master is. This is currently internal-only, but it could be exported later perhaps. --- src/devices/nm-device.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/devices/nm-device.h | 4 ++++ 2 files changed, 44 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6d1d5cd7ee..c4e7ace3cc 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -137,6 +137,7 @@ enum { PROP_AVAILABLE_CONNECTIONS, PROP_PHYSICAL_PORT_ID, PROP_IS_MASTER, + PROP_MASTER, PROP_HW_ADDRESS, PROP_HAS_PENDING_ACTION, LAST_PROP @@ -1415,6 +1416,27 @@ nm_device_master_release_slaves (NMDevice *self, gboolean failed) } } +/** + * nm_device_get_master: + * @dev: the device + * + * If @dev has been enslaved by another device, this returns that + * device. Otherwise it returns %NULL. (In particular, note that if + * @dev is in the process of activating as a slave, but has not yet + * been enslaved by its master, this will return %NULL.) + * + * Returns: (transfer none): @dev's master, or %NULL + */ +NMDevice * +nm_device_get_master (NMDevice *dev) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); + + if (priv->enslaved) + return priv->master; + else + return NULL; +} /** * nm_device_slave_notify_enslave: @@ -1441,6 +1463,7 @@ nm_device_slave_notify_enslave (NMDevice *dev, gboolean success) nm_connection_get_id (connection)); priv->enslaved = TRUE; + g_object_notify (G_OBJECT (dev), NM_DEVICE_MASTER); } else { nm_log_warn (LOGD_DEVICE, "Activation (%s) connection '%s' could not be enslaved", @@ -1494,6 +1517,11 @@ nm_device_slave_notify_release (NMDevice *dev, gboolean master_failed) nm_device_queue_state (dev, new_state, reason); } + + if (priv->enslaved) { + priv->enslaved = FALSE; + g_object_notify (G_OBJECT (dev), NM_DEVICE_MASTER); + } } /** @@ -4551,6 +4579,7 @@ nm_device_deactivate (NMDevice *self, NMDeviceStateReason reason) /* slave: mark no longer enslaved */ g_clear_object (&priv->master); priv->enslaved = FALSE; + g_object_notify (G_OBJECT (self), NM_DEVICE_MASTER); /* Tear down an existing activation request */ clear_act_request (self); @@ -5629,6 +5658,9 @@ get_property (GObject *object, guint prop_id, case PROP_IS_MASTER: g_value_set_boolean (value, priv->is_master); break; + case PROP_MASTER: + g_value_set_object (value, priv->master); + break; case PROP_HW_ADDRESS: if (priv->hw_addr_len) g_value_take_string (value, nm_utils_hwaddr_ntoa_len (priv->hw_addr, priv->hw_addr_len)); @@ -5906,6 +5938,14 @@ nm_device_class_init (NMDeviceClass *klass) FALSE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); + g_object_class_install_property + (object_class, PROP_MASTER, + g_param_spec_object (NM_DEVICE_MASTER, + "Master", + "Master", + NM_TYPE_DEVICE, + G_PARAM_READABLE)); + g_object_class_install_property (object_class, PROP_HW_ADDRESS, g_param_spec_string (NM_DEVICE_HW_ADDRESS, diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 4573db000c..e8f1f6a4ce 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -66,6 +66,7 @@ #define NM_DEVICE_RFKILL_TYPE "rfkill-type" /* Internal only */ #define NM_DEVICE_IFINDEX "ifindex" /* Internal only */ #define NM_DEVICE_IS_MASTER "is-master" /* Internal only */ +#define NM_DEVICE_MASTER "master" /* Internal only */ #define NM_DEVICE_HW_ADDRESS "hw-address" /* Internal only */ #define NM_DEVICE_HAS_PENDING_ACTION "has-pending-action" /* Internal only */ @@ -239,6 +240,9 @@ gboolean nm_device_master_add_slave (NMDevice *dev, NMDevice *slave, gbo GSList * nm_device_master_get_slaves (NMDevice *dev); gboolean nm_device_is_master (NMDevice *dev); +/* Slave */ +NMDevice * nm_device_get_master (NMDevice *dev); + NMActRequest * nm_device_get_act_request (NMDevice *dev); NMConnection * nm_device_get_connection (NMDevice *dev); From 072dca8ad08b26bf05e14992cfc8443cf7b91585 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 17 Jan 2014 15:33:18 -0500 Subject: [PATCH 10/12] core: properly deactivate active connections that fail early If a master activation failed early (eg, because the virtual device could not be created), then the slaves were not being notified of the failure. Fix that. --- src/nm-manager.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 1edcda1622..4fa1991225 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2942,6 +2942,22 @@ _new_active_connection (NMManager *self, device); } +static void +_internal_activation_failed (NMManager *self, + NMActiveConnection *active, + const char *error_desc) +{ + nm_log_warn (LOGD_CORE, "Failed to activate '%s': %s", + nm_connection_get_id (nm_active_connection_get_connection (active)), + error_desc); + + if (nm_active_connection_get_state (active) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + nm_active_connection_set_state (active, NM_ACTIVE_CONNECTION_STATE_DEACTIVATING); + nm_active_connection_set_state (active, NM_ACTIVE_CONNECTION_STATE_DEACTIVATED); + } + active_connection_remove (self, active); +} + static void _internal_activation_auth_done (NMActiveConnection *active, gboolean success, @@ -2956,12 +2972,9 @@ _internal_activation_auth_done (NMActiveConnection *active, if (_internal_activate_generic (self, active, &error)) return; } - g_assert (error_desc || error); - active_connection_remove (self, active); - nm_log_warn (LOGD_CORE, "Failed to activate '%s': %s", - nm_connection_get_id (nm_active_connection_get_connection (active)), - error_desc ? error_desc : error->message); + g_assert (error_desc || error); + _internal_activation_failed (self, active, error_desc ? error_desc : error->message); g_clear_error (&error); } @@ -3130,8 +3143,8 @@ activation_auth_done (NMActiveConnection *active, error_desc); } - active_connection_remove (self, active); dbus_g_method_return_error (context, error); + _internal_activation_failed (self, active, error->message); g_error_free (error); } @@ -3275,10 +3288,8 @@ activation_add_done (NMSettings *self, error = local; } - active_connection_remove (info->manager, info->active); - - if (error) - dbus_g_method_return_error (context, error); + _internal_activation_failed (info->manager, info->active, error->message); + dbus_g_method_return_error (context, error); g_clear_error (&local); done: From 26cfe9f5ce938327b1ed9c2d98d5592450429b09 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 2 Jan 2014 14:59:46 -0500 Subject: [PATCH 11/12] core: fix master deactivation NMActiveConnection was categorizing all deactivation of master connections as "failure", and NMActRequest was deactivating all of the master's slaves with REASON_DEPENDENCY_FAILED no matter what the real reason was. In fact, NMActiveConnection only needs to handle the cases where the master fails before enslaving the device; any failure after that point will be caught by existing master/slave checks in NMDevice. So update the code accordingly (and remove the master_failed code from NMVpnConnection entirely, since no master supports having VPN slaves). --- src/nm-active-connection.c | 77 ++++++++++++++++++----------- src/vpn-manager/nm-vpn-connection.c | 12 ----- 2 files changed, 48 insertions(+), 41 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 3631470318..f7e275a4cf 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -44,7 +44,6 @@ typedef struct { char *path; char *specific_object; NMDevice *device; - guint32 device_state_id; char *pending_activation_id; @@ -92,7 +91,7 @@ enum { }; static void check_master_ready (NMActiveConnection *self); -static void _device_cleanup (NMActiveConnectionPrivate *priv); +static void _device_cleanup (NMActiveConnection *self); /****************************************************************/ @@ -169,7 +168,7 @@ nm_active_connection_set_state (NMActiveConnection *self, * emit property change notification so clients re-read the value, * which will be NULL due to conditions in get_property(). */ - _device_cleanup (priv); + _device_cleanup (self); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES); } } @@ -344,10 +343,8 @@ device_state_changed (NMDevice *device, /* If the device used to be active, but now is disconnected/failed, we * no longer care about its state. */ - if (new_state <= NM_DEVICE_STATE_DISCONNECTED || new_state == NM_DEVICE_STATE_FAILED) { - g_signal_handler_disconnect (device, priv->device_state_id); - priv->device_state_id = 0; - } + if (new_state <= NM_DEVICE_STATE_DISCONNECTED || new_state == NM_DEVICE_STATE_FAILED) + g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_state_changed), self); } /* Let subclasses handle the state change */ @@ -355,6 +352,31 @@ device_state_changed (NMDevice *device, NM_ACTIVE_CONNECTION_GET_CLASS (self)->device_state_changed (self, device, new_state, old_state); } +static void +device_master_changed (GObject *object, + GParamSpec *pspec, + gpointer user_data) +{ + NMDevice *device = NM_DEVICE (object); + NMActiveConnection *self = NM_ACTIVE_CONNECTION (user_data); + NMActiveConnection *master; + NMActiveConnectionState master_state; + + if (!nm_device_get_master (device)) + return; + g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_master_changed), self); + + master = nm_active_connection_get_master (self); + g_assert (master); + + master_state = nm_active_connection_get_state (master); + if (master_state >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) { + /* Master failed before attaching the slave */ + if (NM_ACTIVE_CONNECTION_GET_CLASS (self)->master_failed) + NM_ACTIVE_CONNECTION_GET_CLASS (self)->master_failed (self); + } +} + gboolean nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device) { @@ -374,10 +396,10 @@ nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device) priv->device = g_object_ref (device); g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_INT_DEVICE); - priv->device_state_id = g_signal_connect (device, - "state-changed", - G_CALLBACK (device_state_changed), - self); + g_signal_connect (device, "state-changed", + G_CALLBACK (device_state_changed), self); + g_signal_connect (device, "notify::master", + G_CALLBACK (device_master_changed), self); priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)self); nm_device_add_pending_action (device, priv->pending_activation_id); @@ -457,7 +479,6 @@ master_state_cb (NMActiveConnection *master, gpointer user_data) { NMActiveConnection *self = NM_ACTIVE_CONNECTION (user_data); - NMActiveConnectionState self_state = nm_active_connection_get_state (self); NMActiveConnectionState master_state = nm_active_connection_get_state (master); check_master_ready (self); @@ -465,15 +486,9 @@ master_state_cb (NMActiveConnection *master, nm_log_dbg (LOGD_DEVICE, "(%p): master ActiveConnection [%p] state now '%s' (%d)", self, master, state_to_string (master_state), master_state); - /* Master is deactivating, so this active connection must also deactivate */ - if (self_state < NM_ACTIVE_CONNECTION_STATE_DEACTIVATING && - master_state >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) { - nm_log_dbg (LOGD_DEVICE, "(%p): master ActiveConnection [%p] '%s' failed", - self, master, nm_active_connection_get_name (master)); - - g_signal_handlers_disconnect_by_func (master, - (GCallback) master_state_cb, - self); + if (master_state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATING && + nm_active_connection_get_device (master) == NULL) { + /* Master failed without ever creating its device */ if (NM_ACTIVE_CONNECTION_GET_CLASS (self)->master_failed) NM_ACTIVE_CONNECTION_GET_CLASS (self)->master_failed (self); } @@ -761,24 +776,28 @@ get_property (GObject *object, guint prop_id, } static void -_device_cleanup (NMActiveConnectionPrivate *priv) +_device_cleanup (NMActiveConnection *self) { - if (priv->device_state_id) { - g_assert (priv->device); - g_signal_handler_disconnect (priv->device, priv->device_state_id); - priv->device_state_id = 0; + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + + if (priv->device) { + g_signal_handlers_disconnect_by_func (priv->device, G_CALLBACK (device_state_changed), self); + g_signal_handlers_disconnect_by_func (priv->device, G_CALLBACK (device_master_changed), self); } + if (priv->pending_activation_id) { nm_device_remove_pending_action (priv->device, priv->pending_activation_id); g_clear_pointer (&priv->pending_activation_id, g_free); } + g_clear_object (&priv->device); } static void dispose (GObject *object) { - NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (object); + NMActiveConnection *self = NM_ACTIVE_CONNECTION (object); + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); if (priv->chain) { nm_auth_chain_unref (priv->chain); @@ -792,12 +811,12 @@ dispose (GObject *object) g_clear_object (&priv->connection); - _device_cleanup (priv); + _device_cleanup (self); if (priv->master) { g_signal_handlers_disconnect_by_func (priv->master, (GCallback) master_state_cb, - NM_ACTIVE_CONNECTION (object)); + self); } g_clear_object (&priv->master); g_clear_object (&priv->subject); diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index a4bc431300..8541f10851 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -300,17 +300,6 @@ device_state_changed (NMActiveConnection *active, } } -static void -master_failed (NMActiveConnection *self) -{ - NMVPNConnection *connection = NM_VPN_CONNECTION (self); - - /* Master failure fails the VPN */ - nm_vpn_connection_set_vpn_state (connection, - NM_VPN_CONNECTION_STATE_FAILED, - NM_VPN_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); -} - static void add_ip4_vpn_gateway_route (NMDevice *parent_device, guint32 vpn_gw) { @@ -1828,7 +1817,6 @@ nm_vpn_connection_class_init (NMVPNConnectionClass *connection_class) object_class->constructed = constructed; object_class->dispose = dispose; object_class->finalize = finalize; - active_class->master_failed = master_failed; active_class->device_state_changed = device_state_changed; g_object_class_override_property (object_class, PROP_MASTER, NM_ACTIVE_CONNECTION_MASTER); From e2ab0eaf75c9cf64c100b8a45ce4f8ac1b369004 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 5 Dec 2013 09:13:26 -0500 Subject: [PATCH 12/12] devices: when disconnecting master, propagate reason to the slaves When disconnecting a master device, propagate its NMDeviceStateReason to the slaves. That way, if the reason is USER_REQUESTED, then the slaves will be blocked from re-autoconnecting as well. --- src/devices/nm-device.c | 58 +++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c4e7ace3cc..0454ced8ae 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -365,7 +365,7 @@ static void update_ip_config (NMDevice *self, gboolean initial); static void device_ip_changed (NMPlatform *platform, int ifindex, gpointer platform_object, NMPlatformReason reason, gpointer user_data); static void nm_device_slave_notify_enslave (NMDevice *dev, gboolean success); -static void nm_device_slave_notify_release (NMDevice *dev, gboolean master_failed); +static void nm_device_slave_notify_release (NMDevice *dev, NMDeviceStateReason reason); static void addrconf6_start_with_link_ready (NMDevice *self); @@ -1007,7 +1007,6 @@ nm_device_enslave_slave (NMDevice *dev, NMDevice *slave, NMConnection *connectio * nm_device_release_one_slave: * @dev: the master device * @slave: the slave device to release - * @master_failed: %TRUE if the release was unexpected, ie the master failed * * If @dev is capable of enslaving other devices (ie it's a bridge, bond, team, * etc) then this function releases the previously enslaved @slave. @@ -1016,11 +1015,12 @@ nm_device_enslave_slave (NMDevice *dev, NMDevice *slave, NMConnection *connectio * other devices, or if @slave was never enslaved. */ static gboolean -nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean master_failed) +nm_device_release_one_slave (NMDevice *dev, NMDevice *slave) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); SlaveInfo *info; gboolean success = FALSE; + NMDeviceStateReason reason; g_return_val_if_fail (slave != NULL, FALSE); g_return_val_if_fail (NM_DEVICE_GET_CLASS (dev)->release_slave != NULL, FALSE); @@ -1033,7 +1033,12 @@ nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean master_fai success = NM_DEVICE_GET_CLASS (dev)->release_slave (dev, slave); g_warn_if_fail (success); } - nm_device_slave_notify_release (info->slave, master_failed); + + if (priv->state == NM_DEVICE_STATE_FAILED) + reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; + else + reason = priv->state_reason; + nm_device_slave_notify_release (info->slave, reason); priv->slaves = g_slist_remove (priv->slaves, info); free_slave_info (info); @@ -1263,7 +1268,7 @@ slave_state_changed (NMDevice *slave, } if (release) { - nm_device_release_one_slave (self, slave, FALSE); + nm_device_release_one_slave (self, slave); /* Bridge/bond/team interfaces are left up until manually deactivated */ if (priv->slaves == NULL && priv->state == NM_DEVICE_STATE_ACTIVATED) { nm_log_dbg (LOGD_DEVICE, "(%s): last slave removed; remaining activated", @@ -1405,14 +1410,14 @@ nm_device_is_master (NMDevice *dev) /* release all slaves */ static void -nm_device_master_release_slaves (NMDevice *self, gboolean failed) +nm_device_master_release_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); while (priv->slaves) { SlaveInfo *info = priv->slaves->data; - nm_device_release_one_slave (self, info->slave, failed); + nm_device_release_one_slave (self, info->slave); } } @@ -1481,40 +1486,37 @@ nm_device_slave_notify_enslave (NMDevice *dev, gboolean success) /** * nm_device_slave_notify_release: * @dev: the slave device - * @master_failed: indicates whether the release was unexpected, - * ie the master device failed. + * @reason: the reason associated with the state change * - * Notifies a slave that it has been released, and whether this was expected - * or not. + * Notifies a slave that it has been released, and why. */ static void -nm_device_slave_notify_release (NMDevice *dev, gboolean master_failed) +nm_device_slave_notify_release (NMDevice *dev, NMDeviceStateReason reason) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); NMConnection *connection = nm_device_get_connection (dev); NMDeviceState new_state; - NMDeviceStateReason reason; + const char *master_status; if ( priv->state > NM_DEVICE_STATE_DISCONNECTED && priv->state <= NM_DEVICE_STATE_ACTIVATED) { - if (master_failed) { + if (reason == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) { new_state = NM_DEVICE_STATE_FAILED; - reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - - nm_log_warn (LOGD_DEVICE, - "Activation (%s) connection '%s' master failed", - nm_device_get_iface (dev), - nm_connection_get_id (connection)); + master_status = "failed"; + } else if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) { + new_state = NM_DEVICE_STATE_DEACTIVATING; + master_status = "deactivated by user request"; } else { new_state = NM_DEVICE_STATE_DISCONNECTED; - reason = NM_DEVICE_STATE_REASON_NONE; - - nm_log_dbg (LOGD_DEVICE, - "Activation (%s) connection '%s' master deactivated", - nm_device_get_iface (dev), - nm_connection_get_id (connection)); + master_status = "deactivated"; } + nm_log_dbg (LOGD_DEVICE, + "Activation (%s) connection '%s' master %s", + nm_device_get_iface (dev), + nm_connection_get_id (connection), + master_status); + nm_device_queue_state (dev, new_state, reason); } @@ -4574,7 +4576,7 @@ nm_device_deactivate (NMDevice *self, NMDeviceStateReason reason) NM_DEVICE_GET_CLASS (self)->deactivate (self); /* master: release slaves */ - nm_device_master_release_slaves (self, FALSE); + nm_device_master_release_slaves (self); /* slave: mark no longer enslaved */ g_clear_object (&priv->master); @@ -6357,7 +6359,7 @@ nm_device_state_changed (NMDevice *device, connection ? nm_connection_get_id (connection) : ""); /* Notify any slaves of the unexpected failure */ - nm_device_master_release_slaves (device, TRUE); + nm_device_master_release_slaves (device); /* If the connection doesn't yet have a timestamp, set it to zero so that * we can distinguish between connections we've tried to activate and have