manager: merge VPN handling into _new_active_connection()

Merge _new_vpn_active_connection() into _new_active_connection(). It was the
only caller, and it is simpler to have all the code visible at one place.

That also shows, that the device argument is ignored and not handled.
Ensure that no device is specified for VPN type activations.

src/nm-manager.c

@@ -3986,16 +3986,53 @@ _internal_activate_generic (NMManager *self, NMActiveConnection *active, GError
 }
 
 static NMActiveConnection *
-_new_vpn_active_connection (NMManager *self,
+_new_active_connection (NMManager *self,
-                            NMSettingsConnection *settings_connection,
+                        gboolean is_vpn,
+                        NMConnection *connection,
+                        NMConnection *applied,
                         const char *specific_object,
+                        NMDevice *device,
                         NMAuthSubject *subject,
+                        NMActivationType activation_type,
                         NMActivationReason activation_reason,
                         GError **error)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+	NMSettingsConnection *settings_connection = NULL;
+
+	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
+	g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL);
+
+	nm_assert (is_vpn == _connection_is_vpn (connection));
+
+	nm_assert (   ( is_vpn && !device)
+	           || (!is_vpn && NM_IS_DEVICE (device)));
+
+	if (is_vpn) {
+		/* FIXME: for VPN connections, we don't allow re-activating an
+		 * already active connection. It's a bug, and should be fixed together
+		 * when reworking VPN handling. */
+		if (active_connection_find_first_by_connection (self, connection)) {
+			g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_ALREADY_ACTIVE,
+			             "Connection '%s' is already active",
+			             nm_connection_get_id (connection));
+			return NULL;
+		}
+	}
+
+	specific_object = nm_utils_dbus_normalize_object_path (specific_object);
+
+	if (NM_IS_SETTINGS_CONNECTION (connection))
+		settings_connection = (NMSettingsConnection *) connection;
+
+	if (is_vpn) {
 		NMActiveConnection *parent = NULL;
-	NMDevice *device = NULL;
+
+		/* FIXME: apparently, activation here only works if @connection is
+		 * a settings-connection. Which is not the case during AddAndActivatate.
+		 * Probably, AddAndActivate is broken for VPN. */
+		if (activation_type != NM_ACTIVATION_TYPE_MANAGED)
+			g_return_val_if_reached (NULL);
 
 		g_return_val_if_fail (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection), NULL);
 
@@ -4019,7 +4056,7 @@ _new_vpn_active_connection (NMManager *self,
 		device = nm_active_connection_get_device (parent);
 		if (!device) {
 			g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE,
-		                     "Source connection had no active device.");
+			                     "Source connection had no active device");
 			return NULL;
 		}
 
@@ -4030,57 +4067,7 @@ _new_vpn_active_connection (NMManager *self,
 		                                                     subject);
 	}
 
-static NMActiveConnection *
+	if (activation_type == NM_ACTIVATION_TYPE_MANAGED)
-_new_active_connection (NMManager *self,
-                        gboolean is_vpn,
-                        NMConnection *connection,
-                        NMConnection *applied,
-                        const char *specific_object,
-                        NMDevice *device,
-                        NMAuthSubject *subject,
-                        NMActivationType activation_type,
-                        NMActivationReason activation_reason,
-                        GError **error)
-{
-	NMSettingsConnection *settings_connection = NULL;
-
-	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
-	g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL);
-
-	nm_assert (is_vpn == _connection_is_vpn (connection));
-
-	if (is_vpn) {
-		/* FIXME: for VPN connections, we don't allow re-activating an
-		 * already active connection. It's a bug, and should be fixed together
-		 * when reworking VPN handling. */
-		if (active_connection_find_first_by_connection (self, connection)) {
-			g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_ALREADY_ACTIVE,
-			             "Connection '%s' is already active",
-			             nm_connection_get_id (connection));
-			return NULL;
-		}
-	}
-
-	specific_object = nm_utils_dbus_normalize_object_path (specific_object);
-
-	if (NM_IS_SETTINGS_CONNECTION (connection))
-		settings_connection = (NMSettingsConnection *) connection;
-
-	if (is_vpn) {
-		/* FIXME: apparently, activation here only works if @connection is
-		 * a settings-connection. Which is not the case during AddAndActivatate.
-		 * Probably, AddAndActivate is broken for VPN. */
-		if (activation_type != NM_ACTIVATION_TYPE_MANAGED)
-			g_return_val_if_reached (NULL);
-		return _new_vpn_active_connection (self,
-		                                   settings_connection,
-		                                   specific_object,
-		                                   subject,
-		                                   activation_reason,
-		                                   error);
-	}
-
-	if (device && (activation_type == NM_ACTIVATION_TYPE_MANAGED))
 		nm_device_sys_iface_state_set (device, NM_DEVICE_SYS_IFACE_STATE_MANAGED);
 
 	return (NMActiveConnection *) nm_act_request_new (settings_connection,
@@ -4178,14 +4165,17 @@ nm_manager_activate_connection (NMManager *self,
                                 NMActivationReason activation_reason,
                                 GError **error)
 {
-	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+	NMManagerPrivate *priv;
 	NMActiveConnection *active;
 	GSList *iter;
 
-	g_return_val_if_fail (self, NULL);
+	g_return_val_if_fail (NM_IS_MANAGER (self), NULL);
-	g_return_val_if_fail (connection, NULL);
+	g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (connection), NULL);
+	g_return_val_if_fail (NM_IS_DEVICE (device), NULL);
 	g_return_val_if_fail (!error || !*error, NULL);
 
+	priv = NM_MANAGER_GET_PRIVATE (self);
+
 	if (!nm_auth_is_subject_in_acl_set_error (NM_CONNECTION (connection),
 	                                          subject,
 	                                          NM_MANAGER_ERROR,
@@ -4298,10 +4288,9 @@ validate_activation_request (NMManager *self,
 			                     "Device not found");
 			return NULL;
 		}
-	} else {
+	} else if (!is_vpn) {
 		device = nm_manager_get_best_device_for_connection (self, connection, TRUE, NULL);
-		if (   !device
+		if (!device) {
-		    && !is_vpn) {
 			gs_free char *iface = NULL;
 
 			/* VPN and software-device connections don't need a device yet,
@@ -4330,6 +4319,22 @@ validate_activation_request (NMManager *self,
 		}
 	}
 
+	if (is_vpn && device) {
+		/* VPN's are treated specially. Maybe the should accept a device as well,
+		 * however, later on during activation, we don't handle the device.
+		 *
+		 * Maybe we should, and maybe it makes sense to specify a device
+		 * when activating a VPN. But for now, just error out.  */
+		g_set_error_literal (error,
+		                     NM_MANAGER_ERROR,
+		                     NM_MANAGER_ERROR_UNKNOWN_DEVICE,
+		                     "Cannot specify device when activating VPN");
+		return NULL;
+	}
+
+	nm_assert (   ( is_vpn && !device)
+	           || (!is_vpn && NM_IS_DEVICE (device)));
+
 	*out_device = device;
 	*out_is_vpn = is_vpn;
 	return g_steal_pointer (&subject);