From 8732914815ef315cd94ba67ba4b8247d90412b13 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 31 Jul 2013 09:14:39 -0400 Subject: [PATCH] core: improve NMManager:state transitions with connectivity checking The connectivity-checking code would generally result in NMManager:state going CONNECTING -> CONNECTED_GLOBAL -> CONNECTED_SITE in the case where the connectivity check failed. The brief incorrect CONNECTED_GLOBAL is bad, because clients might see it and do the wrong thing. Instead, when we are ready to switch from CONNECTING to CONNECTED_*, do a connectivity check first, and switch to either CONNECTED_SITE or CONNECTED_GLOBAL based on the result of that. --- src/nm-connectivity.c | 144 +++++++++++++++++++++++++++--------------- src/nm-connectivity.h | 16 +++-- src/nm-manager.c | 104 ++++++++++++++++++++++-------- 3 files changed, 182 insertions(+), 82 deletions(-) diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index fca5649b29..2b6c25944b 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -75,16 +75,7 @@ update_connected (NMConnectivity *self, gboolean connected) NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); gboolean old_connected = priv->connected; -#if WITH_CONCHECK - if (priv->uri == NULL || priv->interval == 0) { - /* Default to connected if no checks are to be run */ - priv->connected = TRUE; - } else - priv->connected = connected; -#else - priv->connected = TRUE; -#endif - + priv->connected = connected; if (priv->connected != old_connected) g_object_notify (G_OBJECT (self), NM_CONNECTIVITY_CONNECTED); } @@ -93,11 +84,16 @@ update_connected (NMConnectivity *self, gboolean connected) static void nm_connectivity_check_cb (SoupSession *session, SoupMessage *msg, gpointer user_data) { - NMConnectivity *self = NM_CONNECTIVITY (user_data); - NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); + GSimpleAsyncResult *simple = user_data; + NMConnectivity *self; + NMConnectivityPrivate *priv; gboolean connected_new = FALSE; const char *nm_header; + self = NM_CONNECTIVITY (g_async_result_get_source_object (G_ASYNC_RESULT (simple))); + g_object_unref (self); + priv = NM_CONNECTIVITY_GET_PRIVATE (self); + /* Check headers; if we find the NM-specific one we're done */ nm_header = soup_message_headers_get_one (msg->response_headers, "X-NetworkManager-Status"); if (g_strcmp0 (nm_header, "online") == 0) { @@ -118,71 +114,119 @@ nm_connectivity_check_cb (SoupSession *session, SoupMessage *msg, gpointer user_ priv->uri, msg->status_code, msg->reason_phrase); } - /* update connectivity and emit signal */ - update_connected (self, connected_new); + g_simple_async_result_set_op_res_gboolean (simple, connected_new); + g_simple_async_result_complete (simple); + update_connected (self, connected_new); +} + +static void +run_check_complete (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + NMConnectivity *self = NM_CONNECTIVITY (object); + NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); + GError *error = NULL; + + nm_connectivity_check_finish (self, result, &error); priv->running = FALSE; + if (error) { + nm_log_err (LOGD_CONCHECK, "Connectivity check failed: %s", error->message); + g_error_free (error); + } } static gboolean run_check (gpointer user_data) { NMConnectivity *self = NM_CONNECTIVITY (user_data); - NMConnectivityPrivate *priv; - SoupMessage *msg; - - g_return_val_if_fail (NM_IS_CONNECTIVITY (self), FALSE); - priv = NM_CONNECTIVITY_GET_PRIVATE (self); - - msg = soup_message_new ("GET", priv->uri); - g_return_val_if_fail (msg != NULL, FALSE); - - soup_message_set_flags (msg, SOUP_MESSAGE_NO_REDIRECT); - soup_session_queue_message (priv->soup_session, - msg, - nm_connectivity_check_cb, - self); + NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); + nm_connectivity_check_async (self, run_check_complete, NULL); priv->running = TRUE; nm_log_dbg (LOGD_CONCHECK, "Connectivity check with uri '%s' started.", priv->uri); + return TRUE; } #endif void -nm_connectivity_start_check (NMConnectivity *self) +nm_connectivity_set_online (NMConnectivity *self, + gboolean online) { #if WITH_CONCHECK NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - if (!priv->uri || !priv->interval) { - nm_connectivity_stop_check (self); + if (online && priv->uri && priv->interval) { + if (!priv->check_id) + priv->check_id = g_timeout_add_seconds (priv->interval, run_check, self); + if (!priv->running) + run_check (self); + return; - } - - if (priv->check_id == 0) - priv->check_id = g_timeout_add_seconds (priv->interval, run_check, self); - - if (priv->running == FALSE) - run_check (self); -#endif -} - -void -nm_connectivity_stop_check (NMConnectivity *self) -{ -#if WITH_CONCHECK - NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - - if (priv->check_id) { + } else if (priv->check_id) { g_source_remove (priv->check_id); priv->check_id = 0; } - - update_connected (self, FALSE); #endif + + /* Either @online is %TRUE but we aren't checking connectivity, or + * @online is %FALSE. Either way we can update our status immediately. + */ + update_connected (self, online); } +void +nm_connectivity_check_async (NMConnectivity *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ + NMConnectivityPrivate *priv; +#if WITH_CONCHECK + SoupMessage *msg; +#endif + GSimpleAsyncResult *simple; + + g_return_val_if_fail (NM_IS_CONNECTIVITY (self), FALSE); + priv = NM_CONNECTIVITY_GET_PRIVATE (self); + + simple = g_simple_async_result_new (G_OBJECT (self), callback, user_data, + nm_connectivity_check_async); + +#if WITH_CONCHECK + if (priv->uri && priv->interval) { + msg = soup_message_new ("GET", priv->uri); + soup_message_set_flags (msg, SOUP_MESSAGE_NO_REDIRECT); + soup_session_queue_message (priv->soup_session, + msg, + nm_connectivity_check_cb, + simple); + + return; + } +#endif + + g_simple_async_result_set_op_res_gboolean (simple, TRUE); + g_simple_async_result_complete_in_idle (simple); +} + +gboolean +nm_connectivity_check_finish (NMConnectivity *self, + GAsyncResult *result, + GError **error) +{ + GSimpleAsyncResult *simple; + + g_return_val_if_fail (g_simple_async_result_is_valid (result, G_OBJECT (self), nm_connectivity_check_async), FALSE); + + simple = G_SIMPLE_ASYNC_RESULT (result); + if (g_simple_async_result_propagate_error (simple, error)) + return FALSE; + return g_simple_async_result_get_op_res_gboolean (simple); +} + + NMConnectivity * nm_connectivity_new (void) { diff --git a/src/nm-connectivity.h b/src/nm-connectivity.h index 0a7a39f7f5..36d6e8d12a 100644 --- a/src/nm-connectivity.h +++ b/src/nm-connectivity.h @@ -1,4 +1,3 @@ - /* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ /* NetworkManager -- Network link manager * @@ -24,6 +23,7 @@ #include #include +#include #include "NetworkManager.h" @@ -40,7 +40,6 @@ #define NM_CONNECTIVITY_RESPONSE "response" #define NM_CONNECTIVITY_CONNECTED "connected" - typedef struct { GObject parent; } NMConnectivity; @@ -51,13 +50,18 @@ typedef struct { GType nm_connectivity_get_type (void); - NMConnectivity *nm_connectivity_new (void); -void nm_connectivity_start_check (NMConnectivity *connectivity); +void nm_connectivity_set_online (NMConnectivity *self, + gboolean online); -void nm_connectivity_stop_check (NMConnectivity *connectivity); +gboolean nm_connectivity_get_connected (NMConnectivity *self); -gboolean nm_connectivity_get_connected (NMConnectivity *connectivity); +void nm_connectivity_check_async (NMConnectivity *self, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean nm_connectivity_check_finish (NMConnectivity *self, + GAsyncResult *result, + GError **error); #endif /* NM_CONNECTIVITY_H */ diff --git a/src/nm-manager.c b/src/nm-manager.c index 1215b59840..df7d8cfb67 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -527,12 +527,74 @@ modem_added (NMModemManager *modem_manager, add_device (self, device); } +static void +set_state (NMManager *manager, NMState state) +{ + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); + const char *state_str; + + if (priv->state == state) + return; + + priv->state = state; + + switch (state) { + case NM_STATE_ASLEEP: + state_str = "ASLEEP"; + break; + case NM_STATE_DISCONNECTED: + state_str = "DISCONNECTED"; + break; + case NM_STATE_DISCONNECTING: + state_str = "DISCONNECTING"; + break; + case NM_STATE_CONNECTING: + state_str = "CONNECTING"; + break; + case NM_STATE_CONNECTED_LOCAL: + state_str = "CONNECTED_LOCAL"; + break; + case NM_STATE_CONNECTED_SITE: + state_str = "CONNECTED_SITE"; + break; + case NM_STATE_CONNECTED_GLOBAL: + state_str = "CONNECTED_GLOBAL"; + break; + case NM_STATE_UNKNOWN: + default: + state_str = "UNKNOWN"; + break; + } + + nm_log_info (LOGD_CORE, "NetworkManager state is now %s", state_str); + + g_object_notify (G_OBJECT (manager), NM_MANAGER_STATE); + g_signal_emit (manager, signals[STATE_CHANGED], 0, priv->state); +} + +static void +checked_connectivity (GObject *object, GAsyncResult *result, gpointer user_data) +{ + NMManager *manager = user_data; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); + + if (priv->state == NM_STATE_CONNECTING || priv->state == NM_STATE_CONNECTED_SITE) { + if (nm_connectivity_check_finish (priv->connectivity, result, NULL)) + set_state (manager, NM_STATE_CONNECTED_GLOBAL); + else + set_state (manager, NM_STATE_CONNECTED_SITE); + } + + g_object_unref (manager); +} + static void nm_manager_update_state (NMManager *manager) { NMManagerPrivate *priv; NMState new_state = NM_STATE_DISCONNECTED; GSList *iter; + gboolean want_connectivity_check = FALSE; g_return_if_fail (NM_IS_MANAGER (manager)); @@ -546,11 +608,14 @@ nm_manager_update_state (NMManager *manager) NMDeviceState state = nm_device_get_state (dev); if (state == NM_DEVICE_STATE_ACTIVATED) { - new_state = NM_STATE_CONNECTED_GLOBAL; - /* Connectivity check might have a better idea */ - if (nm_connectivity_get_connected (priv->connectivity) == FALSE) - new_state = NM_STATE_CONNECTED_SITE; - break; + nm_connectivity_set_online (priv->connectivity, TRUE); + if (!nm_connectivity_get_connected (priv->connectivity)) { + new_state = NM_STATE_CONNECTING; + want_connectivity_check = TRUE; + } else { + new_state = NM_STATE_CONNECTED_GLOBAL; + break; + } } if (nm_device_is_activating (dev)) @@ -562,12 +627,15 @@ nm_manager_update_state (NMManager *manager) } } - if (priv->state != new_state) { - priv->state = new_state; - g_object_notify (G_OBJECT (manager), NM_MANAGER_STATE); - - g_signal_emit (manager, signals[STATE_CHANGED], 0, priv->state); + if (new_state == NM_STATE_CONNECTING && want_connectivity_check) { + nm_connectivity_check_async (priv->connectivity, + checked_connectivity, + g_object_ref (manager)); + return; } + + nm_connectivity_set_online (priv->connectivity, new_state >= NM_STATE_CONNECTED_LOCAL); + set_state (manager, new_state); } static void @@ -578,7 +646,6 @@ manager_device_state_changed (NMDevice *device, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); switch (new_state) { case NM_DEVICE_STATE_UNMANAGED: @@ -593,21 +660,6 @@ manager_device_state_changed (NMDevice *device, } nm_manager_update_state (self); - - if (priv->state >= NM_STATE_CONNECTED_LOCAL) { - if (old_state == NM_DEVICE_STATE_ACTIVATED || new_state == NM_DEVICE_STATE_ACTIVATED) { - /* Still connected, but a device activated or deactivated; make sure - * we still have connectivity on the other activated devices. - */ - nm_log_dbg (LOGD_CORE, "(%s): triggered connectivity check due to state change", - nm_device_get_iface (device)); - nm_connectivity_start_check (priv->connectivity); - } - } else { - /* Cannot be connected if no devices are activated */ - nm_log_dbg (LOGD_CORE, "stopping connectivity checks"); - nm_connectivity_stop_check (priv->connectivity); - } } static void device_has_pending_action_changed (NMDevice *device,