firewall: merge branch 'th/firewall-async'

(cherry picked from commit ec3a9c0607)

src/nm-firewall-manager.c

@@ -28,22 +28,19 @@
 
 /*****************************************************************************/
 
-NM_GOBJECT_PROPERTIES_DEFINE (NMFirewallManager,
-	PROP_AVAILABLE,
-);
-
 enum {
-	STARTED,
+	STATE_CHANGED,
 	LAST_SIGNAL
 };
 
 static guint signals[LAST_SIGNAL] = { 0 };
 
 typedef struct {
-	GDBusProxy *    proxy;
+	GDBusProxy     *proxy;
-	gboolean        running;
+	GCancellable   *proxy_cancellable;
 
 	GHashTable     *pending_calls;
+	bool            running;
 } NMFirewallManagerPrivate;
 
 struct _NMFirewallManager {
@@ -73,6 +70,7 @@ typedef enum {
 
 typedef enum {
 	CB_INFO_MODE_IDLE = 1,
+	CB_INFO_MODE_DBUS_WAITING,
 	CB_INFO_MODE_DBUS,
 	CB_INFO_MODE_DBUS_COMPLETED,
 } CBInfoMode;
@@ -80,7 +78,10 @@ typedef enum {
 struct _NMFirewallManagerCallId {
 	NMFirewallManager *self;
 	CBInfoOpsType ops_type;
-	CBInfoMode mode;
+	union {
+		const CBInfoMode mode;
+		CBInfoMode mode_mutable;
+	};
 	char *iface;
 	NMFirewallManagerAddRemoveCallback callback;
 	gpointer user_data;
@@ -88,6 +89,7 @@ struct _NMFirewallManagerCallId {
 	union {
 		struct {
 			GCancellable *cancellable;
+			GVariant *arg;
 		} dbus;
 		struct {
 			guint id;
@@ -129,7 +131,7 @@ _ops_type_to_string (CBInfoOpsType ops_type)
                      __info \
                         ? ({ \
                                 g_snprintf (__prefix_info, sizeof (__prefix_info), "[%p,%s%s:%s%s%s]: ", __info, \
-                                            _ops_type_to_string (__info->ops_type), _cb_info_is_idle (__info) ? "*" : "", \
+                                            _ops_type_to_string (__info->ops_type), __info->mode == CB_INFO_MODE_IDLE ? "*" : "", \
                                             NM_PRINT_FMT_QUOTE_STRING (__info->iface)); \
                                 __prefix_info; \
                            }) \
@@ -140,16 +142,21 @@ _ops_type_to_string (CBInfoOpsType ops_type)
 
 /*****************************************************************************/
 
-static gboolean
+gboolean
-_cb_info_is_idle (CBInfo *info)
+nm_firewall_manager_get_running (NMFirewallManager *self)
 {
-	return info->mode == CB_INFO_MODE_IDLE;
+	g_return_val_if_fail (NM_IS_FIREWALL_MANAGER (self), FALSE);
+
+	return NM_FIREWALL_MANAGER_GET_PRIVATE (self)->running;
 }
 
+/*****************************************************************************/
+
 static CBInfo *
 _cb_info_create (NMFirewallManager *self,
                  CBInfoOpsType ops_type,
                  const char *iface,
+                 const char *zone,
                  NMFirewallManagerAddRemoveCallback callback,
                  gpointer user_data)
 {
@@ -163,14 +170,14 @@ _cb_info_create (NMFirewallManager *self,
 	info->callback = callback;
 	info->user_data = user_data;
 
-	if (priv->running) {
+	if (priv->running || priv->proxy_cancellable) {
-		info->mode = CB_INFO_MODE_DBUS;
+		info->mode_mutable = CB_INFO_MODE_DBUS_WAITING;
-		info->dbus.cancellable = g_cancellable_new ();
+		info->dbus.arg = g_variant_new ("(ss)", zone ? zone : "", iface);
 	} else
-		info->mode = CB_INFO_MODE_IDLE;
+		info->mode_mutable = CB_INFO_MODE_IDLE;
 
 	if (!nm_g_hash_table_add (priv->pending_calls, info))
-		g_return_val_if_reached (NULL);
+		nm_assert_not_reached ();
 
 	return info;
 }
@@ -178,8 +185,11 @@ _cb_info_create (NMFirewallManager *self,
 static void
 _cb_info_free (CBInfo *info)
 {
-	if (!_cb_info_is_idle (info))
+	if (info->mode != CB_INFO_MODE_IDLE) {
-		g_object_unref (info->dbus.cancellable);
+		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);
@@ -268,6 +278,50 @@ _handle_dbus (GObject *proxy, GAsyncResult *result, gpointer user_data)
 	_cb_info_complete_normal (info, error);
 }
 
+static void
+_handle_dbus_start (NMFirewallManager *self,
+                    CBInfo *info)
+{
+	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
+	const char *dbus_method;
+	GVariant *arg;
+
+	nm_assert (info);
+	nm_assert (priv->running);
+	nm_assert (info->mode == CB_INFO_MODE_DBUS_WAITING);
+
+	switch (info->ops_type) {
+	case CB_INFO_OPS_ADD:
+		dbus_method = "addInterface";
+		break;
+	case CB_INFO_OPS_CHANGE:
+		dbus_method = "changeZone";
+		break;
+	case CB_INFO_OPS_REMOVE:
+		dbus_method = "removeInterface";
+		break;
+	default:
+		nm_assert_not_reached ();
+		break;
+	}
+
+	arg = info->dbus.arg;
+	info->dbus.arg = NULL;
+
+	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);
+}
+
 static NMFirewallManagerCallId
 _start_request (NMFirewallManager *self,
                 CBInfoOpsType ops_type,
@@ -278,45 +332,27 @@ _start_request (NMFirewallManager *self,
 {
 	NMFirewallManagerPrivate *priv;
 	CBInfo *info;
-	const char *dbus_method;
 
 	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, callback, user_data);
+	info = _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),
 	       iface,
 	       NM_PRINT_FMT_QUOTED (zone, "\"", zone, "\"", "default"),
-	       _cb_info_is_idle (info) ? " (not running, simulate success)" : "");
+	       info->mode == CB_INFO_MODE_IDLE
-
+	         ? " (not running, simulate success)"
-	if (!_cb_info_is_idle (info)) {
+	         : (!priv->running
-
+	              ? " (waiting to initialize)"
-		switch (ops_type) {
+	              : ""));
-		case CB_INFO_OPS_ADD:
-			dbus_method = "addInterface";
-			break;
-		case CB_INFO_OPS_CHANGE:
-			dbus_method = "changeZone";
-			break;
-		case CB_INFO_OPS_REMOVE:
-			dbus_method = "removeInterface";
-			break;
-		default:
-			g_assert_not_reached ();
-		}
-
-		g_dbus_proxy_call (priv->proxy,
-		                   dbus_method,
-		                   g_variant_new ("(ss)", zone ? zone : "", iface),
-		                   G_DBUS_CALL_FLAGS_NONE, 10000,
-		                   info->dbus.cancellable,
-		                   _handle_dbus,
-		                   info);
 
+	if (info->mode == CB_INFO_MODE_DBUS_WAITING) {
+		if (priv->running)
+			_handle_dbus_start (self, info);
 		if (!info->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,
@@ -326,15 +362,18 @@ _start_request (NMFirewallManager *self,
 			 * (the request will always be started). */
 			return NULL;
 		}
-	} else if (!info->callback) {
+	} else if (info->mode == CB_INFO_MODE_IDLE) {
-		/* if the user did not provide a callback and firewalld is not running,
+		if (!info->callback) {
-		 * there is no point in scheduling an idle-request to fake success. Just
+			/* if the user did not provide a callback and firewalld is not running,
-		 * return right away. */
+			 * there is no point in scheduling an idle-request to fake success. Just
-		_LOGD (info, "complete: drop request simulating success");
+			 * return right away. */
-		_cb_info_complete_normal (info, NULL);
+			_LOGD (info, "complete: drop request simulating success");
-		return NULL;
+			_cb_info_complete_normal (info, NULL);
+			return NULL;
+		} else
+			info->idle.id = g_idle_add (_handle_idle, info);
 	} else
-		info->idle.id = g_idle_add (_handle_idle, info);
+		nm_assert_not_reached ();
 
 	return info;
 }
@@ -393,11 +432,13 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call)
 
 	_cb_info_callback (info, error);
 
-	if (_cb_info_is_idle (info)) {
+	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 = CB_INFO_MODE_DBUS_COMPLETED;
+		info->mode_mutable = CB_INFO_MODE_DBUS_COMPLETED;
 		g_cancellable_cancel (info->dbus.cancellable);
 		g_clear_object (&info->self);
 	}
@@ -405,49 +446,92 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call)
 
 /*****************************************************************************/
 
-static void
+static gboolean
-set_running (NMFirewallManager *self, gboolean now_running)
+name_owner_changed (NMFirewallManager *self)
 {
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
-	gboolean old_running = priv->running;
+	gs_free char *owner = NULL;
+	gboolean now_running;
+
+	owner = g_dbus_proxy_get_name_owner (priv->proxy);
+	now_running = !!owner;
+
+	if (now_running == priv->running)
+		return FALSE;
 
 	priv->running = now_running;
-	if (old_running != priv->running)
+	_LOGD (NULL, "firewall %s", now_running ? "started" : "stopped");
-		_notify (self, PROP_AVAILABLE);
+	return TRUE;
 }
 
 static void
-name_owner_changed (GObject    *object,
+name_owner_changed_cb (GObject    *object,
-                    GParamSpec *pspec,
+                       GParamSpec *pspec,
-                    gpointer    user_data)
+                       gpointer    user_data)
 {
-	NMFirewallManager *self = NM_FIREWALL_MANAGER (user_data);
+	NMFirewallManager *self = user_data;
-	gs_free char *owner = NULL;
 
-	owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object));
+	nm_assert (NM_IS_FIREWALL_MANAGER (self));
-	if (owner) {
+	nm_assert (G_IS_DBUS_PROXY (object));
-		_LOGD (NULL, "firewall started");
+	nm_assert (NM_FIREWALL_MANAGER_GET_PRIVATE (self)->proxy == G_DBUS_PROXY (object));
-		set_running (self, TRUE);
+
-		g_signal_emit (self, signals[STARTED], 0);
+	if (name_owner_changed (self))
-	} else {
+		g_signal_emit (self, signals[STATE_CHANGED], 0, FALSE);
-		_LOGD (NULL, "firewall stopped");
-		set_running (self, FALSE);
-	}
 }
 
-/*****************************************************************************/
-
 static void
-get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
+_proxy_new_cb (GObject *source_object,
+               GAsyncResult *result,
+               gpointer user_data)
 {
-	switch (prop_id) {
+	NMFirewallManager *self;
-	case PROP_AVAILABLE:
+	NMFirewallManagerPrivate *priv;
-		g_value_set_boolean (value, NM_FIREWALL_MANAGER_GET_PRIVATE ((NMFirewallManager *) object)->running);
+	GDBusProxy *proxy;
-		break;
+	gs_free_error GError *error = NULL;
-	default:
+	GHashTableIter iter;
-		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+	CBInfo *info;
-		break;
+
+	proxy = g_dbus_proxy_new_for_bus_finish (result, &error);
+	if (   !proxy
+	    && 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;
 	}
+
+	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:
+	g_hash_table_iter_init (&iter, priv->pending_calls);
+	while (g_hash_table_iter_next (&iter, (gpointer *) &info, NULL)) {
+		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");
+			g_hash_table_iter_remove (&iter);
+			_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);
 }
 
 /*****************************************************************************/
@@ -465,28 +549,21 @@ constructed (GObject *object)
 {
 	NMFirewallManager *self = (NMFirewallManager *) object;
 	NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self);
-	gs_free char *owner = NULL;
+
-	gs_free_error GError *error = NULL;
+	priv->proxy_cancellable = g_cancellable_new ();
+
+	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);
 
 	G_OBJECT_CLASS (nm_firewall_manager_parent_class)->constructed (object);
-
-	priv->proxy = g_dbus_proxy_new_for_bus_sync (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,
-	                                             NULL, &error);
-	if (priv->proxy) {
-		g_signal_connect (priv->proxy, "notify::g-name-owner",
-		                  G_CALLBACK (name_owner_changed), self);
-		owner = g_dbus_proxy_get_name_owner (priv->proxy);
-		priv->running = (owner != NULL);
-	} else
-		_LOGW (NULL, "could not connect to system D-Bus (%s)", error->message);
-
-	_LOGD (NULL, "firewall constructed (%srunning)", priv->running ? "" : "not");
 }
 
 static void
@@ -503,6 +580,7 @@ dispose (GObject *object)
 		priv->pending_calls = NULL;
 	}
 
+	nm_clear_g_cancellable (&priv->proxy_cancellable);
 	g_clear_object (&priv->proxy);
 
 	G_OBJECT_CLASS (nm_firewall_manager_parent_class)->dispose (object);
@@ -514,23 +592,15 @@ nm_firewall_manager_class_init (NMFirewallManagerClass *klass)
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
 	object_class->constructed = constructed;
-	object_class->get_property = get_property;
 	object_class->dispose = dispose;
 
-	obj_properties[PROP_AVAILABLE] =
+	signals[STATE_CHANGED] =
-	     g_param_spec_boolean (NM_FIREWALL_MANAGER_AVAILABLE, "", "",
+	    g_signal_new (NM_FIREWALL_MANAGER_STATE_CHANGED,
-	                           FALSE,
-	                           G_PARAM_READABLE |
-	                           G_PARAM_STATIC_STRINGS);
-
-	g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties);
-
-	signals[STARTED] =
-	    g_signal_new (NM_FIREWALL_MANAGER_STARTED,
 	                  G_OBJECT_CLASS_TYPE (object_class),
 	                  G_SIGNAL_RUN_FIRST,
 	                  0,
 	                  NULL, NULL,
-	                  g_cclosure_marshal_VOID__VOID,
+	                  g_cclosure_marshal_VOID__BOOLEAN,
-	                  G_TYPE_NONE, 0);
+	                  G_TYPE_NONE, 1,
+	                  G_TYPE_BOOLEAN /* initialized_now */);
 }

src/nm-firewall-manager.h

@@ -33,9 +33,7 @@
 #define NM_IS_FIREWALL_MANAGER_CLASS(klass)     (G_TYPE_CHECK_CLASS_TYPE ((klass),  NM_TYPE_FIREWALL_MANAGER))
 #define NM_FIREWALL_MANAGER_GET_CLASS(obj)      (G_TYPE_INSTANCE_GET_CLASS ((obj),  NM_TYPE_FIREWALL_MANAGER, NMFirewallManagerClass))
 
-#define NM_FIREWALL_MANAGER_AVAILABLE "available"
+#define NM_FIREWALL_MANAGER_STATE_CHANGED "state-changed"
-
-#define NM_FIREWALL_MANAGER_STARTED "started"
 
 typedef struct _NMFirewallManagerCallId *NMFirewallManagerCallId;
 
@@ -46,6 +44,8 @@ GType nm_firewall_manager_get_type (void);
 
 NMFirewallManager *nm_firewall_manager_get (void);
 
+gboolean nm_firewall_manager_get_running (NMFirewallManager *self);
+
 typedef void (*NMFirewallManagerAddRemoveCallback) (NMFirewallManager *self,
                                                     NMFirewallManagerCallId call_id,
                                                     GError *error,

src/nm-policy.c

@@ -71,8 +71,6 @@ typedef struct {
 
 	GSList *pending_secondaries;
 
-	gulong fw_started_id;
-
 	NMSettings *settings;
 
 	NMDevice *default_device4, *activating_device4;
@@ -2058,13 +2056,24 @@ connection_added (NMSettings *settings,
 }
 
 static void
-firewall_started (NMFirewallManager *manager,
+firewall_state_changed (NMFirewallManager *manager,
-                  gpointer user_data)
+                        gboolean initialized_now,
+                        gpointer user_data)
 {
 	NMPolicy *self = (NMPolicy *) user_data;
 	NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self);
 	const GSList *iter;
 
+	if (initialized_now) {
+		/* the firewall manager was initializing, but all requests
+		 * so fare were queued and are already sent. No need to
+		 * re-update the firewall zone of the devices. */
+		return;
+	}
+
+	if (!nm_firewall_manager_get_running (manager))
+		return;
+
 	/* add interface of each device to correct zone */
 	for (iter = nm_manager_get_devices (priv->manager); iter; iter = g_slist_next (iter))
 		nm_device_update_firewall_zone (iter->data);
@@ -2328,9 +2337,8 @@ constructed (GObject *object)
 	}
 
 	priv->firewall_manager = g_object_ref (nm_firewall_manager_get ());
-
+	g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STATE_CHANGED,
-	priv->fw_started_id = g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STARTED,
+	                  G_CALLBACK (firewall_state_changed), self);
-	                                        G_CALLBACK (firewall_started), self);
 
 	priv->dns_manager = g_object_ref (nm_dns_manager_get ());
 	nm_dns_manager_set_initial_hostname (priv->dns_manager, priv->orig_hostname);
@@ -2389,8 +2397,7 @@ dispose (GObject *object)
 	priv->pending_secondaries = NULL;
 
 	if (priv->firewall_manager) {
-		g_assert (priv->fw_started_id);
+		g_signal_handlers_disconnect_by_func (priv->firewall_manager, firewall_state_changed, self);
-		nm_clear_g_signal_handler (priv->firewall_manager, &priv->fw_started_id);
 		g_clear_object (&priv->firewall_manager);
 	}