libnm-core: team: rework defaults management on runner properties

till now when no explicit value was set on a property, the default value
for that property was returned, also if the property was not applicable
to the selected runner.
Fix this, showing default values for properties only when relevant and
showing instead -1 or null when the property is not relevant for the
selected runner.
Moreover, reset all the properties but the link-watchers when the team.runner
is changed: this is required to clean up the properties unrelated to the
new runner and start with the runner-specific defaults.

2 files changed, 218 insertions, 72 deletions

diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c
index 1f9cfbe4d4..c907fb5ec0 100644
--- a/libnm-core/nm-setting-team.c
+++ b/libnm-core/nm-setting-team.c
@@ -623,15 +623,12 @@ static const _NMUtilsTeamPropertyKeys _prop_to_keys[LAST_PROP] = {
 	[PROP_RUNNER_HWADDR_POLICY] =        { "runner", "hwaddr_policy", NULL, 0 },
 	[PROP_RUNNER_TX_HASH] =              { "runner", "tx_hash", NULL, 0 },
 	[PROP_RUNNER_TX_BALANCER] =          { "runner", "tx_balancer", "name", 0 },
-	[PROP_RUNNER_TX_BALANCER_INTERVAL] = { "runner", "tx_balancer", "balancing_interval",
-	                                       NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL_DEFAULT },
+	[PROP_RUNNER_TX_BALANCER_INTERVAL] = { "runner", "tx_balancer", "balancing_interval", -1 },
 	[PROP_RUNNER_ACTIVE] =               { "runner", "active", NULL, 0 },
 	[PROP_RUNNER_FAST_RATE] =            { "runner", "fast_rate", NULL, 0 },
-	[PROP_RUNNER_SYS_PRIO] =             { "runner", "sys_prio", NULL,
-	                                       NM_SETTING_TEAM_RUNNER_SYS_PRIO_DEFAULT },
-	[PROP_RUNNER_MIN_PORTS] =            { "runner", "min_ports", NULL, 0 },
-	[PROP_RUNNER_AGG_SELECT_POLICY] =    { "runner", "agg_select_policy", NULL,
-	                                       {.default_str = NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY_DEFAULT} },
+	[PROP_RUNNER_SYS_PRIO] =             { "runner", "sys_prio", NULL, -1 },
+	[PROP_RUNNER_MIN_PORTS] =            { "runner", "min_ports", NULL, -1 },
+	[PROP_RUNNER_AGG_SELECT_POLICY] =    { "runner", "agg_select_policy", NULL, 0 },
 	[PROP_LINK_WATCHERS] =               { "link_watch", NULL, NULL, 0 }
 };
 
@@ -1289,8 +1286,9 @@ nm_setting_team_init (NMSettingTeam *setting)
 	NMSettingTeamPrivate *priv = NM_SETTING_TEAM_GET_PRIVATE (setting);
 
 	priv->runner = g_strdup (NM_SETTING_TEAM_RUNNER_ROUNDROBIN);
-	priv->runner_sys_prio = NM_SETTING_TEAM_RUNNER_SYS_PRIO_DEFAULT;
-	priv->runner_tx_balancer_interval = NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL_DEFAULT;
+	priv->runner_tx_balancer_interval = -1;
+	priv->runner_sys_prio = -1;
+	priv->runner_min_ports = -1;
 	priv->link_watchers = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_team_link_watcher_unref);
 }
 
@@ -1376,51 +1374,45 @@ set_property (GObject *object, guint prop_id,
 		if (priv->notify_peers_count == g_value_get_int (value))
 			break;
 		priv->notify_peers_count = g_value_get_int (value);
-		if (priv->notify_peers_count)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_NOTIFY_PEERS_INTERVAL:
 		if (priv->notify_peers_interval == g_value_get_int (value))
 			break;
 		priv->notify_peers_interval = g_value_get_int (value);
-		if (priv->notify_peers_interval)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_MCAST_REJOIN_COUNT:
 		if (priv->mcast_rejoin_count == g_value_get_int (value))
 			break;
 		priv->mcast_rejoin_count = g_value_get_int (value);
-		if (priv->mcast_rejoin_count)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_MCAST_REJOIN_INTERVAL:
 		if (priv->mcast_rejoin_interval == g_value_get_int (value))
 			break;
 		priv->mcast_rejoin_interval = g_value_get_int (value);
-		if (priv->mcast_rejoin_interval)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER:
+		if (   !g_value_get_string (value)
+		    || nm_streq (priv->runner, g_value_get_string (value)))
+			break;
 		g_free (priv->runner);
 		priv->runner = g_value_dup_string (value);
-		if (   priv->runner
-		    && !nm_streq (priv->runner,
-		                  NM_SETTING_TEAM_RUNNER_DEFAULT))
-			align_value = value;
-		align_config = TRUE;
+		_nm_utils_json_append_gvalue (&priv->config, _prop_to_keys[prop_id], value);
+		_align_team_properties (setting);
 		break;
 	case PROP_RUNNER_HWADDR_POLICY:
+		if (nm_streq0 (priv->runner_hwaddr_policy, g_value_get_string (value)))
+			break;
 		g_free (priv->runner_hwaddr_policy);
 		priv->runner_hwaddr_policy = g_value_dup_string (value);
-		if (   priv->runner_hwaddr_policy
-		    && !nm_streq (priv->runner_hwaddr_policy,
-		                  NM_SETTING_TEAM_RUNNER_HWADDR_POLICY_SAME_ALL)) {
-			align_value = value;
-		}
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_TX_HASH:
@@ -1435,60 +1427,54 @@ set_property (GObject *object, guint prop_id,
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_TX_BALANCER:
+		if (nm_streq0 (priv->runner_tx_balancer, g_value_get_string (value)))
+			break;
 		g_free (priv->runner_tx_balancer);
 		priv->runner_tx_balancer = g_value_dup_string (value);
-		if (priv->runner_tx_balancer)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_TX_BALANCER_INTERVAL:
 		if (priv->runner_tx_balancer_interval == g_value_get_int (value))
 			break;
 		priv->runner_tx_balancer_interval = g_value_get_int (value);
-		if (priv->runner_tx_balancer_interval !=
-		    NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL_DEFAULT)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_ACTIVE:
 		if (priv->runner_active == g_value_get_boolean (value))
 			break;
 		priv->runner_active = g_value_get_boolean (value);
-		if (priv->runner_active)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_FAST_RATE:
 		if (priv->runner_fast_rate == g_value_get_boolean (value))
 			break;
 		priv->runner_fast_rate = g_value_get_boolean (value);
-		if (priv->runner_fast_rate)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_SYS_PRIO:
 		if (priv->runner_sys_prio == g_value_get_int (value))
 			break;
 		priv->runner_sys_prio = g_value_get_int (value);
-		if (priv->runner_sys_prio != NM_SETTING_TEAM_RUNNER_SYS_PRIO_DEFAULT)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_MIN_PORTS:
 		if (priv->runner_min_ports == g_value_get_int (value))
 			break;
 		priv->runner_min_ports = g_value_get_int (value);
-		if (priv->runner_min_ports)
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_RUNNER_AGG_SELECT_POLICY:
+		if (nm_streq0 (priv->runner_agg_select_policy, g_value_get_string (value)))
+			break;
 		g_free (priv->runner_agg_select_policy);
 		priv->runner_agg_select_policy = g_value_dup_string (value);
-		if (   priv->runner_agg_select_policy
-		    && !nm_streq (priv->runner_agg_select_policy,
-		                  NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY_LACP_PRIO))
-			align_value = value;
+		align_value = value;
 		align_config = TRUE;
 		break;
 	case PROP_LINK_WATCHERS:
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 8ca6ac74c7..da297f7a2f 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -4396,10 +4396,13 @@ _json_del_object (json_t *json,
 	return TRUE;
 }
 
-/* Adds in place to json the defaults for missing properties */
+/* Adds in place to json the defaults for missing properties;
+ * the "add_implicit" allows to add to the json also the default
+ * values used but not shown with teamdctl */
 static void
 _json_team_add_defaults (json_t *json,
-                         gboolean port_config)
+                         gboolean port_config,
+                         gboolean add_implicit)
 {
 	json_t *json_element;
 	const char *runner = NULL;
@@ -4407,38 +4410,193 @@ _json_team_add_defaults (json_t *json,
 	if (port_config) {
 		_json_add_object (json, "link_watch", "name", NULL,
 		                  json_string (NM_TEAM_LINK_WATCHER_ETHTOOL));
+		return;
+	}
+
+	/* Retrieve runner or add default one */
+	json_element = json_object_get (json, "runner");
+	if (json_element) {
+		runner = json_string_value (json_object_get (json_element, "name"));
 	} else {
-		/* Retrieve runner or add default one */
-		json_element = json_object_get (json, "runner");
-		if (json_element) {
-			runner = json_string_value (json_object_get (json_element, "name"));
-		} else {
-			json_element = json_object ();
-			json_object_set_new (json, "runner", json_element);
-		}
-		if (!runner) {
-			runner = NM_SETTING_TEAM_RUNNER_DEFAULT;
-			json_object_set_new (json_element, "name", json_string (runner));
-		}
+		json_element = json_object ();
+		json_object_set_new (json, "runner", json_element);
+	}
+	if (!runner) {
+		runner = NM_SETTING_TEAM_RUNNER_DEFAULT;
+		json_object_set_new (json_element, "name", json_string (runner));
+	}
 
 
-		if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_ACTIVEBACKUP)) {
-			_json_add_object (json, "notify_peers", "count", NULL,
-			                  json_integer (NM_SETTING_TEAM_NOTIFY_PEERS_COUNT_ACTIVEBACKUP_DEFAULT));
-			_json_add_object (json, "mcast_rejoin", "count", NULL,
-			                  json_integer (NM_SETTING_TEAM_NOTIFY_MCAST_COUNT_ACTIVEBACKUP_DEFAULT));
-		} else if (   nm_streq (runner, NM_SETTING_TEAM_RUNNER_LOADBALANCE)
-		           || nm_streq (runner, NM_SETTING_TEAM_RUNNER_LACP)) {
-			json_element = json_array ();
-			json_array_append_new (json_element, json_string ("eth"));
-			json_array_append_new (json_element, json_string ("ipv4"));
-			json_array_append_new (json_element, json_string ("ipv6"));
-			_json_add_object (json, "runner", "tx_hash", NULL, json_element);
+	if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_ACTIVEBACKUP)) {
+		_json_add_object (json, "notify_peers", "count", NULL,
+				  json_integer (NM_SETTING_TEAM_NOTIFY_PEERS_COUNT_ACTIVEBACKUP_DEFAULT));
+		_json_add_object (json, "mcast_rejoin", "count", NULL,
+				  json_integer (NM_SETTING_TEAM_NOTIFY_MCAST_COUNT_ACTIVEBACKUP_DEFAULT));
+	} else if (   nm_streq (runner, NM_SETTING_TEAM_RUNNER_LOADBALANCE)
+		   || nm_streq (runner, NM_SETTING_TEAM_RUNNER_LACP)) {
+		json_element = json_array ();
+		json_array_append_new (json_element, json_string ("eth"));
+		json_array_append_new (json_element, json_string ("ipv4"));
+		json_array_append_new (json_element, json_string ("ipv6"));
+		_json_add_object (json, "runner", "tx_hash", NULL, json_element);
+	}
+
+	if (!add_implicit)
+		return;
+
+	if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_ACTIVEBACKUP))
+		_json_add_object (json, "runner", "hwaddr_policy", NULL, json_string ("same_all"));
+	else if (NM_IN_STRSET (runner,
+	                       NM_SETTING_TEAM_RUNNER_LOADBALANCE,
+	                       NM_SETTING_TEAM_RUNNER_LACP)) {
+		_json_add_object (json, "runner", "tx_balancer", "balancing_interval",
+		                  json_integer (NM_SETTING_TEAM_RUNNER_TX_BALANCER_INTERVAL_DEFAULT));
+		if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_LACP)) {
+			_json_add_object (json, "runner", "active", NULL, json_boolean (TRUE));
+			_json_add_object (json, "runner", "sys_prio", NULL,
+			                  json_integer (NM_SETTING_TEAM_RUNNER_SYS_PRIO_DEFAULT));
+			_json_add_object (json, "runner", "min_ports", NULL, json_integer (0));
+			_json_add_object (json, "runner", "agg_select_policy", NULL,
+			                  json_string (NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY_DEFAULT));
 		}
 	}
 }
 
+static json_t *
+_json_find_object (json_t *json,
+                   const char *key1,
+                   const char *key2,
+                   const char *key3)
+{
+	json_t *json_element;
+
+	if (!key1)
+		return NULL;
+	json_element = json_object_get (json, key1);
+	if (!key2 || !json_element)
+		return json_element;
+
+	json_element = json_object_get (json_element, key2);
+	if (!key3 || !json_element)
+		return json_element;
 
+	json_element = json_object_get (json_element, key3);
+	return json_element;
+}
+
+static inline void
+_json_delete_object_on_int_match (json_t *json,
+                                  const char *key1,
+                                  const char *key2,
+                                  const char *key3,
+                                  int val)
+{
+	json_t *json_element;
+
+	json_element = _json_find_object (json, key1, key2, key3);
+	if (!json_element || !json_is_integer (json_element))
+		return;
+	if (json_integer_value (json_element) == val)
+		_json_del_object (json, key1, key2, key3);
+}
+
+static inline void
+_json_delete_object_on_bool_match (json_t *json,
+                                   const char *key1,
+                                   const char *key2,
+                                   const char *key3,
+                                   gboolean val)
+{
+	json_t *json_element;
+
+	json_element = _json_find_object (json, key1, key2, key3);
+	if (!json_element || !json_is_boolean (json_element))
+		return;
+	if (json_boolean_value (json_element) == val)
+		_json_del_object (json, key1, key2, key3);
+}
+
+static inline void
+_json_delete_object_on_string_match (json_t *json,
+                                     const char *key1,
+                                     const char *key2,
+                                     const char *key3,
+                                     const char *val)
+{
+	json_t *json_element;
+
+	json_element = _json_find_object (json, key1, key2, key3);
+	if (!json_element || !json_is_string (json_element))
+		return;
+	if (nm_streq0 (json_string_value (json_element), val))
+		_json_del_object (json, key1, key2, key3);
+}
+
+static void
+_json_team_normalize_defaults (json_t *json, gboolean reset)
+{
+	json_t *json_element;
+	const char *runner = NM_SETTING_TEAM_RUNNER_DEFAULT;
+	int notify_peers_count = 0, notify_peers_interval = 0;
+	int mcast_rejoin_count = 0, mcast_rejoin_interval = 0;
+	int runner_tx_balancer_interval = -1;
+	gboolean runner_active = FALSE, runner_fast_rate = FALSE;
+	int runner_sys_prio = -1, runner_min_ports = -1;
+
+	json_element = _json_find_object (json, "runner", "name", NULL);
+	if (json_element) {
+		runner = json_string_value (json_element);
+		_json_delete_object_on_string_match (json, "runner", "name", NULL,
+		                                     NM_SETTING_TEAM_RUNNER_DEFAULT);
+	}
+
+	/* the runner changed: clear all the properties. Then team.config will be saved
+	 * and reloaded triggering the reset of the values through _nm_utils_team_config_get
+	 */
+	if (reset) {
+		_json_del_object (json, "notify_peers", "count", NULL);
+		_json_del_object (json, "notify_peers", "interval", NULL);
+		_json_del_object (json, "mcast_rejoin", "count", NULL);
+		_json_del_object (json, "mcast_rejoin", "interval", NULL);
+		_json_del_object (json, "runner", "hwaddr_policy", NULL);
+		_json_del_object (json, "runner", "tx_hash", NULL);
+		_json_del_object (json, "runner", "tx_balancer", "name");
+		_json_del_object (json, "runner", "tx_balancer", "balancing_interval");
+		_json_del_object (json, "runner", "active", NULL);
+		_json_del_object (json, "runner", "fast_rate", NULL);
+		_json_del_object (json, "runner", "sys_prio", NULL);
+		_json_del_object (json, "runner", "min_ports", NULL);
+		_json_del_object (json, "runner", "agg_select_policy", NULL);
+		return;
+	}
+
+	if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_ACTIVEBACKUP)) {
+		notify_peers_count = 1;
+		mcast_rejoin_count = 1;
+		_json_delete_object_on_string_match (json, "runner", "hwaddr_policy", NULL,
+		                                     NM_SETTING_TEAM_RUNNER_HWADDR_POLICY_DEFAULT);
+	} else if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_LACP)) {
+		runner_tx_balancer_interval = 50;
+		runner_active = TRUE;
+		runner_sys_prio = 255;
+		runner_min_ports = 0;
+		_json_delete_object_on_string_match (json, "runner", "agg_select_policy", NULL,
+		                                     NM_SETTING_TEAM_RUNNER_AGG_SELECT_POLICY_DEFAULT);
+	} else if (nm_streq (runner, NM_SETTING_TEAM_RUNNER_LOADBALANCE))
+		runner_tx_balancer_interval = 50;
+
+	_json_delete_object_on_int_match (json, "notify_peers", "count", NULL, notify_peers_count);
+	_json_delete_object_on_int_match (json, "notify_peers", "interval", NULL, notify_peers_interval);
+	_json_delete_object_on_int_match (json, "mcast_rejoin", "count", NULL, mcast_rejoin_count);
+	_json_delete_object_on_int_match (json, "macst_rejoin", "interval", NULL, mcast_rejoin_interval);
+	_json_delete_object_on_int_match (json, "runner", "tx_balancer", "balancing_interval",
+	                                  runner_tx_balancer_interval);
+	_json_delete_object_on_int_match (json, "runner", "sys_prio", NULL, runner_sys_prio);
+	_json_delete_object_on_int_match (json, "runner", "min_ports", NULL, runner_min_ports);
+	_json_delete_object_on_bool_match (json, "runner", "active", NULL, runner_active);
+	_json_delete_object_on_bool_match (json, "runner", "active", NULL, runner_active);
+	_json_delete_object_on_bool_match (json, "runner", "fast_rate", NULL, runner_fast_rate);
+}
 
 static NMTeamLinkWatcher *
 _nm_utils_team_link_watcher_from_json (json_t *json_element)
@@ -4638,7 +4796,7 @@ _nm_utils_team_config_equal (const char *conf1,
 	 * on the configuration type.
 	 */
 	for (i = 0, json = json1; i < 2; i++, json = json2)
-		_json_team_add_defaults (json, port_config);
+		_json_team_add_defaults (json, port_config, FALSE);
 
 	/* Only consider a given subset of nodes, others can change depending on
 	 * current state */
@@ -4695,7 +4853,7 @@ _nm_utils_team_config_get (const char *conf,
 	 * fine to show the default value only if explicitly set.
 	 */
 	if (!port_config)
-		_json_team_add_defaults (json, port_config);
+		_json_team_add_defaults (json, port_config, TRUE);
 
 	/* Now search the property to retrieve */
 	json_element = json_object_get (json, key);
@@ -4882,6 +5040,8 @@ _nm_utils_team_config_set (char **conf,
 
 done:
 	if (updated) {
+		_json_team_normalize_defaults (json, (   nm_streq0 (key, "runner")
+		                                      && nm_streq0 (key2, "name")));
 		g_free (*conf);
 		*conf = json_dumps (json, JSON_PRESERVE_ORDER);
 		/* Don't save an empty config */