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

@@ -67,18 +67,12 @@ typedef struct {
 
 /**************************************************************/
 
-static void
+/* Update the settings of this connection to match that of 'new', taking care to
-ignore_cb (NMSettingsConnectionInterface *connection,
+ * make a private copy of secrets. */
-           GError *error,
-           gpointer user_data)
-{
-}
-
 gboolean
-nm_sysconfig_connection_update (NMSysconfigConnection *self,
+nm_sysconfig_connection_replace_settings (NMSysconfigConnection *self,
-                                NMConnection *new,
+                                          NMConnection *new,
-                                gboolean signal_update,
+                                          GError **error)
-                                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
+	/* Update and commit our settings. */
-	 * ones.  We don't let nm_sysconfig_connection_update() handle the update
+	nm_sysconfig_connection_replace_and_commit (self, 
-	 * signal since we need our own callback after the update is done.
+	                                            call->connection,
-	 */
+	                                            con_update_cb,
-	if (!nm_sysconfig_connection_update (self, call->connection, FALSE, &error)) {
+	                                            call);
-		/* 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);
 
 out:
 	g_object_unref (pk_result);

src/system-settings/nm-sysconfig-connection.h

@@ -22,6 +22,7 @@
 #define NM_SYSCONFIG_CONNECTION_H
 
 #include <nm-connection.h>
+#include <nm-settings-connection-interface.h>
 #include <dbus/dbus-glib.h>
 
 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
+gboolean nm_sysconfig_connection_replace_settings (NMSysconfigConnection *self,
- * with it's backing storage.
+                                                   NMConnection *new_settings,
- */
+                                                   GError **error);
-gboolean nm_sysconfig_connection_update (NMSysconfigConnection *self,
+
-                                         NMConnection *new_settings,
+void nm_sysconfig_connection_replace_and_commit (NMSysconfigConnection *self,
-                                         gboolean signal_update,
+                                                 NMConnection *new_settings,
-                                         GError **error);
+                                                 NMSettingsConnectionInterfaceUpdateFunc callback,
+                                                 gpointer user_data);
+
 
 G_END_DECLS
 

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,

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);

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_sysconfig_connection_replace_and_commit (NM_SYSCONFIG_CONNECTION (connection),
-		                                     NM_CONNECTION (new),
+		                                            NM_CONNECTION (new),
-		                                     TRUE,
+		                                            commit_cb, NULL);
-		                                     &error)) {
-			PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    error updating: %s",
-			             (error && error->message) ? error->message : "(unknown)");
-			g_clear_error (&error);
-		}
 
 		/* Update unmanaged status */
 		g_object_set (connection, "unmanaged", new_unmanaged, NULL);

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 */

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,
+update_connection_settings_commit_cb (NMSettingsConnectionInterface *orig, GError *error, gpointer user_data) {
-                            NMKeyfileConnection *new)
+	if (error) {
-{
-	GError *error = NULL;
-
-	if (!nm_sysconfig_connection_update (NM_SYSCONFIG_CONNECTION (orig),
-	                                     NM_CONNECTION (new),
-	                                     TRUE,
-	                                     &error)) {
 		g_warning ("%s: '%s' / '%s' invalid: %d",
-		           __func__,
+		       	   __func__,
-		           error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)",
+		       	   error ? g_type_name (nm_connection_lookup_setting_type_by_quark (error->domain)) : "(none)",
-		           (error && error->message) ? error->message : "(none)",
+		       	   (error && error->message) ? error->message : "(none)",
-		           error ? error->code : -1);
+		       	   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