libnm-core: drop the ability to verify settings from property overrides

It needs to be possible to deserialize a connection hash into an
invalid NMConnection; in particular, AddAndActivateConnection()
explicitly allows this.

Previously, the SetFunc and NotSetFunc passed to
_nm_setting_class_override_property() could return a verification
error immediately, but this functionality has to go away if we're
going to be able to deserialize invalid connections.

That functionality was only used in the handling of invalid virtual
interface names; reorganize how that code works so that
NMSettingConnection does all of the verification itself. (The code to
make sure that it returned the "correct" error domain in that case
turned out to be irrelevant, since the setting error domains don't get
serialized over D-Bus correctly anyway.)

8 files changed, 54 insertions, 111 deletions

diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index 22fa19f3e4..22ef5dd213 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -740,5 +740,5 @@ nm_setting_bond_class_init (NMSettingBondClass *setting_class)
 
 	 _nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING,
 	                                           _nm_setting_get_deprecated_virtual_interface_name,
-	                                           _nm_setting_set_deprecated_virtual_interface_name);
+	                                           NULL);
 }
diff --git a/libnm-core/nm-setting-bridge.c b/libnm-core/nm-setting-bridge.c
index cd682a49d8..49bca2e813 100644
--- a/libnm-core/nm-setting-bridge.c
+++ b/libnm-core/nm-setting-bridge.c
@@ -480,5 +480,5 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 
 	_nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING,
 	                                          _nm_setting_get_deprecated_virtual_interface_name,
-	                                          _nm_setting_set_deprecated_virtual_interface_name);
+	                                          NULL);
 }
diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index 9fea63b783..403c03653e 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/libnm-core/nm-setting-connection.c
@@ -948,17 +948,12 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 	return TRUE;
 }
 
-static gboolean
-nm_setting_connection_no_interface_name (NMSetting *setting,
-                                         GHashTable *connection_hash,
-                                         const char *property,
-                                         GError **error)
+static const char *
+find_virtual_interface_name (GHashTable *connection_hash)
 {
 	GHashTable *setting_hash;
-	const char *interface_name;
 	GValue *value;
 
-	/* Check if there's a deprecated virtual interface-name property to steal */
 	setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_BOND_SETTING_NAME);
 	if (!setting_hash)
 		setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_BRIDGE_SETTING_NAME);
@@ -968,30 +963,48 @@ nm_setting_connection_no_interface_name (NMSetting *setting,
 		setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_VLAN_SETTING_NAME);
 
 	if (!setting_hash)
-		return TRUE;
+		return NULL;
 
 	/* All of the deprecated virtual interface name properties were named "interface-name". */
 	value = g_hash_table_lookup (setting_hash, "interface-name");
-	if (!value)
-		return TRUE;
+	if (!value || !G_VALUE_HOLDS_STRING (value))
+		return NULL;
 
-	interface_name = g_value_get_string (value);
-	if (!interface_name)
-		return TRUE;
+	return g_value_get_string (value);
+}
 
-	if (!nm_utils_iface_valid_name (interface_name)) {
-		g_set_error_literal (error,
-		                     NM_SETTING_CONNECTION_ERROR,
-		                     NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
-		                     _("property is invalid"));
-		g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
-		return FALSE;
-	}
+static void
+nm_setting_connection_set_interface_name (NMSetting *setting,
+                                          GHashTable *connection_hash,
+                                          const char *property,
+                                          const GValue *value)
+{
+	const char *interface_name;
+
+	/* For compatibility reasons, if there is an invalid virtual interface name,
+	 * we need to make verification fail, even if that virtual name would be
+	 * overridden by a valid connection.interface-name.
+	 */
+	interface_name = find_virtual_interface_name (connection_hash);
+	if (!interface_name || nm_utils_iface_valid_name (interface_name))
+		interface_name = g_value_get_string (value);
 
 	g_object_set (G_OBJECT (setting),
 	              NM_SETTING_CONNECTION_INTERFACE_NAME, interface_name,
 	              NULL);
-	return TRUE;
+}
+
+static void
+nm_setting_connection_no_interface_name (NMSetting *setting,
+                                         GHashTable *connection_hash,
+                                         const char *property)
+{
+	const char *virtual_interface_name;
+
+	virtual_interface_name = find_virtual_interface_name (connection_hash);
+	g_object_set (G_OBJECT (setting),
+	              NM_SETTING_CONNECTION_INTERFACE_NAME, virtual_interface_name,
+	              NULL);
 }
 
 static gboolean
@@ -1256,7 +1269,8 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class)
 		                      G_PARAM_STATIC_STRINGS));
 	_nm_setting_class_override_property (parent_class, NM_SETTING_CONNECTION_INTERFACE_NAME,
 	                                     G_TYPE_STRING,
-	                                     NULL, NULL,
+	                                     NULL,
+	                                     nm_setting_connection_set_interface_name,
 	                                     nm_setting_connection_no_interface_name);
 
 	/**
diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h
index aad7e51697..e7c141a05d 100644
--- a/libnm-core/nm-setting-private.h
+++ b/libnm-core/nm-setting-private.h
@@ -108,11 +108,6 @@ gboolean _nm_setting_get_deprecated_virtual_interface_name (NMSetting *setting,
                                                             NMConnection *connection,
                                                             const char *property,
                                                             GValue *value);
-gboolean _nm_setting_set_deprecated_virtual_interface_name (NMSetting *setting,
-                                                            GHashTable *connection_hash,
-                                                            const char *property,
-                                                            const GValue *value,
-                                                            GError **error);
 
 NMSettingVerifyResult _nm_setting_verify (NMSetting *setting,
                                           GSList    *all_settings,
@@ -135,15 +130,13 @@ typedef gboolean (*NMSettingPropertyGetFunc)    (NMSetting     *setting,
                                                  NMConnection  *connection,
                                                  const char    *property,
                                                  GValue        *value);
-typedef gboolean (*NMSettingPropertySetFunc)    (NMSetting     *setting,
+typedef void     (*NMSettingPropertySetFunc)    (NMSetting     *setting,
                                                  GHashTable    *connection_hash,
                                                  const char    *property,
-                                                 const GValue  *value,
-                                                 GError       **error);
-typedef gboolean (*NMSettingPropertyNotSetFunc) (NMSetting     *setting,
+                                                 const GValue  *value);
+typedef void     (*NMSettingPropertyNotSetFunc) (NMSetting     *setting,
                                                  GHashTable    *connection_hash,
-                                                 const char    *property,
-                                                 GError       **error);
+                                                 const char    *property);
 
 void _nm_setting_class_add_dbus_only_property (NMSettingClass *setting_class,
                                                const char *property_name,
diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c
index 5946ec75a8..8fb94c3992 100644
--- a/libnm-core/nm-setting-team.c
+++ b/libnm-core/nm-setting-team.c
@@ -185,5 +185,5 @@ nm_setting_team_class_init (NMSettingTeamClass *setting_class)
 
 	_nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING,
 	                                          _nm_setting_get_deprecated_virtual_interface_name,
-	                                          _nm_setting_set_deprecated_virtual_interface_name);
+	                                          NULL);
 }
diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c
index f1781027d0..55262264b2 100644
--- a/libnm-core/nm-setting-vlan.c
+++ b/libnm-core/nm-setting-vlan.c
@@ -779,5 +779,5 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
 
 	_nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING,
 	                                          _nm_setting_get_deprecated_virtual_interface_name,
-	                                          _nm_setting_set_deprecated_virtual_interface_name);
+	                                          NULL);
 }
diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index 535706d82d..04eee75117 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -274,19 +274,6 @@ nm_setting_lookup_type_by_quark (GQuark error_quark)
 	return G_TYPE_INVALID;
 }
 
-static GQuark
-_nm_setting_lookup_error_quark (const char *name)
-{
-	SettingInfo *info;
-
-	g_return_val_if_fail (name != NULL, 0);
-
-	_ensure_registered ();
-
-	info = g_hash_table_lookup (registered_settings, name);
-	return info ? info->error_quark : 0;
-}
-
 gint
 _nm_setting_compare_priority (gconstpointer a, gconstpointer b)
 {
@@ -764,7 +751,7 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnection *connection, NMConnectionS
  * property names and value types.
  *
  * Returns: a new #NMSetting object populated with the properties from the
- * hash table, or %NULL on failure
+ * hash table, or %NULL if @setting_hash could not be deserialized.
  **/
 NMSetting *
 _nm_setting_new_from_dbus (GType setting_type,
@@ -807,24 +794,14 @@ _nm_setting_new_from_dbus (GType setting_type,
 		GValue *value = g_hash_table_lookup (setting_hash, property->name);
 
 		if (value && property->set_func) {
-			if (!property->set_func (setting,
-			                         connection_hash,
-			                         property->name,
-			                         value,
-			                         error)) {
-				g_object_unref (setting);
-				setting = NULL;
-				break;
-			}
+			property->set_func (setting,
+			                    connection_hash,
+			                    property->name,
+			                    value);
 		} else if (!value && property->not_set_func) {
-			if (!property->not_set_func (setting,
-			                             connection_hash,
-			                             property->name,
-			                             error)) {
-				g_object_unref (setting);
-				setting = NULL;
-				break;
-			}
+			property->not_set_func (setting,
+			                        connection_hash,
+			                        property->name);
 		} else if (value && property->param_spec) {
 			if (!(property->param_spec->flags & G_PARAM_WRITABLE))
 				continue;
@@ -1748,47 +1725,6 @@ _nm_setting_get_deprecated_virtual_interface_name (NMSetting *setting,
 		return FALSE;
 }
 
-gboolean
-_nm_setting_set_deprecated_virtual_interface_name (NMSetting *setting,
-                                                   GHashTable *connection_hash,
-                                                   const char *property,
-                                                   const GValue *value,
-                                                   GError **error)
-{
-	const char *interface_name;
-	GQuark error_domain;
-	char *error_enum_name;
-	GEnumClass *enum_class;
-	GEnumValue *enum_val;
-	int error_code = 0;
-
-	/* If the virtual setting type hash contains an interface name, it must be
-	 * valid (even if it's going to be ignored in favor of
-	 * NMSettingConnection:interface-name). Other than that, we don't have to
-	 * check anything here; NMSettingConnection:interface-name will do the rest.
-	 */
-	interface_name = g_value_get_string (value);
-	if (!interface_name || nm_utils_iface_valid_name (interface_name))
-		return TRUE;
-
-	/* For compatibility reasons, we have to use the right error domain... */
-	error_domain = _nm_setting_lookup_error_quark (nm_setting_get_name (setting));
-	error_enum_name = g_strdup_printf ("%sError", G_OBJECT_TYPE_NAME (setting));
-	enum_class = g_type_class_ref (g_type_from_name (error_enum_name));
-	g_free (error_enum_name);
-	if (enum_class) {
-		enum_val = g_enum_get_value_by_nick (enum_class, "InvalidProperty");
-		if (enum_val)
-			error_code = enum_val->value;
-		g_type_class_unref (enum_class);
-	}
-
-	g_set_error_literal (error, error_domain, error_code,
-	                     _("invalid value in compatibility property"));
-	g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), property);
-	return FALSE;
-}
-
 /*****************************************************************************/
 
 static void
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index f0fe4ae0a1..55abf4a3f7 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -2689,7 +2689,7 @@ test_connection_normalize_virtual_iface_name (void)
 	 */
 	g_value_set_string (value, ":::this-is-not-a-valid-interface-name:::");
 	con = nm_simple_connection_new_from_dbus (connection_hash, &error);
-	g_assert_error (error, NM_SETTING_VLAN_ERROR, NM_SETTING_VLAN_ERROR_INVALID_PROPERTY);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
 	g_clear_error (&error);
 
 	/* If vlan.interface-name is valid, but doesn't match, it will be ignored. */