firewall: refactor "nm-firewall-manager.c" to not use GDBusProxyth/firewall-manager-rework

- Don't use GDBusProxy but plain GDBusConnection. NMFirewallManager
  is very simple, it doesn't use any of the features that GDBusProxy
  provides.

- make NMFirewallManagerCallId typedef a pointer to the opaque call-id
  struct, instead of the struct itself. It's confusing to have a
  variable that does not look like a pointer and assigning %NULL to
  it.

- internally drop the CBInfo typename and name the call-id variable
  constsistantly as "call_id".

- no need to keep the call-id struct alive after cancelling it. That
  simplifies the lifetime managment of the pending call because the
  completion callback is always invoked shortly before destroying
  the call-id.

- note that the caller is no longer allowed to cancel a call-id from
  inside the completion callback. That just complicates the
  implementation and is not necessary. Assert against that.

4 files changed, 323 insertions, 280 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index f07c4f09a5..c08797ca4f 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -489,7 +489,7 @@ typedef struct _NMDevicePrivate {
 	/* Firewall */
 	FirewallState fw_state:4;
 	NMFirewallManager *fw_mgr;
-	NMFirewallManagerCallId fw_call;
+	NMFirewallManagerCallId *fw_call;
 
 	/* IPv4LL stuff */
 	sd_ipv4ll *    ipv4ll;
@@ -10303,7 +10303,7 @@ activate_stage3_ip_config_start (NMDevice *self)
 
 static void
 fw_change_zone_cb (NMFirewallManager *firewall_manager,
-                   NMFirewallManagerCallId call_id,
+                   NMFirewallManagerCallId *call_id,
                    GError *error,
                    gpointer user_data)
 {
@@ -10316,6 +10316,7 @@ fw_change_zone_cb (NMFirewallManager *firewall_manager,
 
 	if (priv->fw_call != call_id)
 		g_return_if_reached ();
+
 	priv->fw_call = NULL;
 
 	if (nm_utils_error_is_cancelled (error, FALSE))
diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c
index 9b0d49c924..e068438fb1 100644
--- a/src/nm-firewall-manager.c
+++ b/src/nm-firewall-manager.c
@@ -21,9 +21,16 @@
 
 #include "nm-firewall-manager.h"
 
-#include "NetworkManagerUtils.h"
+#include "nm-glib-aux/nm-dbus-aux.h"
 #include "c-list/src/c-list.h"
 
+#include "NetworkManagerUtils.h"
+#include "nm-dbus-manager.h"
+
+#define FIREWALL_DBUS_SERVICE         "org.fedoraproject.FirewallD1"
+#define FIREWALL_DBUS_PATH            "/org/fedoraproject/FirewallD1"
+#define FIREWALL_DBUS_INTERFACE_ZONE  "org.fedoraproject.FirewallD1.zone"
+
 /*****************************************************************************/
 
 enum {
@@ -34,11 +41,16 @@ enum {
 static guint signals[LAST_SIGNAL] = { 0 };
 
 typedef struct {
-	GDBusProxy     *proxy;
-	GCancellable   *proxy_cancellable;
+	GDBusConnection *dbus_connection;
+
+	GCancellable *get_name_owner_cancellable;
+
+	CList pending_calls;
 
-	CList           pending_calls;
-	bool            running;
+	guint name_owner_changed_id;
+
+	bool dbus_inited:1;
+	bool running:1;
 } NMFirewallManagerPrivate;
 
 struct _NMFirewallManager {
@@ -61,27 +73,18 @@ NM_DEFINE_SINGLETON_GETTER (NMFirewallManager, nm_firewall_manager_get, NM_TYPE_
 /*****************************************************************************/
 
 typedef enum {
-	CB_INFO_OPS_ADD = 1,
-	CB_INFO_OPS_CHANGE,
-	CB_INFO_OPS_REMOVE,
-} CBInfoOpsType;
-
-typedef enum {
-	CB_INFO_MODE_IDLE = 1,
-	CB_INFO_MODE_DBUS_WAITING,
-	CB_INFO_MODE_DBUS,
-	CB_INFO_MODE_DBUS_COMPLETED,
-} CBInfoMode;
+	OPS_TYPE_ADD = 1,
+	OPS_TYPE_CHANGE,
+	OPS_TYPE_REMOVE,
+} OpsType;
 
 struct _NMFirewallManagerCallId {
 	CList lst;
+
 	NMFirewallManager *self;
-	CBInfoOpsType ops_type;
-	union {
-		const CBInfoMode mode;
-		CBInfoMode mode_mutable;
-	};
+
 	char *iface;
+
 	NMFirewallManagerAddRemoveCallback callback;
 	gpointer user_data;
 
@@ -94,45 +97,57 @@ struct _NMFirewallManagerCallId {
 			guint id;
 		} idle;
 	};
+
+	OpsType ops_type;
+
+	bool is_idle:1;
 };
-typedef struct _NMFirewallManagerCallId CBInfo;
 
 /*****************************************************************************/
 
 static const char *
-_ops_type_to_string (CBInfoOpsType ops_type)
+_ops_type_to_string (OpsType ops_type)
 {
 	switch (ops_type) {
-	case CB_INFO_OPS_ADD:    return "add";
-	case CB_INFO_OPS_REMOVE: return "remove";
-	case CB_INFO_OPS_CHANGE: return "change";
-	default: g_return_val_if_reached ("unknown");
+	case OPS_TYPE_ADD:    return "add";
+	case OPS_TYPE_REMOVE: return "remove";
+	case OPS_TYPE_CHANGE: return "change";
 	}
+	nm_assert_not_reached ();
+	return NULL;
 }
 
 #define _NMLOG_DOMAIN      LOGD_FIREWALL
 #define _NMLOG_PREFIX_NAME "firewall"
-#define _NMLOG(level, info, ...) \
+#define _NMLOG(level, call_id, ...) \
     G_STMT_START { \
         if (nm_logging_enabled ((level), (_NMLOG_DOMAIN))) { \
-            CBInfo *__info = (info); \
-            char __prefix_name[30]; \
-            char __prefix_info[64]; \
+            NMFirewallManagerCallId *_call_id = (call_id); \
+            char _prefix_name[30]; \
+            char _prefix_info[100]; \
             \
             _nm_log ((level), (_NMLOG_DOMAIN), 0, NULL, NULL, \
                      "%s: %s" _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \
                      (self) != singleton_instance \
                         ? ({ \
-                                g_snprintf (__prefix_name, sizeof (__prefix_name), "%s[%p]", ""_NMLOG_PREFIX_NAME, (self)); \
-                                __prefix_name; \
+                                g_snprintf (_prefix_name, \
+                                            sizeof (_prefix_name), \
+                                            "%s["NM_HASH_OBFUSCATE_PTR_FMT"]", \
+                                            ""_NMLOG_PREFIX_NAME,\
+                                            NM_HASH_OBFUSCATE_PTR (self)); \
+                                _prefix_name; \
                            }) \
                         : _NMLOG_PREFIX_NAME, \
-                     __info \
+                     _call_id \
                         ? ({ \
-                                g_snprintf (__prefix_info, sizeof (__prefix_info), "[%p,%s%s:%s%s%s]: ", __info, \
-                                            _ops_type_to_string (__info->ops_type), __info->mode == CB_INFO_MODE_IDLE ? "*" : "", \
-                                            NM_PRINT_FMT_QUOTE_STRING (__info->iface)); \
-                                __prefix_info; \
+                                g_snprintf (_prefix_info, \
+                                            sizeof (_prefix_info), \
+                                            "["NM_HASH_OBFUSCATE_PTR_FMT",%s%s:%s%s%s]: ", \
+                                            NM_HASH_OBFUSCATE_PTR (_call_id), \
+                                            _ops_type_to_string (_call_id->ops_type), \
+                                            _call_id->is_idle ? "*" : "", \
+                                            NM_PRINT_FMT_QUOTE_STRING (_call_id->iface)); \
+                                _prefix_info; \
                            }) \
                         : "" \
                      _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
@@ -141,125 +156,152 @@ _ops_type_to_string (CBInfoOpsType ops_type)
 
 /*****************************************************************************/
 
+static gboolean
+_get_running (NMFirewallManagerPrivate *priv)
+{
+	/* when starting, we need to asynchronously check whether there is
+	 * a name owner. During that time we optimistially assume that the
+	 * service is indeed running. That is the time when we queue the
+	 * requests, and they will be started once the get-name-owner call
+	 * returns. */
+	return    priv->running
+	       || (   priv->dbus_connection
+	           && !priv->dbus_inited);
+}
+
 gboolean
 nm_firewall_manager_get_running (NMFirewallManager *self)
 {
 	g_return_val_if_fail (NM_IS_FIREWALL_MANAGER (self), FALSE);
 
-	return NM_FIREWALL_MANAGER_GET_PRIVATE (self)->running;
+	return _get_running (NM_FIREWALL_MANAGER_GET_PRIVATE (self));
 }
 
 /*****************************************************************************/
 
-static CBInfo *
+static NMFirewallManagerCallId *
 _cb_info_create (NMFirewallManager *self,
-                 CBInfoOpsType ops_type,
+                 OpsType ops_type,
                  const char *iface,
                  const char *zone,
                  NMFirewallManagerAddRemoveCallback callback,
                  gpointer user_data)
 {
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
-	CBInfo *info;
-
-	info = g_slice_new0 (CBInfo);
-	info->self = g_object_ref (self);
-	info->ops_type = ops_type;
-	info->iface = g_strdup (iface);
-	info->callback = callback;
-	info->user_data = user_data;
-
-	if (priv->running || priv->proxy_cancellable) {
-		info->mode_mutable = CB_INFO_MODE_DBUS_WAITING;
-		info->dbus.arg = g_variant_new ("(ss)", zone ?: "", iface);
-	} else
-		info->mode_mutable = CB_INFO_MODE_IDLE;
+	NMFirewallManagerCallId *call_id;
 
-	c_list_link_tail (&priv->pending_calls, &info->lst);
+	call_id = g_slice_new0 (NMFirewallManagerCallId);
 
-	return info;
-}
+	call_id->self = g_object_ref (self);
+	call_id->ops_type = ops_type;
+	call_id->iface = g_strdup (iface);
+	call_id->callback = callback;
+	call_id->user_data = user_data;
 
-static void
-_cb_info_free (CBInfo *info)
-{
-	c_list_unlink_stale (&info->lst);
-	if (info->mode != CB_INFO_MODE_IDLE) {
-		if (info->dbus.arg)
-			g_variant_unref (info->dbus.arg);
-		g_clear_object (&info->dbus.cancellable);
-	}
-	g_free (info->iface);
-	if (info->self)
-		g_object_unref (info->self);
-	g_slice_free (CBInfo, info);
-}
+	if (_get_running (priv)) {
+		call_id->is_idle = FALSE;
+		call_id->dbus.arg = g_variant_new ("(ss)", zone ?: "", iface);
+	} else
+		call_id->is_idle = TRUE;
 
-static void
-_cb_info_callback (CBInfo *info,
-                   GError *error)
-{
-	if (info->callback)
-		info->callback (info->self, info, error, info->user_data);
+	c_list_link_tail (&priv->pending_calls, &call_id->lst);
+
+	return call_id;
 }
 
 static void
-_cb_info_complete_normal (CBInfo *info, GError *error)
+_cb_info_complete (NMFirewallManagerCallId *call_id,
+                   GError *error)
 {
-	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (info->self);
+	c_list_unlink (&call_id->lst);
 
-	nm_assert (c_list_contains (&priv->pending_calls, &info->lst));
+	if (call_id->callback)
+		call_id->callback (call_id->self, call_id, error, call_id->user_data);
 
-	c_list_unlink (&info->lst);
-
-	_cb_info_callback (info, error);
-	_cb_info_free (info);
+	if (call_id->is_idle)
+		nm_clear_g_source (&call_id->idle.id);
+	else {
+		nm_g_variant_unref (call_id->dbus.arg);
+		nm_clear_g_cancellable (&call_id->dbus.cancellable);
+	}
+	g_free (call_id->iface);
+	g_object_unref (call_id->self);
+	nm_g_slice_free (call_id);
 }
 
 static gboolean
-_handle_idle (gpointer user_data)
+_handle_idle_cb (gpointer user_data)
 {
 	NMFirewallManager *self;
-	CBInfo *info = user_data;
+	NMFirewallManagerCallId *call_id = user_data;
+
+	nm_assert (call_id);
+	nm_assert (NM_IS_FIREWALL_MANAGER (call_id->self));
+	nm_assert (call_id->is_idle);
+	nm_assert (c_list_contains (&NM_FIREWALL_MANAGER_GET_PRIVATE (call_id->self)->pending_calls, &call_id->lst));
 
-	nm_assert (info && NM_IS_FIREWALL_MANAGER (info->self));
+	self = call_id->self;
 
-	self = info->self;
+	_LOGD (call_id, "complete: fake success");
 
-	_LOGD (info, "complete: fake success");
+	call_id->idle.id = 0;
 
-	_cb_info_complete_normal (info, NULL);
+	_cb_info_complete (call_id, NULL);
 	return G_SOURCE_REMOVE;
 }
 
+static gboolean
+_handle_idle_start (NMFirewallManager *self,
+                    NMFirewallManagerCallId *call_id)
+{
+	if (!call_id->callback) {
+		/* if the user did not provide a callback and firewalld is not running,
+		 * there is no point in scheduling an idle-request to fake success. Just
+		 * return right away. */
+		_LOGD (call_id, "complete: drop request simulating success");
+		_cb_info_complete (call_id, NULL);
+		return FALSE;
+	}
+	call_id->idle.id = g_idle_add (_handle_idle_cb, call_id);
+	return TRUE;
+}
+
 static void
-_handle_dbus (GObject *proxy, GAsyncResult *result, gpointer user_data)
+_handle_dbus_cb (GObject *source,
+                 GAsyncResult *result,
+                 gpointer user_data)
 {
 	NMFirewallManager *self;
-	CBInfo *info = user_data;
+	NMFirewallManagerCallId *call_id;
 	gs_free_error GError *error = NULL;
 	gs_unref_variant GVariant *ret = NULL;
 
-	if (info->mode != CB_INFO_MODE_DBUS) {
-		_cb_info_free (info);
+	ret = g_dbus_connection_call_finish (G_DBUS_CONNECTION (source), result, &error);
+
+	if (   !ret
+	    && nm_utils_error_is_cancelled (error, FALSE))
 		return;
-	}
 
-	self = info->self;
+	call_id = user_data;
 
-	ret = g_dbus_proxy_call_finish (G_DBUS_PROXY (proxy), result, &error);
+	nm_assert (call_id);
+	nm_assert (NM_IS_FIREWALL_MANAGER (call_id->self));
+	nm_assert (!call_id->is_idle);
+	nm_assert (c_list_contains (&NM_FIREWALL_MANAGER_GET_PRIVATE (call_id->self)->pending_calls, &call_id->lst));
+
+	self = call_id->self;
 
 	if (error) {
 		const char *non_error = NULL;
 
 		g_dbus_error_strip_remote_error (error);
 
-		switch (info->ops_type) {
-		case CB_INFO_OPS_ADD:
-		case CB_INFO_OPS_CHANGE:
+		switch (call_id->ops_type) {
+		case OPS_TYPE_ADD:
+		case OPS_TYPE_CHANGE:
 			non_error = "ZONE_ALREADY_SET";
 			break;
-		case CB_INFO_OPS_REMOVE:
+		case OPS_TYPE_REMOVE:
 			non_error = "UNKNOWN_INTERFACE";
 			break;
 		}
@@ -267,118 +309,122 @@ _handle_dbus (GObject *proxy, GAsyncResult *result, gpointer user_data)
 		    && non_error
 		    && g_str_has_prefix (error->message, non_error)
 		    && NM_IN_SET (error->message[strlen (non_error)], '\0', ':')) {
-			_LOGD (info, "complete: request failed with a non-error (%s)", error->message);
+			_LOGD (call_id, "complete: request failed with a non-error (%s)", error->message);
 
 			/* The operation failed with an error reason that we don't want
 			 * to propagate. Instead, signal success. */
 			g_clear_error (&error);
 		} else
-			_LOGW (info, "complete: request failed (%s)", error->message);
+			_LOGW (call_id, "complete: request failed (%s)", error->message);
 	} else
-		_LOGD (info, "complete: success");
+		_LOGD (call_id, "complete: success");
+
+	g_clear_object (&call_id->dbus.cancellable);
 
-	_cb_info_complete_normal (info, error);
+	_cb_info_complete (call_id, error);
 }
 
 static void
 _handle_dbus_start (NMFirewallManager *self,
-                    CBInfo *info)
+                    NMFirewallManagerCallId *call_id)
 {
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
 	const char *dbus_method = NULL;
 	GVariant *arg;
 
-	nm_assert (info);
+	nm_assert (call_id);
 	nm_assert (priv->running);
-	nm_assert (info->mode == CB_INFO_MODE_DBUS_WAITING);
+	nm_assert (!call_id->is_idle);
+	nm_assert (c_list_contains (&priv->pending_calls, &call_id->lst));
 
-	switch (info->ops_type) {
-	case CB_INFO_OPS_ADD:
+	switch (call_id->ops_type) {
+	case OPS_TYPE_ADD:
 		dbus_method = "addInterface";
 		break;
-	case CB_INFO_OPS_CHANGE:
+	case OPS_TYPE_CHANGE:
 		dbus_method = "changeZone";
 		break;
-	case CB_INFO_OPS_REMOVE:
+	case OPS_TYPE_REMOVE:
 		dbus_method = "removeInterface";
 		break;
 	}
 	nm_assert (dbus_method);
 
-	arg = info->dbus.arg;
-	info->dbus.arg = NULL;
+	arg = g_steal_pointer (&call_id->dbus.arg);
 
 	nm_assert (arg && g_variant_is_floating (arg));
 
-	info->mode_mutable = CB_INFO_MODE_DBUS;
-	info->dbus.cancellable = g_cancellable_new ();
-
-	g_dbus_proxy_call (priv->proxy,
-	                   dbus_method,
-	                   arg,
-	                   G_DBUS_CALL_FLAGS_NONE, 10000,
-	                   info->dbus.cancellable,
-	                   _handle_dbus,
-	                   info);
+	nm_assert (!call_id->dbus.cancellable);
+
+	call_id->dbus.cancellable = g_cancellable_new ();
+
+	g_dbus_connection_call (priv->dbus_connection,
+	                        FIREWALL_DBUS_SERVICE,
+	                        FIREWALL_DBUS_PATH,
+	                        FIREWALL_DBUS_INTERFACE_ZONE,
+	                        dbus_method,
+	                        arg,
+	                        NULL,
+	                        G_DBUS_CALL_FLAGS_NONE,
+	                        10000,
+	                        call_id->dbus.cancellable,
+	                        _handle_dbus_cb,
+	                        call_id);
 }
 
-static NMFirewallManagerCallId
+static NMFirewallManagerCallId *
 _start_request (NMFirewallManager *self,
-                CBInfoOpsType ops_type,
+                OpsType ops_type,
                 const char *iface,
                 const char *zone,
                 NMFirewallManagerAddRemoveCallback callback,
                 gpointer user_data)
 {
 	NMFirewallManagerPrivate *priv;
-	CBInfo *info;
+	NMFirewallManagerCallId *call_id;
 
 	g_return_val_if_fail (NM_IS_FIREWALL_MANAGER (self), NULL);
 	g_return_val_if_fail (iface && *iface, NULL);
 
 	priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
 
-	info = _cb_info_create (self, ops_type, iface, zone, callback, user_data);
+	call_id = _cb_info_create (self, ops_type, iface, zone, callback, user_data);
 
-	_LOGD (info, "firewall zone %s %s:%s%s%s%s",
-	       _ops_type_to_string (info->ops_type),
+	_LOGD (call_id, "firewall zone %s %s:%s%s%s%s",
+	       _ops_type_to_string (call_id->ops_type),
 	       iface,
 	       NM_PRINT_FMT_QUOTED (zone, "\"", zone, "\"", "default"),
-	       info->mode == CB_INFO_MODE_IDLE
+	       call_id->is_idle
 	         ? " (not running, simulate success)"
 	         : (!priv->running
 	              ? " (waiting to initialize)"
 	              : ""));
 
-	if (info->mode == CB_INFO_MODE_DBUS_WAITING) {
+	if (!call_id->is_idle) {
 		if (priv->running)
-			_handle_dbus_start (self, info);
-		if (!info->callback) {
+			_handle_dbus_start (self, call_id);
+		if (!call_id->callback) {
 			/* if the user did not provide a callback, the call_id is useless.
 			 * Especially, the user cannot use the call-id to cancel the request,
 			 * because he cannot know whether the request is still pending.
 			 *
 			 * Hence, returning %NULL doesn't mean that the request could not be started
-			 * (the request will always be started). */
+			 * (this function never fails and always starts a request). */
 			return NULL;
 		}
-	} else if (info->mode == CB_INFO_MODE_IDLE) {
-		if (!info->callback) {
+	} else {
+		if (!_handle_idle_start (self, call_id)) {
 			/* if the user did not provide a callback and firewalld is not running,
 			 * there is no point in scheduling an idle-request to fake success. Just
 			 * return right away. */
-			_LOGD (info, "complete: drop request simulating success");
-			_cb_info_complete_normal (info, NULL);
 			return NULL;
-		} else
-			info->idle.id = g_idle_add (_handle_idle, info);
-	} else
-		nm_assert_not_reached ();
+		}
+	}
 
-	return info;
+	return call_id;
 }
 
-NMFirewallManagerCallId
+NMFirewallManagerCallId *
 nm_firewall_manager_add_or_change_zone (NMFirewallManager *self,
                                         const char *iface,
                                         const char *zone,
@@ -387,14 +433,14 @@ nm_firewall_manager_add_or_change_zone (NMFirewallManager *self,
                                         gpointer user_data)
 {
 	return _start_request (self,
-	                       add ? CB_INFO_OPS_ADD : CB_INFO_OPS_CHANGE,
+	                       add ? OPS_TYPE_ADD : OPS_TYPE_CHANGE,
 	                       iface,
 	                       zone,
 	                       callback,
 	                       user_data);
 }
 
-NMFirewallManagerCallId
+NMFirewallManagerCallId *
 nm_firewall_manager_remove_from_zone (NMFirewallManager *self,
                                       const char *iface,
                                       const char *zone,
@@ -402,7 +448,7 @@ nm_firewall_manager_remove_from_zone (NMFirewallManager *self,
                                       gpointer user_data)
 {
 	return _start_request (self,
-	                       CB_INFO_OPS_REMOVE,
+	                       OPS_TYPE_REMOVE,
 	                       iface,
 	                       zone,
 	                       callback,
@@ -410,162 +456,161 @@ nm_firewall_manager_remove_from_zone (NMFirewallManager *self,
 }
 
 void
-nm_firewall_manager_cancel_call (NMFirewallManagerCallId call)
+nm_firewall_manager_cancel_call (NMFirewallManagerCallId *call_id)
 {
 	NMFirewallManager *self;
 	NMFirewallManagerPrivate *priv;
-	CBInfo *info = call;
 	gs_free_error GError *error = NULL;
 
-	g_return_if_fail (info);
-	g_return_if_fail (NM_IS_FIREWALL_MANAGER (info->self));
+	g_return_if_fail (call_id);
+	g_return_if_fail (NM_IS_FIREWALL_MANAGER (call_id->self));
+	g_return_if_fail (!c_list_is_empty (&call_id->lst));
 
-	self = info->self;
+	self = call_id->self;
 	priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
 
-	nm_assert (c_list_contains (&priv->pending_calls, &info->lst));
-
-	c_list_unlink (&info->lst);
+	nm_assert (c_list_contains (&priv->pending_calls, &call_id->lst));
 
 	nm_utils_error_set_cancelled (&error, FALSE, "NMFirewallManager");
 
-	_LOGD (info, "complete: cancel (%s)", error->message);
-
-	_cb_info_callback (info, error);
+	_LOGD (call_id, "complete: cancel (%s)", error->message);
 
-	if (info->mode == CB_INFO_MODE_DBUS_WAITING)
-		_cb_info_free (info);
-	else if (info->mode == CB_INFO_MODE_IDLE) {
-		g_source_remove (info->idle.id);
-		_cb_info_free (info);
-	} else {
-		info->mode_mutable = CB_INFO_MODE_DBUS_COMPLETED;
-		g_cancellable_cancel (info->dbus.cancellable);
-		g_clear_object (&info->self);
-	}
+	_cb_info_complete (call_id, error);
 }
 
 /*****************************************************************************/
 
-static gboolean
-name_owner_changed (NMFirewallManager *self)
+static void
+name_owner_changed (NMFirewallManager *self,
+                    const char *owner)
 {
+	_nm_unused gs_unref_object NMFirewallManager *self_keep_alive = g_object_ref (self);
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
-	gs_free char *owner = NULL;
+	gboolean was_running;
 	gboolean now_running;
+	gboolean just_initied;
+
+	owner = nm_str_not_empty (owner);
+
+	if (!owner)
+		_LOGT (NULL, "D-Bus name for firewalld has no owner (firewall stopped)");
+	else
+		_LOGT (NULL, "D-Bus name for firewalld has owner %s (firewall started)", owner);
+
+	was_running = _get_running (priv);
+	just_initied = !priv->dbus_inited;
+
+	priv->dbus_inited = TRUE;
+	priv->running = !!owner;
+
+	now_running = _get_running (priv);
+
+	if (just_initied) {
+		NMFirewallManagerCallId *call_id_safe;
+		NMFirewallManagerCallId *call_id;
+
+		/* We kick of the requests that we have pending. Note that this is
+		 * entirely asynchrnous and also we don't invoke any callbacks for
+		 * the user.
+		 * Even _handle_idle_start() just schedules an idle handler. */
+		c_list_for_each_entry_safe (call_id, call_id_safe, &priv->pending_calls, lst) {
+
+			nm_assert (!call_id->is_idle);
+			nm_assert (call_id->dbus.arg);
+
+			if (priv->running) {
+				_LOGD (call_id, "initalizing: make D-Bus call");
+				_handle_dbus_start (self, call_id);
+			} else {
+				/* we don't want to invoke callbacks to the user right away. That is because
+				 * the user might schedule/cancel more calls, which messes up the order.
+				 *
+				 * Instead, convert the pending calls to idle requests... */
+				nm_clear_pointer (&call_id->dbus.arg, g_variant_unref);
+				call_id->is_idle = TRUE;
+				_LOGD (call_id, "initializing: fake success on idle");
+				_handle_idle_start (self, call_id);
+			}
+		}
+	}
 
-	owner = g_dbus_proxy_get_name_owner (priv->proxy);
-	now_running = !!owner;
-
-	if (now_running == priv->running)
-		return FALSE;
-
-	priv->running = now_running;
-	_LOGD (NULL, "firewall %s", now_running ? "started" : "stopped");
-	return TRUE;
+	if (was_running != now_running)
+		g_signal_emit (self, signals[STATE_CHANGED], 0, FALSE);
 }
 
 static void
-name_owner_changed_cb (GObject    *object,
-                       GParamSpec *pspec,
-                       gpointer    user_data)
+name_owner_changed_cb (GDBusConnection *connection,
+                       const char *sender_name,
+                       const char *object_path,
+                       const char *interface_name,
+                       const char *signal_name,
+                       GVariant *parameters,
+                       gpointer user_data)
 {
 	NMFirewallManager *self = user_data;
+	const char *new_owner;
 
-	nm_assert (NM_IS_FIREWALL_MANAGER (self));
-	nm_assert (G_IS_DBUS_PROXY (object));
-	nm_assert (NM_FIREWALL_MANAGER_GET_PRIVATE (self)->proxy == G_DBUS_PROXY (object));
+	if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(sss)")))
+		return;
 
-	if (name_owner_changed (self))
-		g_signal_emit (self, signals[STATE_CHANGED], 0, FALSE);
+	g_variant_get (parameters,
+	               "(&s&s&s)",
+	               NULL,
+	               NULL,
+	               &new_owner);
+
+	name_owner_changed (self, new_owner);
 }
 
 static void
-_proxy_new_cb (GObject *source_object,
-               GAsyncResult *result,
-               gpointer user_data)
+get_name_owner_cb (const char *name_owner,
+                   GError *error,
+                   gpointer user_data)
 {
 	NMFirewallManager *self;
 	NMFirewallManagerPrivate *priv;
-	GDBusProxy *proxy;
-	gs_free_error GError *error = NULL;
-	CBInfo *info;
-	CList *iter;
 
-	proxy = g_dbus_proxy_new_for_bus_finish (result, &error);
-	if (   !proxy
+	if (   !name_owner
 	    && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
 		return;
 
 	self = user_data;
 	priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
-	g_clear_object (&priv->proxy_cancellable);
 
-	if (!proxy) {
-		_LOGW (NULL, "could not connect to system D-Bus (%s)", error->message);
-		return;
-	}
+	g_clear_object (&priv->get_name_owner_cancellable);
 
-	priv->proxy = proxy;
-	g_signal_connect (priv->proxy, "notify::g-name-owner",
-	                  G_CALLBACK (name_owner_changed_cb), self);
-
-	if (!name_owner_changed (self))
-		_LOGD (NULL, "firewall %s", "initialized (not running)");
-
-again:
-	c_list_for_each (iter, &priv->pending_calls) {
-		info = c_list_entry (iter, CBInfo, lst);
-
-		if (info->mode != CB_INFO_MODE_DBUS_WAITING)
-			continue;
-		if (priv->running) {
-			_LOGD (info, "make D-Bus call");
-			_handle_dbus_start (self, info);
-		} else {
-			_LOGD (info, "complete: fake success");
-			c_list_unlink (&info->lst);
-			_cb_info_callback (info, NULL);
-			_cb_info_free (info);
-			goto again;
-		}
-	}
-
-	/* we always emit a state-changed signal, even if the
-	 * "running" property is still false. */
-	g_signal_emit (self, signals[STATE_CHANGED], 0, TRUE);
+	name_owner_changed (self, name_owner);
 }
 
 /*****************************************************************************/
 
 static void
-nm_firewall_manager_init (NMFirewallManager * self)
+nm_firewall_manager_init (NMFirewallManager *self)
 {
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
 
 	c_list_init (&priv->pending_calls);
-}
-
-static void
-constructed (GObject *object)
-{
-	NMFirewallManager *self = (NMFirewallManager *) object;
-	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
 
-	priv->proxy_cancellable = g_cancellable_new ();
+	priv->dbus_connection = nm_g_object_ref (NM_MAIN_DBUS_CONNECTION_GET);
 
-	g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,
-	                            G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES
-	                          | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
-	                          NULL,
-	                          FIREWALL_DBUS_SERVICE,
-	                          FIREWALL_DBUS_PATH,
-	                          FIREWALL_DBUS_INTERFACE_ZONE,
-	                          priv->proxy_cancellable,
-	                          _proxy_new_cb,
-	                          self);
+	if (!priv->dbus_connection) {
+		_LOGD (NULL, "no D-Bus connection");
+		return;
+	}
 
-	G_OBJECT_CLASS (nm_firewall_manager_parent_class)->constructed (object);
+	priv->name_owner_changed_id = nm_dbus_connection_signal_subscribe_name_owner_changed (priv->dbus_connection,
+	                                                                                      FIREWALL_DBUS_SERVICE,
+	                                                                                      name_owner_changed_cb,
+	                                                                                      self,
+	                                                                                      NULL);
+
+	priv->get_name_owner_cancellable = g_cancellable_new ();
+	nm_dbus_connection_call_get_name_owner (priv->dbus_connection,
+	                                        FIREWALL_DBUS_SERVICE,
+	                                        -1,
+	                                        priv->get_name_owner_cancellable,
+	                                        get_name_owner_cb,
+	                                        self);
 }
 
 static void
@@ -578,10 +623,14 @@ dispose (GObject *object)
 	 * we don't expect pending operations at this point. */
 	nm_assert (c_list_is_empty (&priv->pending_calls));
 
-	nm_clear_g_cancellable (&priv->proxy_cancellable);
-	g_clear_object (&priv->proxy);
+	nm_clear_g_dbus_connection_signal (priv->dbus_connection,
+	                                   &priv->name_owner_changed_id);
+
+	nm_clear_g_cancellable (&priv->get_name_owner_cancellable);
 
 	G_OBJECT_CLASS (nm_firewall_manager_parent_class)->dispose (object);
+
+	g_clear_object (&priv->dbus_connection);
 }
 
 static void
@@ -589,7 +638,6 @@ nm_firewall_manager_class_init (NMFirewallManagerClass *klass)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
-	object_class->constructed = constructed;
 	object_class->dispose = dispose;
 
 	signals[STATE_CHANGED] =
diff --git a/src/nm-firewall-manager.h b/src/nm-firewall-manager.h
index ceca58bea6..e7d10338bc 100644
--- a/src/nm-firewall-manager.h
+++ b/src/nm-firewall-manager.h
@@ -20,11 +20,6 @@
 #ifndef __NETWORKMANAGER_FIREWALL_MANAGER_H__
 #define __NETWORKMANAGER_FIREWALL_MANAGER_H__
 
-#define FIREWALL_DBUS_SERVICE         "org.fedoraproject.FirewallD1"
-#define FIREWALL_DBUS_PATH            "/org/fedoraproject/FirewallD1"
-#define FIREWALL_DBUS_INTERFACE       "org.fedoraproject.FirewallD1"
-#define FIREWALL_DBUS_INTERFACE_ZONE  "org.fedoraproject.FirewallD1.zone"
-
 #define NM_TYPE_FIREWALL_MANAGER                (nm_firewall_manager_get_type ())
 #define NM_FIREWALL_MANAGER(obj)                (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_FIREWALL_MANAGER, NMFirewallManager))
 #define NM_FIREWALL_MANAGER_CLASS(klass)        (G_TYPE_CHECK_CLASS_CAST ((klass),  NM_TYPE_FIREWALL_MANAGER, NMFirewallManagerClass))
@@ -34,7 +29,7 @@
 
 #define NM_FIREWALL_MANAGER_STATE_CHANGED "state-changed"
 
-typedef struct _NMFirewallManagerCallId *NMFirewallManagerCallId;
+typedef struct _NMFirewallManagerCallId NMFirewallManagerCallId;
 
 typedef struct _NMFirewallManager NMFirewallManager;
 typedef struct _NMFirewallManagerClass NMFirewallManagerClass;
@@ -46,22 +41,22 @@ NMFirewallManager *nm_firewall_manager_get (void);
 gboolean nm_firewall_manager_get_running (NMFirewallManager *self);
 
 typedef void (*NMFirewallManagerAddRemoveCallback) (NMFirewallManager *self,
-                                                    NMFirewallManagerCallId call_id,
+                                                    NMFirewallManagerCallId *call_id,
                                                     GError *error,
                                                     gpointer user_data);
 
-NMFirewallManagerCallId nm_firewall_manager_add_or_change_zone (NMFirewallManager *mgr,
-                                                                const char *iface,
-                                                                const char *zone,
-                                                                gboolean add,
-                                                                NMFirewallManagerAddRemoveCallback callback,
-                                                                gpointer user_data);
-NMFirewallManagerCallId nm_firewall_manager_remove_from_zone (NMFirewallManager *mgr,
-                                                              const char *iface,
-                                                              const char *zone,
-                                                              NMFirewallManagerAddRemoveCallback callback,
-                                                              gpointer user_data);
-
-void nm_firewall_manager_cancel_call (NMFirewallManagerCallId fw_call);
+NMFirewallManagerCallId *nm_firewall_manager_add_or_change_zone (NMFirewallManager *mgr,
+                                                                 const char *iface,
+                                                                 const char *zone,
+                                                                 gboolean add,
+                                                                 NMFirewallManagerAddRemoveCallback callback,
+                                                                 gpointer user_data);
+NMFirewallManagerCallId *nm_firewall_manager_remove_from_zone (NMFirewallManager *mgr,
+                                                               const char *iface,
+                                                               const char *zone,
+                                                               NMFirewallManagerAddRemoveCallback callback,
+                                                               gpointer user_data);
+
+void nm_firewall_manager_cancel_call (NMFirewallManagerCallId *call_id);
 
 #endif /* __NETWORKMANAGER_FIREWALL_MANAGER_H__ */
diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c
index 598af04e4b..045648a2de 100644
--- a/src/vpn/nm-vpn-connection.c
+++ b/src/vpn/nm-vpn-connection.c
@@ -111,8 +111,7 @@ typedef struct {
 	NMVpnPluginInfo *plugin_info;
 	char *bus_name;
 
-	/* Firewall */
-	NMFirewallManagerCallId fw_call;
+	NMFirewallManagerCallId *fw_call;
 
 	NMNetns *netns;
 
@@ -1195,7 +1194,7 @@ _cleanup_failed_config (NMVpnConnection *self)
 
 static void
 fw_change_zone_cb (NMFirewallManager *firewall_manager,
-                   NMFirewallManagerCallId call_id,
+                   NMFirewallManagerCallId *call_id,
                    GError *error,
                    gpointer user_data)
 {