From a370faeb59a90fa1550aa8df55d80974dd5220ac Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Nov 2018 11:32:27 +0100 Subject: [PATCH] cli: wait for changed signal after updating a connection In editor_menu_main(), after saving a connection we wait that the Update2() D-Bus call returns and then we copy the NMRemoteConnection instance over to @connection. This assumes that when Update2() returns the remote connection instance is already updated with new settings. Indeed, on server side the NMSettingsConnection first emits the "Updated" signal and then returns to Update2(). However, the Updated signal doesn't include the new setting values and so libnm has to fire an asynchronous nmdbus_settings_connection_call_get_setting() to fetch the new settings, which terminates after the Update2(). So, to be sure that the remote connection got updated we have also to listen to the connection-changed signal, which is always emitted after an update. https://bugzilla.redhat.com/show_bug.cgi?id=1546805 --- clients/cli/connections.c | 45 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index b8eccaaea..841d0f938 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -6588,6 +6588,12 @@ static gboolean nmc_editor_cb_called; static GError *nmc_editor_error; static MonitorACInfo *nmc_editor_monitor_ac; +static void +editor_connection_changed_cb (NMConnection *connection, gboolean *changed) +{ + *changed = TRUE; +} + /* * Store 'error' to shared 'nmc_editor_error' and monitoring info to * 'nmc_editor_monitor_ac' and signal the condition so that @@ -7194,6 +7200,16 @@ menu_switch_to_level1 (const NmcConfig *nmc_config, menu_ctx->valid_props_str = g_strjoinv (", ", menu_ctx->valid_props); } +static gboolean +editor_save_timeout (gpointer user_data) +{ + gboolean *timeout = user_data; + + *timeout = TRUE; + + return G_SOURCE_REMOVE; +} + static gboolean editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_type) { @@ -7726,6 +7742,10 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t /* Save the connection */ if (nm_connection_verify (connection, &err1)) { gboolean persistent = TRUE; + gboolean connection_changed; + nm_auto_unref_gsource GSource *source = NULL; + gboolean timeout = FALSE; + gulong handler_id = 0; /* parse argument */ if (cmd_arg) { @@ -7757,23 +7777,44 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t connection, add_connection_editor_cb, info); + connection_changed = TRUE; } else { /* Save/update already saved (existing) connection */ nm_connection_replace_settings_from_connection (NM_CONNECTION (rem_con), connection); update_connection (persistent, rem_con, update_connection_editor_cb, NULL); + + handler_id = g_signal_connect (rem_con, + NM_CONNECTION_CHANGED, + G_CALLBACK (editor_connection_changed_cb), + &connection_changed); + connection_changed = FALSE; } - //FIXME: add also a timeout for cases the callback is not called - while (!nmc_editor_cb_called) + source = g_timeout_source_new (10 * NM_UTILS_MSEC_PER_SECOND); + g_source_set_callback (source, editor_save_timeout, &timeout, NULL); + g_source_attach (source, g_main_loop_get_context (loop)); + + while (!nmc_editor_cb_called && !timeout) g_main_context_iteration (NULL, TRUE); + while (!connection_changed && !timeout) + g_main_context_iteration (NULL, TRUE); + + if (handler_id) + g_signal_handler_disconnect (rem_con, handler_id); + g_source_destroy (source); + if (nmc_editor_error) { g_print (_("Error: Failed to save '%s' (%s) connection: %s\n"), nm_connection_get_id (connection), nm_connection_get_uuid (connection), nmc_editor_error->message); g_error_free (nmc_editor_error); + } else if (timeout) { + g_print (_("Error: Timeout saving '%s' (%s) connection\n"), + nm_connection_get_id (connection), + nm_connection_get_uuid (connection)); } else { g_print (!rem_con ? _("Connection '%s' (%s) successfully saved.\n") :