nm-setting-bond: don't take default values into account when comparing optionsac/fix_add_miimon

This solves a bug exposed by the following cmds:
$ nmcli c add type bond ifname bond0 con-name bond0
$ nmcli c modify bond0 +bond.options miimon=100
$ nmcli -f bond.options c show bond0
bond.options:                           mode=balance-rr

Here we just added the option 'miimon=100', but it doesn't get saved in
because nm_settings_connection_set_connection() which is responsible for
actually updating the connection compares the new connection with old
one and if and only if the 2 are different the update is carried out.

The bug is triggered because when comparing, if default values are taken into
account, then having 'miimon=100' or not having it it's essentially the
same for compare(). While this doesn't cause a bond to have a wrong
setting when activated it's wrong from a user experience point of view
and thus must be fixed.

When this patch is applied, the above
commands will give the following results:
$ nmcli c add type bond ifname bond0 con-name bond0
$ nmcli c modify bond0 +bond.options miimon=100
$ nmcli -f bond.options c show bond0
bond.options:                           mode=balance-rr,miimon=100

Fix unit tests and also add a new case covering this bug.

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

2 files changed, 9 insertions, 18 deletions

diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index f53f2c8cf2..652d55487b 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -1033,7 +1033,7 @@ options_equal_asym (NMSettingBond *s_bond,
                     NMSettingCompareFlags flags)
 {
 	GHashTableIter iter;
-	const char *key, *value, *value2;
+	const char *key, *value;
 
 	g_hash_table_iter_init (&iter, NM_SETTING_BOND_GET_PRIVATE (s_bond)->options);
 	while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) {
@@ -1048,18 +1048,7 @@ options_equal_asym (NMSettingBond *s_bond,
 				continue;
 		}
 
-		value2 = _bond_get_option (s_bond2, key);
-
-		if (!value2) {
-			if (nm_streq (key, "num_grat_arp"))
-				value2 = _bond_get_option (s_bond2, "num_unsol_na");
-			else if (nm_streq (key, "num_unsol_na"))
-				value2 = _bond_get_option (s_bond2, "num_grat_arp");
-		}
-
-		if (!value2)
-			value2 = _bond_get_option_default (s_bond2, key);
-		if (!nm_streq (value, value2))
+		if (!nm_streq0 (value, _bond_get_option (s_bond2, key)))
 			return FALSE;
 	}
 
diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c
index 71f484bab5..2dc76f1000 100644
--- a/libnm-core/tests/test-setting.c
+++ b/libnm-core/tests/test-setting.c
@@ -641,21 +641,23 @@ test_bond_compare (void)
 	                           ((const char *[]){ "mode", "balance-rr", "miimon", "1", NULL }),
 	                           ((const char *[]){ "mode", "balance-rr", "miimon", "2", NULL }));
 
-	/* ignore default values */
-	test_bond_compare_options (TRUE,
+	test_bond_compare_options (FALSE,
 	                           ((const char *[]){ "miimon", "1", NULL }),
 	                           ((const char *[]){ "miimon", "1", "updelay", "0", NULL }));
 
-	/* special handling of num_grat_arp, num_unsol_na */
 	test_bond_compare_options (FALSE,
 	                           ((const char *[]){ "num_grat_arp", "2", NULL }),
 	                           ((const char *[]){ "num_grat_arp", "1", NULL }));
-	test_bond_compare_options (TRUE,
+	test_bond_compare_options (FALSE,
 	                           ((const char *[]){ "num_grat_arp", "3", NULL }),
 	                           ((const char *[]){ "num_unsol_na", "3", NULL }));
-	test_bond_compare_options (TRUE,
+	test_bond_compare_options (FALSE,
 	                           ((const char *[]){ "num_grat_arp", "4", NULL }),
 	                           ((const char *[]){ "num_unsol_na", "4", "num_grat_arp", "4", NULL }));
+
+	test_bond_compare_options (FALSE,
+	                           ((const char *[]){ "mode", "balance-rr", "miimon", "100", NULL }),
+	                           ((const char *[]){ "mode", "balance-rr", NULL }));
 }
 
 static void