diff options
author | Antonio Cardace <acardace@redhat.com> | 2020-03-17 17:36:06 +0100 |
---|---|---|
committer | Antonio Cardace <acardace@redhat.com> | 2020-03-18 09:51:55 +0100 |
commit | ea0e7e7222089be7e5152005262a9c697354d6c0 (patch) | |
tree | 411addadfa6f30484a9c014f07f70b3da7187249 | |
parent | c8941911adf0074b480304d2aee506e339c3c69e (diff) | |
download | NetworkManager-ea0e7e7222089be7e5152005262a9c697354d6c0.tar.gz |
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
-rw-r--r-- | libnm-core/nm-setting-bond.c | 15 | ||||
-rw-r--r-- | libnm-core/tests/test-setting.c | 12 |
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 |