merge: branch 'lr/ifcfg-device-rh1369008'

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

libnm-core/nm-setting-connection.c

@@ -1696,9 +1696,11 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class)
 	 **/
 	/* ---ifcfg-rh---
 	 * property: master
-	 * variable: MASTER, TEAM_MASTER, BRIDGE
+	 * variable: MASTER, MASTER_UUID, TEAM_MASTER, TEAM_MASTER_UUID, BRIDGE, BRIDGE_UUID
 	 * description: Reference to master connection. The variable used depends on
-	 *   the connection type.
+	 *   the connection type and the value. In general, if the *_UUID variant is present,
+	 *   the variant without *_UUID is ignored. NetworkManager attempts to write both
+	 *   for compatibility with legacy tooling.
 	 * ---end---
 	 */
 	g_object_class_install_property
@@ -1719,10 +1721,12 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class)
 	 **/
 	/* ---ifcfg-rh---
 	 * property: slave-type
-	 * variable: MASTER, TEAM_MASTER, DEVICETYPE, BRIDGE
+	 * variable: MASTER, MASTER_UUID, TEAM_MASTER, TEAM_MASTER_UUID, DEVICETYPE,
+	 *   BRIDGE, BRIDGE_UUID
 	 * description: Slave type doesn't map directly to a variable, but it is
-	 *   recognized using different variables.  MASTER for bonding,
+	 *   recognized using different variables.  MASTER and MASTER_UUID for bonding,
-	 *   TEAM_MASTER and DEVICETYPE for teaming, BRIDGE for bridging.
+	 *   TEAM_MASTER, TEAM_MASTER_UUID and DEVICETYPE for teaming, BRIDGE
+	 *   and BRIDGE_UUID for bridging.
 	 * ---end---
 	 */
 	g_object_class_install_property

src/nm-manager.c

@@ -1145,6 +1145,30 @@ nm_manager_get_connection_iface (NMManager *self,
 	return iface;
 }
 
+/**
+ * nm_manager_iface_for_uuid:
+ * @self: the #NMManager
+ * @uuid: the connection uuid
+ *
+ * Gets a link name for the given UUID. Useful for the settings plugins that
+ * wish to write configuration files compatible with tooling that can't
+ * interpret our UUIDs.
+ *
+ * Returns: An interface name; %NULL if none matches
+ */
+const char *
+nm_manager_iface_for_uuid (NMManager *self, const char *uuid)
+{
+	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+	NMSettingsConnection *connection;
+
+	connection = nm_settings_get_connection_by_uuid (priv->settings, uuid);
+	if (!connection)
+		return NULL;
+
+	return nm_connection_get_interface_name (NM_CONNECTION (connection));
+}
+
 /**
  * system_create_virtual_device:
  * @self: the #NMManager

src/nm-manager.h

@@ -103,6 +103,9 @@ char *              nm_manager_get_connection_iface (NMManager *self,
                                                      NMDevice **out_parent,
                                                      GError **error);
 
+const char *        nm_manager_iface_for_uuid          (NMManager *self,
+                                                        const char *uuid);
+
 NMActiveConnection *nm_manager_activate_connection     (NMManager *manager,
                                                         NMSettingsConnection *connection,
                                                         NMConnection *applied_connection,

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c

@@ -231,7 +231,9 @@ make_connection_setting (const char *file,
 		g_strfreev (items);
 	}
 
-	value = svGetValueString (ifcfg, "BRIDGE");
+	value = svGetValueString (ifcfg, "BRIDGE_UUID");
+	if (!value)
+		value = svGetValueString (ifcfg, "BRIDGE");
 	if (value) {
 		const char *old_value;
 
@@ -1670,7 +1672,10 @@ check_if_bond_slave (shvarFile *ifcfg,
 {
 	char *value;
 
-	value = svGetValueString (ifcfg, "MASTER");
+	value = svGetValueString (ifcfg, "MASTER_UUID");
+	if (!value)
+		value = svGetValueString (ifcfg, "MASTER");
+
 	if (value) {
 		g_object_set (s_con, NM_SETTING_CONNECTION_MASTER, value, NULL);
 		g_object_set (s_con,
@@ -1690,9 +1695,12 @@ check_if_team_slave (shvarFile *ifcfg,
 {
 	gs_free char *value = NULL;
 
-	value = svGetValueString (ifcfg, "TEAM_MASTER");
+	value = svGetValueString (ifcfg, "TEAM_MASTER_UUID");
+	if (!value)
+		value = svGetValueString (ifcfg, "TEAM_MASTER");
 	if (!value)
 		return FALSE;
+
 	g_object_set (s_con, NM_SETTING_CONNECTION_MASTER, value, NULL);
 	g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_TEAM_SETTING_NAME, NULL);
 	return TRUE;
@@ -4646,7 +4654,9 @@ make_bridge_port_setting (shvarFile *ifcfg)
 
 	g_return_val_if_fail (ifcfg != NULL, FALSE);
 
-	value = svGetValueString (ifcfg, "BRIDGE");
+	value = svGetValueString (ifcfg, "BRIDGE_UUID");
+	if (!value)
+		value = svGetValueString (ifcfg, "BRIDGE");
 	if (value) {
 		g_free (value);
 

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c

@@ -32,6 +32,7 @@
 #include <unistd.h>
 #include <stdio.h>
 
+#include "nm-manager.h"
 #include "nm-setting-connection.h"
 #include "nm-setting-wired.h"
 #include "nm-setting-wireless.h"
@@ -1741,13 +1742,9 @@ write_connection_setting (NMSettingConnection *s_con, shvarFile *ifcfg)
 {
 	guint32 n, i;
 	GString *str;
-	const char *master, *type;
+	const char *master, *master_iface = NULL, *type;
 	char *tmp;
 	gint i_int;
-	const char *v_master = NULL;
-	const char *v_slave = NULL;
-	const char *v_bridge = NULL;
-	const char *v_team_master = NULL;
 
 	svSetValueString (ifcfg, "NAME", nm_setting_connection_get_id (s_con));
 	svSetValueString (ifcfg, "UUID", nm_setting_connection_get_uuid (s_con));
@@ -1815,27 +1812,45 @@ write_connection_setting (NMSettingConnection *s_con, shvarFile *ifcfg)
 
 	svSetValueString (ifcfg, "ZONE", nm_setting_connection_get_zone(s_con));
 
+	svSetValueString (ifcfg, "MASTER_UUID", NULL);
+	svSetValueString (ifcfg, "MASTER", NULL);
+	svSetValueString (ifcfg, "SLAVE", NULL);
+	svSetValueString (ifcfg, "BRIDGE_UUID", NULL);
+	svSetValueString (ifcfg, "BRIDGE", NULL);
+	svSetValueString (ifcfg, "TEAM_MASTER_UUID", NULL);
+	svSetValueString (ifcfg, "TEAM_MASTER", NULL);
+
 	master = nm_setting_connection_get_master (s_con);
 	if (master) {
+		/* The reader prefers the *_UUID variants, however we still try to resolve
+		 * it into an interface name, so that legacy tooling is not confused. */
+		if (!nm_utils_get_testing ()) {
+			/* This is conditional for easier testing. */
+			master_iface = nm_manager_iface_for_uuid (nm_manager_get (), master);
+		}
+		if (!master_iface) {
+			master_iface = master;
+			master = NULL;
+
+		}
+
 		if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_BOND_SETTING_NAME)) {
-			v_master = master;
+			svSetValueString (ifcfg, "MASTER_UUID", master);
-			v_slave = "yes";
+			svSetValueString (ifcfg, "MASTER", master_iface);
-		} else if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_BRIDGE_SETTING_NAME))
+			svSetValueString (ifcfg, "SLAVE", "yes");
-			v_bridge = master;
+		} else if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_BRIDGE_SETTING_NAME)) {
-		else if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_TEAM_SETTING_NAME)) {
+			svSetValueString (ifcfg, "BRIDGE_UUID", master);
-			v_team_master = master;
+			svSetValueString (ifcfg, "BRIDGE", master_iface);
+		} else if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_TEAM_SETTING_NAME)) {
+			svSetValueString (ifcfg, "TEAM_MASTER_UUID", master);
+			svSetValueString (ifcfg, "TEAM_MASTER", master_iface);
 			svUnsetValue (ifcfg, "TYPE");
 		}
 	}
 
-	svSetValueString (ifcfg, "MASTER", v_master);
-	svSetValueString (ifcfg, "SLAVE", v_slave);
-	svSetValueString (ifcfg, "BRIDGE", v_bridge);
-	svSetValueString (ifcfg, "TEAM_MASTER", v_team_master);
-
 	if (nm_streq0 (type, NM_SETTING_TEAM_SETTING_NAME))
 		svSetValueString (ifcfg, "DEVICETYPE", TYPE_TEAM);
-	else if (master && nm_setting_connection_is_slave_type (s_con, NM_SETTING_TEAM_SETTING_NAME))
+	else if (master_iface && nm_setting_connection_is_slave_type (s_con, NM_SETTING_TEAM_SETTING_NAME))
 		svSetValueString (ifcfg, "DEVICETYPE", TYPE_TEAM_PORT);
 	else
 		svUnsetValue (ifcfg, "DEVICETYPE");

src/settings/plugins/ifcfg-rh/shvar.c

@@ -40,6 +40,11 @@
 
 /*****************************************************************************/
 
+typedef struct {
+	gchar *original;
+	gchar *value;
+} shvarLine;
+
 struct _shvarFile {
 	char      *fileName;    /* read-only */
 	int        fd;          /* read-only */
@@ -626,6 +631,28 @@ svFileSetModified (shvarFile *s)
 
 /*****************************************************************************/
 
+static shvarLine *
+line_new (char *value)
+{
+	shvarLine *line;
+
+	line = g_slice_new0 (shvarLine);
+	line->original = line->value = value;
+
+	return line;
+}
+
+static void
+line_free (shvarLine *line)
+{
+	g_free (line->value);
+	if (line->original != line->value)
+		g_free (line->original);
+	g_slice_free (shvarLine, line);
+}
+
+/*****************************************************************************/
+
 /* Open the file <name>, returning a shvarFile on success and NULL on failure.
  * Add a wrinkle to let the caller specify whether or not to create the file
  * (actually, return a structure anyway) if it doesn't exist.
@@ -679,9 +706,9 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
 
 		/* we'd use g_strsplit() here, but we want a list, not an array */
 		for (p = arena; (q = strchr (p, '\n')) != NULL; p = q + 1)
-			s->lineList = g_list_prepend (s->lineList, g_strndup (p, q - p));
+			s->lineList = g_list_prepend (s->lineList, line_new (g_strndup (p, q - p)));
 		if (p[0])
-			s->lineList = g_list_prepend (s->lineList, g_strdup (p));
+			s->lineList = g_list_prepend (s->lineList, line_new (g_strdup (p)));
 		g_free (arena);
 		s->lineList = g_list_reverse (s->lineList);
 
@@ -739,14 +766,25 @@ shlist_find (const GList *current, const char *key, const char **out_value)
 	if (current) {
 		len = strlen (key);
 		do {
-			const char *line = current->data;
+			shvarLine *line = current->data;
+			const char *original = line->original;
 
 			/* skip over leading spaces */
-			while (g_ascii_isspace (line[0]))
+			while (g_ascii_isspace (original[0]))
-				line++;
+				original++;
 
-			if (!strncmp (key, line, len) && line[len] == '=') {
+			if (!strncmp (key, original, len) && original[len] == '=') {
-				NM_SET_OUT (out_value, line + len + 1);
+				if (out_value) {
+					const char *value = line->value;
+
+					if (value) {
+						while (g_ascii_isspace (value[0]))
+							value++;
+						nm_assert (!strncmp (key, value, len) && value[len] == '=');
+						value += len + 1;
+					}
+					*out_value = value;
+				}
 				return current;
 			}
 			current = current->next;
@@ -758,24 +796,27 @@ shlist_find (const GList *current, const char *key, const char **out_value)
 }
 
 static void
-shlist_delete (GList **head, GList *current)
+shlist_delete (GList *head, GList *current)
 {
-	nm_assert (head && *head);
+	shvarLine *line = current->data;
+
+	nm_assert (head);
 	nm_assert (current);
 	nm_assert (current->data);
-	nm_assert (g_list_position (*head, current) >= 0);
+	nm_assert (g_list_position (head, current) >= 0);
 
-	g_free (current->data);
+	if (line->value != line->original)
-	*head = g_list_delete_link (*head, current);
+		g_free (line->value);
+	line->value = NULL;
 }
 
 static gboolean
-shlist_delete_all (GList **head, const char *key, gboolean including_last)
+shlist_delete_all (GList *head, const char *key, gboolean including_last)
 {
 	GList *current, *last;
 	gboolean changed = FALSE;
 
-	last = (GList *) shlist_find (*head, key, NULL);
+	last = (GList *) shlist_find (head, key, NULL);
 	if (last) {
 		while ((current = (GList *) shlist_find (last->next, key, NULL))) {
 			shlist_delete (head, last);
@@ -790,18 +831,6 @@ shlist_delete_all (GList **head, const char *key, gboolean including_last)
 	return changed;
 }
 
-static char *
-line_construct (const char *key, const char *value)
-{
-	gs_free char *newval_free = NULL;
-
-	nm_assert (_shell_is_name (key));
-	nm_assert (value);
-
-	return g_strdup_printf ("%s=%s", key,
-	                        svEscape (value, &newval_free));
-}
-
 /*****************************************************************************/
 
 static const char *
@@ -923,50 +952,43 @@ svGetValueInt64 (shvarFile *s, const char *key, guint base, gint64 min, gint64 m
 void
 svSetValue (shvarFile *s, const char *key, const char *value)
 {
-	gs_free char *oldval_free = NULL;
-	const char *oldval, *oldval_tmp;
 	GList *current, *last;
-	gboolean has_multiple = FALSE;
+	shvarLine *line;
+	gchar *new_value;
+	gs_free char *newval_free = NULL;
 
 	g_return_if_fail (s != NULL);
 	g_return_if_fail (key != NULL);
 
 	nm_assert (_shell_is_name (key));
 
-	if (!value) {
+	if (shlist_delete_all (s->lineList, key, TRUE))
-		if (shlist_delete_all (&s->lineList, key, TRUE))
+		s->modified = TRUE;
-			s->modified = TRUE;
+
+	if (!value)
 		return;
-	}
 
-	current = (GList *) shlist_find (s->lineList, key, &oldval);
+	new_value = g_strdup_printf ("%s=%s", key, svEscape (value, &newval_free));
 
+	current = (GList *) shlist_find (s->lineList, key, NULL);
 	if (!current) {
-		s->lineList = g_list_append (s->lineList,
+		s->lineList = g_list_append (s->lineList, line_new (new_value));
-		                             line_construct (key, value));
 		s->modified = TRUE;
 		return;
 	}
 
 	last = current;
-	while ((current = (GList *) shlist_find (current->next, key, &oldval_tmp))) {
+	while ((current = (GList *) shlist_find (current->next, key, NULL)))
 		last = current;
-		oldval = oldval_tmp;
-		has_multiple = TRUE;
-	}
 	current = last;
 
-	oldval = svUnescape (oldval, &oldval_free);
+	line = current->data;
-	if (!nm_streq0 (oldval, value)) {
+	if (line->value != line->original)
-		g_free (current->data);
+		g_free (line->value);
-		current->data = line_construct (key, value);
+	line->value = new_value;
-		s->modified = TRUE;
-	}
 
-	if (has_multiple) {
+	if (!nm_streq0 (line->original, line->value))
-		if (shlist_delete_all (&s->lineList, key, FALSE))
+		s->modified = TRUE;
-			s->modified = TRUE;
-	}
 }
 
 /* Set the variable <key> equal to the value <value>.
@@ -1047,11 +1069,14 @@ svWriteFile (shvarFile *s, int mode, GError **error)
 		f = fdopen (tmpfd, "w");
 		fseek (f, 0, SEEK_SET);
 		for (current = s->lineList; current; current = current->next) {
-			const char *line = current->data;
+			shvarLine *line = current->data;
 			const char *str;
 			const char *value;
 
-			str = line;
+			str = line->value;
+			if (!str)
+				continue;
+
 			while (g_ascii_isspace (str[0]))
 				str++;
 			if (NM_IN_SET (str[0], '\0', '#'))
@@ -1068,10 +1093,10 @@ svWriteFile (shvarFile *s, int mode, GError **error)
 				s_tmp = g_strndup (str, value - str);
 				fprintf (f, "%s\n", s_tmp);
 			}
-			fprintf (f, "#NM: %s\n", line);
+			fprintf (f, "#NM: %s\n", line->value);
 			continue;
 write_regular:
-			fprintf (f, "%s\n", line);
+			fprintf (f, "%s\n", line->value);
 		}
 		fclose (f);
 	}
@@ -1090,6 +1115,6 @@ svCloseFile (shvarFile *s)
 		close (s->fd);
 
 	g_free (s->fileName);
-	g_list_free_full (s->lineList, g_free);
+	g_list_free_full (s->lineList, (GDestroyNotify) line_free);
 	g_slice_free (shvarFile, s);
 }

src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

@@ -8616,6 +8616,7 @@ test_write_unknown (gconstpointer test_data)
 		_svGetValue_check (sv, "NAME3", "name3-value");
 
 		svSetValue (sv, "NAME", "set-by-test1");
+		svSetValue (sv, "NAME2", NULL);
 		svSetValue (sv, "NAME2", "set-by-test2");
 		svSetValue (sv, "NAME3", "set-by-test3");