bond: add proper properties to NMSettingBondth/wip/bondprobsth/bondprobs

Make NMSettingBond have individual properties like other settings
types.

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

Co-Authored-By: Thomas Haller <thaller@redhat.com>
Co-Authored-By: Dan Williams <dcbw@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>

4 files changed, 111 insertions, 97 deletions

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index e2cff0b2cf..b46daf9ca1 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -63,10 +63,10 @@
 #include "reader.h"
 
 #define PLUGIN_PRINT(pname, fmt, args...) \
-	{ g_message ("   " pname ": " fmt, ##args); }
+    G_STMT_START { g_message ("   " pname ": " fmt, ##args); } G_STMT_END
 
 #define PLUGIN_WARN(pname, fmt, args...) \
-	{ g_warning ("   " pname ": " fmt, ##args); }
+    G_STMT_START { g_warning ("   " pname ": " fmt, ##args); } G_STMT_END
 
 static gboolean
 get_int (const char *str, int *value)
@@ -4059,29 +4059,93 @@ infiniband_connection_from_ifcfg (const char *file,
 	return connection;
 }
 
+typedef void (*IfcfgRhOptFunc) (NMSetting *setting,
+                                const char *key,
+                                const char *value,
+                                gpointer data);
+
+static void
+handle_ifcfg_rh_opts (NMSetting *setting,
+                      const char *value,
+                      IfcfgRhOptFunc func,
+                      gpointer data)
+{
+	char **items, **iter;
+
+	g_return_if_fail (value);
+	if (!value || !*value)
+		return;
+
+	items = g_strsplit_set (value, " ", -1);
+	for (iter = items; *iter; iter++) {
+		if (**iter) {
+			char **keys, *key, *val;
+
+			keys = g_strsplit_set (*iter, "=", 2);
+			if (*keys) {
+				key = keys[0];
+				val = keys[1];
+				if (val && *key && *val)
+					func (setting, key, val, data);
+			}
+
+			g_strfreev (keys);
+		}
+	}
+	g_strfreev (items);
+}
+
 static void
-handle_bond_option (NMSettingBond *s_bond,
+handle_bond_option (NMSetting *setting,
                     const char *key,
-                    const char *value)
+                    const char *value,
+                    gpointer data)
 {
-	char *sanitized = NULL, *j;
-	const char *p = value;
+	NMSettingBond *s_bond = NM_SETTING_BOND (setting);
+	const char *value0 = value;
+	char *value_cleanup = NULL;
 
-	/* Remove any quotes or +/- from arp_ip_target */
-	if (!g_strcmp0 (key, NM_SETTING_BOND_OPTION_ARP_IP_TARGET) && value && value[0]) {
-		if (*p == '\'' || *p == '"')
-			p++;
-		j = sanitized = g_malloc0 (strlen (p) + 1);
-		while (*p) {
-			if (*p != '+' && *p != '-' && *p != '\'' && *p != '"')
-				*j++ = *p;
-			p++;
+	g_return_if_fail (key);
+
+	if (!strcmp (key, NM_SETTING_BOND_OPTION_ARP_IP_TARGET)) {
+		char **arp_ip_target = NULL;
+		char *sanitized = NULL, *j;
+		const char *p = value;
+
+		/* Remove any quotes or +/- from arp_ip_target */
+		if (value[0]) {
+			if (*p == '\'' || *p == '"')
+				p++;
+			j = sanitized = g_malloc0 (strlen (p) + 1);
+			while (*p) {
+				if (*p != '+' && *p != '-' && *p != '\'' && *p != '"')
+					*j++ = *p;
+				p++;
+			}
+			value = sanitized;
 		}
+
+		if (value && value[0]) {
+			arp_ip_target = g_strsplit (value, ",", -1);
+			/* don't set the GObject property to arp_ip_target, because we want to reuse the
+			 * validation from nm_setting_bond_set_string. */
+			value_cleanup = g_strjoinv (",", arp_ip_target);
+			value = value_cleanup;
+			g_strfreev (arp_ip_target);
+		} else
+			value = NULL;
+		g_free (sanitized);
 	}
 
-	if (!nm_setting_bond_add_option (s_bond, key, sanitized ? sanitized : value))
-		PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: invalid bonding option '%s'", key);
-	g_free (sanitized);
+	if (!nm_setting_bond_set_string (s_bond, key, value)) {
+		if (!nm_setting_bond_get_property_name (key))
+			PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: unrecognized bond property '%s'", key);
+		else
+			PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: invalid bond property '%s'='%s'", key, value0);
+		return;
+	}
+
+	g_free (value_cleanup);
 }
 
 static NMSetting *
@@ -4105,26 +4169,8 @@ make_bond_setting (shvarFile *ifcfg,
 
 	value = svGetValue (ifcfg, "BONDING_OPTS", FALSE);
 	if (value) {
-		char **items, **iter;
-
-		items = g_strsplit_set (value, " ", -1);
-		for (iter = items; iter && *iter; iter++) {
-			if (strlen (*iter)) {
-				char **keys, *key, *val;
-
-				keys = g_strsplit_set (*iter, "=", 2);
-				if (keys && *keys) {
-					key = *keys;
-					val = *(keys + 1);
-					if (val && strlen(key) && strlen(val))
-						handle_bond_option (s_bond, key, val);
-				}
-
-				g_strfreev (keys);
-			}
-		}
+		handle_ifcfg_rh_opts (NM_SETTING (s_bond), value, handle_bond_option, NULL);
 		g_free (value);
-		g_strfreev (items);
 	}
 
 	return (NMSetting *) s_bond;
@@ -4266,17 +4312,13 @@ team_connection_from_ifcfg (const char *file,
 	return connection;
 }
 
-typedef void (*BridgeOptFunc) (NMSetting *setting,
-                               gboolean stp,
-                               const char *key,
-                               const char *value);
-
 static void
 handle_bridge_option (NMSetting *setting,
-                      gboolean stp,
                       const char *key,
-                      const char *value)
+                      const char *value,
+                      gpointer data)
 {
+	gboolean stp = GPOINTER_TO_INT (data);
 	guint32 u = 0;
 
 	if (!strcmp (key, "priority")) {
@@ -4309,33 +4351,6 @@ handle_bridge_option (NMSetting *setting,
 			PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: unhandled bridge option '%s'", key);
 }
 
-static void
-handle_bridging_opts (NMSetting *setting,
-                      gboolean stp,
-                      const char *value,
-                      BridgeOptFunc func)
-{
-	char **items, **iter;
-
-	items = g_strsplit_set (value, " ", -1);
-	for (iter = items; iter && *iter; iter++) {
-		if (strlen (*iter)) {
-			char **keys, *key, *val;
-
-			keys = g_strsplit_set (*iter, "=", 2);
-			if (keys && *keys) {
-				key = *keys;
-				val = *(keys + 1);
-				if (val && strlen(key) && strlen(val))
-					func (setting, stp, key, val);
-			}
-
-			g_strfreev (keys);
-		}
-	}
-	g_strfreev (items);
-}
-
 static NMSetting *
 make_bridge_setting (shvarFile *ifcfg,
                      const char *file,
@@ -4391,7 +4406,8 @@ make_bridge_setting (shvarFile *ifcfg,
 
 	value = svGetValue (ifcfg, "BRIDGING_OPTS", FALSE);
 	if (value) {
-		handle_bridging_opts (NM_SETTING (s_bridge), stp, value, handle_bridge_option);
+		handle_ifcfg_rh_opts (NM_SETTING (s_bridge), value, handle_bridge_option,
+		                      GINT_TO_POINTER (stp));
 		g_free (value);
 	}
 
@@ -4430,7 +4446,7 @@ bridge_connection_from_ifcfg (const char *file,
 		g_object_unref (connection);
 		return NULL;
 	}
-	nm_connection_add_setting (connection, bridge_setting);	
+	nm_connection_add_setting (connection, bridge_setting);
 
 	if (!nm_connection_verify (connection, error)) {
 		g_object_unref (connection);
@@ -4442,9 +4458,9 @@ bridge_connection_from_ifcfg (const char *file,
 
 static void
 handle_bridge_port_option (NMSetting *setting,
-                           gboolean stp,
                            const char *key,
-                           const char *value)
+                           const char *value,
+                           gpointer data)
 {
 	guint32 u = 0;
 
@@ -4484,7 +4500,8 @@ make_bridge_port_setting (shvarFile *ifcfg)
 		s_port = nm_setting_bridge_port_new ();
 		value = svGetValue (ifcfg, "BRIDGING_OPTS", FALSE);
 		if (value)
-			handle_bridging_opts (s_port, FALSE, value, handle_bridge_port_option);
+			handle_ifcfg_rh_opts (NM_SETTING (s_port), value, handle_bridge_port_option, NULL);
+
 		g_free (value);
 	}
 
@@ -4515,7 +4532,7 @@ is_bond_device (const char *name, shvarFile *parsed)
 
 	if (svTrueValue (parsed, "BONDING_MASTER", FALSE))
 		return TRUE;
-	
+
 	/* XXX: Check for "bond[\d]+"? */
 
 	return FALSE;
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 14c4fb9f72..bc8c9740b9 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -12101,9 +12101,9 @@ test_read_bond_main (void)
 	        "bond-main", "failed to verify %s: DEVICE=%s does not match bond0",
 	        TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_interface_name (s_bond));
 
-	ASSERT (g_strcmp0 (nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MIIMON), "100") == 0,
-	        "bond-main", "failed to verify %s: miimon=%s does not match 100",
-	        TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MIIMON));
+	ASSERT (nm_setting_bond_get_miimon (s_bond) == 100,
+	        "bond-main", "failed to verify %s: miimon=%u does not match 100",
+	        TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_miimon (s_bond));
 
 	g_free (unmanaged);
 	g_free (keyfile);
diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c
index 6f302b4f7c..c83ee0a872 100644
--- a/src/settings/plugins/ifcfg-rh/writer.c
+++ b/src/settings/plugins/ifcfg-rh/writer.c
@@ -1250,7 +1250,8 @@ write_bonding_setting (NMConnection *connection, shvarFile *ifcfg, GError **erro
 {
 	NMSettingBond *s_bond;
 	const char *iface;
-	guint32 i, num_opts;
+	const char *const* kernel_names;
+	GString *str;
 
 	s_bond = nm_connection_get_setting_bond (connection);
 	if (!s_bond) {
@@ -1268,25 +1269,21 @@ write_bonding_setting (NMConnection *connection, shvarFile *ifcfg, GError **erro
 	svSetValue (ifcfg, "DEVICE", iface, FALSE);
 	svSetValue (ifcfg, "BONDING_OPTS", NULL, FALSE);
 
-	num_opts = nm_setting_bond_get_num_options (s_bond);
-	if (num_opts > 0) {
-		GString *str = g_string_sized_new (64);
-
-		for (i = 0; i < nm_setting_bond_get_num_options (s_bond); i++) {
-			const char *key, *value;
+	str = g_string_sized_new (64);
 
-			if (!nm_setting_bond_get_option (s_bond, i, &key, &value))
-				continue;
+	kernel_names = nm_setting_bond_get_kernel_names();
+	for (; *kernel_names; kernel_names++) {
+		if (nm_setting_bond_is_default (s_bond, *kernel_names, NULL)) {
+			const char *strval  = nm_setting_bond_get_string (s_bond, *kernel_names);
 
-			if (str->len)
-				g_string_append_c (str, ' ');
-
-			g_string_append_printf (str, "%s=%s", key, value);
+			/* FIXME: how to represent NULL values that are not default values? */
+			g_string_append_printf (str, "%s=%s ", *kernel_names, strval ? strval : "");
 		}
+	}
+	if (str->len > 0) {
+		g_string_truncate (str, str->len-1);
 
-		if (str->len)
-			svSetValue (ifcfg, "BONDING_OPTS", str->str, FALSE);
-
+		svSetValue (ifcfg, "BONDING_OPTS", str->str, FALSE);
 		g_string_free (str, TRUE);
 	}
 
diff --git a/src/settings/plugins/keyfile/reader.c b/src/settings/plugins/keyfile/reader.c
index 6949d1e5b1..d96a98dfbc 100644
--- a/src/settings/plugins/keyfile/reader.c
+++ b/src/settings/plugins/keyfile/reader.c
@@ -581,7 +581,7 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key)
 		}
 		if (NM_IS_SETTING_BOND (setting)) {
 			if (strcmp (*iter, NM_SETTING_BOND_INTERFACE_NAME))
-				nm_setting_bond_add_option (NM_SETTING_BOND (setting), *iter, value);
+				nm_setting_bond_set_string (NM_SETTING_BOND (setting), *iter, value);
 		}
 		g_free (value);
 	}