From 6e94f302b219a3a6bb7fad15c25fefea6f2f4fde Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 3 Feb 2015 16:15:37 +0100 Subject: [PATCH 1/4] manager: reuse a device connection is active on if none was given upon activation If a connection is already active let's keep it on the same device. This makes it possible to reactivate a connection without client knowing which device is it active on. https://bugzilla.gnome.org/show_bug.cgi?id=730492 --- src/nm-manager.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 9ccb7e44d2..b3935c213a 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2263,6 +2263,17 @@ nm_manager_get_devices (NMManager *manager) return NM_MANAGER_GET_PRIVATE (manager)->devices; } +static NMDevice * +nm_manager_get_connection_device (NMManager *self, + NMConnection *connection) +{ + NMActiveConnection *ac = find_ac_for_connection (self, connection); + if (ac == NULL) + return NULL; + + return nm_active_connection_get_device (ac); +} + static gboolean impl_manager_get_devices (NMManager *manager, GPtrArray **devices, GError **err) { @@ -3107,7 +3118,10 @@ validate_activation_request (NMManager *self, "Device not found"); goto error; } - } else { + } else + device = nm_manager_get_connection_device (self, connection); + + if (!device) { gboolean is_software = nm_connection_is_virtual (connection); /* VPN and software-device connections don't need a device yet */ From 6fc3736c7ac139e40cb9f72b722b95b93c474979 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 3 Feb 2015 16:15:37 +0100 Subject: [PATCH 2/4] manager: pick an available device if none was specified upon connection activation This offloads some complexity from the client. https://bugzilla.gnome.org/show_bug.cgi?id=730492 --- src/nm-manager.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index b3935c213a..71e4b4a0fe 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2274,6 +2274,29 @@ nm_manager_get_connection_device (NMManager *self, return nm_active_connection_get_device (ac); } +static NMDevice * +nm_manager_get_best_device_for_connection (NMManager *self, + NMConnection *connection) +{ + const GSList *devices, *iter; + NMDevice *act_device = nm_manager_get_connection_device (self, connection); + + if (act_device) + return act_device; + + /* Pick the first device that's compatible with the connection. */ + devices = nm_manager_get_devices (self); + for (iter = devices; iter; iter = g_slist_next (iter)) { + NMDevice *device = NM_DEVICE (iter->data); + + if (nm_device_check_connection_available (device, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) + return device; + } + + /* No luck. :( */ + return NULL; +} + static gboolean impl_manager_get_devices (NMManager *manager, GPtrArray **devices, GError **err) { @@ -3119,7 +3142,7 @@ validate_activation_request (NMManager *self, goto error; } } else - device = nm_manager_get_connection_device (self, connection); + device = nm_manager_get_best_device_for_connection (self, connection); if (!device) { gboolean is_software = nm_connection_is_virtual (connection); From 4cb97cf66f175857a1910ee926ea61dc507c08e8 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 3 Feb 2015 16:15:37 +0100 Subject: [PATCH 3/4] manager: remove a connection from device if we're activating it on another device The connection now might be being activated on another device. Defer the removal until we're sure the activation request will proceed and only add the active connection afterwards. https://bugzilla.gnome.org/show_bug.cgi?id=730492 --- src/devices/nm-device.c | 47 +++++++++++++++++++++++++++++++++-------- src/devices/nm-device.h | 2 ++ src/nm-manager.c | 29 +++++++------------------ 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 4edbe84503..1083274df8 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5908,6 +5908,26 @@ _carrier_wait_check_act_request_must_queue (NMDevice *self, NMActRequest *req) return TRUE; } +void +nm_device_steal_connection (NMDevice *self, NMConnection *connection) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + _LOGW (LOGD_DEVICE, "disconnecting connection '%s' for new activation request.", + nm_connection_get_id (connection)); + + if ( priv->queued_act_request + && connection == nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (priv->queued_act_request))) + _clear_queued_act_request (priv); + + if ( priv->act_request + && connection == nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (priv->act_request)) + && priv->state < NM_DEVICE_STATE_DEACTIVATING) + nm_device_state_changed (self, + NM_DEVICE_STATE_DEACTIVATING, + NM_DEVICE_STATE_REASON_USER_REQUESTED); +} + void nm_device_queue_activation (NMDevice *self, NMActRequest *req) { @@ -5929,8 +5949,8 @@ nm_device_queue_activation (NMDevice *self, NMActRequest *req) _LOGD (LOGD_DEVICE, "queue activation request waiting for %s", must_queue ? "carrier" : "currently active connection to disconnect"); + /* Deactivate existing activation request first */ if (priv->act_request) { - /* Deactivate existing activation request first */ _LOGI (LOGD_DEVICE, "disconnecting for new activation request."); nm_device_state_changed (self, NM_DEVICE_STATE_DEACTIVATING, @@ -7390,10 +7410,9 @@ _cleanup_ip_pre (NMDevice *self, gboolean deconfigure) } static void -_cleanup_generic_pre (NMDevice *self, gboolean deconfigure) +_cancel_activation (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - NMConnection *connection; /* Clean up when device was deactivated during call to firewall */ if (priv->fw_call) { @@ -7401,6 +7420,20 @@ _cleanup_generic_pre (NMDevice *self, gboolean deconfigure) priv->fw_call = NULL; } + ip_check_gw_ping_cleanup (self); + + /* Break the activation chain */ + activation_source_clear (self, TRUE, AF_INET); + activation_source_clear (self, TRUE, AF_INET6); +} + +static void +_cleanup_generic_pre (NMDevice *self, gboolean deconfigure) +{ + NMConnection *connection; + + _cancel_activation (self); + connection = nm_device_get_connection (self); if ( deconfigure && connection @@ -7410,12 +7443,6 @@ _cleanup_generic_pre (NMDevice *self, gboolean deconfigure) NULL); } - ip_check_gw_ping_cleanup (self); - - /* Break the activation chain */ - activation_source_clear (self, TRUE, AF_INET); - activation_source_clear (self, TRUE, AF_INET6); - /* Clear any queued transitions */ nm_device_queued_state_clear (self); @@ -7951,6 +7978,8 @@ _set_state_full (NMDevice *self, } break; case NM_DEVICE_STATE_DEACTIVATING: + _cancel_activation (self); + if (quitting) { nm_dispatcher_call_sync (DISPATCHER_ACTION_PRE_DOWN, nm_act_request_get_connection (req), diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 479c9bf299..a204bc134f 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -387,6 +387,8 @@ void nm_device_queue_state (NMDevice *self, gboolean nm_device_get_firmware_missing (NMDevice *self); +void nm_device_steal_connection (NMDevice *device, NMConnection *connection); + void nm_device_queue_activation (NMDevice *device, NMActRequest *req); gboolean nm_device_supports_vlans (NMDevice *device); diff --git a/src/nm-manager.c b/src/nm-manager.c index 71e4b4a0fe..6e78dbe6d2 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2684,7 +2684,7 @@ _internal_activate_vpn (NMManager *self, NMActiveConnection *active, GError **er static gboolean _internal_activate_device (NMManager *self, NMActiveConnection *active, GError **error) { - NMDevice *device, *master_device = NULL; + NMDevice *device, *existing, *master_device = NULL; NMConnection *connection; NMConnection *master_connection = NULL; NMActiveConnection *master_ac = NULL; @@ -2836,6 +2836,11 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * nm_active_connection_get_path (master_ac)); } + /* Disconnect the connection if connected or queued on another device */ + existing = nm_manager_get_connection_device (self, connection); + if (existing) + nm_device_steal_connection (existing, connection); + /* Export the new ActiveConnection to clients and start it on the device */ nm_active_connection_export (active); g_object_notify (G_OBJECT (self), NM_MANAGER_ACTIVE_CONNECTIONS); @@ -2873,6 +2878,7 @@ _internal_activate_generic (NMManager *self, NMActiveConnection *active, GError * is exported, make sure the manager's activating-connection property * is up-to-date. */ + active_connection_add (self, active); policy_activating_device_changed (G_OBJECT (priv->policy), NULL, self); } @@ -2942,18 +2948,6 @@ _new_active_connection (NMManager *self, return NULL; } - if (existing_ac) { - NMDevice *existing_device = nm_active_connection_get_device (existing_ac); - - if (existing_device != device) { - g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_ALREADY_ACTIVE, - "Connection '%s' is already active on %s", - nm_connection_get_id (connection), - nm_device_get_iface (existing_device)); - return NULL; - } - } - /* Normalize the specific object */ if (specific_object && g_strcmp0 (specific_object, "/") == 0) specific_object = NULL; @@ -2985,7 +2979,6 @@ _internal_activation_failed (NMManager *self, 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 @@ -3063,10 +3056,8 @@ nm_manager_activate_connection (NMManager *self, device, subject, error); - if (active) { + if (active) nm_active_connection_authorize (active, _internal_activation_auth_done, self, NULL); - active_connection_add (self, active); - } return active; } @@ -3313,7 +3304,6 @@ impl_manager_activate_connection (NMManager *self, goto error; nm_active_connection_authorize (active, _activation_auth_done, self, context); - active_connection_add (self, active); g_clear_object (&subject); return; @@ -3391,8 +3381,6 @@ _add_and_activate_auth_done (NMActiveConnection *active, activation_add_done, info); } else { - active_connection_remove (self, active); - g_assert (error_desc); error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, @@ -3499,7 +3487,6 @@ impl_manager_add_and_activate_connection (NMManager *self, goto error; nm_active_connection_authorize (active, _add_and_activate_auth_done, self, context); - active_connection_add (self, active); g_object_unref (connection); g_object_unref (subject); return; From 0e8a14cc5f7028aec65dd058b777689ceb7703fc Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Sun, 1 Feb 2015 09:09:37 +0100 Subject: [PATCH 4/4] cli: don't look up a device for activation request unless we have to Let the server decide which device to use if the user didn't explicitly specify the interface, wireless access point or a wimax nsp. The server will just reuse the device for an already active connection or potentially do a better guess. https://bugzilla.gnome.org/show_bug.cgi?id=730492 --- clients/cli/connections.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 6de3440652..9223866216 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1723,6 +1723,7 @@ find_device_for_connection (NmCli *nmc, int i, j; g_return_val_if_fail (nmc != NULL, FALSE); + g_return_val_if_fail (iface || ap || nsp, FALSE); g_return_val_if_fail (device != NULL && *device == NULL, FALSE); g_return_val_if_fail (spec_object != NULL && *spec_object == NULL, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); @@ -2208,7 +2209,7 @@ nmc_activate_connection (NmCli *nmc, g_return_val_if_fail (nmc != NULL, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - if (connection) { + if (connection && (ifname || ap || nsp)) { device_found = find_device_for_connection (nmc, connection, ifname, ap, nsp, &device, &spec_object, &local); /* Virtual connection may not have their interfaces created yet */ @@ -2226,7 +2227,7 @@ nmc_activate_connection (NmCli *nmc, _("unknown device '%s'."), ifname); return FALSE; } - } else { + } else if (!connection) { g_set_error_literal (error, NMCLI_ERROR, NMC_RESULT_ERROR_NOT_FOUND, _("neither a valid connection nor device given")); return FALSE;