settings: merge branch 'th/various-settings-cleanup-3'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/187

27 files changed, 365 insertions, 196 deletions

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 68026a81b5..febfdf0439 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -55,51 +55,66 @@ There are different kind of assertions. Use the one that is appropriate.
 1) g_return_*() from glib. This is usually enabled in release builds and
   can be disabled with G_DISABLE_CHECKS define. This uses g_log() with
   G_LOG_LEVEL_CRITICAL level (which allows the program to continue,
-  until G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings is set). As such,
-  this is the preferred way for assertions that are commonly enabled.
-
-  Make a mild attempt to work around such assertion failure, but don't try
-  to hard. A failure of g_return_*() assertion might allow the process
-  to continue, but there is no guarantee.
+  unless G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings is set). As such,
+  this is usually the preferred way for assertions that are supposed to be
+  enabled by default.
+
+  Optimally, after a g_return_*() failure the program can still continue. This is
+  also the reason why g_return_*() is preferable over g_assert().
+  For example, that is often not given for functions that return a GError, because
+  g_return_*() will return failure without setting the error output. That often leads
+  to a crash immidiately after, because the caller requires the GError to be set.
+  Make a reasonable effort so that an assertion failure may allow the process
+  to proceed. But don't put too much effort in it. After all, it's an assertion
+  failure that is not supposed to happen either way.
 
 2) nm_assert() from NetworkManager. This is disabled by default in release
   builds, but enabled if you build --with-more-assertions. See "WITH_MORE_ASSERTS"
   define. This is preferred for assertions that are expensive to check or
-  nor necessary to check frequently (stuff that is really not expected to
-  fail). Use this deliberately and assume it's not present in production builds.
+  nor necessary to check frequently. It's also for conditions that can easily
+  verified to be true and where future refactoring is unlikley to break that
+  condition.
+  Use this deliberately and assume it is removed from production builds.
 
 3) g_assert() from glib. This is used in unit tests and commonly enabled
   in release builds. It can be disabled with G_DISABLE_ASSERT assert
   define. Since this results in a hard crash on assertion failure, you
-  should almost always prefer g_return_*() over this (except unit tests).
+  should almost always prefer g_return_*() over this (except in unit tests).
 
 4) assert() from <assert.h>. It is usually enabled in release builds and
   can be disabled with NDEBUG define. Don't use it in NetworkManager,
   it's basically like g_assert().
 
-5) g_log() from glib. These are always compiled in, depending on the levels
+5) g_log() from glib. These are always compiled in, depending on the logging level
   these are assertions too. G_LOG_LEVEL_ERROR aborts the program, G_LOG_LEVEL_CRITICAL
   logs a critical warning (like g_return_*(), see G_DEBUG=fatal-criticals)
   and G_LOG_LEVEL_WARNING logs a warning (see G_DEBUG=fatal-warnings).
   G_LOG_LEVEL_DEBUG level is usually not printed, unless G_MESSAGES_DEBUG environment
   is set.
-  In general, avoid using g_log() in NetworkManager. We have nm-logging instead.
+  In general, avoid using g_log() in NetworkManager. We have nm-logging instead
+  which logs to syslog/systemd-journald.
   From a library like libnm it might make sense to log warnings (if someting
   is really wrong) or debug messages. But better don't. If it's important,
   find a way to report the notification via the API to the caller. If it's
   not important, keep silent.
+  In particular, don't use levels G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING because
+  these are effectively assertions and we want to run with G_DEBUG=fatal-warnings.
 
 6) g_warn_if_*() from glib. These are always compiled in and log a G_LOG_LEVEL_WARNING
   warning. Don't use this.
 
 7) G_TYPE_CHECK_INSTANCE_CAST() from glib. Unless building with "WITH_MORE_ASSERTS",
-  we disable G_DISABLE_CAST_CHECKS. This means, cast macros like NM_DEVICE(ptr)
-  translate to plain C pointer casts. Use the cast macros deliberately, in production
-  code they are cheap, with debugging enabled they assert that the pointer is valid.
+  we set G_DISABLE_CAST_CHECKS. This means, cast macros like NM_DEVICE(ptr)
+  translate to plain C pointer casts. Use such cast macros deliberately, in production
+  code they are cheap, with more asserts enabled the check that the pointer type is
+  suitable.
+
+Of course, every assertion failure is a bug, and calling it must have no side effects.
 
-Of course, every assertion failure is a bug, and they must not have side effects.
 Theoretically, you are welcome to disable G_DISABLE_CHECKS and G_DISABLE_ASSERT
 in production builds. In practice, nobody tests such a configuration, so beware.
 
-For testing, you also want to run NetworkManager with G_DEBUG=fatal-warnings
-to crash un critical and warn g_log() messages.
+For testing, you also want to run NetworkManager with environment variable
+G_DEBUG=fatal-warnings to crash upon G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING
+g_log() message. NetworkManager won't use these levels for regular logging
+but for assertions.
diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index a2bd72b0d9..c45f29b467 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -1806,23 +1806,36 @@ _nmtst_connection_unchanging_secrets_updated_cb (NMConnection *connection, const
 	nm_assert_not_reached ();
 }
 
+const char _nmtst_connection_unchanging_user_data = 0;
+
 void
 nmtst_connection_assert_unchanging (NMConnection *connection)
 {
 	nm_assert (NM_IS_CONNECTION (connection));
 
+	if (g_signal_handler_find (connection,
+	                           G_SIGNAL_MATCH_DATA,
+	                           0,
+	                           0,
+	                           NULL,
+	                           NULL,
+	                           (gpointer) &_nmtst_connection_unchanging_user_data) != 0) {
+		/* avoid connecting the assertion handler multiple times. */
+		return;
+	}
+
 	g_signal_connect (connection,
 	                  NM_CONNECTION_CHANGED,
 	                  G_CALLBACK (_nmtst_connection_unchanging_changed_cb),
-	                  NULL);
+	                  (gpointer) &_nmtst_connection_unchanging_user_data);
 	g_signal_connect (connection,
 	                  NM_CONNECTION_SECRETS_CLEARED,
 	                  G_CALLBACK (_nmtst_connection_unchanging_changed_cb),
-	                  NULL);
+	                  (gpointer) &_nmtst_connection_unchanging_user_data);
 	g_signal_connect (connection,
 	                  NM_CONNECTION_SECRETS_UPDATED,
 	                  G_CALLBACK (_nmtst_connection_unchanging_secrets_updated_cb),
-	                  NULL);
+	                  (gpointer) &_nmtst_connection_unchanging_user_data);
 }
 #endif
 
@@ -1853,7 +1866,8 @@ nm_connection_update_secrets (NMConnection *connection,
                               GError **error)
 {
 	NMSetting *setting;
-	gboolean success = TRUE, updated = FALSE;
+	gboolean success = TRUE;
+	gboolean updated = FALSE;
 	GVariant *setting_dict = NULL;
 	GVariantIter iter;
 	const char *key;
@@ -1861,13 +1875,13 @@ nm_connection_update_secrets (NMConnection *connection,
 	int success_detail;
 
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
-	g_return_val_if_fail (   g_variant_is_of_type (secrets, NM_VARIANT_TYPE_SETTING)
-	                      || g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION), FALSE);
-	if (error)
-		g_return_val_if_fail (*error == NULL, FALSE);
 
 	full_connection = g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION);
-	g_return_val_if_fail (setting_name != NULL || full_connection, FALSE);
+
+	g_return_val_if_fail (   full_connection
+	                      || g_variant_is_of_type (secrets, NM_VARIANT_TYPE_SETTING), FALSE);
+	g_return_val_if_fail (!error || !*error, FALSE);
+	g_return_val_if_fail (setting_name || full_connection, FALSE);
 
 	/* Empty @secrets means success */
 	if (g_variant_n_children (secrets) == 0)
@@ -1902,8 +1916,10 @@ nm_connection_update_secrets (NMConnection *connection,
 
 		g_clear_pointer (&setting_dict, g_variant_unref);
 
-		if (success_detail == NM_SETTING_UPDATE_SECRET_ERROR)
+		if (success_detail == NM_SETTING_UPDATE_SECRET_ERROR) {
+			nm_assert (!error || *error);
 			return FALSE;
+		}
 		if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED)
 			updated = TRUE;
 	} else {
@@ -1923,17 +1939,27 @@ nm_connection_update_secrets (NMConnection *connection,
 		/* Update each setting with any secrets from the connection dictionary */
 		g_variant_iter_init (&iter, secrets);
 		while (g_variant_iter_next (&iter, "{&s@a{sv}}", &key, &setting_dict)) {
+			gs_free_error GError *local = NULL;
+
 			/* Update the secrets for this setting */
 			setting = nm_connection_get_setting_by_name (connection, key);
 
 			g_signal_handlers_block_by_func (setting, (GCallback) setting_changed_cb, connection);
-			success_detail = _nm_setting_update_secrets (setting, setting_dict, error);
+			success_detail = _nm_setting_update_secrets (setting, setting_dict, error ? &local : NULL);
 			g_signal_handlers_unblock_by_func (setting, (GCallback) setting_changed_cb, connection);
 
 			g_variant_unref (setting_dict);
 
 			if (success_detail == NM_SETTING_UPDATE_SECRET_ERROR) {
-				success = FALSE;
+				if (success) {
+					if (error) {
+						nm_assert (local);
+						g_propagate_error (error, g_steal_pointer (&local));
+						error = NULL;
+					} else
+						nm_assert (!local);
+					success = FALSE;
+				}
 				break;
 			}
 			if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED)
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index eaf29849cf..60e13937de 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -212,6 +212,7 @@ gboolean _nm_connection_ensure_normalized (NMConnection *connection,
 gboolean _nm_connection_remove_setting (NMConnection *connection, GType setting_type);
 
 #if NM_MORE_ASSERTS
+extern const char _nmtst_connection_unchanging_user_data;
 void nmtst_connection_assert_unchanging (NMConnection *connection);
 #else
 static inline void
diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c
index d281aa9725..f58f145f91 100644
--- a/libnm-core/nm-simple-connection.c
+++ b/libnm-core/nm-simple-connection.c
@@ -141,6 +141,10 @@ nm_simple_connection_new_clone (NMConnection *connection)
 static void
 dispose (GObject *object)
 {
+#if NM_MORE_ASSERTS
+	g_signal_handlers_disconnect_by_data (object, (gpointer) &_nmtst_connection_unchanging_user_data);
+#endif
+
 	nm_connection_clear_secrets (NM_CONNECTION (object));
 
 	G_OBJECT_CLASS (nm_simple_connection_parent_class)->dispose (object);
diff --git a/libnm-core/tests/test-secrets.c b/libnm-core/tests/test-secrets.c
index 22183073e9..d657a43c4e 100644
--- a/libnm-core/tests/test-secrets.c
+++ b/libnm-core/tests/test-secrets.c
@@ -648,7 +648,7 @@ test_update_secrets_null_setting_name_with_setting_hash (void)
 
 	secrets = build_wep_secrets (wepkey);
 
-	NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (setting_name != NULL || full_connection));
+	NMTST_EXPECT_LIBNM_CRITICAL (NMTST_G_RETURN_MSG (setting_name || full_connection));
 	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
 	g_test_assert_expected_messages ();
 	g_assert_no_error (error);
diff --git a/shared/nm-glib-aux/nm-c-list.h b/shared/nm-glib-aux/nm-c-list.h
index d835bbc1d4..36f1ff755b 100644
--- a/shared/nm-glib-aux/nm-c-list.h
+++ b/shared/nm-glib-aux/nm-c-list.h
@@ -101,12 +101,24 @@ nm_c_list_elem_find_first (CList *head, gconstpointer needle)
 
 /*****************************************************************************/
 
+/**
+ * nm_c_list_move_before:
+ * @lst: the list element to which @elem will be prepended.
+ * @elem: the list element to move.
+ *
+ * This unlinks @elem from the current list and linkes it before
+ * @lst. This is like c_list_link_before(), except that @elem must
+ * be initialized and linked. Note that @elem may be linked in @lst
+ * or in another list. In both cases it gets moved.
+ *
+ * Returns: %TRUE if there were any changes. %FALSE if elem was already
+ *   linked at the right place.
+ */
 static inline gboolean
 nm_c_list_move_before (CList *lst, CList *elem)
 {
 	nm_assert (lst);
 	nm_assert (elem);
-	nm_assert (c_list_contains (lst, elem));
 
 	if (   lst != elem
 	    && lst->prev != elem) {
@@ -118,12 +130,24 @@ nm_c_list_move_before (CList *lst, CList *elem)
 }
 #define nm_c_list_move_tail(lst, elem) nm_c_list_move_before (lst, elem)
 
+/**
+ * nm_c_list_move_after:
+ * @lst: the list element to which @elem will be prepended.
+ * @elem: the list element to move.
+ *
+ * This unlinks @elem from the current list and linkes it after
+ * @lst. This is like c_list_link_after(), except that @elem must
+ * be initialized and linked. Note that @elem may be linked in @lst
+ * or in another list. In both cases it gets moved.
+ *
+ * Returns: %TRUE if there were any changes. %FALSE if elem was already
+ *   linked at the right place.
+ */
 static inline gboolean
 nm_c_list_move_after (CList *lst, CList *elem)
 {
 	nm_assert (lst);
 	nm_assert (elem);
-	nm_assert (c_list_contains (lst, elem));
 
 	if (   lst != elem
 	    && lst->next != elem) {
diff --git a/shared/nm-glib-aux/nm-glib.h b/shared/nm-glib-aux/nm-glib.h
index 77a0913899..bdb7ea5b3e 100644
--- a/shared/nm-glib-aux/nm-glib.h
+++ b/shared/nm-glib-aux/nm-glib.h
@@ -522,10 +522,18 @@ _nm_g_variant_new_printf (const char *format_string, ...)
 
 /*****************************************************************************/
 
-#if !GLIB_CHECK_VERSION (2, 56, 0)
+/* Recent glib also casts the results to typeof(Obj), but only if
+ *
+ *  ( defined(g_has_typeof) && GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_56 )
+ *
+ * Since we build NetworkManager with older GLIB_VERSION_MAX_ALLOWED, it's
+ * not taking effect.
+ *
+ * Override this. */
+#undef g_object_ref
+#undef g_object_ref_sink
 #define g_object_ref(Obj)      ((typeof(Obj)) g_object_ref (Obj))
 #define g_object_ref_sink(Obj) ((typeof(Obj)) g_object_ref_sink (Obj))
-#endif
 
 /*****************************************************************************/
 
diff --git a/shared/nm-glib-aux/nm-io-utils.c b/shared/nm-glib-aux/nm-io-utils.c
index 7b9ca0218a..23133ec568 100644
--- a/shared/nm-glib-aux/nm-io-utils.c
+++ b/shared/nm-glib-aux/nm-io-utils.c
@@ -436,3 +436,26 @@ nm_utils_file_set_contents (const char *filename,
 
 	return TRUE;
 }
+
+/**
+ * nm_utils_file_stat:
+ * @filename: the filename to stat.
+ * @out_st: (allow-none) (out): if given, this will be passed to stat().
+ *
+ * Just wraps stat() and gives the errno number as function result instead
+ * of setting the errno (though, errno is also set). It's only for convenience
+ * with
+ *
+ *    if (nm_utils_file_stat (filename, NULL) == -ENOENT) {
+ *    }
+ *
+ * Returns: 0 on success a negative errno on failure. */
+int
+nm_utils_file_stat (const char *filename, struct stat *out_st)
+{
+	struct stat st;
+
+	if (stat (filename, out_st ?: &st) != 0)
+		return -NM_ERRNO_NATIVE (errno);
+	return 0;
+}
diff --git a/shared/nm-glib-aux/nm-io-utils.h b/shared/nm-glib-aux/nm-io-utils.h
index 6037242b02..121fc481d9 100644
--- a/shared/nm-glib-aux/nm-io-utils.h
+++ b/shared/nm-glib-aux/nm-io-utils.h
@@ -59,4 +59,8 @@ gboolean nm_utils_file_set_contents (const char *filename,
                                      mode_t mode,
                                      GError **error);
 
+struct stat;
+
+int nm_utils_file_stat (const char *filename, struct stat *out_st);
+
 #endif /* __NM_IO_UTILS_H__ */
diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h
index 5b594a2ab5..d2210db006 100644
--- a/shared/nm-glib-aux/nm-macros-internal.h
+++ b/shared/nm-glib-aux/nm-macros-internal.h
@@ -1160,6 +1160,30 @@ nm_g_object_unref (gpointer obj)
 #define nm_clear_g_object(pp) \
 	nm_clear_pointer (pp, g_object_unref)
 
+/**
+ * nm_clear_error:
+ * @err: a pointer to pointer to a #GError.
+ *
+ * This is like g_clear_error(). The only difference is
+ * that this is an inline function.
+ */
+static inline void
+nm_clear_error (GError **err)
+{
+	if (err && *err) {
+		g_error_free (*err);
+		*err = NULL;
+	}
+}
+
+/* Patch g_clear_error() to use nm_clear_error(), which is inlineable
+ * and visible to the compiler. For example gs_free_error attribute only
+ * frees the error after checking that it's not %NULL. So, in many cases
+ * the compiler knows that gs_free_error has no effect and can optimize
+ * the call away. By making g_clear_error() inlineable, we give the compiler
+ * more chance to detect that the function actually has no effect. */
+#define g_clear_error(ptr) nm_clear_error(ptr)
+
 static inline gboolean
 nm_clear_g_source (guint *id)
 {
diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index 49037ed7b4..472a4c484f 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -2282,6 +2282,35 @@ nm_utils_hash_keys_to_array (GHashTable *hash,
 	return keys;
 }
 
+gboolean
+nm_utils_hashtable_same_keys (const GHashTable *a,
+                              const GHashTable *b)
+{
+	GHashTableIter h;
+	const char *k;
+
+	if (a == b)
+		return TRUE;
+	if (!a || !b)
+		return FALSE;
+	if (g_hash_table_size ((GHashTable *) a) != g_hash_table_size ((GHashTable *) b))
+		return FALSE;
+
+	g_hash_table_iter_init (&h, (GHashTable *) a);
+	while (g_hash_table_iter_next (&h, (gpointer) &k, NULL)) {
+		if (!g_hash_table_contains ((GHashTable *) b, k))
+			return FALSE;
+	}
+
+#if NM_MORE_ASSERTS > 5
+	g_hash_table_iter_init (&h, (GHashTable *) b);
+	while (g_hash_table_iter_next (&h, (gpointer) &k, NULL))
+		nm_assert (g_hash_table_contains ((GHashTable *) a, k));
+#endif
+
+	return TRUE;
+}
+
 char **
 nm_utils_strv_make_deep_copied (const char **strv)
 {
diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h
index a8f2966b38..e65d1be683 100644
--- a/shared/nm-glib-aux/nm-shared-utils.h
+++ b/shared/nm-glib-aux/nm-shared-utils.h
@@ -966,6 +966,9 @@ nm_utils_strdict_get_keys (const GHashTable *hash,
 	                                                    out_length);
 }
 
+gboolean nm_utils_hashtable_same_keys (const GHashTable *a,
+                                       const GHashTable *b);
+
 char **nm_utils_strv_make_deep_copied (const char **strv);
 
 char **nm_utils_strv_make_deep_copied_n (const char **strv, gsize len);
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index a070d50c2a..3618e32f51 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -959,7 +959,8 @@ nm_ip_routing_rule_to_platform (const NMIPRoutingRule *rule,
 struct _NMShutdownWaitObjHandle {
 	CList lst;
 	GObject *watched_obj;
-	const char *msg_reason;
+	char *msg_reason;
+	bool free_msg_reason:1;
 };
 
 static CList _shutdown_waitobj_lst_head;
@@ -968,6 +969,8 @@ static void
 _shutdown_waitobj_unregister (NMShutdownWaitObjHandle *handle)
 {
 	c_list_unlink_stale (&handle->lst);
+	if (handle->free_msg_reason)
+		g_free (handle->msg_reason);
 	g_slice_free (NMShutdownWaitObjHandle, handle);
 
 	/* FIXME(shutdown): check whether the object list is empty, and
@@ -986,13 +989,14 @@ _shutdown_waitobj_cb (gpointer user_data,
 }
 
 /**
- * _nm_shutdown_wait_obj_register:
+ * nm_shutdown_wait_obj_register_full:
  * @watched_obj: the object to watch. Takes a weak reference on the object
  *   to be notified when it gets destroyed.
- * @msg_reason: a reason message, for debugging and logging purposes. It
- *   must be a static string. Or at least, be alive at least as long as
- *   @watched_obj. So, theoretically, if you need a dynamic @msg_reason,
- *   you could attach it to @watched_obj's user-data.
+ * @msg_reason: a reason message, for debugging and logging purposes.
+ * @free_msg_reason: if %TRUE, then ownership of @msg_reason will be taken
+ *   and the string will be freed with g_free() afterwards. If %FALSE,
+ *   the caller must ensure that @msg_reason string outlives the watched
+ *   objects (e.g. being a static strings).
  *
  * Keep track of @watched_obj until it gets destroyed. During shutdown,
  * we wait until all watched objects are destroyed. This is useful, if
@@ -1009,8 +1013,9 @@ _shutdown_waitobj_cb (gpointer user_data,
  *   once it gets destroyed.
  */
 NMShutdownWaitObjHandle *
-_nm_shutdown_wait_obj_register (GObject *watched_obj,
-                                const char *msg_reason)
+nm_shutdown_wait_obj_register_full (GObject *watched_obj,
+                                    char *msg_reason,
+                                    gboolean free_msg_reason)
 {
 	NMShutdownWaitObjHandle *handle;
 
@@ -1020,11 +1025,14 @@ _nm_shutdown_wait_obj_register (GObject *watched_obj,
 		c_list_init (&_shutdown_waitobj_lst_head);
 
 	handle = g_slice_new (NMShutdownWaitObjHandle);
-	handle->watched_obj = watched_obj;
-	/* we don't clone the string. We require the caller to use pass a static message.
-	 * If he really cannot do that, he should attach the string to the watched_obj
-	 * as user-data. */
-	handle->msg_reason = msg_reason;
+	*handle = (NMShutdownWaitObjHandle) {
+		/* depending on @free_msg_reason, we take ownership of @msg_reason.
+		 * In either case, we just reference the string without cloning
+		 * it. */
+		.watched_obj     = watched_obj,
+		.msg_reason      = msg_reason,
+		.free_msg_reason = free_msg_reason,
+	};
 	c_list_link_tail (&_shutdown_waitobj_lst_head, &handle->lst);
 	g_object_weak_ref (watched_obj, _shutdown_waitobj_cb, handle);
 	return handle;
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index 5e3f6673c4..778dc00176 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -89,10 +89,11 @@ NMPlatformRoutingRule *nm_ip_routing_rule_to_platform (const NMIPRoutingRule *ru
 
 typedef struct _NMShutdownWaitObjHandle NMShutdownWaitObjHandle;
 
-NMShutdownWaitObjHandle *_nm_shutdown_wait_obj_register (GObject *watched_obj,
-                                                        const char *msg_reason);
+NMShutdownWaitObjHandle *nm_shutdown_wait_obj_register_full (GObject *watched_obj,
+                                                             char *msg_reason,
+                                                             gboolean free_msg_reason);
 
-#define nm_shutdown_wait_obj_register(watched_obj, msg_reason) _nm_shutdown_wait_obj_register((watched_obj), (""msg_reason""))
+#define nm_shutdown_wait_obj_register(watched_obj, msg_reason) nm_shutdown_wait_obj_register_full((watched_obj), (""msg_reason""), FALSE)
 
 void nm_shutdown_wait_obj_unregister (NMShutdownWaitObjHandle *handle);
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 8fe3e5776f..abcff229bf 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -5753,7 +5753,7 @@ nm_device_complete_connection (NMDevice *self,
 	                                 error))
 		return FALSE;
 
-	if (!nm_connection_verify (connection, error))
+	if (!nm_connection_normalize (connection, NULL, NULL, error))
 		return FALSE;
 
 	return nm_device_check_connection_compatible (self, connection, error);
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 04d2074af0..6784551675 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -126,6 +126,8 @@ static void
 _nm_singleton_instance_weak_cb (gpointer data,
                                 GObject *where_the_object_was)
 {
+	nm_assert (g_slist_find (_singletons, where_the_object_was));
+
 	_singletons = g_slist_remove (_singletons, where_the_object_was);
 }
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 8960079b11..c961ebc045 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -817,14 +817,14 @@ _delete_volatile_connection_do (NMManager *self,
 	if (!NM_FLAGS_HAS (nm_settings_connection_get_flags (connection),
 	                   NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE))
 		return;
+	if (!nm_settings_has_connection (priv->settings, connection))
+		return;
 	if (active_connection_find (self,
 	                            connection,
 	                            NULL,
 	                            NM_ACTIVE_CONNECTION_STATE_DEACTIVATED,
 	                            NULL))
 		return;
-	if (!nm_settings_has_connection (priv->settings, connection))
-		return;
 
 	_LOGD (LOGD_DEVICE, "volatile connection disconnected. Deleting connection '%s' (%s)",
 	       nm_settings_connection_get_id (connection), nm_settings_connection_get_uuid (connection));
@@ -2064,8 +2064,14 @@ static void
 connection_changed (NMManager *self,
                     NMSettingsConnection *sett_conn)
 {
+	NMConnection *connection;
 	NMDevice *device;
-	NMConnection *connection = nm_settings_connection_get_connection (sett_conn);
+
+	if (NM_FLAGS_HAS (nm_settings_connection_get_flags (sett_conn),
+	                  NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE))
+		return;
+
+	connection = nm_settings_connection_get_connection (sett_conn);
 
 	if (!nm_connection_is_virtual (connection))
 		return;
@@ -2094,33 +2100,22 @@ connection_updated_cb (NMSettings *settings,
                        gboolean by_user,
                        NMManager *self)
 {
-	if (by_user)
-		connection_changed (self, sett_conn);
+	connection_changed (self, sett_conn);
 }
 
 /*****************************************************************************/
 
-typedef struct {
-	CList delete_volatile_connection_lst;
-	NMSettingsConnection *connection;
-} DeleteVolatileConnectionData;
-
 static void
 _delete_volatile_connection_all (NMManager *self, gboolean do_delete)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
-	CList *lst;
-	DeleteVolatileConnectionData *data;
+	NMCListElem *elem;
 
-	while ((lst = c_list_first (&priv->delete_volatile_connection_lst_head))) {
+	while ((elem = c_list_first_entry (&priv->delete_volatile_connection_lst_head, NMCListElem, lst))) {
 		gs_unref_object NMSettingsConnection *connection = NULL;
 
-		data = c_list_entry (lst,
-		                     DeleteVolatileConnectionData,
-		                     delete_volatile_connection_lst);
-		connection = data->connection;
-		c_list_unlink_stale (&data->delete_volatile_connection_lst);
-		g_slice_free (DeleteVolatileConnectionData, data);
+		connection = elem->data;
+		nm_c_list_elem_free (elem);
 
 		if (do_delete)
 			_delete_volatile_connection_do (self, connection);
@@ -2145,7 +2140,6 @@ connection_flags_changed (NMSettings *settings,
 {
 	NMManager *self = user_data;
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
-	DeleteVolatileConnectionData *data;
 
 	if (!NM_FLAGS_HAS (nm_settings_connection_get_flags (connection),
 	                   NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE))
@@ -2157,9 +2151,8 @@ connection_flags_changed (NMSettings *settings,
 		return;
 	}
 
-	data = g_slice_new (DeleteVolatileConnectionData);
-	data->connection = g_object_ref (connection);
-	c_list_link_tail (&priv->delete_volatile_connection_lst_head, &data->delete_volatile_connection_lst);
+	c_list_link_tail (&priv->delete_volatile_connection_lst_head,
+	                  &nm_c_list_elem_new_stale (g_object_ref (connection))->lst);
 	if (!priv->delete_volatile_connection_idle_id)
 		priv->delete_volatile_connection_idle_id = g_idle_add (_delete_volatile_connection_cb, self);
 }
@@ -5529,6 +5522,8 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
 			goto error;
 	}
 
+	nm_assert (_nm_connection_verify (incompl_conn, NULL) == NM_SETTING_VERIFY_SUCCESS);
+
 	active = _new_active_connection (self,
 	                                 is_vpn,
 	                                 NULL,
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index dfe10459c4..663e630656 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -111,7 +111,7 @@ typedef struct _NMSettingsConnectionPrivate {
 	 * to re-read them from disk which defeats the purpose of having the
 	 * connection in-memory at all.
 	 */
-	NMConnection *system_secrets;
+	GVariant *system_secrets;
 
 	/* Caches secrets from agents during the activation process; if new system
 	 * secrets are returned from an agent, they get written out to disk,
@@ -119,7 +119,7 @@ typedef struct _NMSettingsConnectionPrivate {
 	 * secrets, and would wipe out any agent-owned or not-saved secrets the
 	 * agent also returned.
 	 */
-	NMConnection *agent_secrets;
+	GVariant *agent_secrets;
 
 	char *filename;
 
@@ -316,30 +316,36 @@ static void
 update_system_secrets_cache (NMSettingsConnection *self)
 {
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
+	gs_unref_object NMConnection *connection_cloned = NULL;
 
-	if (priv->system_secrets)
-		g_object_unref (priv->system_secrets);
-	priv->system_secrets = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
+	nm_clear_pointer (&priv->system_secrets, g_variant_unref);
+
+	connection_cloned = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 
 	/* Clear out non-system-owned and not-saved secrets */
-	_nm_connection_clear_secrets_by_secret_flags (priv->system_secrets,
+	_nm_connection_clear_secrets_by_secret_flags (connection_cloned,
 	                                              NM_SETTING_SECRET_FLAG_NONE);
+
+	priv->system_secrets = nm_g_variant_ref_sink (nm_connection_to_dbus (connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS));
 }
 
 static void
 update_agent_secrets_cache (NMSettingsConnection *self, NMConnection *new)
 {
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
+	gs_unref_object NMConnection *connection_cloned = NULL;
 
-	if (priv->agent_secrets)
-		g_object_unref (priv->agent_secrets);
-	priv->agent_secrets = nm_simple_connection_new_clone (   new
-	                                                      ?: nm_settings_connection_get_connection (self));
+	nm_clear_pointer (&priv->agent_secrets, g_variant_unref);
+
+	connection_cloned = nm_simple_connection_new_clone (   new
+	                                                    ?: nm_settings_connection_get_connection (self));
 
 	/* Clear out non-system-owned secrets */
-	_nm_connection_clear_secrets_by_secret_flags (priv->agent_secrets,
+	_nm_connection_clear_secrets_by_secret_flags (connection_cloned,
 	                                                NM_SETTING_SECRET_FLAG_NOT_SAVED
 	                                              | NM_SETTING_SECRET_FLAG_AGENT_OWNED);
+
+	priv->agent_secrets = nm_g_variant_ref_sink (nm_connection_to_dbus (connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS));
 }
 
 static void
@@ -350,9 +356,7 @@ secrets_cleared_cb (NMConnection *connection, NMSettingsConnection *self)
 	/* Clear agent secrets when connection's secrets are cleared since agent
 	 * secrets are transient.
 	 */
-	if (priv->agent_secrets)
-		g_object_unref (priv->agent_secrets);
-	priv->agent_secrets = NULL;
+	nm_clear_pointer (&priv->agent_secrets, g_variant_unref);
 }
 
 static void
@@ -572,25 +576,23 @@ nm_settings_connection_update (NMSettingsConnection *self,
 		 * in the replacement connection data if it was eg reread from disk.
 		 */
 		if (priv->agent_secrets) {
-			GVariant *dict;
-
-			dict = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-			if (dict) {
-				(void) nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, dict, NULL);
-				g_variant_unref (dict);
-			}
+			/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
+			nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, priv->agent_secrets, NULL);
+		}
+		if (con_agent_secrets) {
+			/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
+			nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, con_agent_secrets, NULL);
 		}
-		if (con_agent_secrets)
-			(void) nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, con_agent_secrets, NULL);
 	}
 
 	/* Apply agent-owned secrets from the new connection so that
 	 * they can be sent to agents */
 	if (new_agent_secrets) {
-		(void) nm_connection_update_secrets (nm_settings_connection_get_connection (self),
-		                                     NULL,
-		                                     new_agent_secrets,
-		                                     NULL);
+		/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
+		nm_connection_update_secrets (nm_settings_connection_get_connection (self),
+		                              NULL,
+		                              new_agent_secrets,
+		                              NULL);
 	}
 
 	nm_settings_connection_recheck_visibility (self);
@@ -857,6 +859,7 @@ nm_settings_connection_new_secrets (NMSettingsConnection *self,
 		return FALSE;
 	}
 
+	/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
 	if (!nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, secrets, error))
 		return FALSE;
 
@@ -889,7 +892,7 @@ get_secrets_done_cb (NMAgentManager *manager,
 	NMSettingsConnectionPrivate *priv;
 	NMConnection *applied_connection;
 	gs_free_error GError *local = NULL;
-	GVariant *dict = NULL;
+	gs_unref_variant GVariant *system_secrets = NULL;
 	gboolean agent_had_system = FALSE;
 	ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE };
 
@@ -952,12 +955,17 @@ get_secrets_done_cb (NMAgentManager *manager,
 	       setting_name,
 	       call_id);
 
-	if (priv->system_secrets)
-		dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
+	system_secrets = nm_g_variant_ref (priv->system_secrets);
 
 	/* Update the connection with our existing secrets from backing storage */
 	nm_connection_clear_secrets (nm_settings_connection_get_connection (self));
-	if (!dict || nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, dict, &local)) {
+
+	if (   !system_secrets
+	       /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
+	    || nm_connection_update_secrets (nm_settings_connection_get_connection (self),
+	                                     setting_name,
+	                                     system_secrets,
+	                                     &local)) {
 		gs_unref_variant GVariant *filtered_secrets = NULL;
 
 		/* Update the connection with the agent's secrets; by this point if any
@@ -966,6 +974,7 @@ get_secrets_done_cb (NMAgentManager *manager,
 		 * system secrets.
 		 */
 		filtered_secrets = validate_secret_flags (nm_settings_connection_get_connection (self), secrets, &cmp_flags);
+		/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
 		if (nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, filtered_secrets, &local)) {
 			/* Now that all secrets are updated, copy and cache new secrets,
 			 * then save them to backing storage.
@@ -1023,7 +1032,8 @@ get_secrets_done_cb (NMAgentManager *manager,
 
 		nm_connection_clear_secrets (applied_connection);
 
-		if (!dict || nm_connection_update_secrets (applied_connection, setting_name, dict, NULL)) {
+		if (   !system_secrets
+		    || nm_connection_update_secrets (applied_connection, setting_name, system_secrets, NULL)) {
 			gs_unref_variant GVariant *filtered_secrets = NULL;
 
 			filtered_secrets = validate_secret_flags (applied_connection, secrets, &cmp_flags);
@@ -1033,8 +1043,6 @@ get_secrets_done_cb (NMAgentManager *manager,
 
 	_get_secrets_info_callback (call_id, agent_username, setting_name, local);
 	g_clear_error (&local);
-	if (dict)
-		g_variant_unref (dict);
 
 out:
 	_get_secrets_info_free (call_id);
@@ -1095,7 +1103,6 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
                                     gpointer callback_data)
 {
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
-	GVariant *existing_secrets = NULL;
 	NMAgentManagerCallId call_id_a;
 	gs_free char *joined_hints = NULL;
 	NMSettingsConnectionCallId *call_id;
@@ -1132,14 +1139,6 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 		goto schedule_dummy;
 	}
 
-	/* Use priv->system_secrets to work around the fact that nm_connection_clear_secrets()
-	 * will clear secrets on this object's settings.
-	 */
-	if (priv->system_secrets)
-		existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-	if (existing_secrets)
-		g_variant_ref_sink (existing_secrets);
-
 	/* we remember the current version-id of the secret-agents. The version-id is strictly increasing,
 	 * as new agents register the number. We know hence, that this request was made against a certain
 	 * set of secret-agents.
@@ -1147,19 +1146,20 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 	 * Then we know that the this request probably did not yet include the latest secret-agent. */
 	priv->last_secret_agent_version_id = nm_agent_manager_get_agent_version_id (priv->agent_mgr);
 
+	/* Use priv->system_secrets to work around the fact that nm_connection_clear_secrets()
+	 * will clear secrets on this object's settings.
+	 */
 	call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr,
 	                                          nm_dbus_object_get_path (NM_DBUS_OBJECT (self)),
 	                                          nm_settings_connection_get_connection (self),
 	                                          subject,
-	                                          existing_secrets,
+	                                          priv->system_secrets,
 	                                          setting_name,
 	                                          flags,
 	                                          hints,
 	                                          get_secrets_done_cb,
 	                                          call_id);
-	g_assert (call_id_a);
-	if (existing_secrets)
-		g_variant_unref (existing_secrets);
+	nm_assert (call_id_a);
 
 	_LOGD ("(%s:%p) secrets requested flags 0x%X hints '%s'",
 	       setting_name,
@@ -1465,28 +1465,6 @@ typedef struct {
 } UpdateInfo;
 
 static void
-cached_secrets_to_connection (NMSettingsConnection *self, NMConnection *connection)
-{
-	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
-	GVariant *secrets_dict;
-
-	if (priv->agent_secrets) {
-		secrets_dict = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-		if (secrets_dict) {
-			(void) nm_connection_update_secrets (connection, NULL, secrets_dict, NULL);
-			g_variant_unref (secrets_dict);
-		}
-	}
-	if (priv->system_secrets) {
-		secrets_dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-		if (secrets_dict) {
-			(void) nm_connection_update_secrets (connection, NULL, secrets_dict, NULL);
-			g_variant_unref (secrets_dict);
-		}
-	}
-}
-
-static void
 update_complete (NMSettingsConnection *self,
                  UpdateInfo *info,
                  GError *error)
@@ -1519,6 +1497,7 @@ update_auth_cb (NMSettingsConnection *self,
                 GError *error,
                 gpointer data)
 {
+	NMSettingsConnectionPrivate *priv;
 	UpdateInfo *info = data;
 	NMSettingsConnectionCommitReason commit_reason;
 	gs_free_error GError *local = NULL;
@@ -1530,13 +1509,18 @@ update_auth_cb (NMSettingsConnection *self,
 		return;
 	}
 
+	priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
+
 	if (info->new_settings) {
 		if (!_nm_connection_aggregate (info->new_settings, NM_CONNECTION_AGGREGATE_ANY_SECRETS, NULL)) {
 			/* If the new connection has no secrets, we do not want to remove all
 			 * secrets, rather we keep all the existing ones. Do that by merging
 			 * them in to the new connection.
 			 */
-			cached_secrets_to_connection (self, info->new_settings);
+			if (priv->agent_secrets)
+				nm_connection_update_secrets (info->new_settings, NULL, priv->agent_secrets, NULL);
+			if (priv->system_secrets)
+				nm_connection_update_secrets (info->new_settings, NULL, priv->system_secrets, NULL);
 		} else {
 			/* Cache the new secrets from the agent, as stuff like inotify-triggered
 			 * changes to connection's backing config files will blow them away if
@@ -2020,11 +2004,12 @@ dbus_clear_secrets_auth_cb (NMSettingsConnection *self,
 	}
 
 	/* Clear secrets in connection and caches */
+
+	/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
 	nm_connection_clear_secrets (nm_settings_connection_get_connection (self));
-	if (priv->system_secrets)
-		nm_connection_clear_secrets (priv->system_secrets);
-	if (priv->agent_secrets)
-		nm_connection_clear_secrets (priv->agent_secrets);
+
+	nm_clear_pointer (&priv->system_secrets, g_variant_unref);
+	nm_clear_pointer (&priv->agent_secrets, g_variant_unref);
 
 	/* Tell agents to remove secrets for this connection */
 	nm_agent_manager_delete_secrets (priv->agent_mgr,
@@ -2785,8 +2770,8 @@ dispose (GObject *object)
 		nm_connection_clear_secrets (priv->connection);
 	}
 
-	g_clear_object (&priv->system_secrets);
-	g_clear_object (&priv->agent_secrets);
+	nm_clear_pointer (&priv->system_secrets, g_variant_unref);
+	nm_clear_pointer (&priv->agent_secrets, g_variant_unref);
 
 	g_clear_pointer (&priv->seen_bssids, g_hash_table_destroy);
 
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 4d1915e24c..6fbda5274b 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -720,7 +720,7 @@ nm_settings_add_connection_dbus (NMSettings *self,
 	g_return_if_fail (G_IS_DBUS_METHOD_INVOCATION (context));
 
 	/* Connection must be valid, of course */
-	if (!nm_connection_verify (connection, &tmp_error)) {
+	if (_nm_connection_verify (connection, &tmp_error) != NM_SETTING_VERIFY_SUCCESS) {
 		error = g_error_new (NM_SETTINGS_ERROR,
 		                     NM_SETTINGS_ERROR_INVALID_CONNECTION,
 		                     "The connection was invalid: %s",
@@ -1255,6 +1255,10 @@ add_plugin (NMSettings *self,
 
 	priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin));
 
+	nm_shutdown_wait_obj_register_full (G_OBJECT (plugin),
+	                                    g_strdup_printf ("%s-settings-plugin", pname),
+	                                    TRUE);
+
 	_LOGI ("Loaded settings plugin: %s (%s%s%s)",
 	       pname,
 	       NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal"));
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
index 9abe26eebf..5caa861de9 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
@@ -206,6 +206,8 @@ commit_changes (NMSettingsConnection *connection,
 	if (!nms_ifcfg_rh_writer_write_connection (new_connection,
 	                                           IFCFG_DIR,
 	                                           filename,
+	                                           NULL,
+	                                           NULL,
 	                                           &ifcfg_path,
 	                                           &reread,
 	                                           &reread_same,
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
index 14e4108f66..6375b6cc14 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
@@ -79,12 +79,6 @@ G_DEFINE_TYPE (SettingsPluginIfcfg, settings_plugin_ifcfg, NM_TYPE_SETTINGS_PLUG
 
 /*****************************************************************************/
 
-static SettingsPluginIfcfg *settings_plugin_ifcfg_get (void);
-
-NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfcfg, settings_plugin_ifcfg_get, SETTINGS_TYPE_PLUGIN_IFCFG);
-
-/*****************************************************************************/
-
 #define _NMLOG_DOMAIN  LOGD_SETTINGS
 #define _NMLOG(level, ...) \
     G_STMT_START { \
@@ -600,7 +594,7 @@ add_connection (NMSettingsPlugin *config,
 	gs_unref_object NMConnection *reread = NULL;
 
 	if (save_to_disk) {
-		if (!nms_ifcfg_rh_writer_write_connection (connection, IFCFG_DIR, NULL, &path, &reread, NULL, error))
+		if (!nms_ifcfg_rh_writer_write_connection (connection, IFCFG_DIR, NULL, NULL, NULL, &path, &reread, NULL, error))
 			return NULL;
 	} else {
 		if (!nms_ifcfg_rh_writer_can_write_connection (connection, error))
@@ -984,5 +978,5 @@ settings_plugin_ifcfg_class_init (SettingsPluginIfcfgClass *klass)
 G_MODULE_EXPORT NMSettingsPlugin *
 nm_settings_plugin_factory (void)
 {
-	return NM_SETTINGS_PLUGIN (g_object_ref (settings_plugin_ifcfg_get ()));
+	return g_object_new (SETTINGS_TYPE_PLUGIN_IFCFG, NULL);
 }
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
index 9a05572660..038f3dacc9 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
@@ -316,36 +316,41 @@ utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg)
 char *
 utils_detect_ifcfg_path (const char *path, gboolean only_ifcfg)
 {
-	gs_free char *base = NULL;
-	char *ptr, *ifcfg = NULL;
+	const char *base;
 
 	g_return_val_if_fail (path != NULL, NULL);
 
 	if (utils_should_ignore_file (path, only_ifcfg))
 		return NULL;
 
-	base = g_path_get_basename (path);
+	base = strrchr (path, '/');
+	if (!base)
+		base = path;
+	else
+		base += 1;
 
-	if (strncmp (base, IFCFG_TAG, NM_STRLEN (IFCFG_TAG)) == 0) {
+	if (NM_STR_HAS_PREFIX (base, IFCFG_TAG)) {
 		if (base[NM_STRLEN (IFCFG_TAG)] == '\0')
 			return NULL;
 		if (utils_is_ifcfg_alias_file (base, NULL)) {
+			gs_free char *ifcfg = NULL;
+			char *ptr;
+
 			ifcfg = g_strdup (path);
 			ptr = strrchr (ifcfg, ':');
-			if (ptr && ptr > ifcfg) {
+			if (   ptr
+			    && ptr > ifcfg
+			    && !strchr (ptr, '/')) {
 				*ptr = '\0';
 				if (g_file_test (ifcfg, G_FILE_TEST_EXISTS)) {
 					/* the file has a colon, so it is probably an alias.
 					 * To be ~more~ certain that this is an alias file,
 					 * check whether a corresponding base file exists. */
-					if (only_ifcfg) {
-						g_free (ifcfg);
+					if (only_ifcfg)
 						return NULL;
-					}
-					return ifcfg;
+					return g_steal_pointer (&ifcfg);
 				}
 			}
-			g_free (ifcfg);
 		}
 		return g_strdup (path);
 	}
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index fc5b1672d5..e6a08f7158 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -3024,6 +3024,8 @@ static gboolean
 do_write_construct (NMConnection *connection,
                     const char *ifcfg_dir,
                     const char *filename,
+                    NMSIfcfgRHWriterAllowFilenameCb allow_filename_cb,
+                    gpointer allow_filename_user_data,
                     shvarFile **out_ifcfg,
                     GHashTable **out_blobs,
                     GHashTable **out_secrets,
@@ -3067,30 +3069,31 @@ do_write_construct (NMConnection *connection,
 
 		ifcfg_name = g_strdup (filename);
 	} else if (ifcfg_dir) {
-		char *escaped;
+		gs_free char *escaped = NULL;
+		int i_path;
 
 		escaped = escape_id (nm_setting_connection_get_id (s_con));
-		ifcfg_name = g_strdup_printf ("%s/ifcfg-%s", ifcfg_dir, escaped);
 
-		/* If a file with this path already exists then we need another name.
-		 * Multiple connections can have the same ID (ie if two connections with
-		 * the same ID are visible to different users) but of course can't have
-		 * the same path.
-		 */
-		if (g_file_test (ifcfg_name, G_FILE_TEST_EXISTS)) {
-			guint32 idx = 0;
-
-			nm_clear_g_free (&ifcfg_name);
-			while (idx++ < 500) {
-				ifcfg_name = g_strdup_printf ("%s/ifcfg-%s-%u", ifcfg_dir, escaped, idx);
-				if (g_file_test (ifcfg_name, G_FILE_TEST_EXISTS) == FALSE)
-					break;
-				nm_clear_g_free (&ifcfg_name);
-			}
+		for (i_path = 0; i_path < 10000; i_path++) {
+			gs_free char *path_candidate = NULL;
+
+			if (i_path == 0)
+				path_candidate = g_strdup_printf ("%s/ifcfg-%s", ifcfg_dir, escaped);
+			else
+				path_candidate = g_strdup_printf ("%s/ifcfg-%s-%d", ifcfg_dir, escaped, i_path);
+
+			if (   allow_filename_cb
+			    && !allow_filename_cb (path_candidate, allow_filename_user_data))
+				continue;
+
+			if (g_file_test (path_candidate, G_FILE_TEST_EXISTS))
+				continue;
+
+			ifcfg_name = g_steal_pointer (&path_candidate);
+			break;
 		}
-		g_free (escaped);
 
-		if (ifcfg_name == NULL) {
+		if (!ifcfg_name) {
 			g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
 			                     "Failed to find usable ifcfg file name");
 			return FALSE;
@@ -3377,6 +3380,8 @@ gboolean
 nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
                                       const char *ifcfg_dir,
                                       const char *filename,
+                                      NMSIfcfgRHWriterAllowFilenameCb allow_filename_cb,
+                                      gpointer allow_filename_user_data,
                                       char **out_filename,
                                       NMConnection **out_reread,
                                       gboolean *out_reread_same,
@@ -3396,6 +3401,8 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
 	if (!do_write_construct (connection,
 	                         ifcfg_dir,
 	                         filename,
+	                         allow_filename_cb,
+	                         allow_filename_user_data,
 	                         &ifcfg,
 	                         &blobs,
 	                         &secrets,
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.h b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.h
index 15bf4386a6..0902daeef1 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.h
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.h
@@ -22,12 +22,18 @@
 
 #include "nm-connection.h"
 
+
+typedef gboolean (*NMSIfcfgRHWriterAllowFilenameCb) (const char *check_filename,
+                                                     gpointer allow_filename_user_data);
+
 gboolean nms_ifcfg_rh_writer_can_write_connection (NMConnection *connection,
                                                    GError **error);
 
 gboolean nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
                                                const char *ifcfg_dir,
                                                const char *filename,
+                                               NMSIfcfgRHWriterAllowFilenameCb allow_filename_cb,
+                                               gpointer allow_filename_user_data,
                                                char **out_filename,
                                                NMConnection **out_reread,
                                                gboolean *out_reread_same,
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 97bba2f14c..9792e20ecd 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -238,7 +238,7 @@ _assert_expected_content (NMConnection *connection, const char *filename, const
 		g_assert (_ifcfg_dir && _ifcfg_dir[0]); \
 		g_assert (_filename && _filename[0]); \
 		\
-		_success = nms_ifcfg_rh_writer_write_connection (_connection, _ifcfg_dir, _filename, NULL, _out_reread, _out_reread_same, &_error); \
+		_success = nms_ifcfg_rh_writer_write_connection (_connection, _ifcfg_dir, _filename, NULL, NULL, NULL, _out_reread, _out_reread_same, &_error); \
 		nmtst_assert_success (_success, _error); \
 		_assert_expected_content (_connection, _filename, _expected); \
 	} G_STMT_END
@@ -319,6 +319,8 @@ _writer_new_connection_reread (NMConnection *connection,
 	success = nms_ifcfg_rh_writer_write_connection (con_verified,
 	                                                ifcfg_dir,
 	                                                NULL,
+	                                                NULL,
+	                                                NULL,
 	                                                &filename,
 	                                                reread,
 	                                                out_reread_same,
@@ -394,6 +396,8 @@ _writer_new_connection_fail (NMConnection *connection,
 	success = nms_ifcfg_rh_writer_write_connection (connection_normalized,
 	                                                ifcfg_dir,
 	                                                NULL,
+	                                                NULL,
+	                                                NULL,
 	                                                &filename,
 	                                                &reread,
 	                                                NULL,
diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
index 8f9be772bb..23f4015609 100644
--- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
+++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
@@ -75,11 +75,6 @@ G_DEFINE_TYPE (SettingsPluginIfupdown, settings_plugin_ifupdown, NM_TYPE_SETTING
 
 /*****************************************************************************/
 
-static SettingsPluginIfupdown *settings_plugin_ifupdown_get (void);
-NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get, SETTINGS_TYPE_PLUGIN_IFUPDOWN);
-
-/*****************************************************************************/
-
 #define _NMLOG_PREFIX_NAME      "ifupdown"
 #define _NMLOG_DOMAIN           LOGD_SETTINGS
 #define _NMLOG(level, ...) \
@@ -339,5 +334,5 @@ settings_plugin_ifupdown_class_init (SettingsPluginIfupdownClass *klass)
 G_MODULE_EXPORT NMSettingsPlugin *
 nm_settings_plugin_factory (void)
 {
-	return NM_SETTINGS_PLUGIN (g_object_ref (settings_plugin_ifupdown_get ()));
+	return g_object_new (SETTINGS_TYPE_PLUGIN_IFUPDOWN, NULL);
 }
diff --git a/tools/create-exports-NetworkManager.sh b/tools/create-exports-NetworkManager.sh
index 052968999a..b089aba2fc 100755
--- a/tools/create-exports-NetworkManager.sh
+++ b/tools/create-exports-NetworkManager.sh
@@ -61,7 +61,7 @@ get_symbols_missing() {
                      ./src/devices/*/${libs} \
                      ./src/ppp/${libs} -name '*.so' 2>/dev/null); do
         call_nm "$f" |
-            sed -n 's/^\([U]\) \(\(nm_\|nmp_\|_nm\|NM\|_NM\|c_siphash_\).*\)$/\2/p'
+            sed -n 's/^\([U]\) \(\(nm_\|nmp_\|_nm\|NM\|_NM\|nmtst_\|c_siphash_\|c_list_\).*\)$/\2/p'
     done) |
         _sort |
         grep -Fx -f <(get_symbols_explict) -v |