nm-setting-bond: don't take default values into account when comparing options

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
This commit is contained in:
Antonio Cardace 2020-03-17 17:36:06 +01:00
parent fcbef9b6d3
commit 97d3f1b4b9
2 changed files with 9 additions and 18 deletions

View file

@ -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;
}

View file

@ -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