libnm: fix normalizing and verifying OVS connections

Normalizing can be complicated, as settings depend on each other and possibly
conflict.

That is, because verify() must exactly anticipate whether normalization will
succeed and how the result will look like. That is because we only want to
modify the connection, if we are sure that the result will verify.

Hence, verify() and normalize() are strongly related. The implementation
should not be spread out between NMSettingOvsInterface:verify(),
NMSettingOvsPatch:verify() and _normalize_ovs_interface_type().

Also, add some unit-tests.

1 files changed, 19 insertions, 13 deletions

diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index 4b2c5d501b..28d7a8f6b7 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/libnm-core/nm-setting-connection.c
@@ -859,6 +859,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	gboolean is_slave;
 	const char *slave_setting_type;
 	NMSetting *normerr_base_type = NULL;
+	const char *type;
+	const char *slave_type;
 	const char *normerr_slave_setting_type = NULL;
 	const char *normerr_missing_slave_type = NULL;
 	const char *normerr_missing_slave_type_port = NULL;
@@ -904,8 +906,10 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
-	if (!priv->type) {
-		if (!connection || !(normerr_base_type = _nm_connection_find_base_type_setting (connection))) {
+	type = priv->type;
+	if (!type) {
+		if (   !connection
+		    || !(normerr_base_type = _nm_connection_find_base_type_setting (connection))) {
 			g_set_error_literal (error,
 			                     NM_CONNECTION_ERROR,
 			                     NM_CONNECTION_ERROR_MISSING_PROPERTY,
@@ -913,10 +917,11 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE);
 			return FALSE;
 		}
+		type = nm_setting_get_name (normerr_base_type);
 	} else {
 		GType base_type;
 
-		if (!priv->type[0]) {
+		if (!type[0]) {
 			g_set_error_literal (error,
 			                     NM_CONNECTION_ERROR,
 			                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -925,21 +930,21 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			return FALSE;
 		}
 
-		base_type = nm_setting_lookup_type (priv->type);
+		base_type = nm_setting_lookup_type (type);
 		if (   base_type == G_TYPE_INVALID
 		    || _nm_setting_type_get_base_type_priority (base_type) == NM_SETTING_PRIORITY_INVALID) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
 			             _("connection type '%s' is not valid"),
-			             priv->type);
+			             type);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE);
 			return FALSE;
 		}
 
 		/* Make sure the corresponding 'type' item is present */
 		if (   connection
-		    && !nm_connection_get_setting_by_name (connection, priv->type)) {
+		    && !nm_connection_get_setting_by_name (connection, type)) {
 			NMSetting *s_base;
 			NMConnection *connection2;
 
@@ -952,7 +957,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 			g_object_unref (connection2);
 
 			if (!normerr_base_setting) {
-				_set_error_missing_base_setting (error, priv->type);
+				_set_error_missing_base_setting (error, type);
 				return FALSE;
 			}
 		}
@@ -960,13 +965,14 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 
 	is_slave = FALSE;
 	slave_setting_type = NULL;
-	if (priv->slave_type) {
-		is_slave = _nm_setting_slave_type_is_valid (priv->slave_type, &slave_setting_type);
+	slave_type = priv->slave_type;
+	if (slave_type) {
+		is_slave = _nm_setting_slave_type_is_valid (slave_type, &slave_setting_type);
 		if (!is_slave) {
 			g_set_error (error,
 			             NM_CONNECTION_ERROR,
 			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             _("Unknown slave type '%s'"), priv->slave_type);
+			             _("Unknown slave type '%s'"), slave_type);
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
 			return FALSE;
 		}
@@ -986,8 +992,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		    && !nm_connection_get_setting_by_name (connection, slave_setting_type))
 			normerr_slave_setting_type = slave_setting_type;
 	} else {
+		nm_assert (!slave_type);
 		if (priv->master) {
-			const char *slave_type;
 			NMSetting *s_port;
 
 			if (   connection
@@ -1006,8 +1012,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
-	if (   g_strcmp0 (priv->type, NM_SETTING_OVS_PORT_SETTING_NAME) != 0
-	    && g_strcmp0 (priv->slave_type, NM_SETTING_OVS_BRIDGE_SETTING_NAME) == 0) {
+	if (   nm_streq0 (type, NM_SETTING_OVS_PORT_SETTING_NAME)
+	    && !nm_streq0 (slave_type, NM_SETTING_OVS_BRIDGE_SETTING_NAME)) {
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
 		             NM_CONNECTION_ERROR_MISSING_PROPERTY,