[PATCH] modem-manager: properly follow name-owner changes

[Aleksander Morgado <aleksander lanedo com>]

We avoid requesting to auto-start the service when the 'MMManager' is created,
so that we can use it to follow name-owner changes (when auto-starting
requested the 'MMManager' creation may fail).

We still handle the periodic poking to the service, but instead of re-creating
the 'MMManager', we just call org.freedesktop.DBus.Peer.Ping().
---
 src/modem-manager/nm-modem-manager.c | 119 ++++++++++++++++++++++++-----------
 1 file changed, 83 insertions(+), 36 deletions(-)

diff --git a/src/modem-manager/nm-modem-manager.c b/src/modem-manager/nm-modem-manager.c
index 553c114..d88a768 100644
--- a/src/modem-manager/nm-modem-manager.c
+++ b/src/modem-manager/nm-modem-manager.c
@@ -532,6 +532,65 @@ modem_manager_1_name_owner_changed (MMManager *modem_manager_1,
 }
 
 static void
+modem_manager_1_poke_cb (GDBusConnection *connection,
+                         GAsyncResult *res,
+                         NMModemManager *self)
+{
+	GError *error = NULL;
+	GVariant *result;
+
+	result = g_dbus_connection_call_finish (connection, res, &error);
+	if (error) {
+		/* Ignore common errors when MM is not installed and such */
+		if (   !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN)
+		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_EXEC_FAILED)
+		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_FORK_FAILED)
+		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_FAILED)
+		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_TIMEOUT)
+		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) {
+			nm_log_warn (LOGD_MB, "error poking ModemManager: %s", error->message);
+		}
+
+		g_error_free (error);
+		/* Setup timeout to relaunch */
+		schedule_modem_manager_1_relaunch (self, MODEM_POKE_INTERVAL);
+	} else
+		g_variant_unref (result);
+
+	/* Balance refcount */
+	g_object_unref (self);
+}
+
+static void
+modem_manager_1_poke (NMModemManager *self)
+{
+	gchar *name_owner;
+
+	/* If there is no current owner right away, ensure we poke to get one */
+	name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (self->priv->modem_manager_1));
+	if (name_owner) {
+		/* Available! */
+		modem_manager_1_available (self);
+		g_free (name_owner);
+		return;
+	}
+
+	/* Poke! */
+	g_dbus_connection_call (self->priv->dbus_connection,
+	                        "org.freedesktop.ModemManager1",
+	                        "/org/freedesktop/ModemManager1",
+	                        "org.freedesktop.DBus.Peer",
+	                        "Ping",
+	                        NULL, /* inputs */
+	                        NULL, /* outputs */
+	                        G_DBUS_CALL_FLAGS_NONE,
+	                        -1,
+	                        NULL, /* cancellable */
+	                        (GAsyncReadyCallback)modem_manager_1_poke_cb, /* callback */
+	                        g_object_ref (self)); /* user_data */
+}
+
+static void
 manager_new_ready (GObject *source,
                    GAsyncResult *res,
                    NMModemManager *self)
@@ -543,14 +602,10 @@ manager_new_ready (GObject *source,
 	g_assert (!self->priv->modem_manager_1);
 	self->priv->modem_manager_1 = mm_manager_new_finish (res, &error);
 	if (!self->priv->modem_manager_1) {
-		if (   !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN)
-		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_EXEC_FAILED)
-		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_FORK_FAILED)
-		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_FAILED)
-		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_TIMEOUT)
-		    && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) {
-			nm_log_warn (LOGD_MB, "error creating ModemManager client: %s", error->message);
-		}
+		/* We're not really supposed to get any error here. If we do get one,
+		 * though, just re-schedule the MMManager creation after some time.
+		 * During this period, name-owner changes won't be followed. */
+		nm_log_warn (LOGD_MB, "error creating ModemManager client: %s", error->message);
 		g_error_free (error);
 		/* Setup timeout to relaunch */
 		schedule_modem_manager_1_relaunch (self, MODEM_POKE_INTERVAL);
@@ -558,8 +613,6 @@ manager_new_ready (GObject *source,
 		/* If we found the old MM, abort */
 		clear_modem_manager_1_support (self);
 	} else {
-		gchar *name_owner;
-
 		g_signal_connect (self->priv->modem_manager_1,
 		                  "notify::name-owner",
 		                  G_CALLBACK (modem_manager_1_name_owner_changed),
@@ -573,17 +626,8 @@ manager_new_ready (GObject *source,
 		                  G_CALLBACK (modem_object_removed),
 		                  self);
 
-		/* If there is no current owner right away, ensure we poke until we get
-		 * one */
-		name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (self->priv->modem_manager_1));
-		if (!name_owner) {
-			/* Setup timeout to wait for an owner */
-			schedule_modem_manager_1_relaunch (self, MODEM_POKE_INTERVAL);
-		} else {
-			/* Available! */
-			modem_manager_1_available (self);
-			g_free (name_owner);
-		}
+		/* Poke the MMManager! */
+		modem_manager_1_poke (self);
 	}
 
 	/* Balance refcount */
@@ -591,19 +635,24 @@ manager_new_ready (GObject *source,
 }
 
 static void
-recreate_client (NMModemManager *self)
+ensure_client (NMModemManager *self)
 {
 	g_assert (self->priv->dbus_connection);
 
-	/* Re-create the GDBusObjectManagerClient so that we request again the owner
-	 * for the well-known name.
-	 * Note that we pass an extra reference always */
-	g_clear_object (&self->priv->modem_manager_1);
-	mm_manager_new (self->priv->dbus_connection,
-	                G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE,
-	                NULL,
-	                (GAsyncReadyCallback)manager_new_ready,
-	                g_object_ref (self));
+	/* Create the GDBusObjectManagerClient. We do not request to autostart, as
+	 * we don't really want the MMManager creation to fail. We can always poke
+	 * later on if we want to request the autostart */
+	if (!self->priv->modem_manager_1) {
+		mm_manager_new (self->priv->dbus_connection,
+		                G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START,
+		                NULL,
+		                (GAsyncReadyCallback)manager_new_ready,
+		                g_object_ref (self));
+		return;
+	}
+
+	/* If already available, poke! */
+	modem_manager_1_poke (self);
 }
 
 static void
@@ -626,7 +675,7 @@ bus_get_ready (GObject *source,
 		clear_modem_manager_1_support (self);
 	} else {
 		/* Got the bus, create new ModemManager client. */
-		recreate_client (self);
+		ensure_client (self);
 	}
 
 	/* Balance refcount */
@@ -636,8 +685,6 @@ bus_get_ready (GObject *source,
 static gboolean
 ensure_bus (NMModemManager *self)
 {
-	nm_log_dbg (LOGD_MB, "Requesting to (re)launch ModemManager...");
-
 	/* Clear poke ID */
 	self->priv->modem_manager_1_poke_id = 0;
 
@@ -647,8 +694,8 @@ ensure_bus (NMModemManager *self)
 		           (GAsyncReadyCallback)bus_get_ready,
 		           g_object_ref (self));
 	else
-		/* If bus is already available, launch client re-creation */
-		recreate_client (self);
+		/* If bus is already available, ensure client */
+		ensure_client (self);
 
 	return FALSE;
 }
-- 
1.8.1