From 7f88f5257323ec2eb767268babe33a8d7bc0a9cc Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 27 Dec 2007 08:06:27 +0000 Subject: [PATCH] 2007-12-27 Dan Williams * src/nm-device-interface.c src/nm-device-interface.h - (nm_device_interface_error_quark, nm_device_interface_error_get_type): normalize and expand errors - (nm_device_interface_init): register errors so they can be marshalled through dbus-glib - (nm_device_interface_activate): ensure that failure of activation returns an error * src/nm-device.c src/nm-device.h - (device_activation_precheck): implementations of check_connection() now take a GError and must fill it in if the check fails. Return more descriptive error if the requested connection is already activating - (nm_device_activate): actually try to return descriptive errors on failures * src/nm-device-802-11-wireless.c src/nm-device-802-3-ethernet.c src/nm-serial-device.c src/nm-gsm-device.c - (real_check_connection): return more descriptive errors on failure * src/NetworkManagerPolicy.c - (nm_policy_device_change_check): print activation errors in the logs * src/nm-manager.c - (nm_manager_error_quark, nm_manager_error_get_type, nm_manager_class_init): new errors - (nm_manager_activate_device): handle errors - (nm_manager_error_new): removed - (wait_for_connection_expired, connection_added_default_handler, impl_manager_activate_device): better error handling git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@3197 4912f4e0-d625-0410-9fb7-b9a5a253dbdc --- ChangeLog | 37 ++++++++ src/NetworkManagerPolicy.c | 20 ++++- src/nm-device-802-11-wireless.c | 2 +- src/nm-device-802-3-ethernet.c | 2 +- src/nm-device-interface.c | 28 ++++-- src/nm-device-interface.h | 11 ++- src/nm-device.c | 23 +++-- src/nm-device.h | 2 +- src/nm-gsm-device.c | 14 ++- src/nm-manager.c | 152 ++++++++++++++++++++++---------- src/nm-manager.h | 3 +- src/nm-serial-device.c | 12 ++- 12 files changed, 230 insertions(+), 76 deletions(-) diff --git a/ChangeLog b/ChangeLog index cefde26710..8cbbaa5fe4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,40 @@ +2007-12-27 Dan Williams + + * src/nm-device-interface.c + src/nm-device-interface.h + - (nm_device_interface_error_quark, nm_device_interface_error_get_type): + normalize and expand errors + - (nm_device_interface_init): register errors so they can be marshalled + through dbus-glib + - (nm_device_interface_activate): ensure that failure of activation + returns an error + + * src/nm-device.c + src/nm-device.h + - (device_activation_precheck): implementations of check_connection() + now take a GError and must fill it in if the check fails. Return + more descriptive error if the requested connection is already + activating + - (nm_device_activate): actually try to return descriptive errors on + failures + + * src/nm-device-802-11-wireless.c + src/nm-device-802-3-ethernet.c + src/nm-serial-device.c + src/nm-gsm-device.c + - (real_check_connection): return more descriptive errors on failure + + * src/NetworkManagerPolicy.c + - (nm_policy_device_change_check): print activation errors in the logs + + * src/nm-manager.c + - (nm_manager_error_quark, nm_manager_error_get_type, + nm_manager_class_init): new errors + - (nm_manager_activate_device): handle errors + - (nm_manager_error_new): removed + - (wait_for_connection_expired, connection_added_default_handler, + impl_manager_activate_device): better error handling + 2007-12-27 Dan Williams * src/supplicant-manager/nm-supplicant-settings-verify.c diff --git a/src/NetworkManagerPolicy.c b/src/NetworkManagerPolicy.c index 1d6aaa4185..d767a3d4d8 100644 --- a/src/NetworkManagerPolicy.c +++ b/src/NetworkManagerPolicy.c @@ -367,8 +367,24 @@ nm_policy_device_change_check (gpointer user_data) if (old_dev) nm_device_interface_deactivate (NM_DEVICE_INTERFACE (old_dev)); - if (new_dev) - nm_manager_activate_device (policy->manager, new_dev, connection, specific_object, FALSE); + if (new_dev) { + GError *error = NULL; + gboolean success; + + success = nm_manager_activate_device (policy->manager, + new_dev, + connection, + specific_object, + FALSE, + &error); + if (!success) { + nm_warning ("Failed to automatically activate device %s: (%d) %s", + nm_device_get_iface (new_dev), + error->code, + error->message); + g_error_free (error); + } + } } out: diff --git a/src/nm-device-802-11-wireless.c b/src/nm-device-802-11-wireless.c index a588ae4ff5..c771add78f 100644 --- a/src/nm-device-802-11-wireless.c +++ b/src/nm-device-802-11-wireless.c @@ -744,7 +744,7 @@ real_deactivate (NMDevice *dev) } static gboolean -real_check_connection (NMDevice *dev, NMConnection *connection) +real_check_connection (NMDevice *dev, NMConnection *connection, GError **error) { return TRUE; } diff --git a/src/nm-device-802-3-ethernet.c b/src/nm-device-802-3-ethernet.c index e137cd2951..3c30e0ec0d 100644 --- a/src/nm-device-802-3-ethernet.c +++ b/src/nm-device-802-3-ethernet.c @@ -328,7 +328,7 @@ real_can_interrupt_activation (NMDevice *dev) } static gboolean -real_check_connection (NMDevice *dev, NMConnection *connection) +real_check_connection (NMDevice *dev, NMConnection *connection, GError **error) { return TRUE; } diff --git a/src/nm-device-interface.c b/src/nm-device-interface.c index d2d62f9725..d2e948bab4 100644 --- a/src/nm-device-interface.c +++ b/src/nm-device-interface.c @@ -11,10 +11,10 @@ static gboolean impl_device_deactivate (NMDeviceInterface *device, GError **err) GQuark nm_device_interface_error_quark (void) { - static GQuark quark = 0; - if (!quark) - quark = g_quark_from_static_string ("nm_device_interface_error"); - return quark; + static GQuark quark = 0; + if (!quark) + quark = g_quark_from_static_string ("nm-device-interface-error"); + return quark; } /* This should really be standard. */ @@ -27,7 +27,10 @@ nm_device_interface_error_get_type (void) if (etype == 0) { static const GEnumValue values[] = { - ENUM_ENTRY (NM_DEVICE_INTERFACE_ERROR_UNKNOWN_CONNECTION, "UnknownConnection"), + /* Connection is already activating. */ + ENUM_ENTRY (NM_DEVICE_INTERFACE_ERROR_CONNECTION_ACTIVATING, "ConnectionActivating"), + /* Connection is invalid for this device. */ + ENUM_ENTRY (NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, "ConnectionInvalid"), { 0, 0, 0 } }; etype = g_enum_register_static ("NMDeviceInterfaceError", values); @@ -140,6 +143,10 @@ nm_device_interface_init (gpointer g_iface) dbus_g_object_type_install_info (iface_type, &dbus_glib_nm_device_interface_object_info); + dbus_g_error_domain_register (NM_DEVICE_INTERFACE_ERROR, + NULL, + NM_TYPE_DEVICE_INTERFACE_ERROR); + initialized = TRUE; } @@ -187,13 +194,20 @@ nm_device_interface_get_iface (NMDeviceInterface *device) gboolean nm_device_interface_activate (NMDeviceInterface *device, - NMActRequest *req) + NMActRequest *req, + GError **error) { + gboolean success; + g_return_val_if_fail (NM_IS_DEVICE_INTERFACE (device), FALSE); g_return_val_if_fail (NM_IS_ACT_REQUEST (req), FALSE); nm_info ("Activating device %s", nm_device_interface_get_iface (device)); - return NM_DEVICE_INTERFACE_GET_INTERFACE (device)->activate (device, req); + success = NM_DEVICE_INTERFACE_GET_INTERFACE (device)->activate (device, req, error); + if (!success) + g_assert (*error); + + return success; } void diff --git a/src/nm-device-interface.h b/src/nm-device-interface.h index 124c91427f..a7bb1d3137 100644 --- a/src/nm-device-interface.h +++ b/src/nm-device-interface.h @@ -14,11 +14,12 @@ typedef enum { - NM_DEVICE_INTERFACE_ERROR_UNKNOWN_CONNECTION = 0, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_ACTIVATING = 0, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, } NMDeviceInterfaceError; #define NM_DEVICE_INTERFACE_ERROR (nm_device_interface_error_quark ()) -#define NM_DEVICE_INTERFACE_TYPE_ERROR (nm_device_interface_error_get_type ()) +#define NM_TYPE_DEVICE_INTERFACE_ERROR (nm_device_interface_error_get_type ()) #define NM_DEVICE_INTERFACE_UDI "udi" #define NM_DEVICE_INTERFACE_IFACE "interface" @@ -52,7 +53,8 @@ struct _NMDeviceInterface { /* Methods */ gboolean (*activate) (NMDeviceInterface *device, - NMActRequest *req); + NMActRequest *req, + GError **error); void (*deactivate) (NMDeviceInterface *device); @@ -67,7 +69,8 @@ GType nm_device_interface_error_get_type (void); GType nm_device_interface_get_type (void); gboolean nm_device_interface_activate (NMDeviceInterface *device, - NMActRequest *req); + NMActRequest *req, + GError **error); void nm_device_interface_deactivate (NMDeviceInterface *device); diff --git a/src/nm-device.c b/src/nm-device.c index 9a83b99c3f..f42c379381 100644 --- a/src/nm-device.c +++ b/src/nm-device.c @@ -83,7 +83,8 @@ struct _NMDevicePrivate }; static gboolean nm_device_activate (NMDeviceInterface *device, - NMActRequest *req); + NMActRequest *req, + GError **error); static void nm_device_activate_schedule_stage5_ip_config_commit (NMDevice *self); static void nm_device_deactivate (NMDeviceInterface *device); @@ -1128,16 +1129,18 @@ connection_secrets_failed_cb (NMActRequest *req, } static gboolean -device_activation_precheck (NMDevice *self, NMConnection *connection) +device_activation_precheck (NMDevice *self, NMConnection *connection, GError **error) { NMConnection *current_connection; g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); - if (!NM_DEVICE_GET_CLASS (self)->check_connection (self, connection)) + if (!NM_DEVICE_GET_CLASS (self)->check_connection (self, connection, error)) { /* connection is invalid */ + g_assert (*error); return FALSE; + } if (nm_device_get_state (self) != NM_DEVICE_STATE_ACTIVATED) return TRUE; @@ -1146,22 +1149,30 @@ device_activation_precheck (NMDevice *self, NMConnection *connection) return TRUE; current_connection = nm_act_request_get_connection (nm_device_get_act_request (self)); - if (nm_connection_compare (connection, current_connection)) + if (nm_connection_compare (connection, current_connection)) { /* Already activating or activated with the same connection */ + g_set_error (error, + NM_DEVICE_INTERFACE_ERROR, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_ACTIVATING, + "%s", "Connection is already activating"); return FALSE; + } return TRUE; } static gboolean nm_device_activate (NMDeviceInterface *device, - NMActRequest *req) + NMActRequest *req, + GError **error) { NMDevice *self = NM_DEVICE (device); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (!device_activation_precheck (self, nm_act_request_get_connection (req))) + if (!device_activation_precheck (self, nm_act_request_get_connection (req), error)) { + g_assert (*error); return FALSE; + } priv->act_request = g_object_ref (req); priv->secrets_updated_id = g_signal_connect (req, diff --git a/src/nm-device.h b/src/nm-device.h index 3ed8750100..dc9036f155 100644 --- a/src/nm-device.h +++ b/src/nm-device.h @@ -98,7 +98,7 @@ struct _NMDeviceClass NMConnection *connection, const char *setting_name); - gboolean (* check_connection) (NMDevice *self, NMConnection *connection); + gboolean (* check_connection) (NMDevice *self, NMConnection *connection, GError **error); NMActStageReturn (* act_stage1_prepare) (NMDevice *self); NMActStageReturn (* act_stage2_config) (NMDevice *self); diff --git a/src/nm-gsm-device.c b/src/nm-gsm-device.c index 0d5d0f90cd..982055981e 100644 --- a/src/nm-gsm-device.c +++ b/src/nm-gsm-device.c @@ -459,22 +459,28 @@ real_get_generic_capabilities (NMDevice *dev) } static gboolean -real_check_connection (NMDevice *dev, NMConnection *connection) +real_check_connection (NMDevice *dev, NMConnection *connection, GError **error) { NMSettingGsm *gsm; gsm = (NMSettingGsm *) nm_connection_get_setting (connection, NM_TYPE_SETTING_GSM); if (!gsm) { - nm_warning ("Connection check failed: gsm setting not present."); + g_set_error (error, + NM_DEVICE_INTERFACE_ERROR, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, + "%s", "Connection invalid: GSM setting not present"); return FALSE; } if (!gsm->number) { - nm_warning ("Connection check failed: Phone number not set."); + g_set_error (error, + NM_DEVICE_INTERFACE_ERROR, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, + "%s", "Connection invalid: Phone number not set"); return FALSE; } - return NM_DEVICE_CLASS (nm_gsm_device_parent_class)->check_connection (dev, connection); + return NM_DEVICE_CLASS (nm_gsm_device_parent_class)->check_connection (dev, connection, error); } static void diff --git a/src/nm-manager.c b/src/nm-manager.c index fc3c8e5422..12ace795a8 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -100,6 +100,56 @@ enum { LAST_PROP }; +typedef enum +{ + NM_MANAGER_ERROR_UNKNOWN_CONNECTION = 0, + NM_MANAGER_ERROR_UNKNOWN_DEVICE, + NM_MANAGER_ERROR_INVALID_SERVICE, + NM_MANAGER_ERROR_SYSTEM_CONNECTION, + NM_MANAGER_ERROR_PERMISSION_DENIED, +} NMManagerError; + +#define NM_MANAGER_ERROR (nm_manager_error_quark ()) +#define NM_TYPE_MANAGER_ERROR (nm_manager_error_get_type ()) + +static GQuark +nm_manager_error_quark (void) +{ + static GQuark quark = 0; + if (!quark) + quark = g_quark_from_static_string ("nm-manager-error"); + return quark; +} + +/* This should really be standard. */ +#define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC } + +static GType +nm_manager_error_get_type (void) +{ + static GType etype = 0; + + if (etype == 0) { + static const GEnumValue values[] = { + /* Connection was not provided by any known settings service. */ + ENUM_ENTRY (NM_MANAGER_ERROR_UNKNOWN_CONNECTION, "UnknownConnection"), + /* Unknown device. */ + ENUM_ENTRY (NM_MANAGER_ERROR_UNKNOWN_DEVICE, "UnknownDevice"), + /* Invalid settings service (not a recognized system or user + * settings service name) + */ + ENUM_ENTRY (NM_MANAGER_ERROR_INVALID_SERVICE, "InvalidService"), + /* Connection was superceded by a system connection. */ + ENUM_ENTRY (NM_MANAGER_ERROR_SYSTEM_CONNECTION, "SystemConnection"), + /* User does not have the permission to activate this connection. */ + ENUM_ENTRY (NM_MANAGER_ERROR_PERMISSION_DENIED, "PermissionDenied"), + { 0, 0, 0 } + }; + etype = g_enum_register_static ("NMManagerError", values); + } + return etype; +} + static void nm_manager_init (NMManager *manager) { @@ -357,6 +407,8 @@ nm_manager_class_init (NMManagerClass *manager_class) dbus_g_object_type_install_info (G_TYPE_FROM_CLASS (manager_class), &dbus_glib_nm_manager_object_info); + + dbus_g_error_domain_register (NM_MANAGER_ERROR, NULL, NM_TYPE_MANAGER_ERROR); } #define DBUS_TYPE_G_STRING_VARIANT_HASHTABLE (dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_VALUE)) @@ -1108,7 +1160,8 @@ nm_manager_activate_device (NMManager *manager, NMDevice *device, NMConnection *connection, const char *specific_object, - gboolean user_requested) + gboolean user_requested, + GError **error) { NMActRequest *req; gboolean success; @@ -1117,8 +1170,10 @@ nm_manager_activate_device (NMManager *manager, g_return_val_if_fail (NM_IS_DEVICE (device), FALSE); g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); + /* Ensure the requested connection is allowed to be activated */ + req = nm_act_request_new (connection, specific_object, user_requested); - success = nm_device_interface_activate (NM_DEVICE_INTERFACE (device), req); + success = nm_device_interface_activate (NM_DEVICE_INTERFACE (device), req, error); g_object_unref (req); return success; @@ -1132,44 +1187,28 @@ nm_manager_activation_pending (NMManager *manager) return NM_MANAGER_GET_PRIVATE (manager)->pending_connection_info != NULL; } -static GError * -nm_manager_error_new (const gchar *format, ...) -{ - GError *err; - va_list args; - gchar *msg; - static GQuark domain_quark = 0; - - if (domain_quark == 0) - domain_quark = g_quark_from_static_string ("nm_manager_error"); - - va_start (args, format); - msg = g_strdup_vprintf (format, args); - va_end (args); - - err = g_error_new_literal (domain_quark, 1, (const gchar *) msg); - - g_free (msg); - - return err; -} - static gboolean wait_for_connection_expired (gpointer data) { NMManager *manager = NM_MANAGER (data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); PendingConnectionInfo *info = priv->pending_connection_info; - GError *err; + GError *error = NULL; g_return_val_if_fail (info != NULL, FALSE); nm_info ("%s: didn't receive connection details soon enough for activation.", nm_device_get_iface (info->device)); - err = nm_manager_error_new ("Could not find connection"); - dbus_g_method_return_error (info->context, err); - g_error_free (err); + g_set_error (&error, + NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION, + "%s", "Connection was not provided by any settings service"); + nm_warning ("Failed to activate device %s: (%d) %s", + nm_device_get_iface (info->device), + error->code, + error->message); + dbus_g_method_return_error (info->context, error); + g_error_free (error); info->timeout_id = 0; pending_connection_info_destroy (priv->pending_connection_info); @@ -1219,6 +1258,8 @@ connection_added_default_handler (NMManager *manager, NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); PendingConnectionInfo *info = priv->pending_connection_info; const char *path; + gboolean success; + GError *error = NULL; if (!info) return; @@ -1236,14 +1277,21 @@ connection_added_default_handler (NMManager *manager, // FIXME: remove old_dev deactivation when multiple device support lands deactivate_old_device (manager); - if (nm_manager_activate_device (manager, info->device, connection, info->specific_object_path, TRUE)) { + success = nm_manager_activate_device (manager, + info->device, + connection, + info->specific_object_path, + TRUE, + &error); + if (success) dbus_g_method_return (info->context, TRUE); - } else { - GError *err; - - err = nm_manager_error_new ("Error in device activation"); - dbus_g_method_return_error (info->context, err); - g_error_free (err); + else { + dbus_g_method_return_error (info->context, error); + nm_warning ("Failed to activate device %s: (%d) %s", + nm_device_get_iface (info->device), + error->code, + error->message); + g_error_free (error); } pending_connection_info_destroy (info); @@ -1260,12 +1308,14 @@ impl_manager_activate_device (NMManager *manager, NMDevice *device; NMConnectionType connection_type; NMConnection *connection; - GError *err = NULL; + GError *error = NULL; char *real_sop = NULL; device = nm_manager_get_device_by_path (manager, device_path); if (!device) { - err = nm_manager_error_new ("Could not find device"); + g_set_error (&error, + NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, + "%s", "Device not found"); goto err; } @@ -1276,7 +1326,9 @@ impl_manager_activate_device (NMManager *manager, else if (!strcmp (service_name, NM_DBUS_SERVICE_SYSTEM_SETTINGS)) connection_type = NM_CONNECTION_TYPE_SYSTEM; else { - err = nm_manager_error_new ("Invalid service name"); + g_set_error (&error, + NM_MANAGER_ERROR, NM_MANAGER_ERROR_INVALID_SERVICE, + "%s", "Invalid settings service name"); goto err; } @@ -1286,15 +1338,19 @@ impl_manager_activate_device (NMManager *manager, connection = nm_manager_get_connection_by_object_path (manager, connection_type, connection_path); if (connection) { + gboolean success; + // FIXME: remove old_dev deactivation when multiple device support lands deactivate_old_device (manager); - if (nm_manager_activate_device (manager, device, connection, real_sop, TRUE)) { + success = nm_manager_activate_device (manager, + device, + connection, + real_sop, + TRUE, + &error); + if (success) dbus_g_method_return (context, TRUE); - } else { - err = nm_manager_error_new ("Error in device activation"); - goto err; - } } else { PendingConnectionInfo *info; NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); @@ -1321,9 +1377,13 @@ impl_manager_activate_device (NMManager *manager, } err: - if (err) { - dbus_g_method_return_error (context, err); - g_error_free (err); + if (error) { + dbus_g_method_return_error (context, error); + nm_warning ("Failed to activate device %s: (%d) %s", + nm_device_get_iface (device), + error->code, + error->message); + g_error_free (error); } g_free (real_sop); diff --git a/src/nm-manager.h b/src/nm-manager.h index 46ca287b99..2ed7a7c5a4 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -74,7 +74,8 @@ gboolean nm_manager_activate_device (NMManager *manager, NMDevice *device, NMConnection *connection, const char *specific_object, - gboolean user_requested); + gboolean user_requested, + GError **error); gboolean nm_manager_activation_pending (NMManager *manager); diff --git a/src/nm-serial-device.c b/src/nm-serial-device.c index d813a81d90..dc9767e4d7 100644 --- a/src/nm-serial-device.c +++ b/src/nm-serial-device.c @@ -868,20 +868,26 @@ real_deactivate_quickly (NMDevice *device) } static gboolean -real_check_connection (NMDevice *dev, NMConnection *connection) +real_check_connection (NMDevice *dev, NMConnection *connection, GError **error) { NMSettingSerial *serial; NMSettingPPP *ppp; serial = (NMSettingSerial *) nm_connection_get_setting (connection, NM_TYPE_SETTING_SERIAL); if (!serial) { - nm_warning ("Connection check failed: serial setting not present."); + g_set_error (error, + NM_DEVICE_INTERFACE_ERROR, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, + "%s", "Connection invalid: serial setting not present"); return FALSE; } ppp = (NMSettingPPP *) nm_connection_get_setting (connection, NM_TYPE_SETTING_PPP); if (!ppp) { - nm_warning ("Connection check failed: ppp setting not present."); + g_set_error (error, + NM_DEVICE_INTERFACE_ERROR, + NM_DEVICE_INTERFACE_ERROR_CONNECTION_INVALID, + "%s", "Connection invalid: PPP setting not present"); return FALSE; }