Attachment 78658 Details for Bug 56635 – Replace ConnMan connectivity support with GNetworkMonitor

[patch] Replace ConnMan connectivity support with GNetworkMonitor

0004-Replace-ConnMan-connectivity-support-with-GNetworkMo.patch (text/plain), 15.73 KB, created by Simon McVittie on 2013-04-30 16:45:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-04-30 16:45:40 UTC

Size: 15.73 KB

patch

obsolete

>From a6e366caea8654151fc482d4186e2a729a6de141 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 30 Apr 2013 17:29:40 +0100
>Subject: [PATCH 4/4] Replace ConnMan connectivity support with
> GNetworkMonitor
>
>The information we're getting from ConnMan is no more informative
>than GNetworkMonitor, and a ConnMan GNetworkMonitor implementation
>exists (e.g. <https://github.com/jukkar/connman-network-monitor>),
>so we might as well go via that.
>
>On NetworkManager systems, we now use NetworkManager to detect
>transient states like DISCONNECTING, but use the GNetworkMonitor
>to detect whether the interface is up. If NetworkManager is the only
>thing managing an interface, that makes no difference, but if there is
>an interface not managed by NetworkManager, the default netlink
>GNetworkMonitor will say we're online even while NM is DISCONNECTED,
>which is what we want.
>
>On non-NetworkManager, non-ConnMan systems where netlink works,
>netlink will tell us at least the basics of what's going on, and we
>can avoid trying to connect while there is no Internet route.
>
>On systems without a working GNetworkMonitor, GIO's default
>implementation will say "yes, you are online" at all times, which is
>no worse than we had previously.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=56635
>Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
>---
> configure.ac                                  |   29 +---
> src/connectivity-monitor.c                    |  182 ++++++-------------------
> tests/twisted/Makefile.am                     |    7 -
> tests/twisted/account-manager/connectivity.py |    4 -
> tests/twisted/fakeconnectivity.py             |    9 +-
> 5 files changed, 54 insertions(+), 177 deletions(-)
>
>diff --git a/configure.ac b/configure.ac
>index 18a1a28..c7a3266 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -283,29 +283,15 @@ fi
> # Connectivity integration
> # -----------------------------------------------------------
> AC_ARG_WITH(connectivity,
>-              AS_HELP_STRING([--with-connectivity=@<:@nm/connman/auto/no@:>@],
>+              AS_HELP_STRING([--with-connectivity=@<:@nm/auto/no@:>@],
>                              [build with connectivity support]), ,
>                              with_connectivity=auto)
> 
>-if test "x$with_connectivity" = "xno"; then
>+# --with-connectivity=connman always used to turn off NM support,
>+# so treat it as "no" now that ConnMan doesn't need anything special.
>+if test "x$with_connectivity" = "xno" || test "x$with_connectivity" = "xconnman"; then
>    have_nm=no
>-   have_connman=no
>-
>-elif test "x$with_connectivity" = "xconnman"; then
>-   have_nm=no
>-
>-   PKG_CHECK_MODULES(CONNMAN,
>-   [
>-      dbus-glib-1
>-   ], have_connman="yes", have_connman="no")
>-
>-   if test "x$have_connman" = "xyes"; then
>-      AC_DEFINE(HAVE_CONNMAN, 1, [Define if you have connman dependencies])
>-   fi
>-
> else
>-   have_connman=no
>-
>    PKG_CHECK_MODULES(NETWORK_MANAGER,
>    [
>       libnm-glib >= 0.7.0
>@@ -316,12 +302,6 @@ else
>    fi
> fi
> 
>-if test "x$with_connectivity" = "xconnman" -a "x$have_connman" != "xyes"; then
>-   AC_MSG_ERROR([Couldn't find connman dependencies:
>-
>-$CONNMAN_PKG_ERRORS])
>-fi
>-
> if test "x$with_connectivity" = "xnm" -a "x$have_nm" != "xyes"; then
>    AC_MSG_ERROR([Couldn't find Network Manager dependencies:
> 
>@@ -329,7 +309,6 @@ $NETWORK_MANAGER_PKG_ERRORS])
> fi
> 
> AM_CONDITIONAL(HAVE_NM, test "x$have_nm" = "xyes")
>-AM_CONDITIONAL(HAVE_CONNMAN, test "x$have_connman" = "xyes")
> 
> AC_ARG_ENABLE([conn-setting],
>     [AS_HELP_STRING([--enable-conn-setting],
>diff --git a/src/connectivity-monitor.c b/src/connectivity-monitor.c
>index eb728ca..618aaac 100644
>--- a/src/connectivity-monitor.c
>+++ b/src/connectivity-monitor.c
>@@ -1,6 +1,7 @@
> /* -*- Mode: C; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2; -*- */
> /*
>  * Copyright Â© 2009â2011 Collabora Ltd.
>+ * Copyright Â© 2013 Intel Corporation
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
>@@ -26,18 +27,9 @@
> 
> 
> #ifdef HAVE_NM
>-# ifdef HAVE_CONNMAN
>-#  error tried to build both Network Manager and ConnMan support simultaneously!
>-/* I mean, we could support both one day, but not for now. */
>-# endif
>-/* Moving swiftly onâ¦ */
> #include <nm-client.h>
> #endif
> 
>-#ifdef HAVE_CONNMAN
>-#include <dbus/dbus-glib.h>
>-#endif
>-
> #ifdef HAVE_UPOWER
> #include <upower.h>
> #endif
>@@ -57,15 +49,13 @@ typedef enum {
> } Connectivity;
> 
> struct _McdConnectivityMonitorPrivate {
>+  GNetworkMonitor *network_monitor;
>+
> #ifdef HAVE_NM
>   NMClient *nm_client;
>   gulong state_change_signal_id;
> #endif
> 
>-#ifdef HAVE_CONNMAN
>-  DBusGProxy *proxy;
>-#endif
>-
> #ifdef HAVE_UPOWER
>   UpClient *upower_client;
> #endif
>@@ -138,112 +128,57 @@ connectivity_monitor_nm_state_change_cb (NMClient *client,
> 
>   state = nm_client_get_state (priv->nm_client);
> 
>+  /* Deliberately not checking for DISCONNECTED. If we are really disconnected,
>+   * the netlink GNetworkMonitor will say so; or if we have non-NM connectivity
>+   * with a default route, we might as well give it a try. */
>   if (state == NM_STATE_CONNECTING
> #if NM_CHECK_VERSION(0,8,992)
>       || state == NM_STATE_DISCONNECTING
> #endif
>       || state == NM_STATE_ASLEEP)
>     {
>-      DEBUG ("New NetworkManager network state %d (unstable)", state);
>+      DEBUG ("New NetworkManager network state %d (unstable state)", state);
>+
>       connectivity_monitor_change_states (connectivity_monitor,
>           0, CONNECTIVITY_STABLE);
>     }
>-  else if (state == NM_STATE_DISCONNECTED)
>-    {
>-      DEBUG ("New NetworkManager network state %d (stable, down)", state);
>-      connectivity_monitor_change_states (connectivity_monitor,
>-          CONNECTIVITY_STABLE, CONNECTIVITY_UP);
>-    }
>   else
>     {
>-      DEBUG ("New NetworkManager network state %d (stable, up)", state);
>+      DEBUG ("New NetworkManager network state %d (stable state)", state);
>       connectivity_monitor_change_states (connectivity_monitor,
>-          CONNECTIVITY_STABLE|CONNECTIVITY_UP, 0);
>+          CONNECTIVITY_STABLE, 0);
>     }
> }
> #endif
> 
>-#ifdef HAVE_CONNMAN
> static void
>-connectivity_monitor_connman_state_changed (DBusGProxy *proxy,
>-    const gchar *new_state,
>+connectivity_monitor_network_changed (GNetworkMonitor *monitor,
>+    gboolean available,
>     McdConnectivityMonitor *connectivity_monitor)
> {
>   McdConnectivityMonitorPrivate *priv;
>-  gboolean new_connected;
> 
>   priv = connectivity_monitor->priv;
> 
>   if (!priv->use_conn)
>     return;
> 
>-  new_connected = (!tp_strdiff (new_state, "online")
>-                   || !tp_strdiff (new_state, "ready"));
>-
>-  DEBUG ("New ConnMan network state %s", new_state);
>-
>-  connectivity_monitor_set_connected (connectivity_monitor, new_connected);
>-}
>-
>-static void
>-connectivity_monitor_connman_property_changed_cb (DBusGProxy *proxy,
>-    const gchar *prop_name,
>-    const GValue *new_value,
>-    McdConnectivityMonitor *connectivity_monitor)
>-{
>-  if (!tp_strdiff (prop_name, "State"))
>-    connectivity_monitor_connman_state_changed (proxy,
>-        g_value_get_string (new_value), connectivity_monitor);
>-}
>-
>-static void
>-connectivity_monitor_connman_check_state_cb (DBusGProxy *proxy,
>-    DBusGProxyCall *call_id,
>-    gpointer user_data)
>-{
>-  McdConnectivityMonitor *connectivity_monitor = (McdConnectivityMonitor *) user_data;
>-  GError *error = NULL;
>-  GHashTable *props;
>-
>-  if (dbus_g_proxy_end_call (proxy, call_id, &error,
>-          dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_VALUE),
>-          &props, G_TYPE_INVALID))
>+  if (available)
>     {
>-      const gchar *state = tp_asv_get_string (props, "State");
>-
>-      if (state != NULL)
>-        {
>-          connectivity_monitor_connman_state_changed (proxy, state,
>-              connectivity_monitor);
>-        }
>-      else
>-        {
>-          DEBUG ("Failed to get State: not in GetProperties return");
>-        }
>-
>-      g_hash_table_unref (props);
>+      DEBUG ("GNetworkMonitor (%s) says we are at least partially online",
>+          G_OBJECT_TYPE_NAME (monitor));
>+      connectivity_monitor_change_states (connectivity_monitor,
>+          CONNECTIVITY_UP, 0);
>     }
>   else
>     {
>-      DEBUG ("Failed to call GetProperties: %s", error->message);
>-      connectivity_monitor_connman_state_changed (proxy, "offline",
>-          connectivity_monitor);
>+      DEBUG ("GNetworkMonitor (%s) says we are offline",
>+          G_OBJECT_TYPE_NAME (monitor));
>+      connectivity_monitor_change_states (connectivity_monitor,
>+          0, CONNECTIVITY_UP);
>     }
> }
> 
>-static void
>-connectivity_monitor_connman_check_state (McdConnectivityMonitor *connectivity_monitor)
>-{
>-  McdConnectivityMonitorPrivate *priv;
>-
>-  priv = connectivity_monitor->priv;
>-
>-  dbus_g_proxy_begin_call (priv->proxy, "GetProperties",
>-      connectivity_monitor_connman_check_state_cb, connectivity_monitor, NULL,
>-      G_TYPE_INVALID);
>-}
>-#endif
>-
> #ifdef HAVE_UPOWER
> static void
> connectivity_monitor_set_awake (
>@@ -285,10 +220,6 @@ static void
> mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor)
> {
>   McdConnectivityMonitorPrivate *priv;
>-#ifdef HAVE_CONNMAN
>-  DBusGConnection *connection;
>-  GError *error = NULL;
>-#endif
> 
>   priv = G_TYPE_INSTANCE_GET_PRIVATE (connectivity_monitor,
>       MCD_TYPE_CONNECTIVITY_MONITOR, McdConnectivityMonitorPrivate);
>@@ -296,7 +227,18 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor)
>   connectivity_monitor->priv = priv;
> 
>   priv->use_conn = TRUE;
>-  priv->connectivity = CONNECTIVITY_AWAKE | CONNECTIVITY_STABLE;
>+  /* Initially, assume everything is good. */
>+  priv->connectivity = CONNECTIVITY_AWAKE | CONNECTIVITY_STABLE |
>+    CONNECTIVITY_UP;
>+
>+  priv->network_monitor = g_network_monitor_get_default ();
>+
>+  tp_g_signal_connect_object (priv->network_monitor, "network-changed",
>+      G_CALLBACK (connectivity_monitor_network_changed),
>+      connectivity_monitor, 0);
>+  connectivity_monitor_network_changed (priv->network_monitor,
>+      g_network_monitor_get_network_available (priv->network_monitor),
>+      connectivity_monitor);
> 
> #ifdef HAVE_NM
>   priv->nm_client = nm_client_new ();
>@@ -314,38 +256,6 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor)
>     }
> #endif
> 
>-#ifdef HAVE_CONNMAN
>-  connection = dbus_g_bus_get (DBUS_BUS_SYSTEM, &error);
>-  if (connection != NULL)
>-    {
>-      priv->proxy = dbus_g_proxy_new_for_name (connection,
>-          "net.connman", "/",
>-          "net.connman.Manager");
>-
>-      dbus_g_object_register_marshaller (
>-          g_cclosure_marshal_generic,
>-          G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOXED, G_TYPE_INVALID);
>-
>-      dbus_g_proxy_add_signal (priv->proxy, "PropertyChanged",
>-          G_TYPE_STRING, G_TYPE_VALUE, G_TYPE_INVALID);
>-
>-      dbus_g_proxy_connect_signal (priv->proxy, "PropertyChanged",
>-          G_CALLBACK (connectivity_monitor_connman_property_changed_cb),
>-          connectivity_monitor, NULL);
>-
>-      connectivity_monitor_connman_check_state (connectivity_monitor);
>-    }
>-  else
>-    {
>-      DEBUG ("Failed to get system bus connection: %s", error->message);
>-      g_error_free (error);
>-    }
>-#endif
>-
>-#if !defined(HAVE_NM) && !defined(HAVE_CONNMAN)
>-  priv->connectivity |= CONNECTIVITY_UP;
>-#endif
>-
> #ifdef HAVE_UPOWER
>   priv->upower_client = up_client_new ();
>   tp_g_signal_connect_object (priv->upower_client,
>@@ -360,7 +270,7 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor)
> static void
> connectivity_monitor_finalize (GObject *object)
> {
>-#if defined(HAVE_NM) || defined(HAVE_CONNMAN) || defined(HAVE_UPOWER)
>+#if defined(HAVE_NM) || defined(HAVE_UPOWER)
>   McdConnectivityMonitor *connectivity_monitor = MCD_CONNECTIVITY_MONITOR (object);
>   McdConnectivityMonitorPrivate *priv = connectivity_monitor->priv;
> #endif
>@@ -376,18 +286,6 @@ connectivity_monitor_finalize (GObject *object)
>     }
> #endif
> 
>-#ifdef HAVE_CONNMAN
>-  if (priv->proxy != NULL)
>-    {
>-      dbus_g_proxy_disconnect_signal (priv->proxy, "PropertyChanged",
>-          G_CALLBACK (connectivity_monitor_connman_property_changed_cb),
>-          connectivity_monitor);
>-
>-      g_object_unref (priv->proxy);
>-      priv->proxy = NULL;
>-    }
>-#endif
>-
> #ifdef HAVE_UPOWER
>   tp_clear_object (&priv->upower_client);
> #endif
>@@ -398,6 +296,10 @@ connectivity_monitor_finalize (GObject *object)
> static void
> connectivity_monitor_dispose (GObject *object)
> {
>+  McdConnectivityMonitor *self = MCD_CONNECTIVITY_MONITOR (object);
>+
>+  g_clear_object (&self->priv->network_monitor);
>+
>   G_OBJECT_CLASS (mcd_connectivity_monitor_parent_class)->dispose (object);
> }
> 
>@@ -534,17 +436,17 @@ mcd_connectivity_monitor_set_use_conn (McdConnectivityMonitor *connectivity_moni
> 
>   priv->use_conn = use_conn;
> 
>-#if defined(HAVE_NM) || defined(HAVE_CONNMAN)
>   if (use_conn)
>     {
> #if defined(HAVE_NM)
>       connectivity_monitor_nm_state_change_cb (priv->nm_client, NULL, connectivity_monitor);
>-#elif defined(HAVE_CONNMAN)
>-      connectivity_monitor_connman_check_state (connectivity_monitor);
> #endif
>+
>+      connectivity_monitor_network_changed (priv->network_monitor,
>+          g_network_monitor_get_network_available (priv->network_monitor),
>+          connectivity_monitor);
>     }
>   else
>-#endif
>     {
>       /* !use_conn basically means "always assume it's stable and up". */
>       connectivity_monitor_change_states (connectivity_monitor,
>diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
>index 7bb3a10..6a34128 100644
>--- a/tests/twisted/Makefile.am
>+++ b/tests/twisted/Makefile.am
>@@ -146,22 +146,15 @@ HAVE_MCE_PYBOOL = False
> endif
> 
> if HAVE_NM
>-HAVE_CONNECTIVITY_PYBOOL = True
> HAVE_NM_PYBOOL = True
> else
> HAVE_NM_PYBOOL = False
>-if HAVE_CONNMAN
>-HAVE_CONNECTIVITY_PYBOOL = True
>-else
>-HAVE_CONNECTIVITY_PYBOOL = False
>-endif
> endif
> 
> config.py: Makefile
> 	$(AM_V_GEN) { \
> 		echo "HAVE_MCE = $(HAVE_MCE_PYBOOL)"; \
> 		echo "HAVE_NM = $(HAVE_NM_PYBOOL)"; \
>-		echo "HAVE_CONNECTIVITY = $(HAVE_CONNECTIVITY_PYBOOL)"; \
> 	} > $@
> 
> BUILT_SOURCES = config.py
>diff --git a/tests/twisted/account-manager/connectivity.py b/tests/twisted/account-manager/connectivity.py
>index 0198545..a8b6514 100644
>--- a/tests/twisted/account-manager/connectivity.py
>+++ b/tests/twisted/account-manager/connectivity.py
>@@ -30,10 +30,6 @@ import constants as cs
> 
> import config
> 
>-if not config.HAVE_CONNECTIVITY:
>-    print "NOTE: built without ConnMan or NM support"
>-    raise SystemExit(77)
>-
> def sync_connectivity_state(mc):
>     # We cannot simply use sync_dbus here, because nm-glib reports property
>     # changes in an idle (presumably to batch them all together). This is fine
>diff --git a/tests/twisted/fakeconnectivity.py b/tests/twisted/fakeconnectivity.py
>index ccc62e5..02d577f 100644
>--- a/tests/twisted/fakeconnectivity.py
>+++ b/tests/twisted/fakeconnectivity.py
>@@ -17,6 +17,10 @@ class FakeConnectivity(object):
>     NM_STATE_CONNECTED_SITE   = 60
>     NM_STATE_CONNECTED_GLOBAL = 70
> 
>+    # Our fake GNetworkMonitor uses the ConnMan 0.79 D-Bus API - we don't
>+    # have any special support for ConnMan any more, but it's as good an
>+    # API as any. The important thing is that it's not NM, because we *do*
>+    # have a bit of special support for that.
>     CONNMAN_BUS_NAME = 'net.connman'
>     CONNMAN_PATH = '/'
>     CONNMAN_INTERFACE = 'net.connman.Manager'
>@@ -101,7 +105,10 @@ class FakeConnectivity(object):
>     def change_state(self, online, indeterminate=False):
>         if indeterminate:
>             self.nm_state = self.NM_STATE_DISCONNECTING
>-            # keep the previous ConnMan state
>+            # keep the previous "ConnMan" (GNetworkMonitor) state;
>+            # any other GNetworkMonitor would probably do the same
>+            # while trying to disconnect, because e.g. netlink will say the
>+            # interface is still up
>         elif online:
>             self.nm_state = self.NM_STATE_CONNECTED_GLOBAL
>             self.connman_state = self.CONNMAN_ONLINE
>-- 
>1.7.10.4
>

Actions: View | Splinter Review

Attachments on bug 56635: 78655 | 78656 | 78657 | 78658

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.