From 57a26fd2aae9c38e56a05f1994feff6774eeba37 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 6 Mar 2017 11:50:06 +0100 Subject: cli/connections: decide about activation success and failure in single place Track both device and active connections at the same time and decide whether activation finished (either successfully or not) in a single place, check_activated(). This makes the already too complex code path a bit more straightforward and also makes it possible to be more reasonable about the diagnostics. Now that the active connection signals the reason, we include it; but if the failure is due to the device disconnection while we're activating, include a device reason instead, since it's often more useful. Like: Before: Error: Connection activation failed. Without considering the device: Error: Connection activation failed: the base network connection was interrupted After: Error: Connection activation failed: The Wi-Fi network could not be found --- clients/cli/connections.c | 277 ++++++++++++++++++++-------------------------- 1 file changed, 118 insertions(+), 159 deletions(-) (limited to 'clients') diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 84cde2b78c..fecec0d403 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -2124,104 +2124,90 @@ typedef struct { static void activate_connection_info_finish (ActivateConnectionInfo *info); static const char * -vpn_connection_state_reason_to_string (NMVpnConnectionStateReason reason) +active_connection_state_reason_to_string (NMActiveConnectionStateReason reason) { switch (reason) { - case NM_VPN_CONNECTION_STATE_REASON_UNKNOWN: - return _("unknown reason"); - case NM_VPN_CONNECTION_STATE_REASON_NONE: - return _("none"); - case NM_VPN_CONNECTION_STATE_REASON_USER_DISCONNECTED: - return _("the user was disconnected"); - case NM_VPN_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED: - return _("the base network connection was interrupted"); - case NM_VPN_CONNECTION_STATE_REASON_SERVICE_STOPPED: - return _("the VPN service stopped unexpectedly"); - case NM_VPN_CONNECTION_STATE_REASON_IP_CONFIG_INVALID: - return _("the VPN service returned invalid configuration"); - case NM_VPN_CONNECTION_STATE_REASON_CONNECT_TIMEOUT: - return _("the connection attempt timed out"); - case NM_VPN_CONNECTION_STATE_REASON_SERVICE_START_TIMEOUT: - return _("the VPN service did not start in time"); - case NM_VPN_CONNECTION_STATE_REASON_SERVICE_START_FAILED: - return _("the VPN service failed to start"); - case NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS: - return _("no valid VPN secrets"); - case NM_VPN_CONNECTION_STATE_REASON_LOGIN_FAILED: - return _("invalid VPN secrets"); - case NM_VPN_CONNECTION_STATE_REASON_CONNECTION_REMOVED: - return _("the connection was removed"); - default: - return _("unknown"); - } + case NM_ACTIVE_CONNECTION_STATE_REASON_UNKNOWN: + return _("Unknown reason"); + case NM_ACTIVE_CONNECTION_STATE_REASON_NONE: + return _("The connection was disconnected"); + case NM_ACTIVE_CONNECTION_STATE_REASON_USER_DISCONNECTED: + return _("Disconnected by user"); + case NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED: + return _("The base network connection was interrupted"); + case NM_ACTIVE_CONNECTION_STATE_REASON_SERVICE_STOPPED: + return _("The VPN service stopped unexpectedly"); + case NM_ACTIVE_CONNECTION_STATE_REASON_IP_CONFIG_INVALID: + return _("The VPN service returned invalid configuration"); + case NM_ACTIVE_CONNECTION_STATE_REASON_CONNECT_TIMEOUT: + return _("The connection attempt timed out"); + case NM_ACTIVE_CONNECTION_STATE_REASON_SERVICE_START_TIMEOUT: + return _("The VPN service did not start in time"); + case NM_ACTIVE_CONNECTION_STATE_REASON_SERVICE_START_FAILED: + return _("The VPN service failed to start"); + case NM_ACTIVE_CONNECTION_STATE_REASON_NO_SECRETS: + return _("No valid secrets"); + case NM_ACTIVE_CONNECTION_STATE_REASON_LOGIN_FAILED: + return _("Invalid secrets"); + case NM_ACTIVE_CONNECTION_STATE_REASON_CONNECTION_REMOVED: + return _("The connection was removed"); + case NM_ACTIVE_CONNECTION_STATE_REASON_DEPENDENCY_FAILED: + return _("Master connection failed"); + case NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_REALIZE_FAILED: + return _("Could not create a software link"); + case NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_REMOVED: + return _("The device disappeared"); + } + + g_return_val_if_reached (_("Invalid reason")); } static void -device_state_cb (NMDevice *device, GParamSpec *pspec, ActivateConnectionInfo *info) +check_activated (ActivateConnectionInfo *info) { NmCli *nmc = info->nmc; - NMActiveConnection *active; - NMDeviceState state; + NMDevice *device = info->device; + NMActiveConnection *active = info->active; NMActiveConnectionState ac_state; + NMActiveConnectionStateReason ac_reason; + NMDeviceState dev_state; + NMDeviceStateReason dev_reason; - active = nm_device_get_active_connection (device); - state = nm_device_get_state (device); - - ac_state = active ? nm_active_connection_get_state (active) : NM_ACTIVE_CONNECTION_STATE_UNKNOWN; + ac_state = nm_active_connection_get_state (active); + ac_reason = nm_active_connection_get_state_reason (active); - if (ac_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { - if (nmc->print_output == NMC_PRINT_PRETTY) - nmc_terminal_erase_line (); - g_print (_("Connection successfully activated (D-Bus active path: %s)\n"), - nm_object_get_path (NM_OBJECT (active))); - activate_connection_info_finish (info); - } else if ( ac_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATING - && state >= NM_DEVICE_STATE_IP_CONFIG - && state <= NM_DEVICE_STATE_ACTIVATED) { - if (nmc->print_output == NMC_PRINT_PRETTY) - nmc_terminal_erase_line (); - g_print (_("Connection successfully activated (master waiting for slaves) (D-Bus active path: %s)\n"), - nm_object_get_path (NM_OBJECT (active))); - activate_connection_info_finish (info); + if (device) { + dev_state = nm_device_get_state (device); + dev_reason = nm_device_get_state_reason (device); } -} - -static void -active_connection_removed_cb (NMClient *client, NMActiveConnection *active, ActivateConnectionInfo *info) -{ - NmCli *nmc = info->nmc; - if (active == info->active) { - g_string_printf (nmc->return_text, _("Error: Connection activation failed.")); - nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; - activate_connection_info_finish (info); - } -} - -static void -active_connection_state_cb (NMActiveConnection *active, GParamSpec *pspec, ActivateConnectionInfo *info) -{ - NmCli *nmc = info->nmc; - NMActiveConnectionState state; - - state = nm_active_connection_get_state (active); - - if (state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + if (ac_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { if (nmc->print_output == NMC_PRINT_PRETTY) nmc_terminal_erase_line (); g_print (_("Connection successfully activated (D-Bus active path: %s)\n"), nm_object_get_path (NM_OBJECT (active))); activate_connection_info_finish (info); - } else if (state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { - g_string_printf (nmc->return_text, _("Error: Connection activation failed.")); - nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; - activate_connection_info_finish (info); - } else if (state == NM_ACTIVE_CONNECTION_STATE_ACTIVATING) { + } else if (ac_state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { + if (device && ac_reason == NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED) { + if (dev_state == NM_DEVICE_STATE_FAILED || dev_state == NM_DEVICE_STATE_DISCONNECTED) { + g_string_printf (nmc->return_text, _("Error: Connection activation failed: %s"), + nmc_device_reason_to_string (dev_reason)); + nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; + activate_connection_info_finish (info); + } else { + /* Just wait for the device to go failed. We'll get a better error message. */ + return; + } + } else { + g_string_printf (nmc->return_text, _("Error: Connection activation failed: %s"), + active_connection_state_reason_to_string (ac_reason)); + nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; + activate_connection_info_finish (info); + } + } else if (ac_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATING) { /* activating master connection does not automatically activate any slaves, so their * active connection state will not progress beyond ACTIVATING state. * Monitor the device instead. */ - const GPtrArray *devices; - NMDevice *device; if (nmc->secret_agent) { NMRemoteConnection *connection = nm_active_connection_get_connection (active); @@ -2230,53 +2216,34 @@ active_connection_state_cb (NMActiveConnection *active, GParamSpec *pspec, Activ nm_connection_get_path (NM_CONNECTION (connection))); } - devices = nm_active_connection_get_devices (active); - device = devices->len ? g_ptr_array_index (devices, 0) : NULL; if ( device && ( NM_IS_DEVICE_BOND (device) || NM_IS_DEVICE_TEAM (device) - || NM_IS_DEVICE_BRIDGE (device))) { - g_signal_connect (device, "notify::" NM_DEVICE_STATE, G_CALLBACK (device_state_cb), info); - device_state_cb (device, NULL, info); + || NM_IS_DEVICE_BRIDGE (device)) + && dev_state >= NM_DEVICE_STATE_IP_CONFIG + && dev_state <= NM_DEVICE_STATE_ACTIVATED) { + if (nmc->print_output == NMC_PRINT_PRETTY) + nmc_terminal_erase_line (); + g_print (_("Connection successfully activated (master waiting for slaves) (D-Bus active path: %s)\n"), + nm_object_get_path (NM_OBJECT (active))); + activate_connection_info_finish (info); } } } static void -vpn_connection_state_cb (NMVpnConnection *vpn, - NMVpnConnectionState state, - NMVpnConnectionStateReason reason, - ActivateConnectionInfo *info) +device_state_cb (NMDevice *device, GParamSpec *pspec, ActivateConnectionInfo *info) { - NmCli *nmc = info->nmc; - - switch (state) { - case NM_VPN_CONNECTION_STATE_PREPARE: - case NM_VPN_CONNECTION_STATE_NEED_AUTH: - case NM_VPN_CONNECTION_STATE_CONNECT: - case NM_VPN_CONNECTION_STATE_IP_CONFIG_GET: - /* no operation */ - break; - - case NM_VPN_CONNECTION_STATE_ACTIVATED: - if (nmc->print_output == NMC_PRINT_PRETTY) - nmc_terminal_erase_line (); - g_print (_("VPN connection successfully activated (D-Bus active path: %s)\n"), - nm_object_get_path (NM_OBJECT (vpn))); - activate_connection_info_finish (info); - break; - - case NM_VPN_CONNECTION_STATE_FAILED: - case NM_VPN_CONNECTION_STATE_DISCONNECTED: - g_string_printf (nmc->return_text, _("Error: Connection activation failed: %s."), - vpn_connection_state_reason_to_string (reason)); - nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; - activate_connection_info_finish (info); - break; + check_activated (info); +} - default: - break; - } +static void +active_connection_state_cb (NMActiveConnection *active, + NMActiveConnectionState state, + NMActiveConnectionStateReason reason, + ActivateConnectionInfo *info) +{ + check_activated (info); } static void @@ -2308,24 +2275,28 @@ progress_cb (gpointer user_data) } static gboolean -progress_device_cb (gpointer user_data) +progress_active_connection_cb (gpointer user_data) { - NMDevice *device = (NMDevice *) user_data; - - nmc_terminal_show_progress (device ? nmc_device_state_to_string (nm_device_get_state (device)) : ""); + NMActiveConnection *active = user_data; + const char *str; + NMDevice *device; + NMActiveConnectionState ac_state; + const GPtrArray *ac_devs; - return TRUE; -} + ac_state = nm_active_connection_get_state (active); -static gboolean -progress_vpn_cb (gpointer user_data) -{ - NMVpnConnection *vpn = (NMVpnConnection *) user_data; - const char *str; + if (ac_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATING) { + /* If the connection is activating, the device state + * is more interesting. */ + ac_devs = nm_active_connection_get_devices (active); + device = ac_devs->len > 0 ? g_ptr_array_index (ac_devs, 0) : NULL; + } else { + device = NULL; + } - str = NM_IS_VPN_CONNECTION (vpn) ? - vpn_connection_state_to_string (nm_vpn_connection_get_vpn_state (vpn)) : - ""; + str = device + ? nmc_device_state_to_string (nm_device_get_state (device)) + : active_connection_state_to_string (ac_state); nmc_terminal_show_progress (str); @@ -2341,14 +2312,9 @@ activate_connection_info_finish (ActivateConnectionInfo *info) } if (info->active) { - if (NM_IS_VPN_CONNECTION (info->active)) - g_signal_handlers_disconnect_by_func (info->active, G_CALLBACK (vpn_connection_state_cb), info); - else - g_signal_handlers_disconnect_by_func (info->active, G_CALLBACK (active_connection_state_cb), info); + g_signal_handlers_disconnect_by_func (info->active, G_CALLBACK (active_connection_state_cb), info); g_object_unref (info->active); - } - g_signal_handlers_disconnect_by_func (info->nmc->client, G_CALLBACK (active_connection_removed_cb), info); g_free (info); quit (); @@ -2393,34 +2359,27 @@ activate_connection_cb (GObject *client, GAsyncResult *result, gpointer user_dat } activate_connection_info_finish (info); } else { - if (NM_IS_VPN_CONNECTION (active)) { - /* Monitor VPN state */ - g_signal_connect (G_OBJECT (active), "vpn-state-changed", G_CALLBACK (vpn_connection_state_cb), info); - - /* Start progress indication showing VPN states */ - if (nmc->print_output == NMC_PRINT_PRETTY) { - if (progress_id) - g_source_remove (progress_id); - progress_id = g_timeout_add (120, progress_vpn_cb, NM_VPN_CONNECTION (active)); - } - } else { - g_signal_connect (active, "notify::state", G_CALLBACK (active_connection_state_cb), info); - active_connection_state_cb (active, NULL, info); - - /* Start progress indication showing device states */ - if (nmc->print_output == NMC_PRINT_PRETTY) { - if (progress_id) - g_source_remove (progress_id); - progress_id = g_timeout_add (120, progress_device_cb, device); - } + /* Monitor the active connection state state */ + g_signal_connect (G_OBJECT (active), "state-changed", G_CALLBACK (active_connection_state_cb), info); + active_connection_state_cb (active, + nm_active_connection_get_state (active), + nm_active_connection_get_state_reason (active), + info); + + if (device) { + g_signal_connect (device, "notify::" NM_DEVICE_STATE, G_CALLBACK (device_state_cb), info); + device_state_cb (device, NULL, info); + } + + /* Start progress indication showing VPN states */ + if (nmc->print_output == NMC_PRINT_PRETTY) { + if (progress_id) + g_source_remove (progress_id); + progress_id = g_timeout_add (120, progress_active_connection_cb, active); } /* Start timer not to loop forever when signals are not emitted */ g_timeout_add_seconds (nmc->timeout, activate_connection_timeout_cb, info); - - /* Fail when the active connection goes away. */ - g_signal_connect (nmc->client, NM_CLIENT_ACTIVE_CONNECTION_REMOVED, - G_CALLBACK (active_connection_removed_cb), info); } } } -- cgit v1.2.1