From 621975949024520c71215fe17c7d2ffc86e4efbd Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 23 Mar 2016 10:35:55 +0100 Subject: [PATCH 1/6] device: _get_available_connections() with _get_best_connection() We'll need to share the best conneciton logic and it's the only caller of nm_device_get_available_connections(). Let's just move it all to NMDevice and provide the best connection from there instead. --- src/devices/nm-device.c | 60 ++++++++++++++++++++++++++--------------- src/devices/nm-device.h | 5 ++-- src/nm-manager.c | 45 ++++++++----------------------- 3 files changed, 52 insertions(+), 58 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f18b9c147f..583218f6d1 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9689,39 +9689,55 @@ nm_device_recheck_available_connections (NMDevice *self) } /** - * nm_device_get_available_connections: + * nm_device_get_best_connection: * @self: the #NMDevice * @specific_object: a specific object path if any + * @error: reason why no connection was returned * - * Returns a list of connections available to activate on the device, taking - * into account any device-specific details given by @specific_object (like - * WiFi access point path). + * Returns a connection that's most suitable for user-initiated activation + * of a device, optionally with a given specific object. * - * Returns: caller-owned #GPtrArray of #NMConnections + * Returns: the #NMSettingsConnection or %NULL (setting an @error) */ -GPtrArray * -nm_device_get_available_connections (NMDevice *self, const char *specific_object) +NMSettingsConnection * +nm_device_get_best_connection (NMDevice *self, + const char *specific_object, + GError **error) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMSettingsConnection *connection = NULL; + NMSettingsConnection *candidate; + guint64 best_timestamp = 0; GHashTableIter iter; - guint num_available; - NMConnection *connection = NULL; - GPtrArray *array = NULL; - num_available = g_hash_table_size (priv->available_connections); - if (num_available > 0) { - array = g_ptr_array_sized_new (num_available); - g_hash_table_iter_init (&iter, priv->available_connections); - while (g_hash_table_iter_next (&iter, (gpointer) &connection, NULL)) { - /* If a specific object is given, only include connections that are - * compatible with it. - */ - if ( !specific_object /* << Optimization: we know that the connection is available without @specific_object. */ - || nm_device_check_connection_available (self, connection, _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, specific_object)) - g_ptr_array_add (array, connection); + g_hash_table_iter_init (&iter, priv->available_connections); + while (g_hash_table_iter_next (&iter, (gpointer) &candidate, NULL)) { + guint64 candidate_timestamp = 0; + + /* If a specific object is given, only include connections that are + * compatible with it. + */ + if ( specific_object /* << Optimization: we know that the connection is available without @specific_object. */ + && !nm_device_check_connection_available (self, + NM_CONNECTION (candidate), + _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, + specific_object)) + continue; + + nm_settings_connection_get_timestamp (candidate, &candidate_timestamp); + if (!connection || (candidate_timestamp > best_timestamp)) { + connection = candidate; + best_timestamp = candidate_timestamp; } } - return array; + + if (!connection) { + g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION, + "The device '%s' has no connections available for activation.", + nm_device_get_iface (self)); + } + + return connection; } static void diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 9b9edda494..bd657e68ca 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -529,8 +529,9 @@ gboolean nm_device_add_pending_action (NMDevice *device, const char *action, gboolean nm_device_remove_pending_action (NMDevice *device, const char *action, gboolean assert_is_pending); gboolean nm_device_has_pending_action (NMDevice *device); -GPtrArray *nm_device_get_available_connections (NMDevice *device, - const char *specific_object); +NMSettingsConnection *nm_device_get_best_connection (NMDevice *device, + const char *specific_object, + GError **error); gboolean nm_device_check_connection_available (NMDevice *device, NMConnection *connection, diff --git a/src/nm-manager.c b/src/nm-manager.c index 0ccb987a1b..2490d91d5d 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -3325,11 +3325,15 @@ impl_manager_activate_connection (NMManager *self, * regardless of whether that connection is autoconnect-enabled or not * (since this is an explicit request, not an auto-activation request). */ - if (!connection_path) { - GPtrArray *available; - guint64 best_timestamp = 0; - guint i; - + if (connection_path) { + connection = nm_settings_get_connection_by_path (priv->settings, connection_path); + if (!connection) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_UNKNOWN_CONNECTION, + "Connection could not be found."); + goto error; + } + } else { /* If no connection is given, find a suitable connection for the given device path */ if (!device_path) { error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, @@ -3343,36 +3347,9 @@ impl_manager_activate_connection (NMManager *self, goto error; } - available = nm_device_get_available_connections (device, specific_object_path); - for (i = 0; available && i < available->len; i++) { - NMSettingsConnection *candidate = g_ptr_array_index (available, i); - guint64 candidate_timestamp = 0; - - nm_settings_connection_get_timestamp (candidate, &candidate_timestamp); - if (!connection_path || (candidate_timestamp > best_timestamp)) { - connection_path = nm_connection_get_path (NM_CONNECTION (candidate)); - best_timestamp = candidate_timestamp; - } - } - - if (available) - g_ptr_array_free (available, TRUE); - - if (!connection_path) { - error = g_error_new_literal (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_UNKNOWN_CONNECTION, - "The device has no connections available."); + connection = nm_device_get_best_connection (device, specific_object_path, &error); + if (!connection) goto error; - } - } - - g_assert (connection_path); - connection = nm_settings_get_connection_by_path (priv->settings, connection_path); - if (!connection) { - error = g_error_new_literal (NM_MANAGER_ERROR, - NM_MANAGER_ERROR_UNKNOWN_CONNECTION, - "Connection could not be found."); - goto error; } subject = validate_activation_request (self, From 2a45d30cc535e1809ab730bb28fd0b571fcd7610 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 23 Mar 2016 14:51:34 +0100 Subject: [PATCH 2/6] device: only clear the activate request when the device disconnects If it's traversing from unavailable to disconnected (e.g. realizing of the device was delayed because it was awaiting the parent connection), then we just want to progress the activation. --- src/devices/nm-device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 583218f6d1..19c15992d2 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -10400,7 +10400,8 @@ _set_state_full (NMDevice *self, if (state <= NM_DEVICE_STATE_UNAVAILABLE) { if (available_connections_del_all (self)) available_connections_notify (self); - _clear_queued_act_request (priv); + if (old_state > NM_DEVICE_STATE_UNAVAILABLE) + _clear_queued_act_request (priv); } /* Update the available connections list when a device first becomes available */ From ce745e098a469900912353e8226aacd6253a5a42 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 23 Mar 2016 14:51:53 +0100 Subject: [PATCH 3/6] device: delay the activation if the backing device is not yet there --- src/devices/nm-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 19c15992d2..4f92ec1f24 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7735,7 +7735,7 @@ nm_device_queue_activation (NMDevice *self, NMActRequest *req) must_queue = _carrier_wait_check_act_request_must_queue (self, req); - if (!priv->act_request && !must_queue) { + if (!priv->act_request && !must_queue && nm_device_is_real (self)) { /* Just activate immediately */ if (!_device_activate (self, req)) g_assert_not_reached (); From 6e382ea91d5f3a97e195c46792a390b8ddaaa433 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 23 Mar 2016 14:47:02 +0100 Subject: [PATCH 4/6] active-connection: add parent active connection tracking Make it possible to let active connection know about an active connection it depends on and emit a signal when the parent is active. --- src/nm-active-connection.c | 78 ++++++++++++++++++++++++++++++++++++++ src/nm-active-connection.h | 6 +++ 2 files changed, 84 insertions(+) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index f275b3ef70..e907613662 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -59,6 +59,8 @@ typedef struct { NMActiveConnection *master; gboolean master_ready; + NMActiveConnection *parent; + gboolean assumed; NMAuthChain *chain; @@ -98,6 +100,7 @@ enum { enum { DEVICE_CHANGED, DEVICE_METERED_CHANGED, + PARENT_ACTIVE, LAST_SIGNAL }; static guint signals[LAST_SIGNAL] = { 0 }; @@ -678,6 +681,69 @@ nm_active_connection_get_assumed (NMActiveConnection *self) /****************************************************************/ +static void unwatch_parent (NMActiveConnection *self); + +static void +parent_destroyed (gpointer user_data, GObject *parent) +{ + NMActiveConnection *self = user_data; + + unwatch_parent (self); + g_signal_emit (self, signals[PARENT_ACTIVE], 0, NULL); +} + +static void +parent_state_cb (NMActiveConnection *parent_ac, + GParamSpec *pspec, + gpointer user_data) +{ + NMActiveConnection *self = user_data; + NMActiveConnectionState parent_state = nm_active_connection_get_state (parent_ac); + + if (parent_state < NM_ACTIVE_CONNECTION_STATE_ACTIVATED) + return; + + unwatch_parent (self); + g_signal_emit (self, signals[PARENT_ACTIVE], 0, parent_ac); +} + +static void +unwatch_parent (NMActiveConnection *self) +{ + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + + g_signal_handlers_disconnect_by_func (priv->parent, + (GCallback) parent_state_cb, + self); + g_object_weak_unref ((GObject *) priv->parent, parent_destroyed, self); + priv->parent = NULL; +} + +/** + * nm_active_connection_set_parent: + * @self: the #NMActiveConnection + * @parent: The #NMActiveConnection that must be active before the manager + * can proceed progressing the device to disconnected state for us. + * + * Sets the parent connection of @self. A "parent-active" signal will be + * emitted when the parent connection becomes active. + */ +void +nm_active_connection_set_parent (NMActiveConnection *self, NMActiveConnection *parent) +{ + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + + g_return_if_fail (priv->parent == NULL); + priv->parent = parent; + g_signal_connect (priv->parent, + "notify::" NM_ACTIVE_CONNECTION_STATE, + (GCallback) parent_state_cb, + self); + g_object_weak_ref ((GObject *) priv->parent, parent_destroyed, self); +} + +/****************************************************************/ + static void auth_done (NMAuthChain *chain, GError *error, @@ -1028,6 +1094,10 @@ dispose (GObject *object) self); } g_clear_object (&priv->master); + + if (priv->parent) + unwatch_parent (self); + g_clear_object (&priv->subject); G_OBJECT_CLASS (nm_active_connection_parent_class)->dispose (object); @@ -1208,6 +1278,14 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class) NULL, NULL, NULL, G_TYPE_NONE, 1, G_TYPE_UINT); + signals[PARENT_ACTIVE] = + g_signal_new (NM_ACTIVE_CONNECTION_PARENT_ACTIVE, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMActiveConnectionClass, parent_active), + NULL, NULL, NULL, + G_TYPE_NONE, 1, NM_TYPE_ACTIVE_CONNECTION); + nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (ac_class), NMDBUS_TYPE_ACTIVE_CONNECTION_SKELETON, NULL); diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index a7b3d0cd34..a66f8e9ffc 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -58,6 +58,7 @@ /* Internal signals*/ #define NM_ACTIVE_CONNECTION_DEVICE_CHANGED "device-changed" #define NM_ACTIVE_CONNECTION_DEVICE_METERED_CHANGED "device-metered-changed" +#define NM_ACTIVE_CONNECTION_PARENT_ACTIVE "parent-active" struct _NMActiveConnection { NMExportedObject parent; @@ -81,6 +82,8 @@ typedef struct { void (*device_metered_changed) (NMActiveConnection *connection, NMMetered new_value); + + void (*parent_active) (NMActiveConnection *connection); } NMActiveConnectionClass; guint64 nm_active_connection_version_id_get (NMActiveConnection *self); @@ -148,6 +151,9 @@ gboolean nm_active_connection_get_master_ready (NMActiveConnection *self); void nm_active_connection_set_master (NMActiveConnection *self, NMActiveConnection *master); +void nm_active_connection_set_parent (NMActiveConnection *self, + NMActiveConnection *parent); + void nm_active_connection_set_assumed (NMActiveConnection *self, gboolean assumed); From da226ae137f913350ef4931594b903abd21b03ad Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 24 Mar 2016 15:20:44 +0100 Subject: [PATCH 5/6] manager: separate the traversal to disconected to a separate routine Will be useful when we'll be able to defer the realization of the device. --- src/nm-manager.c | 55 +++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 2490d91d5d..517adcc629 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2723,6 +2723,37 @@ _internal_activate_vpn (NMManager *self, NMActiveConnection *active, GError **er return success; } +/* Traverse the device to disconnected state. This means that the device is ready + * for connection and will proceed activating if there's an activation request + * enqueued. + */ +static void +unmanaged_to_disconnected (NMDevice *device) +{ + /* when creating the software device, it can happen that the device is + * still unmanaged by NM_UNMANAGED_PLATFORM_INIT because we didn't yet + * get the udev event. At this point, we can no longer delay the activation + * and force the device to be managed. */ + nm_device_set_unmanaged_by_flags (device, NM_UNMANAGED_PLATFORM_INIT, FALSE, NM_DEVICE_STATE_REASON_USER_REQUESTED); + + nm_device_set_unmanaged_by_flags (device, NM_UNMANAGED_USER_EXPLICIT, FALSE, NM_DEVICE_STATE_REASON_USER_REQUESTED); + + g_return_if_fail (nm_device_get_managed (device, FALSE)); + + if (nm_device_get_state (device) == NM_DEVICE_STATE_UNMANAGED) { + nm_device_state_changed (device, + NM_DEVICE_STATE_UNAVAILABLE, + NM_DEVICE_STATE_REASON_USER_REQUESTED); + } + + if ( nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_FOR_USER_REQUEST) + && (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_DISCONNECTED, + NM_DEVICE_STATE_REASON_USER_REQUESTED); + } +} + static gboolean _internal_activate_device (NMManager *self, NMActiveConnection *active, GError **error) { @@ -2846,28 +2877,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * if (existing) nm_device_steal_connection (existing, connection); - /* when creating the software device, it can happen that the device is - * still unmanaged by NM_UNMANAGED_PLATFORM_INIT because we didn't yet - * get the udev event. At this point, we can no longer delay the activation - * and force the device to be managed. */ - nm_device_set_unmanaged_by_flags (device, NM_UNMANAGED_PLATFORM_INIT, FALSE, NM_DEVICE_STATE_REASON_USER_REQUESTED); - - nm_device_set_unmanaged_by_flags (device, NM_UNMANAGED_USER_EXPLICIT, FALSE, NM_DEVICE_STATE_REASON_USER_REQUESTED); - - g_return_val_if_fail (nm_device_get_managed (device, FALSE), FALSE); - - if (nm_device_get_state (device) == NM_DEVICE_STATE_UNMANAGED) { - nm_device_state_changed (device, - NM_DEVICE_STATE_UNAVAILABLE, - NM_DEVICE_STATE_REASON_USER_REQUESTED); - } - - if ( nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_FOR_USER_REQUEST) - && (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) { - nm_device_state_changed (device, - NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_REASON_USER_REQUESTED); - } + /* Make the device ready for activation. */ + unmanaged_to_disconnected (device); /* Export the new ActiveConnection to clients and start it on the device */ nm_exported_object_export (NM_EXPORTED_OBJECT (active)); From b26159b149cd54a06bbf538a5f1bd11b9c31c5d2 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 24 Mar 2016 15:20:44 +0100 Subject: [PATCH 6/6] manager: allow delaying the device activation when the parent is not real Don't try to realize our device when the parent device is not real. Instead, enqueue the activation and wait until it is active before realizing our device and progressing the device to DISCONNECTED so that it can get connected. --- src/nm-manager.c | 75 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 517adcc629..06065536f8 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -226,6 +226,9 @@ static void active_connection_state_changed (NMActiveConnection *active, static void active_connection_default_changed (NMActiveConnection *active, GParamSpec *pspec, NMManager *self); +static void active_connection_parent_active (NMActiveConnection *active, + NMActiveConnection *parent_ac, + NMManager *self); /* Returns: whether to notify D-Bus of the removal or not */ static gboolean @@ -244,6 +247,7 @@ active_connection_remove (NMManager *self, NMActiveConnection *active) g_signal_emit (self, signals[ACTIVE_CONNECTION_REMOVED], 0, active); g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); g_signal_handlers_disconnect_by_func (active, active_connection_default_changed, self); + g_signal_handlers_disconnect_by_func (active, active_connection_parent_active, self); if ( nm_active_connection_get_assumed (active) && (connection = nm_active_connection_get_settings_connection (active)) @@ -2754,6 +2758,40 @@ unmanaged_to_disconnected (NMDevice *device) } } +/* The parent connection is ready; we can proceed realizing the device and + * progressing the device to disconencted state. + */ +static void +active_connection_parent_active (NMActiveConnection *active, + NMActiveConnection *parent_ac, + NMManager *self) +{ + NMDevice *device = nm_active_connection_get_device (active); + GError *error = NULL; + + g_signal_handlers_disconnect_by_func (active, + (GCallback) active_connection_parent_active, + self); + + if (parent_ac) { + NMSettingsConnection *connection = nm_active_connection_get_settings_connection (active); + NMDevice *parent = nm_active_connection_get_device (parent_ac); + + if (nm_device_create_and_realize (device, (NMConnection *) connection, parent, &error)) { + /* We can now proceed to disconnected state so that activation proceeds. */ + unmanaged_to_disconnected (device); + } else { + nm_log_warn (LOGD_CORE, "Could not realize device '%s': %s", + nm_device_get_iface (device), error->message); + nm_active_connection_set_state (active, NM_ACTIVE_CONNECTION_STATE_DEACTIVATED); + } + } else { + nm_log_warn (LOGD_CORE, "The parent connection device '%s' depended on disappeared.", + nm_device_get_iface (device)); + nm_active_connection_set_state (active, NM_ACTIVE_CONNECTION_STATE_DEACTIVATED); + } +} + static gboolean _internal_activate_device (NMManager *self, NMActiveConnection *active, GError **error) { @@ -2813,9 +2851,35 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * NMDevice *parent; parent = find_parent_device_for_connection (self, (NMConnection *) connection, NULL); - if (!nm_device_create_and_realize (device, (NMConnection *) connection, parent, error)) { - g_prefix_error (error, "%s failed to create resources: ", nm_device_get_iface (device)); - return FALSE; + + if (parent && !nm_device_is_real (parent)) { + NMSettingsConnection *parent_con; + NMActiveConnection *parent_ac; + + parent_con = nm_device_get_best_connection (parent, NULL, error); + if (!parent_con) { + g_prefix_error (error, "%s failed to create parent: ", nm_device_get_iface (device)); + return FALSE; + } + + parent_ac = nm_manager_activate_connection (self, parent_con, NULL, parent, subject, error); + if (!parent_ac) { + g_prefix_error (error, "%s failed to activate parent: ", nm_device_get_iface (device)); + return FALSE; + } + + /* We can't realize now; defer until the parent device is ready. */ + g_signal_connect (active, + NM_ACTIVE_CONNECTION_PARENT_ACTIVE, + (GCallback) active_connection_parent_active, + self); + nm_active_connection_set_parent (active, parent_ac); + } else { + /* We can realize now; no need to wait for a parent device. */ + if (!nm_device_create_and_realize (device, (NMConnection *) connection, parent, error)) { + g_prefix_error (error, "%s failed to create resources: ", nm_device_get_iface (device)); + return FALSE; + } } } @@ -2877,8 +2941,9 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * if (existing) nm_device_steal_connection (existing, connection); - /* Make the device ready for activation. */ - unmanaged_to_disconnected (device); + /* If the device is there, we can ready it for the activation. */ + if (nm_device_is_real (device)) + unmanaged_to_disconnected (device); /* Export the new ActiveConnection to clients and start it on the device */ nm_exported_object_export (NM_EXPORTED_OBJECT (active));