libnm-core: normalize slave-type and slave-settings of connections

Some NMSettingConnection:slave-type types require a matching slave #NMSetting.
Add normalization of either the 'slave-type' property or the slave-setting.

Also be more strict in NMSettingConnection:verify() to enforce an
existing slave-setting depending on the slave-type.

Signed-off-by: Thomas Haller <thaller@redhat.com>

10 files changed, 273 insertions, 15 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 549dc6527d..709fc20a64 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -25,7 +25,7 @@
 #include <dbus/dbus-glib.h>
 #include <string.h>
 #include "nm-connection.h"
-#include "nm-utils.h"
+#include "nm-utils-internal.h"
 #include "nm-dbus-glib-types.h"
 #include "nm-setting-private.h"
 
@@ -581,6 +581,46 @@ _normalize_virtual_iface_name (NMConnection *self)
 }
 
 static gboolean
+_normalize_connection_slave_type (NMConnection *self)
+{
+	NMSettingConnection *s_con = nm_connection_get_setting_connection (self);
+	const char *slave_type, *port_type;
+	GSList *all_settings;
+
+	if (!s_con)
+		return FALSE;
+	if (!nm_setting_connection_get_master (s_con))
+		return FALSE;
+
+	slave_type = nm_setting_connection_get_slave_type (s_con);
+	if (slave_type) {
+		if (   _nm_setting_slave_type_is_valid (slave_type, &port_type)
+		    && port_type) {
+			NMSetting *s_port;
+
+			s_port = nm_connection_get_setting_by_name (self, port_type);
+			if (!s_port) {
+				GType p_type = nm_setting_lookup_type (port_type);
+
+				g_return_val_if_fail (p_type, FALSE);
+				nm_connection_add_setting (self, g_object_new (p_type, NULL));
+				return TRUE;
+			}
+		}
+	} else {
+		all_settings = _nm_utils_hash_values_to_slist (NM_CONNECTION_GET_PRIVATE (self)->settings);
+
+		if ((slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, NULL))) {
+			g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, NULL);
+			g_slist_free (all_settings);
+			return TRUE;
+		}
+		g_slist_free (all_settings);
+	}
+	return FALSE;
+}
+
+static gboolean
 _normalize_ip_config (NMConnection *self, GHashTable *parameters)
 {
 	NMSettingConnection *s_con = nm_connection_get_setting_connection (self);
@@ -837,6 +877,7 @@ nm_connection_normalize (NMConnection *connection,
 	 * errors, because in that case we rather fail without touching the settings. */
 
 	was_modified |= _normalize_virtual_iface_name (connection);
+	was_modified |= _normalize_connection_slave_type (connection);
 	was_modified |= _normalize_ip_config (connection, parameters);
 
 	/* Verify anew. */
diff --git a/libnm-core/nm-setting-bridge-port.c b/libnm-core/nm-setting-bridge-port.c
index 243f4668dd..0bc1a1c828 100644
--- a/libnm-core/nm-setting-bridge-port.c
+++ b/libnm-core/nm-setting-bridge-port.c
@@ -159,6 +159,32 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
+
+	if (all_settings) {
+		NMSettingConnection *s_con;
+		const char *slave_type;
+
+		s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
+		                                                                  NM_SETTING_CONNECTION_SETTING_NAME,
+		                                                                  error, NULL, NULL));
+		if (!s_con)
+			 return FALSE;
+
+		slave_type = nm_setting_connection_get_slave_type (s_con);
+		if (   slave_type
+		    && strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)) {
+			g_set_error (error,
+			             NM_SETTING_CONNECTION_ERROR,
+			             NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
+			             _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"),
+			             NM_SETTING_BRIDGE_PORT_SETTING_NAME,
+			             NM_SETTING_BRIDGE_SETTING_NAME,
+			             slave_type);
+			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
+			return FALSE;
+		}
+	}
+
 	return TRUE;
 }
 
diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index c359298250..fdfe5f7214 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/libnm-core/nm-setting-connection.c
@@ -741,7 +741,11 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 {
 	NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting);
 	gboolean is_slave;
+	const char *slave_setting_type = NULL;
 	GSList *iter;
+	const char *normerr_slave_setting_type = NULL;
+	const char *normerr_missing_slave_type = NULL;
+	const char *normerr_missing_slave_type_port = NULL;
 
 	if (!priv->id) {
 		g_set_error_literal (error,
@@ -862,10 +866,9 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		}
 	}
 
-	is_slave = (   priv->slave_type
-	            && (   !strcmp (priv->slave_type, NM_SETTING_BOND_SETTING_NAME)
-	                || !strcmp (priv->slave_type, NM_SETTING_BRIDGE_SETTING_NAME)
-	                || !strcmp (priv->slave_type, NM_SETTING_TEAM_SETTING_NAME)));
+	is_slave = FALSE;
+	if (priv->slave_type)
+		is_slave = _nm_setting_slave_type_is_valid (priv->slave_type, &slave_setting_type);
 
 	if (priv->slave_type && !is_slave) {
 		g_set_error (error,
@@ -873,7 +876,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		             NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
 		             _("Unknown slave type '%s'"), priv->slave_type);
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
-		return NM_SETTING_VERIFY_ERROR;
+		return FALSE;
 	}
 
 	if (is_slave) {
@@ -883,19 +886,54 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 			                     NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
 			                     _("Slave connections need a valid '" NM_SETTING_CONNECTION_MASTER "' property"));
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER);
-			return NM_SETTING_VERIFY_ERROR;
+			return FALSE;
 		}
+		if (   slave_setting_type
+		    && all_settings /* only check for an existing slave-setting when having @all_settings */
+		    && !nm_setting_find_in_list (all_settings, slave_setting_type))
+			normerr_slave_setting_type = slave_setting_type;
 	} else {
 		if (priv->master) {
-			g_set_error_literal (error,
-			                     NM_SETTING_CONNECTION_ERROR,
-			                     NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
-			                     _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'"));
-			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
-			return NM_SETTING_VERIFY_ERROR;
+			const char *slave_type;
+			NMSetting *s_port;
+
+			if (   all_settings
+			    && (slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, &s_port))) {
+				normerr_missing_slave_type = slave_type;
+				normerr_missing_slave_type_port = nm_setting_get_name (s_port);
+			} else {
+				g_set_error_literal (error,
+				                     NM_SETTING_CONNECTION_ERROR,
+				                     NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
+				                     _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'"));
+				g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
+				return FALSE;
+			}
 		}
 	}
 
+	/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
+
+	if (normerr_slave_setting_type) {
+		g_set_error (error,
+		             NM_SETTING_CONNECTION_ERROR,
+		             NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND,
+		             _("slave-type '%s' requires a '%s' setting in the connection"),
+		             priv->slave_type, normerr_slave_setting_type);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
+		return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+	}
+
+	if (normerr_missing_slave_type) {
+		g_set_error (error,
+		             NM_SETTING_CONNECTION_ERROR,
+		             NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
+		             _("Detect a slave connection with '" NM_SETTING_CONNECTION_MASTER "' set and a port type '%s'. '" NM_SETTING_CONNECTION_SLAVE_TYPE "' should be set to '%s'"),
+		             normerr_missing_slave_type_port, normerr_missing_slave_type);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
+		return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+	}
+
 	return TRUE;
 }
 
diff --git a/libnm-core/nm-setting-connection.h b/libnm-core/nm-setting-connection.h
index 7782567e39..169e2c18ee 100644
--- a/libnm-core/nm-setting-connection.h
+++ b/libnm-core/nm-setting-connection.h
@@ -52,6 +52,8 @@ G_BEGIN_DECLS
  *   #NMSettingConnection:type property was not present in the #NMConnection
  * @NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED: ip configuration is not
  *   allowed to be present.
+ * @NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND: the mandatory #NMSetting
+ *   object for the slave is missing. The slave type depends on #NMSettingConnection:slave-type
  *
  * Describes errors that may result from operations involving a
  * #NMSettingConnection.
@@ -64,6 +66,7 @@ typedef enum
 	NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,       /*< nick=MissingProperty >*/
 	NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, /*< nick=TypeSettingNotFound >*/
 	NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED,  /*< nick=IpConfigNotAllowed >*/
+	NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND, /*< nick=SlaveSettingNotFound >*/
 } NMSettingConnectionError;
 
 #define NM_SETTING_CONNECTION_ERROR nm_setting_connection_error_quark ()
diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h
index e68d5a4278..316838a32b 100644
--- a/libnm-core/nm-setting-private.h
+++ b/libnm-core/nm-setting-private.h
@@ -110,4 +110,7 @@ NMSettingVerifyResult _nm_setting_verify (NMSetting *setting,
                                           GSList    *all_settings,
                                           GError    **error);
 
+gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type);
+const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port);
+
 #endif  /* NM_SETTING_PRIVATE_H */
diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c
index cb8d00c38f..e5cac6d61a 100644
--- a/libnm-core/nm-setting-team-port.c
+++ b/libnm-core/nm-setting-team-port.c
@@ -103,6 +103,30 @@ nm_setting_team_port_get_config (NMSettingTeamPort *setting)
 static gboolean
 verify (NMSetting *setting, GSList *all_settings, GError **error)
 {
+	if (all_settings) {
+		NMSettingConnection *s_con;
+		const char *slave_type;
+
+		s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
+		                                                                  NM_SETTING_CONNECTION_SETTING_NAME,
+		                                                                  error, NULL, NULL));
+		if (!s_con)
+			return FALSE;
+
+		slave_type = nm_setting_connection_get_slave_type (s_con);
+		if (   slave_type
+		    && strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME)) {
+			g_set_error (error,
+			             NM_SETTING_CONNECTION_ERROR,
+			             NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
+			             _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"),
+			             NM_SETTING_TEAM_PORT_SETTING_NAME,
+			             NM_SETTING_TEAM_SETTING_NAME,
+			             slave_type);
+			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
+			return FALSE;
+		}
+	}
 	return TRUE;
 }
 
diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index 830484b569..5fc0b64867 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -288,6 +288,65 @@ _nm_setting_compare_priority (gconstpointer a, gconstpointer b)
 
 /*************************************************************/
 
+gboolean
+_nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type)
+{
+	const char *port_type = NULL;
+	gboolean found = TRUE;
+
+	if (!slave_type)
+		found = FALSE;
+	else if (!strcmp (slave_type, NM_SETTING_BOND_SETTING_NAME))
+		;
+	else if (!strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME))
+		port_type = NM_SETTING_BRIDGE_PORT_SETTING_NAME;
+	else if (!strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME))
+		port_type = NM_SETTING_TEAM_PORT_SETTING_NAME;
+	else
+		found = FALSE;
+
+	if (out_port_type)
+		*out_port_type = port_type;
+	return found;
+}
+
+
+const char *
+_nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port)
+{
+	GSList *iter;
+	const char *slave_type = NULL;
+	NMSetting *s_port = NULL;
+
+	for (iter = all_settings; iter; iter = iter->next) {
+		NMSetting *s_iter = NM_SETTING (iter->data);
+		const char *name = nm_setting_get_name (s_iter);
+		const char *i_slave_type = NULL;
+
+		if (!strcmp (name, NM_SETTING_BRIDGE_PORT_SETTING_NAME))
+			i_slave_type = NM_SETTING_BRIDGE_SETTING_NAME;
+		else if (!strcmp (name, NM_SETTING_TEAM_PORT_SETTING_NAME))
+			i_slave_type = NM_SETTING_TEAM_SETTING_NAME;
+		else
+			continue;
+
+		if (slave_type) {
+			/* there are more then one matching port types, cannot detect the slave type. */
+			slave_type = NULL;
+			s_port = NULL;
+			break;
+		}
+		slave_type = i_slave_type;
+		s_port = s_iter;
+	}
+
+	if (out_s_port)
+		*out_s_port = s_port;
+	return slave_type;
+}
+
+/*************************************************************/
+
 static void
 destroy_gvalue (gpointer data)
 {
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index 0c660d0384..f9a7714b8c 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -2658,6 +2658,64 @@ test_connection_normalize_virtual_iface_name (void)
 	g_object_unref (con);
 }
 
+static void
+test_connection_normalize_slave_type_1 (void)
+{
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingConnection *s_con;
+
+	con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_1",
+	                                       "cc4cd5df-45dc-483e-b291-6b76c2338ecb",
+	                                       NM_SETTING_WIRED_SETTING_NAME, &s_con);
+
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_MASTER, "master0",
+	              NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type",
+	              NULL);
+
+	nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_SLAVE_TYPE, "bridge",
+	              NULL);
+
+	g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+	nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND);
+	g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+	g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME);
+}
+
+static void
+test_connection_normalize_slave_type_2 (void)
+{
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingConnection *s_con;
+
+	con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_2",
+	                                       "40bea008-ca72-439a-946b-e65f827656f9",
+	                                       NM_SETTING_WIRED_SETTING_NAME, &s_con);
+
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_MASTER, "master0",
+	              NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type",
+	              NULL);
+
+	nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_SLAVE_TYPE, NULL,
+	              NULL);
+	nm_connection_add_setting (con, nm_setting_bridge_port_new ());
+
+	g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+	g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NULL);
+	nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY);
+	g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME));
+	g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME);
+}
+
 NMTST_DEFINE ();
 
 int main (int argc, char **argv)
@@ -2697,6 +2755,8 @@ int main (int argc, char **argv)
 	test_connection_new_from_hash ();
 	test_connection_verify_sets_interface_name ();
 	test_connection_normalize_virtual_iface_name ();
+	test_connection_normalize_slave_type_1 ();
+	test_connection_normalize_slave_type_2 ();
 
 	test_setting_connection_permissions_helpers ();
 	test_setting_connection_permissions_property ();
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 37fd6199a6..86a0d1834b 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -59,6 +59,7 @@ libnm-core/nm-setting-ip6-config.c
 libnm-core/nm-setting-olpc-mesh.c
 libnm-core/nm-setting-ppp.c
 libnm-core/nm-setting-pppoe.c
+libnm-core/nm-setting-team-port.c
 libnm-core/nm-setting-vlan.c
 libnm-core/nm-setting-vpn.c
 libnm-core/nm-setting-wimax.c
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 4b9c7aa675..d917159efe 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -14409,8 +14409,11 @@ test_read_team_port_empty_config (void)
 	g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_WIRED_SETTING_NAME);
 	g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, "team0");
 
-	/* Empty TEAM_PORT_CONFIG means no team-port setting */
-	g_assert (nm_connection_get_setting_team_port (connection) == NULL);
+	/* Normalization adds a team-port setting */
+	g_assert (nm_connection_get_setting_team_port (connection));
+
+	/* empty/missing config */
+	g_assert (!nm_setting_team_port_get_config (nm_connection_get_setting_team_port (connection)));
 
 	g_object_unref (connection);
 }