rpms
/
NetworkManager
7
0
Fork 0
Code Issues Pull Requests Releases Activity
NetworkManager/SOURCES/1008-checkpoint-preserve-in...

488 lines
22 KiB
Diff
Raw Permalink Blame History

From d6837f0bd30da069d327099cb555854630cd4584 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 2 May 2024 16:40:26 +0200
Subject: [PATCH 1/2] settings: add
nm_settings_connection_persist_mode_to_string()
(cherry picked from commit a48b7fe7b9d8adf4902c7b3cfcc4d89bc46cbbef)
(cherry picked from commit e5837aa1d3960b743adcff0a5041445ccd65fb93)
---
src/core/settings/nm-settings-connection.c | 23 ++++++++++++++++++++++
src/core/settings/nm-settings-connection.h | 4 ++++
2 files changed, 27 insertions(+)
diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c
index 176cc2c252..459c60ad1e 100644
--- a/src/core/settings/nm-settings-connection.c
+++ b/src/core/settings/nm-settings-connection.c
@@ -226,6 +226,29 @@ static guint _get_seen_bssids(NMSettingsConnection *self,
/*****************************************************************************/
+char *
+nm_settings_connection_persist_mode_to_string(NMSettingsConnectionPersistMode mode)
+{
+ switch (mode) {
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY:
+ return "in-memory";
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_DETACHED:
+ return "in-memory-detached";
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_ONLY:
+ return "in-memory-only";
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP:
+ return "keep";
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_NO_PERSIST:
+ return "no-persist";
+ case NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK:
+ return "to-disk";
+ }
+
+ return nm_assert_unreachable_val(NULL);
+}
+
+/*****************************************************************************/
+
NMSettings *
nm_settings_connection_get_settings(NMSettingsConnection *self)
{
diff --git a/src/core/settings/nm-settings-connection.h b/src/core/settings/nm-settings-connection.h
index 835a978e40..d15a75b749 100644
--- a/src/core/settings/nm-settings-connection.h
+++ b/src/core/settings/nm-settings-connection.h
@@ -379,4 +379,8 @@ void _nm_settings_connection_emit_signal_updated_internal(
void _nm_settings_connection_cleanup_after_remove(NMSettingsConnection *self);
+/*****************************************************************************/
+
+char *nm_settings_connection_persist_mode_to_string(NMSettingsConnectionPersistMode mode);
+
#endif /* __NETWORKMANAGER_SETTINGS_CONNECTION_H__ */
--
2.41.0
From c6f9d0a6d5c864ba0141b6e985727cd69c5560fa Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 15 Apr 2024 10:51:24 +0200
Subject: [PATCH 2/2] checkpoint: preserve in-memory state of connections
If a connection is in-memory (i.e. has flag "unsaved"), after a
checkpoint and rollback it can be wrongly persisted to disk:
- if the connection was modified and written to disk after the
rollback, during the rollback we update it again with persist mode
"keep", which keeps it on disk;
- if the connection was deleted after the rollback, during the
rollback we add it again with persist mode "to-disk".
Instead, remember whether the connection had the "unsaved" flag set
and try to restore the previous state.
However, this is not straightforward as there are 4 different possible
states for the settings connection: persistent; in-memory only;
in-memory shadowing a persistent file; in-memory shadowing a detached
persistent file (i.e. the deletion of the connection doesn't delete
the persistent file). Handle all those cases.
Fixes: 3e09aed2a09f ('checkpoint: add create, rollback and destroy D-Bus API')
(cherry picked from commit c979bfeb8b0d3bed19bac2ad01a6a6ed899f924e)
(cherry picked from commit ebf25794d9cd89190775ac401c36d63aa1c108f7)
---
NEWS | 8 ++
src/core/nm-checkpoint.c | 242 ++++++++++++++++++++++++++++++++-------
2 files changed, 211 insertions(+), 39 deletions(-)
diff --git a/NEWS b/NEWS
index 6ac3118db9..e33152c6f4 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,11 @@
+===============================================
+NetworkManager-1.46.2
+Overview of changes since NetworkManager-1.46.0
+===============================================
+
+* Properly restore in-memory connection profiles during the rollback
+ of a checkpoint.
+
=============================================
NetworkManager-1.46
Overview of changes since NetworkManager-1.44
diff --git a/src/core/nm-checkpoint.c b/src/core/nm-checkpoint.c
index cc5c189bf9..ffcf6e3aad 100644
--- a/src/core/nm-checkpoint.c
+++ b/src/core/nm-checkpoint.c
@@ -10,6 +10,7 @@
#include "nm-active-connection.h"
#include "nm-act-request.h"
#include "libnm-core-aux-intern/nm-auth-subject.h"
+#include "libnm-core-intern/nm-keyfile-internal.h"
#include "nm-core-utils.h"
#include "nm-dbus-interface.h"
#include "devices/nm-device.h"
@@ -17,6 +18,7 @@
#include "nm-manager.h"
#include "settings/nm-settings.h"
#include "settings/nm-settings-connection.h"
+#include "settings/plugins/keyfile/nms-keyfile-storage.h"
#include "nm-simple-connection.h"
#include "nm-utils.h"
@@ -29,11 +31,14 @@ typedef struct {
NMDevice *device;
NMConnection *applied_connection;
NMConnection *settings_connection;
+ NMConnection *settings_connection_shadowed;
guint64 ac_version_id;
NMDeviceState state;
bool is_software : 1;
bool realized : 1;
bool activation_lifetime_bound_to_profile_visibility : 1;
+ bool settings_connection_is_unsaved : 1;
+ bool settings_connection_is_shadowed_owned : 1;
NMUnmanFlagOp unmanaged_explicit;
NMActivationReason activation_reason;
gulong dev_exported_change_id;
@@ -150,37 +155,111 @@ nm_checkpoint_includes_devices_of(NMCheckpoint *self, NMCheckpoint *cp_for_devic
return NULL;
}
+static NMConnection *
+parse_connection_from_shadowed_file(const char *path, GError **error)
+{
+ nm_auto_unref_keyfile GKeyFile *keyfile = NULL;
+ gs_free char *base_dir = NULL;
+ char *sep;
+
+ keyfile = g_key_file_new();
+ if (!g_key_file_load_from_file(keyfile, path, G_KEY_FILE_NONE, error))
+ return NULL;
+
+ sep = strrchr(path, '/');
+ base_dir = g_strndup(path, sep - path);
+
+ return nm_keyfile_read(keyfile, base_dir, NM_KEYFILE_HANDLER_FLAGS_NONE, NULL, NULL, error);
+}
+
static NMSettingsConnection *
-find_settings_connection(NMCheckpoint *self,
- DeviceCheckpoint *dev_checkpoint,
- gboolean *need_update,
- gboolean *need_activation)
+find_settings_connection(NMCheckpoint *self,
+ DeviceCheckpoint *dev_checkpoint,
+ gboolean *need_update,
+ gboolean *need_update_shadowed,
+ gboolean *need_activation,
+ NMSettingsConnectionPersistMode *persist_mode)
{
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE(self);
NMActiveConnection *active;
NMSettingsConnection *sett_conn;
+ const char *shadowed_file;
+ NMConnection *shadowed_connection = NULL;
const char *uuid, *ac_uuid;
const CList *tmp_clist;
-
- *need_activation = FALSE;
- *need_update = FALSE;
+ gboolean sett_conn_unsaved;
+ NMSettingsStorage *storage;
+
+ *need_activation = FALSE;
+ *need_update = FALSE;
+ *need_update_shadowed = FALSE;
+
+ /* With regard to storage, there are 4 different possible states for the settings
+ * connection: 1) persistent; 2) in-memory only; 3) in-memory shadowing a persistent
+ * file; 4) in-memory shadowing a detached persistent file (i.e. the deletion of
+ * the connection doesn't delete the persistent file).
+ */
+ if (dev_checkpoint->settings_connection_is_unsaved) {
+ if (dev_checkpoint->settings_connection_shadowed) {
+ if (dev_checkpoint->settings_connection_is_shadowed_owned)
+ *persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY;
+ else
+ *persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_DETACHED;
+ } else
+ *persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_ONLY;
+ } else {
+ *persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK;
+ }
uuid = nm_connection_get_uuid(dev_checkpoint->settings_connection);
sett_conn = nm_settings_get_connection_by_uuid(NM_SETTINGS_GET, uuid);
- if (!sett_conn)
- return NULL;
-
- /* Now check if the connection changed, ... */
- if (!nm_connection_compare(dev_checkpoint->settings_connection,
- nm_settings_connection_get_connection(sett_conn),
- NM_SETTING_COMPARE_FLAG_EXACT)) {
+ /* Check if the connection changed */
+ if (sett_conn
+ && !nm_connection_compare(dev_checkpoint->settings_connection,
+ nm_settings_connection_get_connection(sett_conn),
+ NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP)) {
_LOGT("rollback: settings connection %s changed", uuid);
*need_update = TRUE;
*need_activation = TRUE;
}
- /* ... is active, ... */
+ storage = sett_conn ? nm_settings_connection_get_storage(sett_conn) : NULL;
+ shadowed_file = storage ? nm_settings_storage_get_shadowed_storage(storage, NULL) : NULL;
+ shadowed_connection =
+ shadowed_file ? parse_connection_from_shadowed_file(shadowed_file, NULL) : NULL;
+
+ if (dev_checkpoint->settings_connection_shadowed) {
+ if (!shadowed_connection
+ || !nm_connection_compare(dev_checkpoint->settings_connection_shadowed,
+ shadowed_connection,
+ NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP)) {
+ _LOGT("rollback: shadowed connection changed for %s", uuid);
+ *need_update_shadowed = TRUE;
+ *need_update = TRUE;
+ }
+ } else {
+ if (shadowed_connection) {
+ _LOGT("rollback: shadowed connection changed for %s", uuid);
+ *need_update = TRUE;
+ }
+ }
+
+ if (!sett_conn)
+ return NULL;
+
+ /* Check if the connection unsaved flag changed */
+ sett_conn_unsaved = NM_FLAGS_HAS(nm_settings_connection_get_flags(sett_conn),
+ NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED);
+ if (sett_conn_unsaved != dev_checkpoint->settings_connection_is_unsaved) {
+ _LOGT("rollback: storage changed for settings connection %s: unsaved (%d -> %d)",
+ uuid,
+ dev_checkpoint->settings_connection_is_unsaved,
+ sett_conn_unsaved);
+ *need_update = TRUE;
+ }
+
+ /* Check if the active state changed */
nm_manager_for_each_active_connection (priv->manager, active, tmp_clist) {
ac_uuid =
nm_settings_connection_get_uuid(nm_active_connection_get_settings_connection(active));
@@ -196,7 +275,7 @@ find_settings_connection(NMCheckpoint *self,
return sett_conn;
}
- /* ... or if the connection was reactivated/reapplied */
+ /* Check if the connection was reactivated/reapplied */
if (nm_active_connection_version_id_get(active) != dev_checkpoint->ac_version_id) {
_LOGT("rollback: active connection version id of %s changed", uuid);
*need_activation = TRUE;
@@ -212,12 +291,19 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
NMSettingsConnection *connection;
gs_unref_object NMAuthSubject *subject = NULL;
GError *local_error = NULL;
- gboolean need_update, need_activation;
+ gboolean need_update;
+ gboolean need_update_shadowed;
+ gboolean need_activation;
NMSettingsConnectionPersistMode persist_mode;
NMSettingsConnectionIntFlags sett_flags;
NMSettingsConnectionIntFlags sett_mask;
- connection = find_settings_connection(self, dev_checkpoint, &need_update, &need_activation);
+ connection = find_settings_connection(self,
+ dev_checkpoint,
+ &need_update,
+ &need_update_shadowed,
+ &need_activation,
+ &persist_mode);
/* FIXME: we need to ensure to re-create/update the profile for the
* same settings plugin. E.g. if it was a keyfile in /run or /etc,
@@ -229,9 +315,26 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
sett_mask = NM_SETTINGS_CONNECTION_INT_FLAGS_NONE;
if (connection) {
+ if (need_update_shadowed) {
+ _LOGD("rollback: updating shadowed file for connection %s",
+ nm_connection_get_uuid(dev_checkpoint->settings_connection));
+ nm_settings_connection_update(
+ connection,
+ NULL,
+ dev_checkpoint->settings_connection_shadowed,
+ NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK,
+ sett_flags,
+ sett_mask,
+ NM_SETTINGS_CONNECTION_UPDATE_REASON_RESET_SYSTEM_SECRETS
+ | NM_SETTINGS_CONNECTION_UPDATE_REASON_UPDATE_NON_SECRET,
+ "checkpoint-rollback",
+ NULL);
+ }
+
if (need_update) {
- _LOGD("rollback: updating connection %s", nm_settings_connection_get_uuid(connection));
- persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP;
+ _LOGD("rollback: updating connection %s with persist mode \"%s\"",
+ nm_connection_get_uuid(dev_checkpoint->settings_connection),
+ nm_settings_connection_persist_mode_to_string(persist_mode));
nm_settings_connection_update(
connection,
NULL,
@@ -246,21 +349,54 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
}
} else {
/* The connection was deleted, recreate it */
- _LOGD("rollback: adding connection %s again",
- nm_connection_get_uuid(dev_checkpoint->settings_connection));
-
- persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK;
- if (!nm_settings_add_connection(NM_SETTINGS_GET,
- NULL,
- dev_checkpoint->settings_connection,
- persist_mode,
- NM_SETTINGS_CONNECTION_ADD_REASON_NONE,
- sett_flags,
- &connection,
- &local_error)) {
- _LOGD("rollback: connection add failure: %s", local_error->message);
- g_clear_error(&local_error);
- return FALSE;
+ if (need_update_shadowed) {
+ _LOGD("rollback: adding back shadowed file for connection %s",
+ nm_connection_get_uuid(dev_checkpoint->settings_connection));
+
+ if (!nm_settings_add_connection(NM_SETTINGS_GET,
+ NULL,
+ dev_checkpoint->settings_connection_shadowed,
+ NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK,
+ NM_SETTINGS_CONNECTION_ADD_REASON_NONE,
+ sett_flags,
+ &connection,
+ &local_error)) {
+ _LOGD("rollback: connection add failure: %s", local_error->message);
+ g_clear_error(&local_error);
+ return FALSE;
+ }
+
+ _LOGD("rollback: updating connection %s with persist mode \"%s\"",
+ nm_connection_get_uuid(dev_checkpoint->settings_connection),
+ nm_settings_connection_persist_mode_to_string(persist_mode));
+
+ nm_settings_connection_update(
+ connection,
+ NULL,
+ dev_checkpoint->settings_connection,
+ persist_mode,
+ sett_flags,
+ sett_mask,
+ NM_SETTINGS_CONNECTION_UPDATE_REASON_RESET_SYSTEM_SECRETS
+ | NM_SETTINGS_CONNECTION_UPDATE_REASON_UPDATE_NON_SECRET,
+ "checkpoint-rollback",
+ NULL);
+ } else {
+ _LOGD("rollback: adding back connection %s with persist mode \"%s\"",
+ nm_connection_get_uuid(dev_checkpoint->settings_connection),
+ nm_settings_connection_persist_mode_to_string(persist_mode));
+ if (!nm_settings_add_connection(NM_SETTINGS_GET,
+ NULL,
+ dev_checkpoint->settings_connection,
+ persist_mode,
+ NM_SETTINGS_CONNECTION_ADD_REASON_NONE,
+ sett_flags,
+ &connection,
+ &local_error)) {
+ _LOGD("rollback: connection add failure: %s", local_error->message);
+ g_clear_error(&local_error);
+ return FALSE;
+ }
}
need_activation = TRUE;
}
@@ -362,11 +498,15 @@ nm_checkpoint_rollback(NMCheckpoint *self)
while (g_hash_table_iter_next(&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) {
guint32 result = NM_ROLLBACK_RESULT_OK;
- _LOGD("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d)",
+ _LOGD("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d, "
+ "connection-unsaved %d, connection-shadowed %d, connection-shadowed-owned %d)",
dev_checkpoint->original_dev_name,
(int) dev_checkpoint->state,
dev_checkpoint->realized,
- dev_checkpoint->unmanaged_explicit);
+ dev_checkpoint->unmanaged_explicit,
+ dev_checkpoint->settings_connection_is_unsaved,
+ !!dev_checkpoint->settings_connection_shadowed,
+ dev_checkpoint->settings_connection_is_shadowed_owned);
if (nm_device_is_real(device)) {
if (!dev_checkpoint->realized) {
@@ -518,6 +658,7 @@ device_checkpoint_destroy(gpointer data)
g_clear_object(&dev_checkpoint->applied_connection);
g_clear_object(&dev_checkpoint->settings_connection);
g_clear_object(&dev_checkpoint->device);
+ g_clear_object(&dev_checkpoint->settings_connection_shadowed);
g_free(dev_checkpoint->original_dev_path);
g_free(dev_checkpoint->original_dev_name);
@@ -555,7 +696,7 @@ _dev_exported_changed(NMDBusObject *obj, NMCheckpoint *checkpoint)
}
static DeviceCheckpoint *
-device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
+device_checkpoint_create(NMCheckpoint *self, NMDevice *device)
{
DeviceCheckpoint *dev_checkpoint;
NMConnection *applied_connection;
@@ -579,7 +720,7 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
dev_checkpoint->dev_exported_change_id = g_signal_connect(device,
NM_DBUS_OBJECT_EXPORTED_CHANGED,
G_CALLBACK(_dev_exported_changed),
- checkpoint);
+ self);
if (nm_device_get_unmanaged_mask(device, NM_UNMANAGED_USER_EXPLICIT)) {
dev_checkpoint->unmanaged_explicit =
@@ -589,6 +730,11 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
act_request = nm_device_get_act_request(device);
if (act_request) {
+ NMSettingsStorage *storage;
+ gboolean shadowed_owned = FALSE;
+ const char *shadowed_file;
+ gs_free_error GError *error = NULL;
+
settings_connection = nm_act_request_get_settings_connection(act_request);
applied_connection = nm_act_request_get_applied_connection(act_request);
@@ -602,6 +748,24 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
dev_checkpoint->activation_lifetime_bound_to_profile_visibility =
NM_FLAGS_HAS(nm_active_connection_get_state_flags(NM_ACTIVE_CONNECTION(act_request)),
NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY);
+
+ dev_checkpoint->settings_connection_is_unsaved =
+ NM_FLAGS_HAS(nm_settings_connection_get_flags(settings_connection),
+ NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED);
+
+ storage = nm_settings_connection_get_storage(settings_connection);
+ shadowed_file =
+ storage ? nm_settings_storage_get_shadowed_storage(storage, &shadowed_owned) : NULL;
+ if (shadowed_file) {
+ dev_checkpoint->settings_connection_is_shadowed_owned = shadowed_owned;
+ dev_checkpoint->settings_connection_shadowed =
+ parse_connection_from_shadowed_file(shadowed_file, &error);
+ if (!dev_checkpoint->settings_connection_shadowed) {
+ _LOGW("error reading shadowed connection file for %s: %s",
+ nm_device_get_iface(device),
+ error->message);
+ }
+ }
}
return dev_checkpoint;
--
2.41.0
Reference in New Issue View Git Blame Copy Permalink