core: allow multiple devices with the same interface name

But, of course, only one realized device can have the same
interface name at a time.

This commit effectively reverts most of:

1b37cd0340
core: allow ActiveConnections to be created without a device

But it's not easy to do a separate revert of that code due to
interdependencies with nm-manager.c.

Creating devices when they are defined by a connection also makes
makes it possible to require the NMDevice to be present when
activating it, which means we can remove a bunch of code from
NMManager that had to handle software devices not existing yet at
the time of the activation request.

But it also means we must be more careful when finding master
interfaces during slave activation, since we cannot simply match
by interface name alone.  Instead we must find the master which
matches both the interface name and can control slaves of the type
which is being activated.

src/devices/nm-device.c

@@ -2289,6 +2289,18 @@ nm_device_master_release_slaves (NMDevice *self)
 	}
 }
 
+/**
+ * nm_device_is_master:
+ * @self: the device
+ *
+ * Returns: %TRUE if the device can have slaves
+ */
+gboolean
+nm_device_is_master (NMDevice *self)
+{
+	return NM_DEVICE_GET_PRIVATE (self)->is_master;
+}
+
 /**
  * nm_device_get_master:
  * @self: the device
@@ -2873,6 +2885,34 @@ nm_device_check_connection_compatible (NMDevice *self, NMConnection *connection)
 	return NM_DEVICE_GET_CLASS (self)->check_connection_compatible (self, connection);
 }
 
+gboolean
+nm_device_check_slave_connection_compatible (NMDevice *self, NMConnection *slave)
+{
+	NMDevicePrivate *priv;
+	NMSettingConnection *s_con;
+	const char *connection_type, *slave_type;
+
+	g_return_val_if_fail (NM_IS_DEVICE (self), FALSE);
+	g_return_val_if_fail (NM_IS_CONNECTION (slave), FALSE);
+
+	priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (!priv->is_master)
+		return FALSE;
+
+	/* All masters should have connection type set */
+	connection_type = NM_DEVICE_GET_CLASS (self)->connection_type;
+	g_return_val_if_fail (connection_type, FALSE);
+
+	s_con = nm_connection_get_setting_connection (slave);
+	g_assert (s_con);
+	slave_type = nm_setting_connection_get_slave_type (s_con);
+	if (!slave_type)
+		return FALSE;
+
+	return strcmp (connection_type, slave_type) == 0;
+}
+
 /**
  * nm_device_can_assume_connections:
  * @self: #NMDevice instance

src/devices/nm-device.h

@@ -388,6 +388,7 @@ void            nm_device_capture_initial_config (NMDevice *dev);
 
 /* Master */
 GSList *        nm_device_master_get_slaves (NMDevice *dev);
+gboolean        nm_device_is_master         (NMDevice *dev);
 
 /* Slave */
 NMDevice *      nm_device_get_master        (NMDevice *dev);
@@ -421,6 +422,7 @@ gboolean nm_device_complete_connection (NMDevice *device,
                                         GError **error);
 
 gboolean nm_device_check_connection_compatible (NMDevice *device, NMConnection *connection);
+gboolean nm_device_check_slave_connection_compatible (NMDevice *device, NMConnection *connection);
 
 gboolean nm_device_uses_assumed_connection (NMDevice *device);
 

src/nm-activation-request.c

@@ -468,9 +468,7 @@ master_failed (NMActiveConnection *self)
  * @specific_object: the object path of the specific object (ie, WiFi access point,
  *    etc) that will be used to activate @connection and @device
  * @subject: the #NMAuthSubject representing the requestor of the activation
- * @device: the device/interface to configure according to @connection; or %NULL
+ * @device: the device/interface to configure according to @connection
- * if the connection describes a software device which will be created during
- * connection activation
  *
  * Creates a new device-based activation request.
  *
@@ -483,7 +481,7 @@ nm_act_request_new (NMSettingsConnection *settings_connection,
                     NMDevice *device)
 {
 	g_return_val_if_fail (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection), NULL);
-	g_return_val_if_fail (!device || NM_IS_DEVICE (device), NULL);
+	g_return_val_if_fail (NM_IS_DEVICE (device), NULL);
 	g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL);
 
 	return (NMActRequest *) g_object_new (NM_TYPE_ACT_REQUEST,

src/nm-active-connection.c

@@ -515,8 +515,13 @@ nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device)
 			priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)self);
 			nm_device_add_pending_action (device, priv->pending_activation_id, TRUE);
 		}
-	} else
+	} else {
+		/* The ActiveConnection's device can only be cleared after the
+		 * connection is activated.
+		 */
+		g_warn_if_fail (priv->state > NM_ACTIVE_CONNECTION_STATE_UNKNOWN);
 		priv->device = NULL;
+	}
 	g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_INT_DEVICE);
 
 	g_signal_emit (self, signals[DEVICE_CHANGED], 0, priv->device, old_device);
@@ -831,6 +836,7 @@ set_property (GObject *object, guint prop_id,
 		}
 		break;
 	case PROP_INT_DEVICE:
+		/* construct-only */
 		nm_active_connection_set_device (NM_ACTIVE_CONNECTION (object), g_value_get_object (value));
 		break;
 	case PROP_INT_SUBJECT:
@@ -1129,7 +1135,7 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class)
 		(object_class, PROP_INT_DEVICE,
 		 g_param_spec_object (NM_ACTIVE_CONNECTION_INT_DEVICE, "", "",
 		                      NM_TYPE_DEVICE,
-		                      G_PARAM_READWRITE |
+		                      G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY |
 		                      G_PARAM_STATIC_STRINGS));
 
 	g_object_class_install_property

src/nm-manager.c

@@ -499,16 +499,53 @@ find_device_by_ip_iface (NMManager *self, const gchar *iface)
 	return NULL;
 }
 
+/**
+ * find_device_by_iface:
+ * @self: the #NMManager
+ * @iface: the device interface to find
+ * @connection: a connection to ensure the returned device is compatible with
+ * @slave: a slave connection to ensure a master is compatible with
+ *
+ * Finds a device by interface name, preferring realized devices.  If @slave
+ * is given, this function will only return master devices and will ensure
+ * @slave, when activated, can be a slave of the returned master device.  If
+ * @connection is given, this function will only consider devices that are
+ * compatible with @connection.
+ *
+ * Returns: the matching #NMDevice
+ */
 static NMDevice *
-find_device_by_iface (NMManager *self, const gchar *iface)
+find_device_by_iface (NMManager *self,
+                      const char *iface,
+                      NMConnection *connection,
+                      NMConnection *slave)
 {
+	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+	NMDevice *fallback = NULL;
 	GSList *iter;
 
-	for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = g_slist_next (iter)) {
+	g_return_val_if_fail (iface != NULL, NULL);
-		if (g_strcmp0 (nm_device_get_iface (NM_DEVICE (iter->data)), iface) == 0)
+
-			return NM_DEVICE (iter->data);
+	for (iter = priv->devices; iter; iter = iter->next) {
+		NMDevice *candidate = iter->data;
+
+		if (strcmp (nm_device_get_iface (candidate), iface))
+			continue;
+		if (connection && !nm_device_check_connection_compatible (candidate, connection))
+			continue;
+		if (slave) {
+			if (!nm_device_is_master (candidate))
+				continue;
+			if (!nm_device_check_slave_connection_compatible (candidate, slave))
+				continue;
 		}
-	return NULL;
+
+		if (nm_device_is_real (candidate))
+			return candidate;
+		else if (!fallback)
+			fallback = candidate;
+	}
+	return fallback;
 }
 
 static gboolean
@@ -860,8 +897,8 @@ find_parent_device_for_connection (NMManager *self, NMConnection *connection)
 	if (!parent_name)
 		return NULL;
 
-	/* Try as an interface name */
+	/* Try as an interface name of a parent device */
-	parent = find_device_by_iface (self, parent_name);
+	parent = find_device_by_iface (self, parent_name, NULL, NULL);
 	if (parent)
 		return parent;
 
@@ -988,18 +1025,16 @@ system_create_virtual_device (NMManager *self, NMConnection *connection, GError
 	if (!iface)
 		return NULL;
 
-	/* Make sure we didn't create a device for this connection already */
+	/* If some other device is already compatible with this connection,
+	 * don't create a new device for it.
+	 */
 	for (iter = priv->devices; iter; iter = g_slist_next (iter)) {
 		NMDevice *candidate = iter->data;
 
 		if (   g_strcmp0 (nm_device_get_iface (candidate), iface) == 0
-		    || nm_device_check_connection_compatible (candidate, connection)) {
+		    && nm_device_check_connection_compatible (candidate, connection)) {
 			nm_log_dbg (LOGD_DEVICE, "(%s) already created virtual interface name %s",
 			            nm_connection_get_id (connection), iface);
-			g_set_error (error,
-			             NM_MANAGER_ERROR,
-			             NM_MANAGER_ERROR_FAILED,
-			             "interface name '%s' already created", iface);
 			return NULL;
 		}
 	}
@@ -1665,7 +1700,8 @@ device_ip_iface_changed (NMDevice *device,
 		NMDevice *candidate = NM_DEVICE (iter->data);
 
 		if (   candidate != device
-		    && g_strcmp0 (nm_device_get_iface (candidate), ip_iface) == 0) {
+		    && g_strcmp0 (nm_device_get_iface (candidate), ip_iface) == 0
+		    && nm_device_is_real (candidate)) {
 			remove_device (self, candidate, FALSE, FALSE);
 			break;
 		}
@@ -1728,8 +1764,6 @@ add_device (NMManager *self, NMDevice *device)
 	ifindex = nm_device_get_ifindex (device);
 	if (ifindex > 0 && nm_manager_get_device_by_ifindex (self, ifindex))
 		return;
-	if (find_device_by_iface (self, nm_device_get_iface (device)))
-		return;
 
 	/* Remove existing devices owned by the new device; eg remove ethernet
 	 * ports that are owned by a WWAN modem, since udev may announce them
@@ -1739,9 +1773,11 @@ add_device (NMManager *self, NMDevice *device)
 	 * the child NMDevice entirely
 	 */
 	for (iter = priv->devices; iter; iter = iter->next) {
-		iface = nm_device_get_ip_iface (iter->data);
+		NMDevice *candidate = iter->data;
-		if (nm_device_owns_iface (device, iface))
+
-			remove = g_slist_prepend (remove, iter->data);
+		iface = nm_device_get_ip_iface (candidate);
+		if (nm_device_is_real (candidate) && nm_device_owns_iface (device, iface))
+			remove = g_slist_prepend (remove, candidate);
 	}
 	for (iter = remove; iter; iter = iter->next)
 		remove_device (self, NM_DEVICE (iter->data), FALSE, FALSE);
@@ -1881,37 +1917,37 @@ platform_link_added (NMManager *self,
 	NMDevice *device = NULL;
 	GError *error = NULL;
 	gboolean nm_plugin_missing = FALSE;
+	GSList *iter;
 
 	g_return_if_fail (ifindex > 0);
 
 	if (nm_manager_get_device_by_ifindex (self, ifindex))
 		return;
 
-	device = find_device_by_iface (self, plink->name);
+	/* Let unrealized devices try to realize themselves with the link */
-	if (device) {
+	for (iter = NM_MANAGER_GET_PRIVATE (self)->devices; iter; iter = iter->next) {
-		gboolean compatible = FALSE;
+		NMDevice *candidate = iter->data;
+		gboolean compatible = TRUE;
 
-		if (nm_device_is_real (device))
+		if (strcmp (nm_device_get_iface (candidate), plink->name))
-			return;
+			continue;
 
-		if (nm_device_realize (device, plink, &compatible, &error)) {
+		if (nm_device_is_real (candidate)) {
-			/* Success */
+			/* Ignore the link added event since there's already a realized
-			nm_device_setup_finish (device, plink);
+			 * device with the link's name.
-			return;
-		}
-
-		nm_log_warn (LOGD_DEVICE, "(%s): %s", plink->name, error->message);
-		remove_device (self, device, FALSE, FALSE);
-		g_clear_error (&error);
-
-		if (compatible) {
-			/* Device compatible with platform link, but some other fatal error
-			 * happened during realization.
 			 */
 			return;
+		} else if (nm_device_realize (candidate, plink, &compatible, &error)) {
+			/* Success */
+			nm_device_setup_finish (candidate, plink);
+			return;
 		}
 
-		/* Fall through and create new compatible device for the link */
+		nm_log_dbg (LOGD_DEVICE, "(%s): failed to realize from plink: '%s'",
+		            plink->name, error->message);
+		g_clear_error (&error);
+
+		/* Try next unrealized device */
 	}
 
 	/* Try registered device factories */
@@ -2245,7 +2281,7 @@ find_master (NMManager *self,
 		return TRUE;  /* success, but no master */
 
 	/* Try as an interface name first */
-	master_device = find_device_by_iface (self, master);
+	master_device = find_device_by_iface (self, master, NULL, connection);
 	if (master_device) {
 		if (master_device == device) {
 			g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_DEPENDENCY_FAILED,
@@ -2620,8 +2656,10 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 	NMConnection *applied;
 	NMSettingsConnection *connection;
 	NMSettingsConnection *master_connection = NULL;
+	NMConnection *existing_connection = NULL;
 	NMActiveConnection *master_ac = NULL;
-	GError *local_err = NULL;
+	NMAuthSubject *subject;
+	char *error_desc = NULL;
 
 	g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
 	g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (active), FALSE);
@@ -2635,52 +2673,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 	applied = nm_active_connection_get_applied_connection (active);
 
 	device = nm_active_connection_get_device (active);
-	if (!device) {
+	g_return_val_if_fail (device != NULL, FALSE);
-		if (!nm_connection_is_virtual (applied)) {
-			NMSettingConnection *s_con = nm_connection_get_setting_connection (applied);
-
-			g_assert (s_con);
-			g_set_error (error,
-			             NM_MANAGER_ERROR,
-			             NM_MANAGER_ERROR_UNKNOWN_DEVICE,
-			             "Unsupported virtual interface type '%s'",
-			             nm_setting_connection_get_connection_type (s_con));
-			return FALSE;
-		}
-
-		device = system_create_virtual_device (self, applied, &local_err);
-		if (!device) {
-			g_set_error (error,
-			             NM_MANAGER_ERROR,
-			             NM_MANAGER_ERROR_UNKNOWN_DEVICE,
-			             "Failed to create virtual interface: %s",
-			             local_err ? local_err->message : "(unknown)");
-			g_clear_error (&local_err);
-			return FALSE;
-		}
-
-		if (!nm_active_connection_set_device (active, device)) {
-			g_set_error_literal (error,
-			                     NM_MANAGER_ERROR,
-			                     NM_MANAGER_ERROR_UNKNOWN_DEVICE,
-			                     "The device could not be activated with this connection");
-			return FALSE;
-		}
-
-		/* A newly created device, if allowed to be managed by NM, will be
-		 * in the UNAVAILABLE state here.  To ensure it can be activated
-		 * immediately, we transition it to DISCONNECTED.
-		 */
-		if (   nm_device_is_available (device, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE)
-			&& (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) {
-			nm_device_state_changed (device,
-			                         NM_DEVICE_STATE_DISCONNECTED,
-			                         NM_DEVICE_STATE_REASON_NONE);
-		}
-	} else {
-		NMConnection *existing_connection = NULL;
-		NMAuthSubject *subject;
-		char *error_desc = NULL;
 
 	/* If the device is active and its connection is not visible to the
 	 * user that's requesting this new activation, fail, since other users
@@ -2701,7 +2694,6 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 		g_free (error_desc);
 		return FALSE;
 	}
-	}
 
 	/* Final connection must be available on device */
 	if (!nm_device_check_connection_available (device, applied, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
@@ -2777,9 +2769,10 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 		}
 
 		nm_active_connection_set_master (active, master_ac);
-		nm_log_dbg (LOGD_CORE, "Activation of '%s' depends on active connection %p",
+		nm_log_dbg (LOGD_CORE, "Activation of '%s' depends on active connection %p %s",
 		            nm_settings_connection_get_id (connection),
-		            master_ac);
+		            master_ac,
+		            str_if_set (nm_exported_object_get_path (NM_EXPORTED_OBJECT  (master_ac)), ""));
 	}
 
 	/* Check slaves for master connection and possibly activate them */
@@ -2790,6 +2783,19 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 	if (existing)
 		nm_device_steal_connection (existing, connection);
 
+	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_NONE)
+	    && (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) {
+		nm_device_state_changed (device,
+		                         NM_DEVICE_STATE_DISCONNECTED,
+		                         NM_DEVICE_STATE_REASON_USER_REQUESTED);
+	}
+
 	/* Export the new ActiveConnection to clients and start it on the device */
 	nm_exported_object_export (NM_EXPORTED_OBJECT (active));
 	g_object_notify (G_OBJECT (self), NM_MANAGER_ACTIVE_CONNECTIONS);
@@ -3042,6 +3048,23 @@ nm_manager_activate_connection (NMManager *self,
 	return active;
 }
 
+/**
+ * validate_activation_request:
+ * @self: the #NMManager
+ * @context: the D-Bus context of the requestor
+ * @connection: the partial or complete #NMConnection to be activated
+ * @device_path: the object path of the device to be activated, or "/"
+ * @out_device: on successful reutrn, the #NMDevice to be activated with @connection
+ * @out_vpn: on successful return, %TRUE if @connection is a VPN connection
+ * @error: location to store an error on failure
+ *
+ * Performs basic validation on an activation request, including ensuring that
+ * the requestor is a valid Unix process, is not disallowed in @connection
+ * permissions, and that a device exists that can activate @connection.
+ *
+ * Returns: on success, the #NMAuthSubject representing the requestor, or
+ *   %NULL on error
+ */
 static NMAuthSubject *
 validate_activation_request (NMManager *self,
                              GDBusMethodInvocation *context,
@@ -3116,11 +3139,11 @@ validate_activation_request (NMManager *self,
 	} else
 		device = nm_manager_get_best_device_for_connection (self, connection, TRUE);
 
-	if (!device) {
+	if (!device && !vpn) {
 		gboolean is_software = nm_connection_is_virtual (connection);
 
 		/* VPN and software-device connections don't need a device yet */
-		if (!vpn && !is_software) {
+		if (!is_software) {
 			g_set_error_literal (error,
 			                     NM_MANAGER_ERROR,
 			                     NM_MANAGER_ERROR_UNKNOWN_DEVICE,
@@ -3136,11 +3159,19 @@ validate_activation_request (NMManager *self,
 			if (!iface)
 				goto error;
 
-			device = find_device_by_iface (self, iface);
+			device = find_device_by_iface (self, iface, connection, NULL);
 			g_free (iface);
 		}
 	}
 
+	if ((!vpn || device_path) && !device) {
+		g_set_error_literal (error,
+		                     NM_MANAGER_ERROR,
+		                     NM_MANAGER_ERROR_UNKNOWN_DEVICE,
+		                     "Failed to find a compatible device for this connection");
+		goto error;
+	}
+
 	*out_device = device;
 	*out_vpn = vpn;
 	return subject;
@@ -3454,14 +3485,6 @@ impl_manager_add_and_activate_connection (NMManager *self,
 	if (!subject)
 		goto error;
 
-	/* AddAndActivate() requires a device to complete the connection with */
-	if (!device) {
-		error = g_error_new_literal (NM_MANAGER_ERROR,
-		                             NM_MANAGER_ERROR_UNKNOWN_DEVICE,
-		                             "This connection requires an existing device.");
-		goto error;
-	}
-
 	all_connections = nm_settings_get_connections (priv->settings);
 	if (vpn) {
 		/* Try to fill the VPN's connection setting and name at least */