merge: branch 'bg/slaves-order-rh1420708'

https://bugzilla.redhat.com/show_bug.cgi?id=1420708

2 files changed, 106 insertions, 34 deletions

diff --git a/src/nm-manager.c b/src/nm-manager.c
index 6c74a770ee..5f06f23ff5 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -2473,7 +2473,8 @@ nm_manager_get_device_paths (NMManager *self)
 static NMDevice *
 nm_manager_get_best_device_for_connection (NMManager *self,
                                            NMConnection *connection,
-                                           gboolean for_user_request)
+                                           gboolean for_user_request,
+                                           GHashTable *unavailable_devices)
 {
 	const GSList *devices, *iter;
 	NMActiveConnection *ac;
@@ -2494,6 +2495,9 @@ nm_manager_get_best_device_for_connection (NMManager *self,
 	for (iter = devices; iter; iter = g_slist_next (iter)) {
 		NMDevice *device = NM_DEVICE (iter->data);
 
+		if (unavailable_devices && g_hash_table_contains (unavailable_devices, device))
+			continue;
+
 		if (nm_device_check_connection_available (device, connection, flags, NULL))
 			return device;
 	}
@@ -2855,49 +2859,83 @@ ensure_master_active_connection (NMManager *self,
 	return NULL;
 }
 
+typedef struct {
+	NMSettingsConnection *connection;
+	NMDevice *device;
+} SlaveConnectionInfo;
+
 /**
  * find_slaves:
  * @manager: #NMManager object
  * @connection: the master #NMSettingsConnection to find slave connections for
  * @device: the master #NMDevice for the @connection
+ * @out_n_slaves: on return, the number of slaves found
  *
  * Given an #NMSettingsConnection, attempts to find its slaves. If @connection is not
  * master, or has not any slaves, this will return %NULL.
  *
- * Returns: list of slave connections for given master @connection, or %NULL
+ * Returns: an array of #SlaveConnectionInfo for given master @connection, or %NULL
  **/
-static GSList *
+static SlaveConnectionInfo *
 find_slaves (NMManager *manager,
              NMSettingsConnection *connection,
-             NMDevice *device)
+             NMDevice *device,
+             guint *out_n_slaves)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager);
 	gs_free NMSettingsConnection **all_connections = NULL;
+	guint n_all_connections;
 	guint i;
-	GSList *slaves = NULL;
+	SlaveConnectionInfo *slaves = NULL;
+	guint n_slaves = 0;
 	NMSettingConnection *s_con;
+	gs_unref_hashtable GHashTable *devices = NULL;
+
+	nm_assert (out_n_slaves);
 
 	s_con = nm_connection_get_setting_connection (NM_CONNECTION (connection));
-	g_assert (s_con);
+	g_return_val_if_fail (s_con, NULL);
+
+	devices = g_hash_table_new (g_direct_hash, g_direct_equal);
 
 	/* Search through all connections, not only inactive ones, because
 	 * even if a slave was already active, it might be deactivated during
 	 * master reactivation.
 	 */
-	all_connections = nm_settings_get_connections_sorted (priv->settings, NULL);
-	for (i = 0; all_connections[i]; i++) {
+	all_connections = nm_settings_get_connections_sorted (priv->settings, &n_all_connections);
+	for (i = 0; i < n_all_connections; i++) {
 		NMSettingsConnection *master_connection = NULL;
-		NMDevice *master_device = NULL;
+		NMDevice *master_device = NULL, *slave_device;
 		NMConnection *candidate = NM_CONNECTION (all_connections[i]);
 
 		find_master (manager, candidate, NULL, &master_connection, &master_device, NULL, NULL);
 		if (   (master_connection && master_connection == connection)
 		    || (master_device && master_device == device)) {
-			slaves = g_slist_prepend (slaves, candidate);
+			slave_device = nm_manager_get_best_device_for_connection (manager,
+			                                                          candidate,
+			                                                          FALSE,
+			                                                          devices);
+
+			if (!slaves) {
+				/* what we allocate is quite likely much too large. Don't bother, it is only
+				 * a temporary buffer. */
+				slaves = g_new (SlaveConnectionInfo, n_all_connections);
+			}
+
+			nm_assert (n_slaves < n_all_connections);
+			slaves[n_slaves].connection = NM_SETTINGS_CONNECTION (candidate),
+			slaves[n_slaves].device = slave_device,
+			n_slaves++;
+
+			if (slave_device)
+				g_hash_table_add (devices, slave_device);
 		}
 	}
 
-	return g_slist_reverse (slaves);
+	*out_n_slaves = n_slaves;
+
+	/* Warning: returns NULL if n_slaves is zero. */
+	return slaves;
 }
 
 static gboolean
@@ -2929,39 +2967,56 @@ out:
 	return FALSE;
 }
 
-static gboolean
+static gint
+compare_slaves (gconstpointer a, gconstpointer b, gpointer _unused)
+{
+	const SlaveConnectionInfo *a_info = a;
+	const SlaveConnectionInfo *b_info = b;
+
+	/* Slaves without a device at the end */
+	if (!a_info->device)
+		return 1;
+	if (!b_info->device)
+		return -1;
+
+	return g_strcmp0 (nm_device_get_iface (a_info->device),
+	                  nm_device_get_iface (b_info->device));
+}
+
+static void
 autoconnect_slaves (NMManager *self,
                     NMSettingsConnection *master_connection,
                     NMDevice *master_device,
                     NMAuthSubject *subject)
 {
 	GError *local_err = NULL;
-	gboolean ret = FALSE;
 
 	if (should_connect_slaves (NM_CONNECTION (master_connection), master_device)) {
-		GSList *slaves, *iter;
+		gs_free SlaveConnectionInfo *slaves = NULL;
+		guint i, n_slaves = 0;
 
-		iter = slaves = find_slaves (self, master_connection, master_device);
-		ret = slaves != NULL;
+		slaves = find_slaves (self, master_connection, master_device, &n_slaves);
+		if (n_slaves > 1) {
+			g_qsort_with_data (slaves, n_slaves, sizeof (slaves[0]),
+			                   compare_slaves, NULL);
+		}
 
-		while (iter) {
-			NMSettingsConnection *slave_connection = iter->data;
+		for (i = 0; i < n_slaves; i++) {
+			SlaveConnectionInfo *slave = &slaves[i];
 			const char *uuid;
 
-			iter = iter->next;
-
 			/* To avoid loops when autoconnecting slaves, we propagate
 			 * the UUID of the initial connection down to slaves until
 			 * the same connection is found.
 			 */
 			uuid = g_object_get_qdata (G_OBJECT (master_connection),
 			                           autoconnect_root_quark ());
-			if (nm_streq0 (nm_settings_connection_get_uuid (slave_connection), uuid)) {
+			if (nm_streq0 (nm_settings_connection_get_uuid (slave->connection), uuid)) {
 				_LOGI (LOGD_CORE,
 				       "will NOT activate slave connection '%s' (%s) as a dependency for master '%s' (%s): "
 				       "circular dependency detected",
-				       nm_settings_connection_get_id (slave_connection),
-				       nm_settings_connection_get_uuid (slave_connection),
+				       nm_settings_connection_get_id (slave->connection),
+				       nm_settings_connection_get_uuid (slave->connection),
 				       nm_settings_connection_get_id (master_connection),
 				       nm_settings_connection_get_uuid (master_connection));
 				continue;
@@ -2969,23 +3024,34 @@ autoconnect_slaves (NMManager *self,
 
 			if (!uuid)
 				uuid = nm_settings_connection_get_uuid (master_connection);
-			g_object_set_qdata_full (G_OBJECT (slave_connection),
+			g_object_set_qdata_full (G_OBJECT (slave->connection),
 			                         autoconnect_root_quark (),
 			                         g_strdup (uuid),
 			                         g_free);
 
+			if (!slave->device) {
+				_LOGD (LOGD_CORE,
+				       "will NOT activate slave connection '%s' (%s) as a dependency for master '%s' (%s): "
+				       "no compatible device found",
+				       nm_settings_connection_get_id (slave->connection),
+				       nm_settings_connection_get_uuid (slave->connection),
+				       nm_settings_connection_get_id (master_connection),
+				       nm_settings_connection_get_uuid (master_connection));
+				continue;
+			}
+
 			_LOGD (LOGD_CORE, "will activate slave connection '%s' (%s) as a dependency for master '%s' (%s)",
-			       nm_settings_connection_get_id (slave_connection),
-			       nm_settings_connection_get_uuid (slave_connection),
+			       nm_settings_connection_get_id (slave->connection),
+			       nm_settings_connection_get_uuid (slave->connection),
 			       nm_settings_connection_get_id (master_connection),
 			       nm_settings_connection_get_uuid (master_connection));
 
 			/* Schedule slave activation */
 			nm_manager_activate_connection (self,
-			                                slave_connection,
+			                                slave->connection,
 			                                NULL,
 			                                NULL,
-			                                nm_manager_get_best_device_for_connection (self, NM_CONNECTION (slave_connection), FALSE),
+			                                slave->device,
 			                                subject,
 			                                NM_ACTIVATION_TYPE_MANAGED,
 			                                &local_err);
@@ -2994,9 +3060,7 @@ autoconnect_slaves (NMManager *self,
 				g_clear_error (&local_err);
 			}
 		}
-		g_slist_free (slaves);
 	}
-	return ret;
 }
 
 static gboolean
@@ -3602,7 +3666,7 @@ validate_activation_request (NMManager *self,
 			goto error;
 		}
 	} else
-		device = nm_manager_get_best_device_for_connection (self, connection, TRUE);
+		device = nm_manager_get_best_device_for_connection (self, connection, TRUE, NULL);
 
 	if (!device && !vpn) {
 		gboolean is_software = nm_connection_is_virtual (connection);
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 4ab2cff20b..6eb4a1f807 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -444,11 +444,19 @@ _link_get_all_presort (gconstpointer  p_a,
 	const NMPlatformLink *a = p_a;
 	const NMPlatformLink *b = p_b;
 
-	if (a->ifindex < b->ifindex)
+	/* Loopback always first */
+	if (a->ifindex == 1)
 		return -1;
-	if (a->ifindex > b->ifindex)
+	if (b->ifindex == 1)
 		return 1;
-	return 0;
+
+	/* Initialized links first */
+	if (a->initialized > b->initialized)
+		return -1;
+	if (a->initialized < b->initialized)
+		return 1;
+
+	return strcmp (a->name, b->name);
 }
 
 /**