keyfile: merge branch 'th/keyfile-write-connection-bgo740738'

https://bugzilla.gnome.org/show_bug.cgi?id=740738

10 files changed, 85 insertions, 39 deletions

diff --git a/src/nm-manager.c b/src/nm-manager.c
index cd73125213..d8072d1d69 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -3221,7 +3221,9 @@ activation_add_done (NMSettings *self,
 		nm_active_connection_set_connection (info->active, NM_CONNECTION (new_connection));
 
 		if (_internal_activate_generic (info->manager, info->active, &local)) {
-			nm_settings_connection_commit_changes (new_connection, NULL, NULL);
+			nm_settings_connection_commit_changes (new_connection,
+			                                       NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION | NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED,
+			                                       NULL, NULL);
 			dbus_g_method_return (context,
 			                      nm_connection_get_path (NM_CONNECTION (new_connection)),
 			                      nm_active_connection_get_path (info->active));
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index cc0a5b6b53..c25dfac01a 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -593,9 +593,14 @@ replace_and_commit (NMSettingsConnection *self,
                     gpointer user_data)
 {
 	GError *error = NULL;
+	NMSettingsConnectionCommitReason commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION;
+
+	if (g_strcmp0 (nm_connection_get_id (NM_CONNECTION (self)),
+	               nm_connection_get_id (new_connection)) != 0)
+		commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED;
 
 	if (nm_settings_connection_replace_settings (self, new_connection, TRUE, "replace-and-commit-disk", &error))
-		nm_settings_connection_commit_changes (self, callback, user_data);
+		nm_settings_connection_commit_changes (self, commit_reason, callback, user_data);
 	else {
 		g_assert (error);
 		if (callback)
@@ -618,6 +623,7 @@ nm_settings_connection_replace_and_commit (NMSettingsConnection *self,
 
 static void
 commit_changes (NMSettingsConnection *self,
+                NMSettingsConnectionCommitReason commit_reason,
                 NMSettingsConnectionCommitFunc callback,
                 gpointer user_data)
 {
@@ -633,6 +639,7 @@ commit_changes (NMSettingsConnection *self,
 
 void
 nm_settings_connection_commit_changes (NMSettingsConnection *self,
+                                       NMSettingsConnectionCommitReason commit_reason,
                                        NMSettingsConnectionCommitFunc callback,
                                        gpointer user_data)
 {
@@ -640,6 +647,7 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self,
 
 	if (NM_SETTINGS_CONNECTION_GET_CLASS (self)->commit_changes) {
 		NM_SETTINGS_CONNECTION_GET_CLASS (self)->commit_changes (self,
+		                                                         commit_reason,
 		                                                         callback ? callback : ignore_cb,
 		                                                         user_data);
 	} else {
@@ -916,7 +924,7 @@ agent_secrets_done_cb (NMAgentManager *manager,
 				       setting_name,
 				       call_id);
 
-				nm_settings_connection_commit_changes (self, new_secrets_commit_cb, NULL);
+				nm_settings_connection_commit_changes (self, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, new_secrets_commit_cb, NULL);
 			} else {
 				_LOGD ("(%s:%u) new agent secrets processed",
 				       setting_name,
@@ -1743,7 +1751,7 @@ dbus_clear_secrets_auth_cb (NMSettingsConnection *self,
 		/* Tell agents to remove secrets for this connection */
 		nm_agent_manager_delete_secrets (priv->agent_mgr, NM_CONNECTION (self));
 
-		nm_settings_connection_commit_changes (self, clear_secrets_cb, context);
+		nm_settings_connection_commit_changes (self, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, clear_secrets_cb, context);
 	}
 }
 
diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h
index 512112f032..0d3963d3df 100644
--- a/src/settings/nm-settings-connection.h
+++ b/src/settings/nm-settings-connection.h
@@ -78,6 +78,11 @@ typedef enum
 	NM_SETTINGS_CONNECTION_FLAGS_ALL = ((__NM_SETTINGS_CONNECTION_FLAGS_LAST - 1) << 1) - 1,
 } NMSettingsConnectionFlags;
 
+typedef enum { /*< skip >*/
+	NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE                       = 0,
+	NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION                = (1LL << 0),
+	NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED                 = (1LL << 1),
+} NMSettingsConnectionCommitReason;
 
 typedef struct _NMSettingsConnectionClass NMSettingsConnectionClass;
 
@@ -103,6 +108,7 @@ struct _NMSettingsConnectionClass {
 	                            gpointer user_data);
 
 	void (*commit_changes) (NMSettingsConnection *self,
+	                        NMSettingsConnectionCommitReason commit_reason,
 	                        NMSettingsConnectionCommitFunc callback,
 	                        gpointer user_data);
 
@@ -117,6 +123,7 @@ struct _NMSettingsConnectionClass {
 GType nm_settings_connection_get_type (void);
 
 void nm_settings_connection_commit_changes (NMSettingsConnection *self,
+                                            NMSettingsConnectionCommitReason commit_reason,
                                             NMSettingsConnectionCommitFunc callback,
                                             gpointer user_data);
 
diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
index a18920c685..277e9d7a20 100644
--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
+++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c
@@ -365,6 +365,7 @@ replace_and_commit (NMSettingsConnection *connection,
 
 static void
 commit_changes (NMSettingsConnection *connection,
+                NMSettingsConnectionCommitReason commit_reason,
                 NMSettingsConnectionCommitFunc callback,
                 gpointer user_data)
 {
@@ -392,7 +393,7 @@ commit_changes (NMSettingsConnection *connection,
 			/* Don't bother writing anything out if in-memory and on-disk data are the same */
 			if (same) {
 				/* But chain up to parent to handle success - emits updated signal */
-				NM_SETTINGS_CONNECTION_CLASS (nm_ifcfg_connection_parent_class)->commit_changes (connection, callback, user_data);
+				NM_SETTINGS_CONNECTION_CLASS (nm_ifcfg_connection_parent_class)->commit_changes (connection, commit_reason, callback, user_data);
 				return;
 			}
 		}
@@ -415,7 +416,7 @@ commit_changes (NMSettingsConnection *connection,
 
 	if (success) {
 		/* Chain up to parent to handle success */
-		NM_SETTINGS_CONNECTION_CLASS (nm_ifcfg_connection_parent_class)->commit_changes (connection, callback, user_data);
+		NM_SETTINGS_CONNECTION_CLASS (nm_ifcfg_connection_parent_class)->commit_changes (connection, commit_reason, callback, user_data);
 	} else {
 		/* Otherwise immediate error */
 		callback (connection, error, user_data);
diff --git a/src/settings/plugins/ifnet/nm-ifnet-connection.c b/src/settings/plugins/ifnet/nm-ifnet-connection.c
index 693e653288..b31246408e 100644
--- a/src/settings/plugins/ifnet/nm-ifnet-connection.c
+++ b/src/settings/plugins/ifnet/nm-ifnet-connection.c
@@ -102,8 +102,9 @@ nm_ifnet_connection_get_conn_name (NMIfnetConnection *connection)
 
 static void
 commit_changes (NMSettingsConnection *connection,
+                NMSettingsConnectionCommitReason commit_reason,
                 NMSettingsConnectionCommitFunc callback,
-	            gpointer user_data)
+                gpointer user_data)
 {
 	GError *error = NULL;
 	NMIfnetConnectionPrivate *priv = NM_IFNET_CONNECTION_GET_PRIVATE (connection);
@@ -139,7 +140,7 @@ commit_changes (NMSettingsConnection *connection,
 		g_free (priv->conn_name);
 		priv->conn_name = new_name;
 
-		NM_SETTINGS_CONNECTION_CLASS (nm_ifnet_connection_parent_class)->commit_changes (connection, callback, user_data);
+		NM_SETTINGS_CONNECTION_CLASS (nm_ifnet_connection_parent_class)->commit_changes (connection, commit_reason, callback, user_data);
 		nm_log_info (LOGD_SETTINGS, "Successfully updated %s", priv->conn_name);
 	} else {
 		nm_log_warn (LOGD_SETTINGS, "Failed to update %s",
diff --git a/src/settings/plugins/ifupdown/plugin.c b/src/settings/plugins/ifupdown/plugin.c
index c3cb180b58..d67405b037 100644
--- a/src/settings/plugins/ifupdown/plugin.c
+++ b/src/settings/plugins/ifupdown/plugin.c
@@ -193,7 +193,7 @@ bind_device_to_connection (SCPluginIfupdown *self,
 		g_object_set (s_wifi, NM_SETTING_WIRELESS_MAC_ADDRESS, address, NULL);
 	}
 
-	nm_settings_connection_commit_changes (NM_SETTINGS_CONNECTION (exported), NULL, NULL);
+	nm_settings_connection_commit_changes (NM_SETTINGS_CONNECTION (exported), NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, NULL, NULL);
 }    
 
 static void
diff --git a/src/settings/plugins/keyfile/nm-keyfile-connection.c b/src/settings/plugins/keyfile/nm-keyfile-connection.c
index 8fb94302d7..744ae9ccf3 100644
--- a/src/settings/plugins/keyfile/nm-keyfile-connection.c
+++ b/src/settings/plugins/keyfile/nm-keyfile-connection.c
@@ -33,6 +33,9 @@
 #include "reader.h"
 #include "writer.h"
 #include "common.h"
+#include "utils.h"
+#include "gsystem-local-alloc.h"
+#include "nm-logging.h"
 
 G_DEFINE_TYPE (NMKeyfileConnection, nm_keyfile_connection, NM_TYPE_SETTINGS_CONNECTION)
 
@@ -88,6 +91,7 @@ nm_keyfile_connection_new (NMConnection *source,
 
 static void
 commit_changes (NMSettingsConnection *connection,
+                NMSettingsConnectionCommitReason commit_reason,
                 NMSettingsConnectionCommitFunc callback,
                 gpointer user_data)
 {
@@ -96,6 +100,8 @@ commit_changes (NMSettingsConnection *connection,
 
 	if (!nm_keyfile_plugin_write_connection (NM_CONNECTION (connection),
 	                                         nm_settings_connection_get_filename (connection),
+	                                         NM_FLAGS_ALL (commit_reason,   NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION
+	                                                                      | NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED),
 	                                         &path,
 	                                         &error)) {
 		callback (connection, error, user_data);
@@ -104,10 +110,28 @@ commit_changes (NMSettingsConnection *connection,
 	}
 
 	/* Update the filename if it changed */
-	if (path)
+	if (   path
+	    && g_strcmp0 (path, nm_settings_connection_get_filename (connection)) != 0) {
+		gs_free char *old_path = g_strdup (nm_settings_connection_get_filename (connection));
+
 		nm_settings_connection_set_filename (connection, path);
+		if (old_path) {
+			nm_log_info (LOGD_SETTINGS, "keyfile: update "NM_KEYFILE_CONNECTION_LOG_FMT" and rename from \"%s\"",
+			             NM_KEYFILE_CONNECTION_LOG_ARG (connection),
+			             old_path);
+		} else {
+			nm_log_info (LOGD_SETTINGS, "keyfile: update "NM_KEYFILE_CONNECTION_LOG_FMT" and persist connection",
+			             NM_KEYFILE_CONNECTION_LOG_ARG (connection));
+		}
+	} else {
+		nm_log_info (LOGD_SETTINGS, "keyfile: update "NM_KEYFILE_CONNECTION_LOG_FMT,
+		             NM_KEYFILE_CONNECTION_LOG_ARG (connection));
+	}
+
+	g_free (path);
 
 	NM_SETTINGS_CONNECTION_CLASS (nm_keyfile_connection_parent_class)->commit_changes (connection,
+	                                                                                   commit_reason,
 	                                                                                   callback,
 	                                                                                   user_data);
 }
diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c
index 57b0d094fb..71f407dd57 100644
--- a/src/settings/plugins/keyfile/plugin.c
+++ b/src/settings/plugins/keyfile/plugin.c
@@ -514,7 +514,7 @@ add_connection (NMSystemConfigInterface *config,
 	gs_free char *path = NULL;
 
 	if (save_to_disk) {
-		if (!nm_keyfile_plugin_write_connection (connection, NULL, &path, error))
+		if (!nm_keyfile_plugin_write_connection (connection, NULL, FALSE, &path, error))
 			return NULL;
 	}
 	return NM_SETTINGS_CONNECTION (update_connection (self, connection, path, NULL, FALSE, NULL, error));
diff --git a/src/settings/plugins/keyfile/writer.c b/src/settings/plugins/keyfile/writer.c
index b6a8786dc4..e6ee7f97cc 100644
--- a/src/settings/plugins/keyfile/writer.c
+++ b/src/settings/plugins/keyfile/writer.c
@@ -34,6 +34,7 @@
 #include "common.h"
 #include "utils.h"
 #include "nm-keyfile-internal.h"
+#include "gsystem-local-alloc.h"
 
 
 typedef struct {
@@ -233,17 +234,18 @@ _internal_write_connection (NMConnection *connection,
                             uid_t owner_uid,
                             pid_t owner_grp,
                             const char *existing_path,
+                            gboolean force_rename,
                             char **out_path,
                             GError **error)
 {
 	GKeyFile *key_file;
-	char *data;
+	gs_free char *data = NULL;
 	gsize len;
-	gboolean success = FALSE;
-	char *path;
+	gs_free char *path = NULL;
 	const char *id;
 	WriteInfo info = { 0 };
 	GError *local_err = NULL;
+	int errsv;
 
 	g_return_val_if_fail (!out_path || !*out_path, FALSE);
 	g_return_val_if_fail (keyfile_dir && keyfile_dir[0] == '/', FALSE);
@@ -267,7 +269,7 @@ _internal_write_connection (NMConnection *connection,
 	/* If we have existing file path, use it. Else generate one from
 	 * connection's ID.
 	 */
-	if (existing_path != NULL) {
+	if (existing_path != NULL && !force_rename) {
 		path = g_strdup (existing_path);
 	} else {
 		char *filename_escaped = nm_keyfile_plugin_utils_escape_filename (id);
@@ -312,8 +314,7 @@ _internal_write_connection (NMConnection *connection,
 				/* this really should not happen, we tried hard to find an unused name... bail out. */
 				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 				                    "could not find suitable keyfile file name (%s already used)", path);
-				g_free (path);
-				goto out;
+				return FALSE;
 			}
 			/* Both our preferred path based on connection id and id-uuid are taken.
 			 * Fallback to @existing_path */
@@ -331,42 +332,41 @@ _internal_write_connection (NMConnection *connection,
 	g_file_set_contents (path, data, len, &local_err);
 	if (local_err) {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "%s.%d: error writing to file '%s': %s", __FILE__, __LINE__,
+		             "error writing to file '%s': %s",
 		             path, local_err->message);
 		g_error_free (local_err);
-		g_free (path);
-		goto out;
+		return FALSE;
 	}
 
 	if (chown (path, owner_uid, owner_grp) < 0) {
+		errsv = errno;
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "%s.%d: error chowning '%s': %d", __FILE__, __LINE__,
-		             path, errno);
+		             "error chowning '%s': %s (%d)",
+		             path, g_strerror (errsv), errsv);
 		unlink (path);
-	} else {
-		if (chmod (path, S_IRUSR | S_IWUSR) < 0) {
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-			             "%s.%d: error setting permissions on '%s': %d", __FILE__,
-			             __LINE__, path, errno);
-			unlink (path);
-		} else {
-			if (out_path && g_strcmp0 (existing_path, path)) {
-				*out_path = path;  /* pass path out to caller */
-				path = NULL;
-			}
-			success = TRUE;
-		}
+		return FALSE;
 	}
-	g_free (path);
 
-out:
-	g_free (data);
-	return success;
+	if (chmod (path, S_IRUSR | S_IWUSR) < 0) {
+		errsv = errno;
+		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+		             "error setting permissions on '%s': %s (%d)",
+		             path, g_strerror (errsv), errsv);
+		unlink (path);
+		return FALSE;
+	}
+
+	if (out_path && g_strcmp0 (existing_path, path)) {
+		*out_path = path;  /* pass path out to caller */
+		path = NULL;
+	}
+	return TRUE;
 }
 
 gboolean
 nm_keyfile_plugin_write_connection (NMConnection *connection,
                                     const char *existing_path,
+                                    gboolean force_rename,
                                     char **out_path,
                                     GError **error)
 {
@@ -374,6 +374,7 @@ nm_keyfile_plugin_write_connection (NMConnection *connection,
 	                                   KEYFILE_DIR,
 	                                   0, 0,
 	                                   existing_path,
+	                                   force_rename,
 	                                   out_path,
 	                                   error);
 }
@@ -390,6 +391,7 @@ nm_keyfile_plugin_write_test_connection (NMConnection *connection,
 	                                   keyfile_dir,
 	                                   owner_uid, owner_grp,
 	                                   NULL,
+	                                   FALSE,
 	                                   out_path,
 	                                   error);
 }
diff --git a/src/settings/plugins/keyfile/writer.h b/src/settings/plugins/keyfile/writer.h
index 8b08812d86..95885106df 100644
--- a/src/settings/plugins/keyfile/writer.h
+++ b/src/settings/plugins/keyfile/writer.h
@@ -27,6 +27,7 @@
 
 gboolean nm_keyfile_plugin_write_connection (NMConnection *connection,
                                              const char *existing_path,
+                                             gboolean force_rename,
                                              char **out_path,
                                              GError **error);