From bbd4c232137723d9f241ee33ca40264046f95a4e Mon Sep 17 00:00:00 2001 From: Daniel Gnoutcheff Date: Wed, 4 Aug 2010 04:45:43 -0400 Subject: [PATCH] split nm_sysconfig_connection_update The various "update" functions implemented by NMSysconfigConnection have become confusing. Depending on how you count, we've wound up with about 4 functions that all share the name "update" but nonetheless do different things. These functions used to be distributed over several interfaces implemented by NMSysconfigConnection, but now that we've removed NMExportedConnection and are about to remove NMSettingsConnectionInterface, they will be all crammed into a single interface and will be even more confusing than before. It's time to give better names to these guys. The renames planned are: - nm_settings_connection_interface_update() --> nm_sysconfig_connection_commit_changes() - nm_sysconfig_connection_update() with signal_update==FALSE --> nm_sysconfig_connection_replace_settings() - nm_sysconfig_connection_update() with signal_update==TRUE --> nm_sysconfig_connection_replace_and_commit() This commit performs the last two renames. The first will be performed when removing NMSettingsConnectionInterface. We also have nm_sysconfig_connection_replace_and_commit() have an async-ish API that accepts a callback. This fits nicely with the async-ish API of nm_settings_connection_interface_update(), and it lets us clean up pk_update_cb() a bit. --- src/system-settings/nm-sysconfig-connection.c | 95 +++++++++++-------- src/system-settings/nm-sysconfig-connection.h | 17 ++-- src/system-settings/nm-sysconfig-settings.c | 3 +- .../plugins/ifcfg-rh/nm-ifcfg-connection.c | 2 +- system-settings/plugins/ifcfg-rh/plugin.c | 22 +++-- .../plugins/keyfile/nm-keyfile-connection.c | 2 +- system-settings/plugins/keyfile/plugin.c | 28 +++--- 7 files changed, 99 insertions(+), 70 deletions(-) diff --git a/src/system-settings/nm-sysconfig-connection.c b/src/system-settings/nm-sysconfig-connection.c index c55ea70124..8d0c061e8a 100644 --- a/src/system-settings/nm-sysconfig-connection.c +++ b/src/system-settings/nm-sysconfig-connection.c @@ -67,18 +67,12 @@ typedef struct { /**************************************************************/ -static void -ignore_cb (NMSettingsConnectionInterface *connection, - GError *error, - gpointer user_data) -{ -} - +/* Update the settings of this connection to match that of 'new', taking care to + * make a private copy of secrets. */ gboolean -nm_sysconfig_connection_update (NMSysconfigConnection *self, - NMConnection *new, - gboolean signal_update, - GError **error) +nm_sysconfig_connection_replace_settings (NMSysconfigConnection *self, + NMConnection *new, + GError **error) { NMSysconfigConnectionPrivate *priv; GHashTable *new_settings; @@ -91,12 +85,6 @@ nm_sysconfig_connection_update (NMSysconfigConnection *self, priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (self); - /* Do nothing if there's nothing to update */ - if (nm_connection_compare (NM_CONNECTION (self), - NM_CONNECTION (new), - NM_SETTING_COMPARE_FLAG_EXACT)) - return TRUE; - new_settings = nm_connection_to_hash (new); g_assert (new_settings); if (nm_connection_replace_settings (NM_CONNECTION (self), new_settings, error)) { @@ -107,17 +95,57 @@ nm_sysconfig_connection_update (NMSysconfigConnection *self, g_object_unref (priv->secrets); priv->secrets = nm_connection_duplicate (NM_CONNECTION (self)); - if (signal_update) { - nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (self), - ignore_cb, - NULL); - } success = TRUE; } g_hash_table_destroy (new_settings); return success; } +static void +ignore_cb (NMSettingsConnectionInterface *connection, + GError *error, + gpointer user_data) +{ +} + +/* Replaces the settings in this connection with those in 'new'. If any changes + * are made, commits them to permanent storage and to any other subsystems + * watching this connection. Before returning, 'callback' is run with the given + * 'user_data' along with any errors encountered. + */ +void +nm_sysconfig_connection_replace_and_commit (NMSysconfigConnection *self, + NMConnection *new, + NMSettingsConnectionInterfaceUpdateFunc callback, + gpointer user_data) +{ + GError *error = NULL; + + g_return_if_fail (self != NULL); + g_return_if_fail (NM_IS_SYSCONFIG_CONNECTION (self)); + g_return_if_fail (new != NULL); + g_return_if_fail (NM_IS_CONNECTION (new)); + + if (!callback) + callback = ignore_cb; + + /* Do nothing if there's nothing to update */ + if (nm_connection_compare (NM_CONNECTION (self), + NM_CONNECTION (new), + NM_SETTING_COMPARE_FLAG_EXACT)) { + callback (NM_SETTINGS_CONNECTION_INTERFACE (self), NULL, user_data); + return; + } + + if (nm_sysconfig_connection_replace_settings (self, new, &error)) { + nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (self), + callback, user_data); + } else { + callback (NM_SETTINGS_CONNECTION_INTERFACE (self), error, user_data); + g_clear_error (&error); + } +} + /**************************************************************/ static gboolean @@ -236,7 +264,7 @@ get_secrets (NMSettingsConnectionInterface *connection, /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() * will clear secrets on this object's settings. priv->secrets should be * a complete copy of this object and kept in sync by - * nm_sysconfig_connection_update(). + * nm_sysconfig_connection_replace_settings(). */ if (!priv->secrets) { error = g_error_new (NM_SYSCONFIG_SETTINGS_ERROR, @@ -452,22 +480,11 @@ pk_update_cb (GObject *object, GAsyncResult *result, gpointer user_data) goto out; } - /* Update our settings internally so the update() call will save the new - * ones. We don't let nm_sysconfig_connection_update() handle the update - * signal since we need our own callback after the update is done. - */ - if (!nm_sysconfig_connection_update (self, call->connection, FALSE, &error)) { - /* Shouldn't really happen since we've already validated the settings */ - dbus_g_method_return_error (call->context, error); - g_error_free (error); - polkit_call_free (call); - goto out; - } - - /* Caller is authenticated, now we can finally try to commit the update */ - nm_settings_connection_interface_update (NM_SETTINGS_CONNECTION_INTERFACE (self), - con_update_cb, - call); + /* Update and commit our settings. */ + nm_sysconfig_connection_replace_and_commit (self, + call->connection, + con_update_cb, + call); out: g_object_unref (pk_result); diff --git a/src/system-settings/nm-sysconfig-connection.h b/src/system-settings/nm-sysconfig-connection.h index 3c43bf914d..253c28750e 100644 --- a/src/system-settings/nm-sysconfig-connection.h +++ b/src/system-settings/nm-sysconfig-connection.h @@ -22,6 +22,7 @@ #define NM_SYSCONFIG_CONNECTION_H #include +#include #include G_BEGIN_DECLS @@ -43,13 +44,15 @@ typedef struct { GType nm_sysconfig_connection_get_type (void); -/* Called by a system-settings plugin to update a connection is out of sync - * with it's backing storage. - */ -gboolean nm_sysconfig_connection_update (NMSysconfigConnection *self, - NMConnection *new_settings, - gboolean signal_update, - GError **error); +gboolean nm_sysconfig_connection_replace_settings (NMSysconfigConnection *self, + NMConnection *new_settings, + GError **error); + +void nm_sysconfig_connection_replace_and_commit (NMSysconfigConnection *self, + NMConnection *new_settings, + NMSettingsConnectionInterfaceUpdateFunc callback, + gpointer user_data); + G_END_DECLS diff --git a/src/system-settings/nm-sysconfig-settings.c b/src/system-settings/nm-sysconfig-settings.c index 2084db0c1b..c6294cbf86 100644 --- a/src/system-settings/nm-sysconfig-settings.c +++ b/src/system-settings/nm-sysconfig-settings.c @@ -71,7 +71,8 @@ EXPORT(nm_inotify_helper_add_watch) EXPORT(nm_inotify_helper_remove_watch) EXPORT(nm_sysconfig_connection_get_type) -EXPORT(nm_sysconfig_connection_update) +EXPORT(nm_sysconfig_connection_replace_settings) +EXPORT(nm_sysconfig_connection_replace_and_commit) /* END LINKER CRACKROCK */ static void claim_connection (NMSysconfigSettings *self, diff --git a/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c index 52ce6e152d..64f29ecaa7 100644 --- a/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c +++ b/system-settings/plugins/ifcfg-rh/nm-ifcfg-connection.c @@ -133,7 +133,7 @@ nm_ifcfg_connection_new (const char *filename, } /* Update our settings with what was read from the file */ - nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (object), tmp, FALSE, NULL); + nm_sysconfig_connection_replace_settings (NM_SYSCONFIG_CONNECTION (object), tmp, NULL); g_object_unref (tmp); priv = NM_IFCFG_CONNECTION_GET_PRIVATE (object); diff --git a/system-settings/plugins/ifcfg-rh/plugin.c b/system-settings/plugins/ifcfg-rh/plugin.c index acfed7652d..f6db495726 100644 --- a/system-settings/plugins/ifcfg-rh/plugin.c +++ b/system-settings/plugins/ifcfg-rh/plugin.c @@ -201,6 +201,17 @@ read_connections (SCPluginIfcfg *plugin) /* Monitoring */ +/* Callback for nm_sysconfig_connection_replace_and_commit. Report any errors + * encountered when commiting connection settings updates. */ +static void +commit_cb (NMSettingsConnectionInterface *connection, GError *error, gpointer unused) +{ + if (error) { + PLUGIN_WARN (IFCFG_PLUGIN_NAME, " error updating: %s", + (error && error->message) ? error->message : "(unknown)"); + } +} + static void connection_changed_handler (SCPluginIfcfg *plugin, const char *path, @@ -261,14 +272,9 @@ connection_changed_handler (SCPluginIfcfg *plugin, g_signal_emit_by_name (plugin, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection); } - if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (connection), - NM_CONNECTION (new), - TRUE, - &error)) { - PLUGIN_WARN (IFCFG_PLUGIN_NAME, " error updating: %s", - (error && error->message) ? error->message : "(unknown)"); - g_clear_error (&error); - } + nm_sysconfig_connection_replace_and_commit (NM_SYSCONFIG_CONNECTION (connection), + NM_CONNECTION (new), + commit_cb, NULL); /* Update unmanaged status */ g_object_set (connection, "unmanaged", new_unmanaged, NULL); diff --git a/system-settings/plugins/keyfile/nm-keyfile-connection.c b/system-settings/plugins/keyfile/nm-keyfile-connection.c index 1c90961bef..b5e5b5c141 100644 --- a/system-settings/plugins/keyfile/nm-keyfile-connection.c +++ b/system-settings/plugins/keyfile/nm-keyfile-connection.c @@ -147,7 +147,7 @@ constructor (GType type, return NULL; } - nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (object), tmp, FALSE, NULL); + nm_sysconfig_connection_replace_settings (NM_SYSCONFIG_CONNECTION (object), tmp, NULL); g_object_unref (tmp); /* if for some reason the connection didn't have a UUID, add one */ diff --git a/system-settings/plugins/keyfile/plugin.c b/system-settings/plugins/keyfile/plugin.c index 36f47ccdc5..5b82b5b6fa 100644 --- a/system-settings/plugins/keyfile/plugin.c +++ b/system-settings/plugins/keyfile/plugin.c @@ -125,26 +125,28 @@ find_by_uuid (gpointer key, gpointer data, gpointer user_data) } static void -update_connection_settings (NMKeyfileConnection *orig, - NMKeyfileConnection *new) -{ - GError *error = NULL; - - if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (orig), - NM_CONNECTION (new), - TRUE, - &error)) { +update_connection_settings_commit_cb (NMSettingsConnectionInterface *orig, GError *error, gpointer user_data) { + if (error) { g_warning ("%s: '%s' / '%s' invalid: %d", - __func__, - error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)", - (error && error->message) ? error->message : "(none)", - error ? error->code : -1); + __func__, + error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)", + (error && error->message) ? error->message : "(none)", + error ? error->code : -1); g_clear_error (&error); g_signal_emit_by_name (orig, "removed"); } } +static void +update_connection_settings (NMKeyfileConnection *orig, + NMKeyfileConnection *new) +{ + nm_sysconfig_connection_replace_and_commit (NM_SYSCONFIG_CONNECTION (orig), + NM_CONNECTION (new), + update_connection_settings_commit_cb, NULL); +} + /* Monitoring */ static void