From da38fa32f0c438347cfeb41a5735653d7deb5bc6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Apr 2017 18:17:42 +0200 Subject: [PATCH 1/5] firewall: create firewall D-Bus proxy asynchronously Creating it asynchronously changes that on the first call to nm_firewall_manager_get() the instance is not yet running. Note that NMPolicy already connects to the "STARTED" signal and reapplies the zones when firewalld appears. So, this delayed change of the running state is handled mostly fine already. One part is still missing, it's to queue add_or_change/remove calls while the firewall manager is initializing. That follows next. (cherry picked from commit 753f39fa826405d4f50793647899059303b0f0cf) --- src/nm-firewall-manager.c | 90 +++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 27 deletions(-) diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c index 05e5a3f833..cf3d7d79bc 100644 --- a/src/nm-firewall-manager.c +++ b/src/nm-firewall-manager.c @@ -40,10 +40,11 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { - GDBusProxy * proxy; - gboolean running; + GDBusProxy *proxy; + GCancellable *proxy_cancellable; GHashTable *pending_calls; + bool running; } NMFirewallManagerPrivate; struct _NMFirewallManager { @@ -417,14 +418,12 @@ set_running (NMFirewallManager *self, gboolean now_running) } static void -name_owner_changed (GObject *object, - GParamSpec *pspec, - gpointer user_data) +name_owner_changed (NMFirewallManager *self) { - NMFirewallManager *self = NM_FIREWALL_MANAGER (user_data); + NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self); gs_free char *owner = NULL; - owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object)); + owner = g_dbus_proxy_get_name_owner (priv->proxy); if (owner) { _LOGD (NULL, "firewall started"); set_running (self, TRUE); @@ -435,6 +434,49 @@ name_owner_changed (GObject *object, } } +static void +name_owner_changed_cb (GObject *object, + GParamSpec *pspec, + gpointer user_data) +{ + nm_assert (NM_IS_FIREWALL_MANAGER (user_data)); + nm_assert (G_IS_DBUS_PROXY (object)); + nm_assert (NM_FIREWALL_MANAGER_GET_PRIVATE (NM_FIREWALL_MANAGER (user_data))->proxy == G_DBUS_PROXY (object)); + + name_owner_changed (NM_FIREWALL_MANAGER (user_data)); +} + +static void +_proxy_new_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) +{ + NMFirewallManager *self; + NMFirewallManagerPrivate *priv; + GDBusProxy *proxy; + gs_free_error GError *error = NULL; + + 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); + + name_owner_changed (self); +} + /*****************************************************************************/ static void @@ -465,28 +507,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 +538,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); From 0a668deb78a8cdab71263e5430ce9ab149aa2f50 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Apr 2017 18:30:01 +0200 Subject: [PATCH 2/5] firewall: merge "started" signal and "available" property The GObject property NM_FIREWALL_MANAGER_AVAILABLE is basically unused. Drop it. (cherry picked from commit db576b848ab029b9a232fe9fda2f816660c6a9b8) --- src/nm-firewall-manager.c | 72 ++++++++++++--------------------------- src/nm-firewall-manager.h | 6 ++-- src/nm-policy.c | 17 +++++---- 3 files changed, 33 insertions(+), 62 deletions(-) diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c index cf3d7d79bc..1d87a21efc 100644 --- a/src/nm-firewall-manager.c +++ b/src/nm-firewall-manager.c @@ -28,12 +28,8 @@ /*****************************************************************************/ -NM_GOBJECT_PROPERTIES_DEFINE (NMFirewallManager, - PROP_AVAILABLE, -); - enum { - STARTED, + STATE_CHANGED, LAST_SIGNAL }; @@ -141,6 +137,16 @@ _ops_type_to_string (CBInfoOpsType ops_type) /*****************************************************************************/ +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; +} + +/*****************************************************************************/ + static gboolean _cb_info_is_idle (CBInfo *info) { @@ -406,32 +412,22 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call) /*****************************************************************************/ -static void -set_running (NMFirewallManager *self, gboolean now_running) -{ - NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self); - gboolean old_running = priv->running; - - priv->running = now_running; - if (old_running != priv->running) - _notify (self, PROP_AVAILABLE); -} - static void name_owner_changed (NMFirewallManager *self) { NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self); gs_free char *owner = NULL; + gboolean now_running; owner = g_dbus_proxy_get_name_owner (priv->proxy); - if (owner) { - _LOGD (NULL, "firewall started"); - set_running (self, TRUE); - g_signal_emit (self, signals[STARTED], 0); - } else { - _LOGD (NULL, "firewall stopped"); - set_running (self, FALSE); - } + now_running = !!owner; + + if (now_running == priv->running) + return; + + priv->running = now_running; + _LOGD (NULL, "firewall %s", now_running ? "started" : "stopped"); + g_signal_emit (self, signals[STATE_CHANGED], 0); } static void @@ -479,21 +475,6 @@ _proxy_new_cb (GObject *source_object, /*****************************************************************************/ -static void -get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) -{ - switch (prop_id) { - case PROP_AVAILABLE: - g_value_set_boolean (value, NM_FIREWALL_MANAGER_GET_PRIVATE ((NMFirewallManager *) object)->running); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - -/*****************************************************************************/ - static void nm_firewall_manager_init (NMFirewallManager * self) { @@ -550,19 +531,10 @@ 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] = - g_param_spec_boolean (NM_FIREWALL_MANAGER_AVAILABLE, "", "", - 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, + signals[STATE_CHANGED] = + g_signal_new (NM_FIREWALL_MANAGER_STATE_CHANGED, G_OBJECT_CLASS_TYPE (object_class), G_SIGNAL_RUN_FIRST, 0, diff --git a/src/nm-firewall-manager.h b/src/nm-firewall-manager.h index a00ee451e7..8bbf82a7a7 100644 --- a/src/nm-firewall-manager.h +++ b/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_STARTED "started" +#define NM_FIREWALL_MANAGER_STATE_CHANGED "state-changed" 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, diff --git a/src/nm-policy.c b/src/nm-policy.c index 7ae50e7621..a480bf2cbb 100644 --- a/src/nm-policy.c +++ b/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,16 @@ connection_added (NMSettings *settings, } static void -firewall_started (NMFirewallManager *manager, - gpointer user_data) +firewall_state_changed (NMFirewallManager *manager, + gpointer user_data) { NMPolicy *self = (NMPolicy *) user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); const GSList *iter; + 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 +2329,8 @@ constructed (GObject *object) } priv->firewall_manager = g_object_ref (nm_firewall_manager_get ()); - - priv->fw_started_id = g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STARTED, - G_CALLBACK (firewall_started), self); + g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STATE_CHANGED, + G_CALLBACK (firewall_state_changed), 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 +2389,7 @@ dispose (GObject *object) priv->pending_secondaries = NULL; if (priv->firewall_manager) { - g_assert (priv->fw_started_id); - nm_clear_g_signal_handler (priv->firewall_manager, &priv->fw_started_id); + g_signal_handlers_disconnect_by_func (priv->firewall_manager, firewall_state_changed, self); g_clear_object (&priv->firewall_manager); } From 2d8b6118ce83b20dbac0868ba45c025825d3d901 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Apr 2017 19:00:22 +0200 Subject: [PATCH 3/5] firewall: factor out D-Bus call from _start_request() Will be used in the next commit. (cherry picked from commit d8bf05d3e695f043eeb0fac4646fc6babad1bee3) --- src/nm-firewall-manager.c | 72 +++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c index 1d87a21efc..3e57e3109a 100644 --- a/src/nm-firewall-manager.c +++ b/src/nm-firewall-manager.c @@ -85,6 +85,7 @@ struct _NMFirewallManagerCallId { union { struct { GCancellable *cancellable; + GVariant *arg; } dbus; struct { guint id; @@ -157,6 +158,7 @@ static CBInfo * _cb_info_create (NMFirewallManager *self, CBInfoOpsType ops_type, const char *iface, + const char *zone, NMFirewallManagerAddRemoveCallback callback, gpointer user_data) { @@ -173,6 +175,7 @@ _cb_info_create (NMFirewallManager *self, if (priv->running) { info->mode = CB_INFO_MODE_DBUS; info->dbus.cancellable = g_cancellable_new (); + info->dbus.arg = g_variant_new ("(ss)", zone ? zone : "", iface); } else info->mode = CB_INFO_MODE_IDLE; @@ -185,8 +188,11 @@ _cb_info_create (NMFirewallManager *self, static void _cb_info_free (CBInfo *info) { - if (!_cb_info_is_idle (info)) + if (!_cb_info_is_idle (info)) { + if (info->dbus.arg) + g_variant_unref (info->dbus.arg); g_object_unref (info->dbus.cancellable); + } g_free (info->iface); if (info->self) g_object_unref (info->self); @@ -275,6 +281,43 @@ _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; + + 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)); + + 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, @@ -285,14 +328,13 @@ _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), @@ -301,29 +343,7 @@ _start_request (NMFirewallManager *self, _cb_info_is_idle (info) ? " (not running, simulate success)" : ""); if (!_cb_info_is_idle (info)) { - - 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); - + _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, From 3489b07e59dc6300528900650c8df303971a62b0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Apr 2017 19:07:49 +0200 Subject: [PATCH 4/5] firewall: drop _cb_info_is_idle() Next we will get another mode, so an is-idle doesn't cut it. It can be confusing where the mode is set and where it is only accessed read-only. For that, add mode_mutable. (cherry picked from commit 04f4e327a9be606de5de009e0b5e1cc88abb8d6a) --- src/nm-firewall-manager.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c index 3e57e3109a..fd64e46124 100644 --- a/src/nm-firewall-manager.c +++ b/src/nm-firewall-manager.c @@ -77,7 +77,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; @@ -127,7 +130,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; \ }) \ @@ -148,12 +151,6 @@ nm_firewall_manager_get_running (NMFirewallManager *self) /*****************************************************************************/ -static gboolean -_cb_info_is_idle (CBInfo *info) -{ - return info->mode == CB_INFO_MODE_IDLE; -} - static CBInfo * _cb_info_create (NMFirewallManager *self, CBInfoOpsType ops_type, @@ -173,11 +170,11 @@ _cb_info_create (NMFirewallManager *self, info->user_data = user_data; if (priv->running) { - info->mode = CB_INFO_MODE_DBUS; + info->mode_mutable = CB_INFO_MODE_DBUS; 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); @@ -188,7 +185,7 @@ _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) { if (info->dbus.arg) g_variant_unref (info->dbus.arg); g_object_unref (info->dbus.cancellable); @@ -340,9 +337,9 @@ _start_request (NMFirewallManager *self, _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)) { + if (info->mode != CB_INFO_MODE_IDLE) { _handle_dbus_start (self, info); if (!info->callback) { /* if the user did not provide a callback, the call_id is useless. @@ -420,11 +417,11 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call) _cb_info_callback (info, error); - if (_cb_info_is_idle (info)) { + 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); } From b504c6de24c31a01ef316bd42b0583cd8685ade3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Apr 2017 18:52:06 +0200 Subject: [PATCH 5/5] firewall: queue operations while NMFirewallManager instance is initializing We now initialize the NMFirewallManager asynchronously. That means, at first firewalld appears as "not running", for which we usually would fake-success right away. It would be complex for callers to wait for firewall-manager to be ready. So instead, have the asynchronous requests be queued and complete them once the D-Bus proxy is initialized. (cherry picked from commit fb7815df6e691c6e22769a4703204c010836fca9) --- src/nm-firewall-manager.c | 99 ++++++++++++++++++++++++++++----------- src/nm-policy.c | 8 ++++ 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/nm-firewall-manager.c b/src/nm-firewall-manager.c index fd64e46124..045d5abc4c 100644 --- a/src/nm-firewall-manager.c +++ b/src/nm-firewall-manager.c @@ -70,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; @@ -169,15 +170,14 @@ _cb_info_create (NMFirewallManager *self, info->callback = callback; info->user_data = user_data; - if (priv->running) { - info->mode_mutable = CB_INFO_MODE_DBUS; - info->dbus.cancellable = g_cancellable_new (); + if (priv->running || priv->proxy_cancellable) { + info->mode_mutable = CB_INFO_MODE_DBUS_WAITING; info->dbus.arg = g_variant_new ("(ss)", zone ? zone : "", iface); } else 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; } @@ -188,7 +188,7 @@ _cb_info_free (CBInfo *info) if (info->mode != CB_INFO_MODE_IDLE) { if (info->dbus.arg) g_variant_unref (info->dbus.arg); - g_object_unref (info->dbus.cancellable); + g_clear_object (&info->dbus.cancellable); } g_free (info->iface); if (info->self) @@ -286,6 +286,10 @@ _handle_dbus_start (NMFirewallManager *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"; @@ -306,6 +310,9 @@ _handle_dbus_start (NMFirewallManager *self, 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, @@ -337,10 +344,15 @@ _start_request (NMFirewallManager *self, _ops_type_to_string (info->ops_type), iface, NM_PRINT_FMT_QUOTED (zone, "\"", zone, "\"", "default"), - info->mode == CB_INFO_MODE_IDLE ? " (not running, simulate success)" : ""); + info->mode == CB_INFO_MODE_IDLE + ? " (not running, simulate success)" + : (!priv->running + ? " (waiting to initialize)" + : "")); - if (info->mode != CB_INFO_MODE_IDLE) { - _handle_dbus_start (self, 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, @@ -350,15 +362,18 @@ _start_request (NMFirewallManager *self, * (the request will always be started). */ return NULL; } - } else if (!info->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 (info, "complete: drop request simulating success"); - _cb_info_complete_normal (info, NULL); - return NULL; + } else if (info->mode == CB_INFO_MODE_IDLE) { + if (!info->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 (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 - info->idle.id = g_idle_add (_handle_idle, info); + nm_assert_not_reached (); return info; } @@ -417,7 +432,9 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call) _cb_info_callback (info, error); - if (info->mode == CB_INFO_MODE_IDLE) { + 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 { @@ -429,7 +446,7 @@ nm_firewall_manager_cancel_call (NMFirewallManagerCallId call) /*****************************************************************************/ -static void +static gboolean name_owner_changed (NMFirewallManager *self) { NMFirewallManagerPrivate *priv = NM_FIREWALL_MANAGER_GET_PRIVATE (self); @@ -440,11 +457,11 @@ name_owner_changed (NMFirewallManager *self) now_running = !!owner; if (now_running == priv->running) - return; + return FALSE; priv->running = now_running; _LOGD (NULL, "firewall %s", now_running ? "started" : "stopped"); - g_signal_emit (self, signals[STATE_CHANGED], 0); + return TRUE; } static void @@ -452,11 +469,14 @@ name_owner_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) { - nm_assert (NM_IS_FIREWALL_MANAGER (user_data)); - nm_assert (G_IS_DBUS_PROXY (object)); - nm_assert (NM_FIREWALL_MANAGER_GET_PRIVATE (NM_FIREWALL_MANAGER (user_data))->proxy == G_DBUS_PROXY (object)); + NMFirewallManager *self = user_data; - name_owner_changed (NM_FIREWALL_MANAGER (user_data)); + 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 (name_owner_changed (self)) + g_signal_emit (self, signals[STATE_CHANGED], 0, FALSE); } static void @@ -468,6 +488,8 @@ _proxy_new_cb (GObject *source_object, NMFirewallManagerPrivate *priv; GDBusProxy *proxy; gs_free_error GError *error = NULL; + GHashTableIter iter; + CBInfo *info; proxy = g_dbus_proxy_new_for_bus_finish (result, &error); if ( !proxy @@ -487,7 +509,29 @@ _proxy_new_cb (GObject *source_object, g_signal_connect (priv->proxy, "notify::g-name-owner", G_CALLBACK (name_owner_changed_cb), self); - name_owner_changed (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); } /*****************************************************************************/ @@ -556,6 +600,7 @@ nm_firewall_manager_class_init (NMFirewallManagerClass *klass) G_SIGNAL_RUN_FIRST, 0, NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); + g_cclosure_marshal_VOID__BOOLEAN, + G_TYPE_NONE, 1, + G_TYPE_BOOLEAN /* initialized_now */); } diff --git a/src/nm-policy.c b/src/nm-policy.c index a480bf2cbb..f82260f97f 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2057,12 +2057,20 @@ connection_added (NMSettings *settings, static void firewall_state_changed (NMFirewallManager *manager, + 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;