fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-10 21:00:23 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

cli: normalize profile when setting bond options "active_slave" or "primary"

Browse source 
"active_slave" is by now deprecated and became an alias for "primary".
If a profile specifies both properties, only "primary" is honored, without
failing validation (to not break existing behavior).
Maybe we should introduce a normalization for such cases. But normalize
might not do the right thing, if a profile currently has "primary" set,
and the user modifies it to set "active_slave" to a different value,
normalize would not know which setting was set first and remove
"active_slave" again.

In the past, nm_setting_bond_add_option() performed some simple
normalization, but this was dropped, because (such incompatible) settings
can also be created via the GObject property. Our C accessor function
should not be less flexible than other ways of creating a profile.

In the end, whenever a user (or a tool) creates a profile, the tool must
be aware of the semantics. E.g. setting an IP route without a suitable
IP address is unlike to make sense, the tool must understand what it's
doing. The same is true for the bond options. When a tool (or user) sets
the "active_slave" property, then it must clear out the redundant
information from the "primary" setting. There is no alternative to this
problem than having tools smart enough to understand what they are
doing.
This commit is contained in:
Thomas Haller 2020-09-10 09:27:59 +02:00
parent 58da09439a
commit 2e2e2f92df
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
1 changed files with 31 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
clients/common/nm-meta-setting-desc.c
View file

@ -2376,12 +2376,14 @@ _nm_meta_setting_bond_add_option (NMSetting *setting,
const char *value,
GError **error)
{
NMSettingBond *s_bond = NM_SETTING_BOND (setting);
gs_free char *tmp_value = NULL;
const char *val2;
char *p;
if ( !value
|| !value[0]) {
if (!nm_setting_bond_remove_option (NM_SETTING_BOND (setting), name)) {
if (!nm_setting_bond_remove_option (s_bond, name)) {
nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT,
_("failed to unset bond option \"%s\""),
name);
@ -2401,7 +2403,7 @@ _nm_meta_setting_bond_add_option (NMSetting *setting,
*p = ',';
}
if (!nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value)) {
if (!nm_setting_bond_add_option (s_bond, name, value)) {
nm_utils_error_set (error, NM_UTILS_ERROR_INVALID_ARGUMENT,
_("failed to set bond option \"%s\""),
name);
@ -2410,10 +2412,35 @@ _nm_meta_setting_bond_add_option (NMSetting *setting,
if (nm_streq (name, NM_SETTING_BOND_OPTION_ARP_INTERVAL)) {
if (_nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT, 0) > 0)
_nm_setting_bond_remove_options_miimon (NM_SETTING_BOND (setting));
_nm_setting_bond_remove_options_miimon (s_bond);
} else if (nm_streq (name, NM_SETTING_BOND_OPTION_MIIMON)) {
if (_nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT, 0) > 0)
_nm_setting_bond_remove_options_arp_interval (NM_SETTING_BOND (setting));
_nm_setting_bond_remove_options_arp_interval (s_bond);
} else if (nm_streq (name, NM_SETTING_BOND_OPTION_PRIMARY)) {
if ( (val2 = nm_setting_bond_get_option_by_name (s_bond,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE))
&& !nm_streq (val2, value)) {
/* "active_slave" option is deprecated and an alias for "primary". When
* setting "primary" to a different value, remove the deprecated "active_slave"
* setting.
*
* If we wouldn't do this, then the profile would work as requested, but ignoring
* the (redundant, differing) "active_slave" option. That is confusing, thus clean
* it up. */
nm_setting_bond_remove_option (s_bond,
NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
}
} else if (nm_streq (name, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE)) {
if ( (val2 = nm_setting_bond_get_option_by_name (s_bond,
NM_SETTING_BOND_OPTION_PRIMARY))
&& !nm_streq (val2, value)) {
/* "active_slave" is a deprecated alias for "primary". NetworkManager will ignore
* "active_slave" if "primary" is set, we thus need to coerce the primary option
* too. */
nm_setting_bond_add_option (s_bond,
NM_SETTING_BOND_OPTION_PRIMARY,
value);
}
}
return TRUE;