From 2b6191eb63825ef6faf3da38c18d5dbd7111f514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 4 Mar 2026 10:15:32 +0100 Subject: [PATCH] checkpoint: rollback devices' "permanently managed" configuration If a device's "managed" configuration is changed persistently (stored to NM-intern), it needs to be undone in a rollback. --- src/core/devices/nm-device.c | 2 +- src/core/nm-checkpoint.c | 73 ++++++++++++++++++++++++++--- src/core/nm-config.c | 23 ++++++--- src/core/nm-config.h | 1 + src/core/tests/config/test-config.c | 21 +++++---- 5 files changed, 98 insertions(+), 22 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 0ffbf84102..ceccf6c98e 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -15015,7 +15015,7 @@ set_managed(NMDevice *self, NMDeviceManaged managed, NMDeviceManagedFlags flags, gboolean by_mac; managed_to_disk = managed == NM_DEVICE_MANAGED_RESET ? NM_TERNARY_DEFAULT : !!managed; - nm_config_get_device_managed(nm_manager_get_config(priv->manager), self, &old, error); + nm_config_get_device_managed(nm_manager_get_config(priv->manager), self, &old, NULL, error); if (!get_managed_match_by_mac(self, flags, &by_mac, error)) return FALSE; diff --git a/src/core/nm-checkpoint.c b/src/core/nm-checkpoint.c index 45ea3a73b6..10db0dcbab 100644 --- a/src/core/nm-checkpoint.c +++ b/src/core/nm-checkpoint.c @@ -39,7 +39,9 @@ typedef struct { bool activation_lifetime_bound_to_profile_visibility : 1; bool settings_connection_is_unsaved : 1; bool settings_connection_is_shadowed_owned : 1; + bool permanent_managed_by_mac : 1; NMUnmanFlagOp unmanaged_explicit; + NMTernary permanent_managed; NMActivationReason activation_reason; gulong dev_exported_change_id; } DeviceCheckpoint; @@ -496,14 +498,19 @@ nm_checkpoint_rollback(NMCheckpoint *self) /* Start rolling-back each device */ g_hash_table_iter_init(&iter, priv->devices); while (g_hash_table_iter_next(&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) { - guint32 result = NM_ROLLBACK_RESULT_OK; + guint32 result = NM_ROLLBACK_RESULT_OK; + NMTernary perm_managed = NM_TERNARY_DEFAULT; + gboolean perm_managed_by_mac = FALSE; + gboolean force_perm_managed; _LOGD("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d, " - "connection-unsaved %d, connection-shadowed %d, connection-shadowed-owned %d)", + "permanently managed %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->permanent_managed, dev_checkpoint->settings_connection_is_unsaved, !!dev_checkpoint->settings_connection_shadowed, dev_checkpoint->settings_connection_is_shadowed_owned); @@ -541,6 +548,43 @@ nm_checkpoint_rollback(NMCheckpoint *self) NM_DEVICE_STATE_REASON_NOW_MANAGED); } + force_perm_managed = !nm_config_get_device_managed(nm_config_get(), + device, + &perm_managed, + &perm_managed_by_mac, + NULL); + + if (force_perm_managed || (perm_managed != dev_checkpoint->permanent_managed) + || (dev_checkpoint->permanent_managed != NM_TERNARY_DEFAULT + && perm_managed_by_mac != dev_checkpoint->permanent_managed_by_mac)) { + gs_free_error GError *error = NULL; + NMUnmanFlagOp set_op; + + _LOGD("rollback: restore permanent managed state"); + + if (!nm_config_set_device_managed(nm_config_get(), + device, + dev_checkpoint->permanent_managed, + dev_checkpoint->permanent_managed_by_mac, + &error)) { + _LOGE("rollback: failed to restore permanent managed state: %s", error->message); + result = NM_ROLLBACK_RESULT_ERR_FAILED; + /* even if this failed, we try to continue the rollback */ + } + + if (dev_checkpoint->permanent_managed == NM_TERNARY_TRUE) + set_op = NM_UNMAN_FLAG_OP_SET_MANAGED; + else if (dev_checkpoint->permanent_managed == NM_TERNARY_FALSE) + set_op = NM_UNMAN_FLAG_OP_SET_UNMANAGED; + else + set_op = NM_UNMAN_FLAG_OP_FORGET; + + nm_device_set_unmanaged_by_flags_queue(device, + NM_UNMANAGED_USER_CONF, + set_op, + NM_DEVICE_STATE_REASON_NOW_MANAGED); + } + if (dev_checkpoint->state == NM_DEVICE_STATE_UNMANAGED) { if (nm_device_get_state(device) != NM_DEVICE_STATE_UNMANAGED || dev_checkpoint->unmanaged_explicit == NM_UNMAN_FLAG_OP_SET_UNMANAGED) { @@ -703,6 +747,8 @@ device_checkpoint_create(NMCheckpoint *self, NMDevice *device) NMSettingsConnection *settings_connection; const char *path; NMActRequest *act_request; + gboolean perm_managed_by_mac; + gs_free_error GError *error = NULL; nm_assert(NM_IS_DEVICE(device)); nm_assert(nm_device_is_real(device)); @@ -728,12 +774,26 @@ device_checkpoint_create(NMCheckpoint *self, NMDevice *device) } else dev_checkpoint->unmanaged_explicit = NM_UNMAN_FLAG_OP_FORGET; + if (nm_config_get_device_managed(nm_config_get(), + device, + &dev_checkpoint->permanent_managed, + &perm_managed_by_mac, + NULL)) { + dev_checkpoint->permanent_managed_by_mac = perm_managed_by_mac; + } else { + dev_checkpoint->permanent_managed = NM_TERNARY_DEFAULT; + dev_checkpoint->permanent_managed_by_mac = FALSE; + _LOGW("error getting permanent managed state for %s: %s", + nm_device_get_iface(device), + error->message); + g_clear_error(&error); + } + 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; + NMSettingsStorage *storage; + gboolean shadowed_owned = FALSE; + const char *shadowed_file; settings_connection = nm_act_request_get_settings_connection(act_request); applied_connection = nm_act_request_get_applied_connection(act_request); @@ -764,6 +824,7 @@ device_checkpoint_create(NMCheckpoint *self, NMDevice *device) _LOGW("error reading shadowed connection file for %s: %s", nm_device_get_iface(device), error->message); + g_clear_error(&error); } } } diff --git a/src/core/nm-config.c b/src/core/nm-config.c index e9a2681320..83816e1e10 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -2106,7 +2106,11 @@ normalize_hwaddr_for_group_name(const char *hwaddr, char *out, GError **error) * Returns: TRUE if there were no errors, FALSE otherwise. */ gboolean -nm_config_get_device_managed(NMConfig *self, NMDevice *device, NMTernary *out, GError **error) +nm_config_get_device_managed(NMConfig *self, + NMDevice *device, + NMTernary *out_managed, + gboolean *out_by_mac, + GError **error) { NMConfigPrivate *priv; const GKeyFile *keyfile = NULL; @@ -2119,14 +2123,15 @@ nm_config_get_device_managed(NMConfig *self, NMDevice *device, NMTernary *out, G g_return_val_if_fail(NM_IS_CONFIG(self), FALSE); g_return_val_if_fail(NM_CONFIG_GET_PRIVATE(self)->config_data, FALSE); - g_return_val_if_fail(out, FALSE); + g_return_val_if_fail(out_managed, FALSE); g_return_val_if_fail(ifname, FALSE); priv = NM_CONFIG_GET_PRIVATE(self); keyfile = _nm_config_data_get_keyfile_intern(priv->config_data); if (!keyfile) { - *out = NM_TERNARY_DEFAULT; + NM_SET_OUT(out_managed, NM_TERNARY_DEFAULT); + NM_SET_OUT(out_by_mac, FALSE); return TRUE; } @@ -2153,16 +2158,20 @@ nm_config_get_device_managed(NMConfig *self, NMDevice *device, NMTernary *out, G } if (val_by_name != NM_TERNARY_DEFAULT && val_by_mac == NM_TERNARY_DEFAULT) { - *out = val_by_name; + NM_SET_OUT(out_managed, val_by_name); + NM_SET_OUT(out_by_mac, FALSE); return TRUE; } else if (val_by_mac != NM_TERNARY_DEFAULT && val_by_name == NM_TERNARY_DEFAULT) { - *out = val_by_mac; + NM_SET_OUT(out_managed, val_by_mac); + NM_SET_OUT(out_by_mac, TRUE); return TRUE; } else if (val_by_name == NM_TERNARY_DEFAULT && val_by_mac == NM_TERNARY_DEFAULT) { - *out = NM_TERNARY_DEFAULT; + NM_SET_OUT(out_managed, NM_TERNARY_DEFAULT); + NM_SET_OUT(out_by_mac, FALSE); return TRUE; } else if (val_by_name == val_by_mac) { - *out = val_by_name; + NM_SET_OUT(out_managed, val_by_name); + NM_SET_OUT(out_by_mac, FALSE); return TRUE; } diff --git a/src/core/nm-config.h b/src/core/nm-config.h index a1b1463418..0cfbdfe40e 100644 --- a/src/core/nm-config.h +++ b/src/core/nm-config.h @@ -145,6 +145,7 @@ void nm_config_set_connectivity_check_enabled(NMConfig *self, gboolean enabled); gboolean nm_config_get_device_managed(NMConfig *self, NMDevice *device, NMTernary *out_managed, + gboolean *out_by_mac, GError **error); gboolean nm_config_set_device_managed(NMConfig *self, NMDevice *device, diff --git a/src/core/tests/config/test-config.c b/src/core/tests/config/test-config.c index 26588d1a1d..b2f29821e8 100644 --- a/src/core/tests/config/test-config.c +++ b/src/core/tests/config/test-config.c @@ -321,6 +321,7 @@ test_config_managed(void) gs_free char *group_by_name = NULL; const char *ifname, *group_by_mac; NMTernary managed; + gboolean by_mac; GKeyFile *kf = nm_config_create_keyfile(); dev = nm_test_device_new("11:11:11:11:11:11"); @@ -334,14 +335,15 @@ test_config_managed(void) config = setup_config(NULL, CONFIG_USER, CONFIG_INTERN, NULL, "/no/such/dir", "", NULL); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, NULL, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_DEFAULT); /* Matching by name */ NMTST_EXPECT_NM_INFO("config: signal: *"); g_assert(nm_config_set_device_managed(config, dev, NM_TERNARY_TRUE, FALSE, NULL)); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, &by_mac, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_TRUE); + g_assert_false(by_mac); g_key_file_load_from_file(kf, CONFIG_INTERN, G_KEY_FILE_NONE, NULL); g_assert_false(g_key_file_has_key(kf, group_by_mac, "managed", NULL)); g_assert_true(g_key_file_has_key(kf, group_by_name, "managed", NULL)); @@ -349,8 +351,9 @@ test_config_managed(void) NMTST_EXPECT_NM_INFO("config: signal: *"); g_assert(nm_config_set_device_managed(config, dev, NM_TERNARY_FALSE, FALSE, NULL)); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, &by_mac, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_FALSE); + g_assert_false(by_mac); g_key_file_load_from_file(kf, CONFIG_INTERN, G_KEY_FILE_NONE, NULL); g_assert_false(g_key_file_has_key(kf, group_by_mac, "managed", NULL)); g_assert_true(g_key_file_has_key(kf, group_by_name, "managed", NULL)); @@ -359,8 +362,9 @@ test_config_managed(void) /* Matching by MAC address */ NMTST_EXPECT_NM_INFO("config: signal: *"); g_assert(nm_config_set_device_managed(config, dev, NM_TERNARY_TRUE, TRUE, NULL)); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, &by_mac, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_TRUE); + g_assert_true(by_mac); g_key_file_load_from_file(kf, CONFIG_INTERN, G_KEY_FILE_NONE, NULL); g_assert_false(g_key_file_has_key(kf, group_by_name, "managed", NULL)); g_assert_true(g_key_file_has_key(kf, group_by_mac, "managed", NULL)); @@ -368,8 +372,9 @@ test_config_managed(void) NMTST_EXPECT_NM_INFO("config: signal: *"); g_assert(nm_config_set_device_managed(config, dev, NM_TERNARY_FALSE, TRUE, NULL)); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, &by_mac, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_FALSE); + g_assert_true(by_mac); g_key_file_load_from_file(kf, CONFIG_INTERN, G_KEY_FILE_NONE, NULL); g_assert_false(g_key_file_has_key(kf, group_by_name, "managed", NULL)); g_assert_true(g_key_file_has_key(kf, group_by_mac, "managed", NULL)); @@ -378,7 +383,7 @@ test_config_managed(void) /* Resetting the managed state */ NMTST_EXPECT_NM_INFO("config: signal: *"); g_assert(nm_config_set_device_managed(config, dev, NM_TERNARY_DEFAULT, FALSE, NULL)); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, NULL, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_DEFAULT); g_key_file_load_from_file(kf, CONFIG_INTERN, G_KEY_FILE_NONE, NULL); g_assert_false(g_key_file_has_key(kf, group_by_name, "managed", NULL)); @@ -391,7 +396,7 @@ test_config_managed(void) g_key_file_set_string(kf, group_by_mac, "managed", "0"); g_assert(g_key_file_save_to_file(kf, CONFIG_INTERN, NULL)); config = setup_config(NULL, CONFIG_USER, CONFIG_INTERN, NULL, "/no/such/dir", "", NULL); - g_assert(!nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(!nm_config_get_device_managed(config, dev, &managed, NULL, NULL)); g_object_unref(config); @@ -400,7 +405,7 @@ test_config_managed(void) g_key_file_set_string(kf, group_by_mac, "managed", "1"); g_assert(g_key_file_save_to_file(kf, CONFIG_INTERN, NULL)); config = setup_config(NULL, CONFIG_USER, CONFIG_INTERN, NULL, "/no/such/dir", "", NULL); - g_assert(nm_config_get_device_managed(config, dev, &managed, NULL)); + g_assert(nm_config_get_device_managed(config, dev, &managed, NULL, NULL)); g_assert_cmpint(managed, ==, NM_TERNARY_TRUE); g_key_file_unref(kf);