From c3dd5d8df2388813c22d2662985725b188450b85 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Nov 2017 20:57:42 +0100 Subject: [PATCH 01/35] settings: log pretty names for settings-connection flags --- src/settings/nm-settings-connection.c | 13 ++++++++++++- src/settings/nm-settings-connection.h | 6 +++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 9b289f9f21..5e6c93da4a 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2115,6 +2115,13 @@ nm_settings_connection_get_unsaved (NMSettingsConnection *self) /*****************************************************************************/ +NM_UTILS_FLAGS2STR_DEFINE_STATIC (_settings_connection_flags_to_string, NMSettingsConnectionFlags, + NM_UTILS_FLAGS2STR (NM_SETTINGS_CONNECTION_FLAGS_NONE, "none"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_CONNECTION_FLAGS_UNSAVED, "unsaved"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED, "nm-generatd"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_CONNECTION_FLAGS_VOLATILE, "volatile"), +); + NMSettingsConnectionFlags nm_settings_connection_get_flags (NMSettingsConnection *self) { @@ -2151,7 +2158,11 @@ nm_settings_connection_set_flags_all (NMSettingsConnection *self, NMSettingsConn old_flags = priv->flags; if (old_flags != flags) { - _LOGT ("update settings-connection flags to 0x%x (was 0x%x)", (guint) flags, (guint) priv->flags); + char buf1[255], buf2[255]; + + _LOGT ("update settings-connection flags to %s (was %s)", + _settings_connection_flags_to_string (flags, buf1, sizeof (buf1)), + _settings_connection_flags_to_string (priv->flags, buf2, sizeof (buf2))); priv->flags = flags; _notify (self, PROP_FLAGS); if (NM_FLAGS_HAS (old_flags, NM_SETTINGS_CONNECTION_FLAGS_UNSAVED) != NM_FLAGS_HAS (flags, NM_SETTINGS_CONNECTION_FLAGS_UNSAVED)) diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 3672bc08c1..dd9e9c213c 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -65,9 +65,9 @@ * * #NMSettingsConnection flags. **/ -typedef enum -{ +typedef enum { NM_SETTINGS_CONNECTION_FLAGS_NONE = 0x00, + NM_SETTINGS_CONNECTION_FLAGS_UNSAVED = 0x01, NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED = 0x02, NM_SETTINGS_CONNECTION_FLAGS_VOLATILE = 0x04, @@ -76,7 +76,7 @@ typedef enum NM_SETTINGS_CONNECTION_FLAGS_ALL = ((__NM_SETTINGS_CONNECTION_FLAGS_LAST - 1) << 1) - 1, } NMSettingsConnectionFlags; -typedef enum { /*< skip >*/ +typedef enum { 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), From 9531da8b3ed80b9067023ea4596235572e58df00 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 15:45:20 +0100 Subject: [PATCH 02/35] settings: add persistent-mode argument for connection-replace The current behavior of update_unsaved is confusing. Give the argument an enum with a name that describes better what's happening. Also, it makes the uses grep-able. --- src/settings/nm-settings-connection.c | 56 +++++++++++-------- src/settings/nm-settings-connection.h | 15 +++-- .../plugins/ibft/nms-ibft-connection.c | 2 +- .../ifcfg-rh/nms-ifcfg-rh-connection.c | 10 +--- .../plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c | 2 +- .../plugins/ifnet/nms-ifnet-connection.c | 4 +- src/settings/plugins/ifnet/nms-ifnet-plugin.c | 2 +- .../plugins/keyfile/nms-keyfile-connection.c | 4 +- .../plugins/keyfile/nms-keyfile-plugin.c | 2 +- 9 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 5e6c93da4a..64fec2b12d 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -552,13 +552,13 @@ nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self, return TRUE; } -gboolean -nm_settings_connection_replace_settings_full (NMSettingsConnection *self, - NMConnection *new_connection, - gboolean prepare_new_connection, - gboolean update_unsaved, - const char *log_diff_name, - GError **error) +static gboolean +_replace_settings_full (NMSettingsConnection *self, + NMConnection *new_connection, + gboolean prepare_new_connection, + NMSettingsConnectionPersistMode persist_mode, + const char *log_diff_name, + GError **error) { NMSettingsConnectionPrivate *priv; @@ -619,8 +619,16 @@ nm_settings_connection_replace_settings_full (NMSettingsConnection *self, /* Manually emit changed signal since we disconnected the handler, but * only update Unsaved if the caller wanted us to. */ - if (update_unsaved) + switch (persist_mode) { + case NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY: set_unsaved (self, TRUE); + break; + case NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK: + set_unsaved (self, FALSE); + break; + case NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP: + break; + } g_signal_handlers_unblock_by_func (self, G_CALLBACK (connection_changed_cb), NULL); @@ -635,16 +643,16 @@ nm_settings_connection_replace_settings_full (NMSettingsConnection *self, gboolean nm_settings_connection_replace_settings (NMSettingsConnection *self, NMConnection *new_connection, - gboolean update_unsaved, + NMSettingsConnectionPersistMode persist_mode, const char *log_diff_name, GError **error) { - return nm_settings_connection_replace_settings_full (self, - new_connection, - TRUE, - update_unsaved, - log_diff_name, - error); + return _replace_settings_full (self, + new_connection, + TRUE, + persist_mode, + log_diff_name, + error); } gboolean @@ -694,14 +702,14 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self, } if (reread_connection || new_connection) { - if (!nm_settings_connection_replace_settings_full (self, - reread_connection ?: new_connection, - !reread_connection, - FALSE, - new_connection - ? "update-during-write" - : "replace-and-commit-disk", - &local)) { + if (!_replace_settings_full (self, + reread_connection ?: new_connection, + !reread_connection, + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, + new_connection + ? "update-during-write" + : "replace-and-commit-disk", + &local)) { /* this can't really happen, because at this point replace-settings * is no longer supposed to fail. It's a bug. */ _LOGE ("write: replacing setting failed: %s", @@ -1711,7 +1719,7 @@ update_auth_cb (NMSettingsConnection *self, if (info->new_settings) { nm_settings_connection_replace_settings (self, info->new_settings, - TRUE, + NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, "replace-unsaved", &local); } diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index dd9e9c213c..cb9d99879e 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -140,19 +140,18 @@ gboolean nm_settings_connection_replace_settings_prepare (NMSettingsConnection * NMConnection *new_connection, GError **error); +typedef enum { + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, + NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, + NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, +} NMSettingsConnectionPersistMode; + gboolean nm_settings_connection_replace_settings (NMSettingsConnection *self, NMConnection *new_connection, - gboolean update_unsaved, + NMSettingsConnectionPersistMode persist_mode, const char *log_diff_name, GError **error); -gboolean nm_settings_connection_replace_settings_full (NMSettingsConnection *self, - NMConnection *new_connection, - gboolean prepare_new_connection, - gboolean update_unsaved, - const char *log_diff_name, - GError **error); - gboolean nm_settings_connection_delete (NMSettingsConnection *self, GError **error); diff --git a/src/settings/plugins/ibft/nms-ibft-connection.c b/src/settings/plugins/ibft/nms-ibft-connection.c index 834ba83408..3c00c99fb7 100644 --- a/src/settings/plugins/ibft/nms-ibft-connection.c +++ b/src/settings/plugins/ibft/nms-ibft-connection.c @@ -62,7 +62,7 @@ nms_ibft_connection_new (const GPtrArray *block, GError **error) /* Update settings with what was read from iscsiadm */ if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), source, - FALSE, + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, NULL, error)) g_clear_object (&object); 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 4c1d02ae08..5fca37fc44 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c @@ -429,18 +429,12 @@ nm_ifcfg_connection_new (NMConnection *source, NMConnection *tmp; char *unhandled_spec = NULL; const char *unmanaged_spec = NULL, *unrecognized_spec = NULL; - gboolean update_unsaved = TRUE; g_assert (source || full_path); if (out_ignore_error) *out_ignore_error = FALSE; - if (full_path) { - /* The connection already is on the disk */ - update_unsaved = FALSE; - } - /* If we're given a connection already, prefer that instead of re-reading */ if (source) tmp = g_object_ref (source); @@ -466,7 +460,9 @@ nm_ifcfg_connection_new (NMConnection *source, /* Update our settings with what was read from the file */ if (nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, - update_unsaved, + full_path + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP /* connection is already on disk */ + : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, NULL, error)) nm_ifcfg_connection_check_devtimeout (NM_IFCFG_CONNECTION (object)); 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 1ebdeafa7f..9671a646b4 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c @@ -315,7 +315,7 @@ update_connection (SettingsPluginIfcfg *self, if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (connection_by_uuid), NM_CONNECTION (connection_new), - FALSE, /* don't set Unsaved */ + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, "ifcfg-update", &local)) { /* Shouldn't ever get here as 'connection_new' was verified by the reader already diff --git a/src/settings/plugins/ifnet/nms-ifnet-connection.c b/src/settings/plugins/ifnet/nms-ifnet-connection.c index 5dbb124c30..ddd8ab7162 100644 --- a/src/settings/plugins/ifnet/nms-ifnet-connection.c +++ b/src/settings/plugins/ifnet/nms-ifnet-connection.c @@ -188,7 +188,9 @@ nm_ifnet_connection_new (NMConnection *source, const char *conn_name) NM_IFNET_CONNECTION_GET_PRIVATE ((NMIfnetConnection *) object)->conn_name = g_strdup (conn_name); if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, - update_unsaved, + update_unsaved + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY + : NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, NULL, NULL)) { g_object_unref (object); diff --git a/src/settings/plugins/ifnet/nms-ifnet-plugin.c b/src/settings/plugins/ifnet/nms-ifnet-plugin.c index 998b04b47a..23a03e5ce3 100644 --- a/src/settings/plugins/ifnet/nms-ifnet-plugin.c +++ b/src/settings/plugins/ifnet/nms-ifnet-plugin.c @@ -271,7 +271,7 @@ reload_connections (NMSettingsPlugin *config) /* Update existing connection with new settings */ if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (old), NM_CONNECTION (new), - FALSE, /* don't set Unsaved */ + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, "ifnet-update", &error)) { /* Shouldn't ever get here as 'new' was verified by the reader already diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.c b/src/settings/plugins/keyfile/nms-keyfile-connection.c index 300aa9f7b3..fe1c651339 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-connection.c +++ b/src/settings/plugins/keyfile/nms-keyfile-connection.c @@ -161,7 +161,9 @@ nms_keyfile_connection_new (NMConnection *source, /* Update our settings with what was read from the file */ if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, - update_unsaved, + update_unsaved + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY + : NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, NULL, error)) { g_object_unref (object); diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index dfc311af33..660990fd3b 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -260,7 +260,7 @@ update_connection (NMSKeyfilePlugin *self, if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (connection_by_uuid), NM_CONNECTION (connection_new), - FALSE, /* don't set Unsaved */ + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, "keyfile-update", &local)) { /* Shouldn't ever get here as 'connection_new' was verified by the reader already From 75f787d1da309166ce6b5a9df4ddfe681b85b6cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Nov 2017 19:23:05 +0100 Subject: [PATCH 03/35] settings: split nm_settings_connection_commit_changes() to call it without preparing the new connection Will be used next. --- src/settings/nm-settings-connection.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 64fec2b12d..54b3de1621 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -655,11 +655,12 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, error); } -gboolean -nm_settings_connection_commit_changes (NMSettingsConnection *self, - NMConnection *new_connection, - NMSettingsConnectionCommitReason commit_reason, - GError **error) +static gboolean +_commit_changes_full (NMSettingsConnection *self, + NMConnection *new_connection, + gboolean prepare_new_connection, + NMSettingsConnectionCommitReason commit_reason, + GError **error) { NMSettingsConnectionClass *klass; gs_free_error GError *local = NULL; @@ -679,7 +680,8 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self, return FALSE; } - if ( new_connection + if ( prepare_new_connection + && new_connection && !nm_settings_connection_replace_settings_prepare (self, new_connection, &local)) { @@ -731,6 +733,19 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self, return TRUE; } +gboolean +nm_settings_connection_commit_changes (NMSettingsConnection *self, + NMConnection *new_connection, + NMSettingsConnectionCommitReason commit_reason, + GError **error) +{ + return _commit_changes_full (self, + new_connection, + TRUE, + commit_reason, + error); +} + static void remove_entry_from_db (NMSettingsConnection *self, const char* db_name) { From 3706fd17ebaf2722fb587c575ecbafefc9b52172 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Nov 2017 19:27:13 +0100 Subject: [PATCH 04/35] settings: refactor update_auth_cb() and prepare connection once Note how nm_settings_connection_commit_changes() would call prepare() a second time. Don't do that. Also, move the prepare step earlier, and call _replace_settings_full() without preparing the new connection again. --- src/settings/nm-settings-connection.c | 34 +++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 54b3de1621..ea8bff4429 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1716,7 +1716,9 @@ update_auth_cb (NMSettingsConnection *self, */ update_agent_secrets_cache (self, info->new_settings); } + } + if (info->new_settings) { if (nm_audit_manager_audit_enabled (nm_audit_manager_get ())) { gs_unref_hashtable GHashTable *diff = NULL; gboolean same; @@ -1730,17 +1732,6 @@ update_auth_cb (NMSettingsConnection *self, } } - if (!info->save_to_disk) { - if (info->new_settings) { - nm_settings_connection_replace_settings (self, - info->new_settings, - NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, - "replace-unsaved", - &local); - } - goto out; - } - if (info->new_settings) { if (!nm_settings_connection_replace_settings_prepare (self, info->new_settings, @@ -1748,16 +1739,29 @@ update_auth_cb (NMSettingsConnection *self, goto out; } + if (!info->save_to_disk) { + if (info->new_settings) { + _replace_settings_full (self, + info->new_settings, + FALSE, + NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, + "replace-unsaved", + &local); + } + goto out; + } + commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; if ( info->new_settings && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), nm_connection_get_id (info->new_settings))) commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; - nm_settings_connection_commit_changes (self, - info->new_settings, - commit_reason, - &local); + _commit_changes_full (self, + info->new_settings, + FALSE, + commit_reason, + &local); out: if (!local) { From 9988f9dbbb4261e28d92b6dc6e0b9d8567028794 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Nov 2017 19:35:39 +0100 Subject: [PATCH 05/35] settings: call replace-settings also without new settings _replace_settings_full() does more then just replace the settings. It at least also sets some flags, like saved/unsaved depending on @persist_mode. _replace_settings_full() also emits the "updated" signal. Note that previously it would just return without signal if nm_connection_compare() indicates that there is no difference. But the caller probably cares about whether the user tries to change the connection at all, not whether the change actually introduced a real difference in the settings. Like, policy might re-set the autoconnect blocked flags. But it should do so on every user-update, regardless of whether a change was actually made. It makes sense to call _replace_settings_full() with a @new_connection of %NULL, to mean: just update the flags, but keep the current connection. Extend _replace_settings_full() to allow for that. Also, update update_auth_cb() to always call _replace_settings_full(), even if no new connection is provided. In this case, only update the connection flags accordingly. --- src/settings/nm-settings-connection.c | 93 ++++++++++++++------------- 1 file changed, 47 insertions(+), 46 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index ea8bff4429..e0275613aa 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -561,56 +561,60 @@ _replace_settings_full (NMSettingsConnection *self, GError **error) { NMSettingsConnectionPrivate *priv; + gboolean replaced = FALSE; g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), FALSE); - g_return_val_if_fail (NM_IS_CONNECTION (new_connection), FALSE); + g_return_val_if_fail (!new_connection || NM_IS_CONNECTION (new_connection), FALSE); priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); if ( prepare_new_connection + && new_connection && !nm_settings_connection_replace_settings_prepare (self, new_connection, error)) return FALSE; - /* Do nothing if there's nothing to update */ - if (nm_connection_compare (NM_CONNECTION (self), - new_connection, - NM_SETTING_COMPARE_FLAG_EXACT)) { - return TRUE; - } - /* Disconnect the changed signal to ensure we don't set Unsaved when * it's not required. */ g_signal_handlers_block_by_func (self, G_CALLBACK (connection_changed_cb), NULL); - if (log_diff_name) - nm_utils_log_connection_diff (new_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ "); + /* Do nothing if there's nothing to update */ + if ( new_connection + && !nm_connection_compare (NM_CONNECTION (self), + new_connection, + NM_SETTING_COMPARE_FLAG_EXACT)) { + if (log_diff_name) + nm_utils_log_connection_diff (new_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ "); - nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection); + nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection); - _LOGD ("replace settings from connection %p (%s)", new_connection, nm_connection_get_id (NM_CONNECTION (self))); + replaced = TRUE; + _LOGD ("replace settings from connection %p (%s)", new_connection, nm_connection_get_id (NM_CONNECTION (self))); + } nm_settings_connection_set_flags (self, NM_SETTINGS_CONNECTION_FLAGS_NM_GENERATED | NM_SETTINGS_CONNECTION_FLAGS_VOLATILE, FALSE); - /* Cache the just-updated system secrets in case something calls - * nm_connection_clear_secrets() and clears them. - */ - update_system_secrets_cache (self); + if (replaced) { + /* Cache the just-updated system secrets in case something calls + * nm_connection_clear_secrets() and clears them. + */ + update_system_secrets_cache (self); - /* Add agent and always-ask secrets back; they won't necessarily be - * in the replacement connection data if it was eg reread from disk. - */ - if (priv->agent_secrets) { - GVariant *dict; + /* Add agent and always-ask secrets back; they won't necessarily be + * 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_CONNECTION (self), NULL, dict, NULL); - g_variant_unref (dict); + dict = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + if (dict) { + (void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, dict, NULL); + g_variant_unref (dict); + } } } @@ -1740,29 +1744,26 @@ update_auth_cb (NMSettingsConnection *self, } if (!info->save_to_disk) { - if (info->new_settings) { - _replace_settings_full (self, - info->new_settings, - FALSE, - NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, - "replace-unsaved", - &local); - } - goto out; + _replace_settings_full (self, + info->new_settings, + FALSE, + NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, + "replace-unsaved", + &local); + } else { + commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; + if ( info->new_settings + && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), + nm_connection_get_id (info->new_settings))) + commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; + + _commit_changes_full (self, + info->new_settings, + FALSE, + commit_reason, + &local); } - commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; - if ( info->new_settings - && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), - nm_connection_get_id (info->new_settings))) - commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; - - _commit_changes_full (self, - info->new_settings, - FALSE, - commit_reason, - &local); - out: if (!local) { gs_unref_object NMConnection *for_agent = NULL; From 4fd5e6e1bb7ee12a389a741212c899d3e15b27f1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 Nov 2017 12:46:18 +0100 Subject: [PATCH 06/35] settings: in _commit_changes_full() mark the connection as saved before emitting signals _commit_changes_full() calls _replace_settings_full() which already emits signals about the changed connection. We should mark the connectin as saved earlier. In fact, we can tell _replace_settings_full() to mark it as not-unsaved via the persist-mode parameter. --- src/settings/nm-settings-connection.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index e0275613aa..394099d61e 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -711,7 +711,7 @@ _commit_changes_full (NMSettingsConnection *self, if (!_replace_settings_full (self, reread_connection ?: new_connection, !reread_connection, - NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, + NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, new_connection ? "update-during-write" : "replace-and-commit-disk", @@ -723,9 +723,8 @@ _commit_changes_full (NMSettingsConnection *self, g_propagate_error (error, g_steal_pointer (&local)); g_return_val_if_reached (FALSE); } - } - - set_unsaved (self, FALSE); + } else + set_unsaved (self, FALSE); if (reread_connection) _LOGI ("write: successfully updated (%s), connection was modified in the process", logmsg_change); From 7a0900be7cfe8d4d2538e16cc3ec8d325d8fc8b5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 Nov 2017 15:56:27 +0100 Subject: [PATCH 07/35] settings: always call _commit_changes_full() instead of _replace_settings_full() All callers of _replace_settings_full() now use _commit_changes_full(). commit-changes should contain all the logic what to do when updating a connection. Now, the connection might optionally be written to disk. --- src/settings/nm-settings-connection.c | 141 +++++++++++++++----------- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 394099d61e..dee8b18321 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -150,6 +150,16 @@ G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_E /*****************************************************************************/ +static gboolean _commit_changes_full (NMSettingsConnection *self, + NMConnection *new_connection, + gboolean prepare_new_connection, + NMSettingsConnectionPersistMode persist_mode, + NMSettingsConnectionCommitReason commit_reason, + const char *log_diff_name, + GError **error); + +/*****************************************************************************/ + static void _emit_updated (NMSettingsConnection *self, gboolean by_user) { @@ -651,37 +661,48 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, const char *log_diff_name, GError **error) { - return _replace_settings_full (self, - new_connection, - TRUE, - persist_mode, - log_diff_name, - error); + return _commit_changes_full (self, + new_connection, + TRUE, + persist_mode, + NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, + log_diff_name, + error); } static gboolean _commit_changes_full (NMSettingsConnection *self, NMConnection *new_connection, gboolean prepare_new_connection, + NMSettingsConnectionPersistMode persist_mode, NMSettingsConnectionCommitReason commit_reason, + const char *log_diff_name, GError **error) { - NMSettingsConnectionClass *klass; + NMSettingsConnectionClass *klass = NULL; gs_free_error GError *local = NULL; gs_unref_object NMConnection *reread_connection = NULL; gs_free char *logmsg_change = NULL; g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), FALSE); - klass = NM_SETTINGS_CONNECTION_GET_CLASS (self); - if (!klass->commit_changes) { - _LOGW ("write: setting plugin %s does not support to write connection", - G_OBJECT_TYPE_NAME (self)); - g_set_error (error, - NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_FAILED, - "writing settings not supported"); - return FALSE; + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP) { + persist_mode = nm_settings_connection_get_unsaved (self) + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY + : NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK; + } + + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { + klass = NM_SETTINGS_CONNECTION_GET_CLASS (self); + if (!klass->commit_changes) { + _LOGW ("write: setting plugin %s does not support to write connection", + G_OBJECT_TYPE_NAME (self)); + g_set_error (error, + NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_FAILED, + "writing settings not supported"); + return FALSE; + } } if ( prepare_new_connection @@ -695,36 +716,27 @@ _commit_changes_full (NMSettingsConnection *self, return FALSE; } - if (!klass->commit_changes (self, - new_connection, - commit_reason, - &reread_connection, - &logmsg_change, - &local)) { - _LOGW ("write: failure to write setting: %s", - local->message); - g_propagate_error (error, g_steal_pointer (&local)); - return FALSE; - } - - if (reread_connection || new_connection) { - if (!_replace_settings_full (self, - reread_connection ?: new_connection, - !reread_connection, - NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, - new_connection - ? "update-during-write" - : "replace-and-commit-disk", - &local)) { - /* this can't really happen, because at this point replace-settings - * is no longer supposed to fail. It's a bug. */ - _LOGE ("write: replacing setting failed: %s", + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { + if (!klass->commit_changes (self, + new_connection, + commit_reason, + &reread_connection, + &logmsg_change, + &local)) { + _LOGW ("write: failure to write setting: %s", local->message); g_propagate_error (error, g_steal_pointer (&local)); - g_return_val_if_reached (FALSE); + return FALSE; } - } else - set_unsaved (self, FALSE); + } + + if (!_replace_settings_full (self, + reread_connection ?: new_connection, + !reread_connection, + persist_mode, + log_diff_name, + error)) + return FALSE; if (reread_connection) _LOGI ("write: successfully updated (%s), connection was modified in the process", logmsg_change); @@ -745,7 +757,11 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self, return _commit_changes_full (self, new_connection, TRUE, + NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, commit_reason, + new_connection + ? "update-during-write" + : "replace-and-commit-disk", error); } @@ -1742,26 +1758,27 @@ update_auth_cb (NMSettingsConnection *self, goto out; } - if (!info->save_to_disk) { - _replace_settings_full (self, - info->new_settings, - FALSE, - NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, - "replace-unsaved", - &local); - } else { - commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; - if ( info->new_settings - && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), - nm_connection_get_id (info->new_settings))) - commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; + commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; + if ( info->new_settings + && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), + nm_connection_get_id (info->new_settings))) + commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; - _commit_changes_full (self, - info->new_settings, - FALSE, - commit_reason, - &local); - } + _commit_changes_full (self, + info->new_settings, + FALSE, + info->save_to_disk + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK + : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, + commit_reason, + !info->save_to_disk + ? (info->new_settings + ? "update-unsaved" + : "make-unsaved") + : (info->new_settings + ? "update-settings" + : "write-out-to-disk"), + &local); out: if (!local) { From ad9deb69682192e2d38405fc079b935e0b201167 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 Nov 2017 16:39:25 +0100 Subject: [PATCH 08/35] settings: merge _replace_settings_full() into _commit_changes_full() It's complicated what happens during a commit/replace/update (whatever you call it). It doesn't get simpler by spreading it out to various functions. Let's have one large function (_commit_changes_full()) which does all the steps necessary. There should be no alternative ways how to update a connection. --- src/settings/nm-settings-connection.c | 215 ++++++++++---------------- 1 file changed, 84 insertions(+), 131 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index dee8b18321..b0bdc0dc04 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -150,16 +150,6 @@ G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_E /*****************************************************************************/ -static gboolean _commit_changes_full (NMSettingsConnection *self, - NMConnection *new_connection, - gboolean prepare_new_connection, - NMSettingsConnectionPersistMode persist_mode, - NMSettingsConnectionCommitReason commit_reason, - const char *log_diff_name, - GError **error); - -/*****************************************************************************/ - static void _emit_updated (NMSettingsConnection *self, gboolean by_user) { @@ -563,27 +553,65 @@ nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self, } static gboolean -_replace_settings_full (NMSettingsConnection *self, - NMConnection *new_connection, - gboolean prepare_new_connection, - NMSettingsConnectionPersistMode persist_mode, - const char *log_diff_name, - GError **error) +_commit_changes_full (NMSettingsConnection *self, + NMConnection *new_connection, + NMSettingsConnectionPersistMode persist_mode, + NMSettingsConnectionCommitReason commit_reason, + const char *log_diff_name, + GError **error) { NMSettingsConnectionPrivate *priv; + NMSettingsConnectionClass *klass = NULL; + gs_unref_object NMConnection *reread_connection = NULL; + NMConnection *replace_connection; gboolean replaced = FALSE; + gs_free char *logmsg_change = NULL; + GError *local = NULL; g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), FALSE); - g_return_val_if_fail (!new_connection || NM_IS_CONNECTION (new_connection), FALSE); priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - if ( prepare_new_connection - && new_connection + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP) { + persist_mode = nm_settings_connection_get_unsaved (self) + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY + : NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK; + } + + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { + klass = NM_SETTINGS_CONNECTION_GET_CLASS (self); + if (!klass->commit_changes) { + g_set_error (&local, + NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_FAILED, + "writing settings not supported"); + goto out; + } + } + + if ( new_connection && !nm_settings_connection_replace_settings_prepare (self, new_connection, - error)) - return FALSE; + &local)) + goto out; + + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { + if (!klass->commit_changes (self, + new_connection, + commit_reason, + &reread_connection, + &logmsg_change, + &local)) + goto out; + + if ( reread_connection + && !nm_settings_connection_replace_settings_prepare (self, + reread_connection, + &local)) + goto out; + } + + replace_connection = reread_connection ?: new_connection; /* Disconnect the changed signal to ensure we don't set Unsaved when * it's not required. @@ -591,17 +619,16 @@ _replace_settings_full (NMSettingsConnection *self, g_signal_handlers_block_by_func (self, G_CALLBACK (connection_changed_cb), NULL); /* Do nothing if there's nothing to update */ - if ( new_connection + if ( replace_connection && !nm_connection_compare (NM_CONNECTION (self), - new_connection, + replace_connection, NM_SETTING_COMPARE_FLAG_EXACT)) { if (log_diff_name) - nm_utils_log_connection_diff (new_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ "); + nm_utils_log_connection_diff (replace_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ "); - nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection); + nm_connection_replace_settings_from_connection (NM_CONNECTION (self), replace_connection); replaced = TRUE; - _LOGD ("replace settings from connection %p (%s)", new_connection, nm_connection_get_id (NM_CONNECTION (self))); } nm_settings_connection_set_flags (self, @@ -648,9 +675,40 @@ _replace_settings_full (NMSettingsConnection *self, _emit_updated (self, TRUE); +out: + if (local) { + _LOGI ("write: failure to update connection: %s", local->message); + g_propagate_error (error, local); + return FALSE; + } + + if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { + if (reread_connection) + _LOGI ("write: successfully updated (%s), connection was modified in the process", logmsg_change); + else if (new_connection) + _LOGI ("write: successfully updated (%s)", logmsg_change); + else + _LOGI ("write: successfully commited (%s)", logmsg_change); + } return TRUE; } +gboolean +nm_settings_connection_commit_changes (NMSettingsConnection *self, + NMConnection *new_connection, + NMSettingsConnectionCommitReason commit_reason, + GError **error) +{ + return _commit_changes_full (self, + new_connection, + NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, + commit_reason, + new_connection + ? "update-during-write" + : "replace-and-commit-disk", + error); +} + /* Update the settings of this connection to match that of 'new_connection', * taking care to make a private copy of secrets. */ @@ -663,108 +721,12 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, { return _commit_changes_full (self, new_connection, - TRUE, persist_mode, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, log_diff_name, error); } -static gboolean -_commit_changes_full (NMSettingsConnection *self, - NMConnection *new_connection, - gboolean prepare_new_connection, - NMSettingsConnectionPersistMode persist_mode, - NMSettingsConnectionCommitReason commit_reason, - const char *log_diff_name, - GError **error) -{ - NMSettingsConnectionClass *klass = NULL; - gs_free_error GError *local = NULL; - gs_unref_object NMConnection *reread_connection = NULL; - gs_free char *logmsg_change = NULL; - - g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), FALSE); - - if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP) { - persist_mode = nm_settings_connection_get_unsaved (self) - ? NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY - : NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK; - } - - if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { - klass = NM_SETTINGS_CONNECTION_GET_CLASS (self); - if (!klass->commit_changes) { - _LOGW ("write: setting plugin %s does not support to write connection", - G_OBJECT_TYPE_NAME (self)); - g_set_error (error, - NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_FAILED, - "writing settings not supported"); - return FALSE; - } - } - - if ( prepare_new_connection - && new_connection - && !nm_settings_connection_replace_settings_prepare (self, - new_connection, - &local)) { - _LOGW ("write: failed to prepare connection for writing: %s", - local->message); - g_propagate_error (error, g_steal_pointer (&local)); - return FALSE; - } - - if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { - if (!klass->commit_changes (self, - new_connection, - commit_reason, - &reread_connection, - &logmsg_change, - &local)) { - _LOGW ("write: failure to write setting: %s", - local->message); - g_propagate_error (error, g_steal_pointer (&local)); - return FALSE; - } - } - - if (!_replace_settings_full (self, - reread_connection ?: new_connection, - !reread_connection, - persist_mode, - log_diff_name, - error)) - return FALSE; - - if (reread_connection) - _LOGI ("write: successfully updated (%s), connection was modified in the process", logmsg_change); - else if (new_connection) - _LOGI ("write: successfully updated (%s)", logmsg_change); - else - _LOGI ("write: successfully commited (%s)", logmsg_change); - - return TRUE; -} - -gboolean -nm_settings_connection_commit_changes (NMSettingsConnection *self, - NMConnection *new_connection, - NMSettingsConnectionCommitReason commit_reason, - GError **error) -{ - return _commit_changes_full (self, - new_connection, - TRUE, - NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, - commit_reason, - new_connection - ? "update-during-write" - : "replace-and-commit-disk", - error); -} - static void remove_entry_from_db (NMSettingsConnection *self, const char* db_name) { @@ -1751,13 +1713,6 @@ update_auth_cb (NMSettingsConnection *self, } } - if (info->new_settings) { - if (!nm_settings_connection_replace_settings_prepare (self, - info->new_settings, - &local)) - goto out; - } - commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION; if ( info->new_settings && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)), @@ -1766,7 +1721,6 @@ update_auth_cb (NMSettingsConnection *self, _commit_changes_full (self, info->new_settings, - FALSE, info->save_to_disk ? NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, @@ -1780,7 +1734,6 @@ update_auth_cb (NMSettingsConnection *self, : "write-out-to-disk"), &local); -out: if (!local) { gs_unref_object NMConnection *for_agent = NULL; From f73e78770e9930cb111ecf5b6b11209daa13f9a0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 Nov 2017 16:49:08 +0100 Subject: [PATCH 09/35] settings/trivial: rename update function in nm-settings-connection.c --- src/settings/nm-settings-connection.c | 88 +++++++++++++-------------- src/settings/nm-settings-connection.h | 4 -- 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index b0bdc0dc04..22c389c872 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -525,10 +525,10 @@ connection_changed_cb (NMSettingsConnection *self, gpointer unused) _emit_updated (self, FALSE); } -gboolean -nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self, - NMConnection *new_connection, - GError **error) +static gboolean +_update_prepare (NMSettingsConnection *self, + NMConnection *new_connection, + GError **error) { NMSettingsConnectionPrivate *priv; @@ -553,12 +553,12 @@ nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self, } static gboolean -_commit_changes_full (NMSettingsConnection *self, - NMConnection *new_connection, - NMSettingsConnectionPersistMode persist_mode, - NMSettingsConnectionCommitReason commit_reason, - const char *log_diff_name, - GError **error) +_update (NMSettingsConnection *self, + NMConnection *new_connection, + NMSettingsConnectionPersistMode persist_mode, + NMSettingsConnectionCommitReason commit_reason, + const char *log_diff_name, + GError **error) { NMSettingsConnectionPrivate *priv; NMSettingsConnectionClass *klass = NULL; @@ -590,9 +590,9 @@ _commit_changes_full (NMSettingsConnection *self, } if ( new_connection - && !nm_settings_connection_replace_settings_prepare (self, - new_connection, - &local)) + && !_update_prepare (self, + new_connection, + &local)) goto out; if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) { @@ -605,9 +605,9 @@ _commit_changes_full (NMSettingsConnection *self, goto out; if ( reread_connection - && !nm_settings_connection_replace_settings_prepare (self, - reread_connection, - &local)) + && !_update_prepare (self, + reread_connection, + &local)) goto out; } @@ -699,14 +699,14 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self, NMSettingsConnectionCommitReason commit_reason, GError **error) { - return _commit_changes_full (self, - new_connection, - NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, - commit_reason, - new_connection - ? "update-during-write" - : "replace-and-commit-disk", - error); + return _update (self, + new_connection, + NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, + commit_reason, + new_connection + ? "update-during-write" + : "replace-and-commit-disk", + error); } /* Update the settings of this connection to match that of 'new_connection', @@ -719,12 +719,12 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, const char *log_diff_name, GError **error) { - return _commit_changes_full (self, - new_connection, - persist_mode, - NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, - log_diff_name, - error); + return _update (self, + new_connection, + persist_mode, + NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, + log_diff_name, + error); } static void @@ -1719,20 +1719,20 @@ update_auth_cb (NMSettingsConnection *self, nm_connection_get_id (info->new_settings))) commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED; - _commit_changes_full (self, - info->new_settings, - info->save_to_disk - ? NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK - : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, - commit_reason, - !info->save_to_disk - ? (info->new_settings - ? "update-unsaved" - : "make-unsaved") - : (info->new_settings - ? "update-settings" - : "write-out-to-disk"), - &local); + _update (self, + info->new_settings, + info->save_to_disk + ? NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK + : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY, + commit_reason, + !info->save_to_disk + ? (info->new_settings + ? "update-unsaved" + : "make-unsaved") + : (info->new_settings + ? "update-settings" + : "write-out-to-disk"), + &local); if (!local) { gs_unref_object NMConnection *for_agent = NULL; diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index cb9d99879e..d4be5a5526 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -136,10 +136,6 @@ gboolean nm_settings_connection_commit_changes (NMSettingsConnection *self, NMSettingsConnectionCommitReason commit_reason, GError **error); -gboolean nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self, - NMConnection *new_connection, - GError **error); - typedef enum { NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, From d26c749ea6188652dc01e7150354635300f7e8ad Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 Nov 2017 17:07:06 +0100 Subject: [PATCH 10/35] checkpoint: don't bypass settings-connection commit code on rollback commit involves more then just replacing the setting and writing them out. What? Dunno. It's complex. But let's not bypass the commit-changes function. That one is supposed to get it right. --- src/nm-checkpoint.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index af56bf26e6..39a6c255cf 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -254,10 +254,8 @@ activate: if (need_update) { _LOGD ("rollback: updating connection %s", nm_settings_connection_get_uuid (connection)); - nm_connection_replace_settings_from_connection (NM_CONNECTION (connection), - dev_checkpoint->settings_connection); nm_settings_connection_commit_changes (connection, - NULL, + dev_checkpoint->settings_connection, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, NULL); } From 98ee18d888df477c35a8f2ce258ac2daf1099b26 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 Nov 2017 19:46:00 +0100 Subject: [PATCH 11/35] all: add new D-Bus API org.freedesktop.NetworkManager.Settings.Connection.Update2() We already have Update(), UpdateUnsaved() and Save(), which serve similar purposes. We will need a form of update with another argument. Most notably, to block autoconnect while doing the update. Other use cases could be to prevent reapplying connection.zone and connection.metered, to to reapply all changes. Instead of adding a specific update function that only serves that new use-case, add a extensible Update2() function. It can be extended to cope with future variants of update. --- ...top.NetworkManager.Settings.Connection.xml | 34 +++++ libnm-core/nm-dbus-interface.h | 14 ++ libnm-core/nm-errors.h | 2 + libnm/libnm.ver | 1 + libnm/nm-remote-connection.c | 61 ++++---- src/settings/nm-settings-connection.c | 134 ++++++++++++++---- 6 files changed, 188 insertions(+), 58 deletions(-) diff --git a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml index 61bd26ddc6..6e518677e9 100644 --- a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml +++ b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml @@ -101,6 +101,40 @@ --> + + + + + + + +