libnm-core, settings: move NMSettingsError to nm-errors

Move the definition of NMSettingsError to nm-errors, register it with
D-Bus, and verify in the tests that it maps correctly.

Remove a few unused error codes, simplify a few others, and rename
GENERAL to FAILED and HOSTNAME_INVALID to INVALID_HOSTNAME, for
consistency.

libnm-core/nm-errors.c

@@ -30,6 +30,7 @@ G_DEFINE_QUARK (nm-connection-error-quark, nm_connection_error)
 G_DEFINE_QUARK (nm-crypto-error-quark, nm_crypto_error)
 G_DEFINE_QUARK (nm-device-error-quark, nm_device_error)
 G_DEFINE_QUARK (nm-manager-error-quark, nm_manager_error)
+G_DEFINE_QUARK (nm-settings-error-quark, nm_settings_error)
 
 static void
 register_error_domain (GQuark domain,
@@ -65,4 +66,7 @@ _nm_dbus_errors_init (void)
 	register_error_domain (NM_MANAGER_ERROR,
 	                       NM_DBUS_INTERFACE,
 	                       NM_TYPE_MANAGER_ERROR);
+	register_error_domain (NM_SETTINGS_ERROR,
+	                       NM_DBUS_INTERFACE_SETTINGS,
+	                       NM_TYPE_SETTINGS_ERROR);
 }

libnm-core/nm-errors.h

@@ -175,4 +175,35 @@ typedef enum {
 GQuark nm_manager_error_quark (void);
 #define NM_MANAGER_ERROR (nm_manager_error_quark ())
 
+/**
+ * NMSettingsError:
+ * @NM_SETTINGS_ERROR_FAILED: unknown or unclassified error
+ * @NM_SETTINGS_ERROR_PERMISSION_DENIED: permission denied
+ * @NM_SETTINGS_ERROR_NOT_SUPPORTED: the requested operation is not supported by any
+ *   active settings backend
+ * @NM_SETTINGS_ERROR_INVALID_CONNECTION: the connection was invalid
+ * @NM_SETTINGS_ERROR_READ_ONLY_CONNECTION: attempted to modify a read-only connection
+ * @NM_SETTINGS_ERROR_UUID_EXISTS: a connection with that UUID already exists
+ * @NM_SETTINGS_ERROR_INVALID_HOSTNAME: attempted to set an invalid hostname
+ *
+ * Errors related to the settings/persistent configuration interface of
+ * NetworkManager.
+ *
+ * These may be returned from #NMClient methods that invoke D-Bus operations on
+ * the "org.freedesktop.NetworkManager.Settings" interface, and correspond to
+ * D-Bus errors in that namespace.
+ */
+typedef enum {
+	NM_SETTINGS_ERROR_FAILED = 0,           /*< nick=Failed >*/
+	NM_SETTINGS_ERROR_PERMISSION_DENIED,    /*< nick=PermissionDenied >*/
+	NM_SETTINGS_ERROR_NOT_SUPPORTED,        /*< nick=NotSupported >*/
+	NM_SETTINGS_ERROR_INVALID_CONNECTION,   /*< nick=InvalidConnection >*/
+	NM_SETTINGS_ERROR_READ_ONLY_CONNECTION, /*< nick=ReadOnlyConnection >*/
+	NM_SETTINGS_ERROR_UUID_EXISTS,          /*< nick=UuidExists >*/
+	NM_SETTINGS_ERROR_INVALID_HOSTNAME,     /*< nick=InvalidHostname >*/
+} NMSettingsError;
+
+GQuark nm_settings_error_quark (void);
+#define NM_SETTINGS_ERROR (nm_settings_error_quark ())
+
 #endif /* __NM_ERRORS_H__ */

libnm/libnm.ver

@@ -792,6 +792,8 @@ global:
 	nm_setting_wireless_security_remove_proto;
 	nm_setting_wireless_security_remove_proto_by_value;
 	nm_setting_wireless_security_set_wep_key;
+	nm_settings_error_get_type;
+	nm_settings_error_quark;
 	nm_simple_connection_get_type;
 	nm_simple_connection_new;
 	nm_simple_connection_new_clone;

libnm/tests/test-remote-settings-client.c

@@ -458,6 +458,45 @@ test_add_bad_connection (void)
 
 /*******************************************************************/
 
+static void
+save_hostname_cb (GObject *s,
+                  GAsyncResult *result,
+                  gpointer user_data)
+{
+	gboolean *done = user_data;
+	GError *error = NULL;
+
+	nm_client_save_hostname_finish (client, result, &error);
+	g_assert_no_error (error);
+
+	*done = TRUE;
+}
+
+static void
+test_save_hostname (void)
+{
+	time_t start, now;
+	gboolean done = FALSE;
+	GError *error = NULL;
+
+	/* test-networkmanager-service.py requires the hostname to contain a '.' */
+	nm_client_save_hostname (client, "foo", NULL, &error);
+	g_assert_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_HOSTNAME);
+	g_clear_error (&error);
+
+	nm_client_save_hostname_async (client, "example.com", NULL, save_hostname_cb, &done);
+
+	start = time (NULL);
+	do {
+		now = time (NULL);
+		g_main_context_iteration (NULL, FALSE);
+	} while ((done == FALSE) && (now - start < 5));
+	g_assert (done == TRUE);
+	g_assert (remote == NULL);
+}
+
+/*******************************************************************/
+
 int
 main (int argc, char **argv)
 {
@@ -490,6 +529,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/client/remove_connection", test_remove_connection);
 	g_test_add_func ("/client/add_remove_connection", test_add_remove_connection);
 	g_test_add_func ("/client/add_bad_connection", test_add_bad_connection);
+	g_test_add_func ("/client/save_hostname", test_save_hostname);
 
 	ret = g_test_run ();
 

src/Makefile.am

@@ -163,8 +163,6 @@ nm_sources = \
 	settings/nm-secret-agent.h \
 	settings/nm-settings-connection.c \
 	settings/nm-settings-connection.h \
-	settings/nm-settings-error.c \
-	settings/nm-settings-error.h \
 	settings/nm-settings.c \
 	settings/nm-settings.h \
 	settings/nm-system-config-interface.c \

src/settings/nm-settings-connection.c

@@ -33,7 +33,6 @@
 #include "nm-settings-connection.h"
 #include "nm-session-monitor.h"
 #include "nm-dbus-manager.h"
-#include "nm-settings-error.h"
 #include "nm-dbus-glib-types.h"
 #include "nm-logging.h"
 #include "nm-auth-utils.h"
@@ -570,7 +569,7 @@ nm_settings_connection_commit_changes (NMSettingsConnection *connection,
 		                                                               user_data);
 	} else {
 		GError *error = g_error_new (NM_SETTINGS_ERROR,
-		                             NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		                             NM_SETTINGS_ERROR_FAILED,
 		                             "%s: %s:%d commit_changes() unimplemented", __func__, __FILE__, __LINE__);
 		if (callback)
 			callback (connection, error, user_data);
@@ -591,7 +590,7 @@ nm_settings_connection_delete (NMSettingsConnection *connection,
 		                                                       user_data);
 	} else {
 		GError *error = g_error_new (NM_SETTINGS_ERROR,
-		                             NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		                             NM_SETTINGS_ERROR_FAILED,
 		                             "%s: %s:%d delete() unimplemented", __func__, __FILE__, __LINE__);
 		if (callback)
 			callback (connection, error, user_data);
@@ -754,7 +753,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
 	}
 
 	if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) {
-		local = g_error_new (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_SETTING,
+		local = g_error_new (NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
 		                     "%s.%d - Connection didn't have requested setting '%s'.",
 		                     __FILE__, __LINE__, setting_name);
 		callback (self, call_id, NULL, setting_name, local, callback_data);
@@ -916,7 +915,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 	 * will clear secrets on this object's settings.
 	 */
 	if (!priv->system_secrets) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
+		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 		             "%s.%d - Internal error; secrets cache invalid.",
 		             __FILE__, __LINE__);
 		return 0;
@@ -924,7 +923,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 
 	/* Make sure the request actually requests something we can return */
 	if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_SETTING,
+		g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
 		             "%s.%d - Connection didn't have requested setting '%s'.",
 		             __FILE__, __LINE__, setting_name);
 		return 0;
@@ -1009,7 +1008,7 @@ pk_auth_cb (NMAuthChain *chain,
 	/* If our NMSettingsConnection is already gone, do nothing */
 	if (chain_error) {
 		error = g_error_new (NM_SETTINGS_ERROR,
-		                     NM_SETTINGS_ERROR_GENERAL,
+		                     NM_SETTINGS_ERROR_FAILED,
 		                     "Error checking authorization: %s",
 		                     chain_error->message ? chain_error->message : "(unknown)");
 	} else if (result != NM_AUTH_CALL_RESULT_YES) {

src/settings/nm-settings-error.c

@@ -1,33 +0,0 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
-/* NetworkManager system settings service
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Copyright (C) 2008 Novell, Inc.
- * Copyright (C) 2008 - 2011 Red Hat, Inc.
- */
-
-#include "nm-settings-error.h"
-
-GQuark
-nm_settings_error_quark (void)
-{
-	static GQuark ret = 0;
-
-	if (ret == 0)
-		ret = g_quark_from_static_string ("nm-settings-error");
-
-	return ret;
-}

src/settings/nm-settings-error.h

@@ -1,53 +0,0 @@
-/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
-/* NetworkManager system settings service
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Copyright (C) 2008 Novell, Inc.
- * Copyright (C) 2008 - 2011 Red Hat, Inc.
- */
-
-#ifndef __NETWORKMANAGER_SETTINGS_ERROR_H__
-#define __NETWORKMANAGER_SETTINGS_ERROR_H__
-
-#include <glib.h>
-#include <glib-object.h>
-
-typedef enum {
-	NM_SETTINGS_ERROR_GENERAL = 0,                 /*< nick=GeneralError >*/
-	NM_SETTINGS_ERROR_INVALID_CONNECTION,          /*< nick=InvalidConnection >*/
-	NM_SETTINGS_ERROR_READ_ONLY_CONNECTION,        /*< nick=ReadOnlyConnection >*/
-	NM_SETTINGS_ERROR_INTERNAL_ERROR,              /*< nick=InternalError >*/
-	NM_SETTINGS_ERROR_SECRETS_UNAVAILABLE,         /*< nick=SecretsUnavailable >*/
-	NM_SETTINGS_ERROR_SECRETS_REQUEST_CANCELED,    /*< nick=SecretsRequestCanceled >*/
-	NM_SETTINGS_ERROR_PERMISSION_DENIED,           /*< nick=PermissionDenied >*/
-	NM_SETTINGS_ERROR_INVALID_SETTING,             /*< nick=InvalidSetting >*/
-	NM_SETTINGS_ERROR_ADD_NOT_SUPPORTED,           /*< nick=AddNotSupported >*/
-	NM_SETTINGS_ERROR_UPDATE_NOT_SUPPORTED,        /*< nick=UpdateNotSupported >*/
-	NM_SETTINGS_ERROR_DELETE_NOT_SUPPORTED,        /*< nick=DeleteNotSupported >*/
-	NM_SETTINGS_ERROR_ADD_FAILED,                  /*< nick=AddFailed >*/
-	NM_SETTINGS_ERROR_SAVE_HOSTNAME_NOT_SUPPORTED, /*< nick=SaveHostnameNotSupported >*/
-	NM_SETTINGS_ERROR_SAVE_HOSTNAME_FAILED,        /*< nick=SaveHostnameFailed >*/
-	NM_SETTINGS_ERROR_HOSTNAME_INVALID,            /*< nick=HostnameInvalid >*/
-	NM_SETTINGS_ERROR_UUID_EXISTS,                 /*< nick=UuidExists >*/
-} NMSettingsError;
-
-#define NM_SETTINGS_ERROR (nm_settings_error_quark ())
-GQuark nm_settings_error_quark (void);
-
-#define NM_TYPE_SETTINGS_ERROR (nm_settings_error_get_type ())
-GType  nm_settings_error_get_type (void);
-
-#endif /* __NETWORKMANAGER_SETTINGS_ERROR_H__ */

src/settings/nm-settings.c

@@ -58,7 +58,6 @@
 #include "nm-dbus-glib-types.h"
 #include "nm-settings.h"
 #include "nm-settings-connection.h"
-#include "nm-settings-error.h"
 #include "nm-system-config-interface.h"
 #include "nm-logging.h"
 #include "nm-dbus-manager.h"
@@ -952,7 +951,7 @@ nm_settings_add_connection (NMSettings *self,
 		g_clear_error (&add_error);
 	}
 
-	g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_ADD_FAILED,
+	g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 	                     "No plugin supported adding this connection");
 	return NULL;
 }
@@ -1034,7 +1033,7 @@ pk_add_cb (NMAuthChain *chain,
 
 	if (chain_error) {
 		error = g_error_new (NM_SETTINGS_ERROR,
-		                     NM_SETTINGS_ERROR_GENERAL,
+		                     NM_SETTINGS_ERROR_FAILED,
 		                     "Error checking authorization: %s",
 		                     chain_error->message ? chain_error->message : "(unknown)");
 	} else if (result != NM_AUTH_CALL_RESULT_YES) {
@@ -1138,7 +1137,7 @@ nm_settings_add_connection_dbus (NMSettings *self,
 	/* Do any of the plugins support adding? */
 	if (!get_plugin (self, NM_SYSTEM_CONFIG_INTERFACE_CAP_MODIFY_CONNECTIONS)) {
 		error = g_error_new_literal (NM_SETTINGS_ERROR,
-		                             NM_SETTINGS_ERROR_ADD_NOT_SUPPORTED,
+		                             NM_SETTINGS_ERROR_NOT_SUPPORTED,
 		                             "None of the registered plugins support add.");
 		goto done;
 	}
@@ -1362,7 +1361,7 @@ pk_hostname_cb (NMAuthChain *chain,
 	/* If our NMSettingsConnection is already gone, do nothing */
 	if (chain_error) {
 		error = g_error_new (NM_SETTINGS_ERROR,
-		                     NM_SETTINGS_ERROR_GENERAL,
+		                     NM_SETTINGS_ERROR_FAILED,
 		                     "Error checking authorization: %s",
 		                     chain_error->message ? chain_error->message : "(unknown)");
 	} else if (result != NM_AUTH_CALL_RESULT_YES) {
@@ -1377,7 +1376,7 @@ pk_hostname_cb (NMAuthChain *chain,
 
 			/* error will be cleared if any plugin supports saving the hostname */
 			error = g_error_new_literal (NM_SETTINGS_ERROR,
-			                             NM_SETTINGS_ERROR_SAVE_HOSTNAME_FAILED,
+			                             NM_SETTINGS_ERROR_FAILED,
 			                             "Saving the hostname failed.");
 
 			g_object_get (G_OBJECT (iter->data), NM_SYSTEM_CONFIG_INTERFACE_CAPABILITIES, &caps, NULL);
@@ -1436,7 +1435,7 @@ impl_settings_save_hostname (NMSettings *self,
 	/* Minimal validation of the hostname */
 	if (!validate_hostname (hostname)) {
 		error = g_error_new_literal (NM_SETTINGS_ERROR,
-		                             NM_SETTINGS_ERROR_HOSTNAME_INVALID,
+		                             NM_SETTINGS_ERROR_INVALID_HOSTNAME,
 		                             "The hostname was too long or contained invalid characters.");
 		goto done;
 	}
@@ -1444,7 +1443,7 @@ impl_settings_save_hostname (NMSettings *self,
 	/* Do any of the plugins support setting the hostname? */
 	if (!get_plugin (self, NM_SYSTEM_CONFIG_INTERFACE_CAP_MODIFY_HOSTNAME)) {
 		error = g_error_new_literal (NM_SETTINGS_ERROR,
-		                             NM_SETTINGS_ERROR_SAVE_HOSTNAME_NOT_SUPPORTED,
+		                             NM_SETTINGS_ERROR_NOT_SUPPORTED,
 		                             "None of the registered plugins support setting the hostname.");
 		goto done;
 	}

src/settings/plugins/ibft/plugin.c

@@ -32,7 +32,6 @@
 
 #include "nm-dbus-glib-types.h"
 #include "nm-system-config-interface.h"
-#include "nm-settings-error.h"
 #include "nm-logging.h"
 #include "NetworkManagerUtils.h"
 

src/settings/plugins/ifcfg-rh/plugin.c

@@ -47,7 +47,6 @@
 #include "nm-dbus-glib-types.h"
 #include "plugin.h"
 #include "nm-system-config-interface.h"
-#include "nm-settings-error.h"
 #include "nm-config.h"
 #include "nm-logging.h"
 #include "NetworkManagerUtils.h"
@@ -789,7 +788,7 @@ impl_ifcfgrh_get_ifcfg_details (SCPluginIfcfg *plugin,
 	if (!s_con) {
 		g_set_error (error,
 		             NM_SETTINGS_ERROR,
-		             NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		             NM_SETTINGS_ERROR_FAILED,
 		             "unable to retrieve the connection setting");
 		return FALSE;
 	}
@@ -798,7 +797,7 @@ impl_ifcfgrh_get_ifcfg_details (SCPluginIfcfg *plugin,
 	if (!uuid) {
 		g_set_error (error,
 		             NM_SETTINGS_ERROR,
-		             NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		             NM_SETTINGS_ERROR_FAILED,
 		             "unable to get the UUID");
 		return FALSE;
 	}
@@ -807,7 +806,7 @@ impl_ifcfgrh_get_ifcfg_details (SCPluginIfcfg *plugin,
 	if (!path) {
 		g_set_error (error,
 		             NM_SETTINGS_ERROR,
-		             NM_SETTINGS_ERROR_INTERNAL_ERROR,
+		             NM_SETTINGS_ERROR_FAILED,
 		             "unable to get the connection D-Bus path");
 		return FALSE;
 	}

src/settings/plugins/ifnet/nm-ifnet-connection.c

@@ -27,7 +27,6 @@
 #include <nm-settings-connection.h>
 #include <nm-system-config-interface.h>
 #include <nm-logging.h>
-#include <nm-settings-error.h>
 #include "nm-ifnet-connection.h"
 #include "connection_parser.h"
 #include "net_parser.h"

src/settings/plugins/ifupdown/nm-ifupdown-connection.c

@@ -28,7 +28,6 @@
 #include <nm-setting-wireless-security.h>
 #include <nm-settings-connection.h>
 #include <nm-system-config-interface.h>
-#include <nm-settings-error.h>
 #include <nm-logging.h>
 #include "nm-ifupdown-connection.h"
 #include "parser.h"

tools/test-networkmanager-service.py

@@ -990,6 +990,9 @@ class Connection(dbus.service.Object):
 ###################################################################
 IFACE_SETTINGS = 'org.freedesktop.NetworkManager.Settings'
 
+class InvalidHostnameException(dbus.DBusException):
+    _dbus_error_name = IFACE_SETTINGS + '.InvalidHostname'
+
 class Settings(dbus.service.Object):
     def __init__(self, bus, object_path):
         dbus.service.Object.__init__(self, bus, object_path)
@@ -1032,6 +1035,14 @@ class Settings(dbus.service.Object):
         self.props['Connections'] = dbus.Array(self.connections.keys(), 'o')
         self.PropertiesChanged({ 'connections': self.props['Connections'] })
 
+    @dbus.service.method(dbus_interface=IFACE_SETTINGS, in_signature='s', out_signature='')
+    def SaveHostname(self, hostname):
+        # Arbitrary requirement to test error handling
+        if hostname.find('.') == -1:
+            raise InvalidHostnameException()
+        self.props['Hostname'] = hostname
+        self.PropertiesChanged({ 'hostname': hostname })
+
     @dbus.service.method(dbus_interface=dbus.PROPERTIES_IFACE, in_signature='s', out_signature='a{sv}')
     def GetAll(self, iface):
         if iface != IFACE_SETTINGS: