From dab114f038f39e07080f71426d70e84449890088 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 May 2023 13:06:22 +0200 Subject: [PATCH 1/2] cloud-setup: fix terminating in the middle of reconfiguring the system Once we start reconfiguring the system, we need to finish on all interfaces. Otherwise, we might reconfigure some interfaces, abort and leave the network broken. When that happens, a subsequent run might also be unable to recover, because we are unable to reach the HTTP meta data service. https://bugzilla.redhat.com/show_bug.cgi?id=2207812 Fixes: 69f048bf0ca3 ('cloud-setup: add tool for automatic IP configuration in cloud') --- src/nm-cloud-setup/main.c | 49 ++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/nm-cloud-setup/main.c b/src/nm-cloud-setup/main.c index 881043f526..caae480847 100644 --- a/src/nm-cloud-setup/main.c +++ b/src/nm-cloud-setup/main.c @@ -15,6 +15,12 @@ /*****************************************************************************/ +typedef struct { + GCancellable *cancellable; + gboolean enabled; + gboolean signal_received; +} SigTermData; + typedef struct { GMainLoop *main_loop; GCancellable *cancellable; @@ -540,7 +546,7 @@ _nmc_mangle_connection(NMDevice *device, /*****************************************************************************/ static gboolean -_config_one(GCancellable *sigterm_cancellable, +_config_one(SigTermData *sigterm_data, NMClient *nmc, const NMCSProviderGetConfigResult *result, guint idx) @@ -560,7 +566,7 @@ _config_one(GCancellable *sigterm_cancellable, g_main_context_iteration(NULL, FALSE); - if (g_cancellable_is_cancelled(sigterm_cancellable)) + if (g_cancellable_is_cancelled(sigterm_data->cancellable)) return FALSE; device = nm_g_object_ref(_nmc_get_device_by_hwaddr(nmc, hwaddr)); @@ -594,7 +600,7 @@ try_again: g_clear_error(&error); applied_connection = nmcs_device_get_applied_connection(device, - sigterm_cancellable, + sigterm_data->cancellable, &applied_version_id, &error); if (!applied_connection) { @@ -655,8 +661,12 @@ try_again: maybe_no_preserved_external_ip = (nmc_client_has_version_info_v(nmc) < NM_ENCODE_VERSION(1, 41, 6)); + /* Once we start reconfiguring the system, we cannot abort in the middle. From now on, + * any SIGTERM gets ignored until we are done. */ + sigterm_data->enabled = FALSE; + if (!nmcs_device_reapply(device, - sigterm_cancellable, + NULL, applied_connection, applied_version_id, maybe_no_preserved_external_ip, @@ -687,15 +697,13 @@ try_again: } static gboolean -_config_all(GCancellable *sigterm_cancellable, - NMClient *nmc, - const NMCSProviderGetConfigResult *result) +_config_all(SigTermData *sigterm_data, NMClient *nmc, const NMCSProviderGetConfigResult *result) { gboolean any_changes = FALSE; guint i; for (i = 0; i < result->n_iface_datas; i++) { - if (_config_one(sigterm_cancellable, nmc, result, i)) + if (_config_one(sigterm_data, nmc, result, i)) any_changes = TRUE; } @@ -707,13 +715,16 @@ _config_all(GCancellable *sigterm_cancellable, static gboolean sigterm_handler(gpointer user_data) { - GCancellable *sigterm_cancellable = user_data; + SigTermData *sigterm_data = user_data; - if (!g_cancellable_is_cancelled(sigterm_cancellable)) { - _LOGD("SIGTERM received"); - g_cancellable_cancel(user_data); - } else - _LOGD("SIGTERM received (again)"); + _LOGD("SIGTERM received (%s) (%s)", + sigterm_data->signal_received ? "first time" : "again", + sigterm_data->enabled ? "cancel operation" : "ignore"); + + sigterm_data->signal_received = TRUE; + + if (sigterm_data->enabled) + g_cancellable_cancel(sigterm_data->cancellable); return G_SOURCE_CONTINUE; } @@ -728,6 +739,7 @@ main(int argc, const char *const *argv) gs_unref_object NMClient *nmc = NULL; nm_auto_free_nmcs_provider_get_config_result NMCSProviderGetConfigResult *result = NULL; gs_free_error GError *error = NULL; + SigTermData sigterm_data; _nm_logging_enabled_init(g_getenv(NMCS_ENV_NM_CLOUD_SETUP_LOG)); @@ -740,7 +752,12 @@ main(int argc, const char *const *argv) sigterm_cancellable = g_cancellable_new(); - sigterm_source = nm_g_unix_signal_add_source(SIGTERM, sigterm_handler, sigterm_cancellable); + sigterm_data = (SigTermData){ + .cancellable = sigterm_cancellable, + .enabled = TRUE, + .signal_received = FALSE, + }; + sigterm_source = nm_g_unix_signal_add_source(SIGTERM, sigterm_handler, &sigterm_data); provider = _provider_detect(sigterm_cancellable); if (!provider) @@ -771,7 +788,7 @@ main(int argc, const char *const *argv) if (!result) goto done; - if (_config_all(sigterm_cancellable, nmc, result)) + if (_config_all(&sigterm_data, nmc, result)) _LOGI("some changes were applied for provider %s", nmcs_provider_get_name(provider)); else _LOGD("no changes were applied for provider %s", nmcs_provider_get_name(provider)); From c70a5470be034c660b426ebdbef9e8e67609ece7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Jun 2023 13:04:53 +0200 Subject: [PATCH 2/2] cloud-setup: clear error variable in nmcs_device_reapply() This is rather bad, because if we reach the "goto again" case, the error variable is not cleared. Subsequently passing the error location to nm_device_reapply_finish() will trigger a glib warning. Fixes: 29b0420be72f ('nm-cloud-setup: set preserve-external-ip flag during reapply') --- src/nm-cloud-setup/nm-cloud-setup-utils.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nm-cloud-setup/nm-cloud-setup-utils.c b/src/nm-cloud-setup/nm-cloud-setup-utils.c index f82cdf9059..75739c7cdb 100644 --- a/src/nm-cloud-setup/nm-cloud-setup-utils.c +++ b/src/nm-cloud-setup/nm-cloud-setup-utils.c @@ -581,6 +581,8 @@ nmcs_device_reapply(NMDevice *device, NMDeviceReapplyFlags reapply_flags = NM_DEVICE_REAPPLY_FLAGS_PRESERVE_EXTERNAL_IP; again: + g_clear_error(&data.error); + nm_device_reapply_async(device, connection, version_id,