bond: merge branch 'jk/numeric-bond-modes-nm-1-0' into nm-1-0danw/nm-1-0

8 files changed, 150 insertions, 13 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 907713bd08..239079e3b1 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -723,6 +723,27 @@ _normalize_infiniband_mtu (NMConnection *self, GHashTable *parameters)
 	return FALSE;
 }
 
+static gboolean
+_normalize_bond_mode (NMConnection *self, GHashTable *parameters)
+{
+	NMSettingBond *s_bond = nm_connection_get_setting_bond (self);
+
+	/* Convert mode from numeric to string notation */
+	if (s_bond) {
+		const char *mode = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE);
+		int mode_int = nm_utils_bond_mode_string_to_int (mode);
+
+		if (mode_int != -1) {
+			const char *mode_new = nm_utils_bond_mode_int_to_string (mode_int);
+			if (g_strcmp0 (mode_new, mode) != 0) {
+				nm_setting_bond_add_option (s_bond, NM_SETTING_BOND_OPTION_MODE, mode_new);
+				return TRUE;
+			}
+		}
+	}
+	return FALSE;
+}
+
 /**
  * nm_connection_verify:
  * @connection: the #NMConnection to verify
@@ -936,6 +957,7 @@ nm_connection_normalize (NMConnection *connection,
 	was_modified |= _normalize_connection_slave_type (connection);
 	was_modified |= _normalize_ip_config (connection, parameters);
 	was_modified |= _normalize_infiniband_mtu (connection, parameters);
+	was_modified |= _normalize_bond_mode (connection, parameters);
 
 	/* Verify anew. */
 	success = _nm_connection_verify (connection, error);
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 5af90267a5..9f4b33e342 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -132,4 +132,8 @@ GByteArray *nm_utils_rsa_key_encrypt (const guint8 *data,
                                       char **out_password,
                                       GError **error);
 
+/* These are public API in NM 1.2, but private on nm-1-0. */
+int nm_utils_bond_mode_string_to_int (const char *mode);
+const char *nm_utils_bond_mode_int_to_string (int mode);
+
 #endif
diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index bb05312539..5441a090da 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -440,15 +440,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
 	GHashTableIter iter;
 	const char *key, *value;
-	const char *valid_modes[] = { "balance-rr",
-	                              "active-backup",
-	                              "balance-xor",
-	                              "broadcast",
-	                              "802.3ad",
-	                              "balance-tlb",
-	                              "balance-alb",
-	                              NULL };
-	int miimon = 0, arp_interval = 0;
+	int mode, miimon = 0, arp_interval = 0;
+	const char *mode_orig, *mode_new;
 	const char *arp_ip_target = NULL;
 	const char *lacp_rate;
 	const char *primary;
@@ -484,7 +477,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
 	}
 
-	value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_MODE);
+	/* Verify bond mode */
+	mode_orig = value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_MODE);
 	if (!value) {
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
@@ -494,7 +488,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
 		return FALSE;
 	}
-	if (!_nm_utils_string_in_list (value, valid_modes)) {
+	mode = nm_utils_bond_mode_string_to_int (value);
+	if (mode == -1) {
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
 		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -503,6 +498,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
 		return FALSE;
 	}
+	mode_new = value = nm_utils_bond_mode_int_to_string (mode);
 
 	/* Make sure mode is compatible with other settings */
 	if (   strcmp (value, "balance-alb") == 0
@@ -645,7 +641,22 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		return FALSE;
 	}
 
-	return _nm_connection_verify_required_interface_name (connection, error);
+	if (!_nm_connection_verify_required_interface_name (connection, error))
+		return FALSE;
+
+	/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
+
+	if (g_strcmp0 (mode_orig, mode_new) != 0) {
+		g_set_error (error,
+		             NM_CONNECTION_ERROR,
+		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		             _("'%s' option should be string"),
+		             NM_SETTING_BOND_OPTION_MODE);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
+		return NM_SETTING_VERIFY_NORMALIZABLE;
+	}
+
+	return TRUE;
 }
 
 static void
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index b3198f4c0b..f55a7dac03 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -3213,3 +3213,64 @@ nm_utils_check_virtual_device_compatibility (GType virtual_type, GType other_typ
 		return FALSE;
 	}
 }
+
+typedef struct {
+	const char *str;
+	const char *num;
+} BondMode;
+
+static BondMode bond_mode_table[] = {
+	[0] = { "balance-rr",    "0" },
+	[1] = { "active-backup", "1" },
+	[2] = { "balance-xor",   "2" },
+	[3] = { "broadcast",     "3" },
+	[4] = { "802.3ad",       "4" },
+	[5] = { "balance-tlb",   "5" },
+	[6] = { "balance-alb",   "6" },
+};
+
+/**
+ * nm_utils_bond_mode_int_to_string:
+ * @mode: bonding mode as a numeric value
+ *
+ * Convert bonding mode from integer value to descriptive name.
+ * See https://www.kernel.org/doc/Documentation/networking/bonding.txt for
+ * available modes.
+ *
+ * Returns: bonding mode string, or NULL on error
+*/
+
+const char *
+nm_utils_bond_mode_int_to_string (int mode)
+{
+	if (mode >= 0 && mode < G_N_ELEMENTS (bond_mode_table))
+		return bond_mode_table[mode].str;
+	return NULL;
+}
+
+/**
+ * nm_utils_bond_mode_string_to_int:
+ * @mode: bonding mode as string
+ *
+ * Convert bonding mode from string representation to numeric value.
+ * See https://www.kernel.org/doc/Documentation/networking/bonding.txt for
+ * available modes.
+ * The @mode string can be either a descriptive name or a number (as string).
+ *
+ * Returns: numeric bond mode, or -1 on error
+*/
+int
+nm_utils_bond_mode_string_to_int (const char *mode)
+{
+	int i;
+
+	if (!mode || !*mode)
+		return -1;
+
+	for (i = 0; i < G_N_ELEMENTS (bond_mode_table); i++) {
+		if (   strcmp (mode, bond_mode_table[i].str) == 0
+		    || strcmp (mode, bond_mode_table[i].num) == 0)
+			return i;
+	}
+	return -1;
+}
diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index 63b4914a47..c43439cb37 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -3858,7 +3858,8 @@ handle_bond_option (NMSettingBond *s_bond,
 	}
 
 	if (!nm_setting_bond_add_option (s_bond, key, sanitized ? sanitized : value))
-		PARSE_WARNING ("invalid bonding option '%s'", key);
+		PARSE_WARNING ("invalid bonding option '%s' = %s",
+		               key, sanitized ? sanitized : value);
 	g_free (sanitized);
 }
 
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
index 509b3b3cb1..a20a78d9a5 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
@@ -108,6 +108,7 @@ EXTRA_DIST = \
 	ifcfg-test-bond-main \
 	ifcfg-test-bond-slave \
 	ifcfg-test-bond-slave-ib \
+	ifcfg-test-bond-mode-numeric \
 	ifcfg-test-dcb \
 	ifcfg-test-dcb-default-app-priorities \
 	ifcfg-test-dcb-bad-booleans \
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bond-mode-numeric b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bond-mode-numeric
new file mode 100644
index 0000000000..9989f58b97
--- /dev/null
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bond-mode-numeric
@@ -0,0 +1,5 @@
+DEVICE=bond0
+ONBOOT=no
+TYPE=Bond
+BOOTPROTO=dhcp
+BONDING_OPTS="mode=4 miimon=100"
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 a2f328f336..9864d251af 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -11522,6 +11522,37 @@ test_write_bond_slave_ib (void)
 	g_object_unref (reread);
 }
 
+static void
+test_read_bond_opts_mode_numeric (void)
+{
+	NMConnection *connection;
+	NMSettingConnection *s_con;
+	NMSettingBond *s_bond;
+	gboolean success;
+	GError *error = NULL;
+
+	connection = connection_from_file_test (TEST_IFCFG_DIR "/network-scripts/ifcfg-test-bond-mode-numeric",
+	                                        NULL, TYPE_ETHERNET, NULL, &error);
+	g_assert_no_error (error);
+	g_assert (connection);
+
+	success = nm_connection_verify (connection, &error);
+	g_assert_no_error (error);
+	g_assert (success);
+
+	g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "bond0");
+
+	s_con = nm_connection_get_setting_connection (connection);
+	g_assert (s_con);
+	g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_BOND_SETTING_NAME);
+
+	s_bond = nm_connection_get_setting_bond (connection);
+	g_assert (s_bond);
+	g_assert_cmpstr (nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE), ==, "802.3ad");
+
+	g_object_unref (connection);
+}
+
 #define DCB_ALL_FLAGS (NM_SETTING_DCB_FLAG_ENABLE | \
                        NM_SETTING_DCB_FLAG_ADVERTISE | \
                        NM_SETTING_DCB_FLAG_WILLING)
@@ -12606,6 +12637,7 @@ int main (int argc, char **argv)
 	test_write_bond_main ();
 	test_write_bond_slave ();
 	test_write_bond_slave_ib ();
+	g_test_add_func (TPATH "bond/bonding-opts-numeric-mode", test_read_bond_opts_mode_numeric);
 
 	/* bridging */
 	test_read_bridge_main ();