From c668d972ea67b86f7b2e6341f0492de04f0bfc3b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 15:47:24 +0100 Subject: [PATCH 01/21] policy: fix disconnecting notify:alive signal from active-connection Fixes: 37e8c53eeed579fe34a68819cd12f3295d581394 --- src/nm-policy.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nm-policy.c b/src/nm-policy.c index 5199afbf1c..4e47b8c902 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2249,6 +2249,9 @@ active_connection_removed (NMManager *manager, g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); + g_signal_handlers_disconnect_by_func (active, + active_connection_keep_alive_changed, + self); } /*****************************************************************************/ From 15033be1a3edd9e57d18ce65e0ab9c56c115bf9d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 11:28:33 +0100 Subject: [PATCH 02/21] keep-alive: add nm_keep_alive_disarm() to silence notifications once we disconnect The NMKeepAlive instance is useful to find out when we should disconnect. The moment when we start disconnecting, we don't care about it anymore. Add a nm_keep_alive_disarm() function to suppress property change events about the alive state, after that point. Emitting further events from that point on is only confusing. Yes, this means, a NMKeepAlive instance shall not be re-used for multiple purposes. Create a separate keep-alive instace for each target that should be guarded. Also, once disarmed, we can release all resources that the NMKeepAlive instance was holding until now. --- src/nm-active-connection.c | 6 ++++++ src/nm-keep-alive.c | 44 ++++++++++++++++++++++++++++++++++---- src/nm-keep-alive.h | 2 ++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index ebe911e477..da478e071e 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -264,6 +264,12 @@ nm_active_connection_set_state (NMActiveConnection *self, state_to_string (new_state), state_to_string (priv->state)); + if (new_state > NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + /* once we are about to deactivate, we don't need the keep-alive instance + * anymore. Freeze/disarm it. */ + nm_keep_alive_disarm (priv->keep_alive); + } + if ( new_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED && priv->activation_type == NM_ACTIVATION_TYPE_ASSUME) { /* assuming connections mean to gracefully take over an externally diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 6949682571..96d817a37c 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -42,6 +42,8 @@ typedef struct { guint subscription_id; bool floating:1; + bool disarmed:1; + bool forced:1; bool alive:1; bool dbus_client_confirmed:1; @@ -100,6 +102,11 @@ _notify_alive (NMKeepAlive *self) { NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + if (priv->disarmed) { + /* once disarmed, the alive state is frozen. */ + return; + } + if (priv->alive == _is_alive (self)) return; priv->alive = !priv->alive; @@ -163,7 +170,8 @@ _set_settings_connection_watch_visible (NMKeepAlive *self, old_connection = g_steal_pointer (&priv->connection); } - if (connection) { + if ( connection + && !priv->disarmed) { priv->connection = g_object_ref (connection); g_signal_connect (priv->connection, NM_SETTINGS_CONNECTION_FLAGS_CHANGED, @@ -300,7 +308,8 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, cleanup_dbus_watch (self); - if (client_address) { + if ( client_address + && !priv->disarmed) { _LOGD ("Registering dbus client watch for keep alive"); priv->dbus_client = g_strdup (client_address); @@ -323,6 +332,33 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, /*****************************************************************************/ +/** + * nm_keep_alive_disarm: + * @self: the #NMKeepAlive instance + * + * Once the instance is disarmed, it will not change its alive state + * anymore and will not emit anymore property changed signals about + * alive state changed. + * + * As such, it will also free internal resources (since they no longer + * affect the externally visible state). + * + * Once disarmed, the instance is frozen and cannot change anymore. + */ +void +nm_keep_alive_disarm (NMKeepAlive *self) +{ + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + priv->disarmed = TRUE; + + /* release internal data. */ + _set_settings_connection_watch_visible (self, NULL, FALSE); + cleanup_dbus_watch (self); +} + +/*****************************************************************************/ + static void get_property (GObject *object, guint prop_id, @@ -366,8 +402,8 @@ dispose (GObject *object) { NMKeepAlive *self = NM_KEEP_ALIVE (object); - _set_settings_connection_watch_visible (self, NULL, FALSE); - cleanup_dbus_watch (self); + /* disarm also happens to free all resources. */ + nm_keep_alive_disarm (self); } static void diff --git a/src/nm-keep-alive.h b/src/nm-keep-alive.h index b8c26e173d..341563b54c 100644 --- a/src/nm-keep-alive.h +++ b/src/nm-keep-alive.h @@ -42,6 +42,8 @@ gboolean nm_keep_alive_is_alive (NMKeepAlive *self); void nm_keep_alive_sink (NMKeepAlive *self); +void nm_keep_alive_disarm (NMKeepAlive *self); + void nm_keep_alive_set_forced (NMKeepAlive *self, gboolean forced); From a1e811b427e63b8db5be01300f3eb50ff72fd37c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 13:16:54 +0100 Subject: [PATCH 03/21] keep-alive: drop "floating" argument from nm_keep_alive_new() All callers only want to create floating instances at first. Also, it seems to make generally more sense this way: you create a floating instance, set it up, and then arm it. This simplifies nm_keep_alive_new(), which previously was adding additional code that wasn't accessible via plain g_object_new(). --- src/nm-active-connection.c | 2 +- src/nm-keep-alive.c | 17 ++++++++--------- src/nm-keep-alive.h | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index da478e071e..5852504a7b 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -1446,7 +1446,7 @@ nm_active_connection_init (NMActiveConnection *self) priv->activation_type = NM_ACTIVATION_TYPE_MANAGED; priv->version_id = _version_id_new (); - priv->keep_alive = nm_keep_alive_new (TRUE); + priv->keep_alive = nm_keep_alive_new (); g_signal_connect_object (priv->keep_alive, "notify::" NM_KEEP_ALIVE_ALIVE, (GCallback) keep_alive_alive_changed, self, diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 96d817a37c..320b9e5b98 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -382,19 +382,18 @@ get_property (GObject *object, static void nm_keep_alive_init (NMKeepAlive *self) { - nm_assert (NM_KEEP_ALIVE_GET_PRIVATE (self)->alive == _is_alive (self)); + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + priv->floating = TRUE; + priv->alive = TRUE; + + nm_assert (priv->alive == _is_alive (self)); } NMKeepAlive * -nm_keep_alive_new (gboolean floating) +nm_keep_alive_new (void) { - NMKeepAlive *self = g_object_new (NM_TYPE_KEEP_ALIVE, NULL); - NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); - - priv->floating = floating; - priv->alive = TRUE; - nm_assert (priv->alive == _is_alive (self)); - return self; + return g_object_new (NM_TYPE_KEEP_ALIVE, NULL); } static void diff --git a/src/nm-keep-alive.h b/src/nm-keep-alive.h index 341563b54c..ef5b9d4456 100644 --- a/src/nm-keep-alive.h +++ b/src/nm-keep-alive.h @@ -36,7 +36,7 @@ typedef struct _NMKeepAliveClass NMKeepAliveClass; GType nm_keep_alive_get_type (void) G_GNUC_CONST; -NMKeepAlive* nm_keep_alive_new (gboolean floating); +NMKeepAlive* nm_keep_alive_new (void); gboolean nm_keep_alive_is_alive (NMKeepAlive *self); From 7578e59ba973a9b51a216d562cc9d1cb83ea3a91 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 13:24:30 +0100 Subject: [PATCH 04/21] keep-alive: rename nm_keep_alive_sink() to nm_keep_alive_arm() The names "floating" and "sink()" are well known and good. However, "disarm()" seems the best name for the counterpart operation, better than "float()", "unsink()", or "freeze()". Since we have "nm_keep_alive_disarm()", for consitency rename - "floating" -> (not) "armed" - "sink()" -> "arm()" --- src/nm-active-connection.c | 4 ++-- src/nm-keep-alive.c | 48 ++++++++++++++++++++++++++------------ src/nm-keep-alive.h | 3 +-- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 5852504a7b..59f7622c77 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -1034,7 +1034,7 @@ nm_active_connection_bind_dbus_client (NMActiveConnection *self, GDBusConnection NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); nm_keep_alive_set_dbus_client_watch (priv->keep_alive, dbus_con, dbus_client); - nm_keep_alive_sink (priv->keep_alive); + nm_keep_alive_arm (priv->keep_alive); } /*****************************************************************************/ @@ -1487,7 +1487,7 @@ constructed (GObject *object) NM_ACTIVATION_REASON_AUTOCONNECT, NM_ACTIVATION_REASON_AUTOCONNECT_SLAVES)) { nm_keep_alive_set_settings_connection_watch_visible (priv->keep_alive, priv->settings_connection.obj); - nm_keep_alive_sink (priv->keep_alive); + nm_keep_alive_arm (priv->keep_alive); } } diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 320b9e5b98..0b3b7fabd7 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -41,7 +41,7 @@ typedef struct { GCancellable *dbus_client_confirm_cancellable; guint subscription_id; - bool floating:1; + bool armed:1; bool disarmed:1; bool forced:1; @@ -79,8 +79,14 @@ _is_alive (NMKeepAlive *self) { NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); - if ( priv->floating - || priv->forced) + nm_assert (!priv->disarmed); + + if (!priv->armed) { + /* before arming, the instance is always alive. */ + return TRUE; + } + + if (priv->forced) return TRUE; if ( priv->connection @@ -121,17 +127,6 @@ nm_keep_alive_is_alive (NMKeepAlive *self) /*****************************************************************************/ -void -nm_keep_alive_sink (NMKeepAlive *self) -{ - NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); - - if (priv->floating) { - priv->floating = FALSE; - _notify_alive (self); - } -} - void nm_keep_alive_set_forced (NMKeepAlive *self, gboolean forced) { @@ -332,6 +327,30 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, /*****************************************************************************/ +/** + * nm_keep_alive_arm: + * @self: the #NMKeepAlive + * + * A #NMKeepAlive instance is unarmed by default. That means, it's + * alive and stays alive until being armed. Arming means, that the conditions + * start to be actively evaluated, that the alive state may change, and + * that property changed signals are emitted. + * + * The opposite is nm_keep_alive_disarm() which freezes the alive state + * for good. Once disarmed, the instance cannot be armed again. Arming an + * instance multiple times has no effect. Arming an already disarmed instance + * also has no effect. */ +void +nm_keep_alive_arm (NMKeepAlive *self) +{ + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + if (!priv->armed) { + priv->armed = TRUE; + _notify_alive (self); + } +} + /** * nm_keep_alive_disarm: * @self: the #NMKeepAlive instance @@ -384,7 +403,6 @@ nm_keep_alive_init (NMKeepAlive *self) { NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); - priv->floating = TRUE; priv->alive = TRUE; nm_assert (priv->alive == _is_alive (self)); diff --git a/src/nm-keep-alive.h b/src/nm-keep-alive.h index ef5b9d4456..3a7cd66858 100644 --- a/src/nm-keep-alive.h +++ b/src/nm-keep-alive.h @@ -40,8 +40,7 @@ NMKeepAlive* nm_keep_alive_new (void); gboolean nm_keep_alive_is_alive (NMKeepAlive *self); -void nm_keep_alive_sink (NMKeepAlive *self); - +void nm_keep_alive_arm (NMKeepAlive *self); void nm_keep_alive_disarm (NMKeepAlive *self); void nm_keep_alive_set_forced (NMKeepAlive *self, From 461bf7aa0c3b9b13a50f8961d599b4dbd8b915b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:18:01 +0100 Subject: [PATCH 05/21] device: add state-change reason argument to nm_device_disconnect_active_connection() nm_device_disconnect_active_connection() is generally useful and a prefered form to fail an active connection. The device's state-change reason is important, so it needs to be injected. --- src/devices/nm-device.c | 5 +++-- src/devices/nm-device.h | 3 ++- src/nm-manager.c | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a6c2ea03e5..c0c174390c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11823,7 +11823,8 @@ _carrier_wait_check_act_request_must_queue (NMDevice *self, NMActRequest *req) } void -nm_device_disconnect_active_connection (NMActiveConnection *active) +nm_device_disconnect_active_connection (NMActiveConnection *active, + NMDeviceStateReason device_reason) { NMDevice *self; NMDevicePrivate *priv; @@ -11850,7 +11851,7 @@ nm_device_disconnect_active_connection (NMActiveConnection *active) if (priv->state < NM_DEVICE_STATE_DEACTIVATING) { nm_device_state_changed (self, NM_DEVICE_STATE_DEACTIVATING, - NM_DEVICE_STATE_REASON_NEW_ACTIVATION); + device_reason); } else { /* it's going down already... */ } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 828c22d94b..1bd95a83bd 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -769,7 +769,8 @@ void nm_device_queue_state (NMDevice *self, gboolean nm_device_get_firmware_missing (NMDevice *self); -void nm_device_disconnect_active_connection (NMActiveConnection *active); +void nm_device_disconnect_active_connection (NMActiveConnection *active, + NMDeviceStateReason device_reason); void nm_device_queue_activation (NMDevice *device, NMActRequest *req); diff --git a/src/nm-manager.c b/src/nm-manager.c index 9200b24fd1..d335cf9df2 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4549,7 +4549,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * for (i = 0; i < n_all; i++) { nm_device_disconnect_active_connection ( all_ac_arr ? all_ac_arr->pdata[i] - : ac); + : ac, + NM_DEVICE_STATE_REASON_NEW_ACTIVATION); } } } From fe5f5f7a0e1279b494d99cba19caf222aa89a9f9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:24:19 +0100 Subject: [PATCH 06/21] device: pass active-connection's state-change reason to _clear_queued_act_request() No change in behavior, yet. --- src/devices/nm-device.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c0c174390c..cf8de90bea 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11619,14 +11619,15 @@ disconnect_cb (NMDevice *self, } static void -_clear_queued_act_request (NMDevicePrivate *priv) +_clear_queued_act_request (NMDevicePrivate *priv, + NMActiveConnectionStateReason active_reason) { if (priv->queued_act_request) { gs_unref_object NMActRequest *ac = NULL; ac = g_steal_pointer (&priv->queued_act_request); nm_active_connection_set_state_fail ((NMActiveConnection *) ac, - NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED, + active_reason, NULL); } } @@ -11768,7 +11769,8 @@ _carrier_wait_check_queued_act_request (NMDevice *self) priv->queued_act_request_is_waiting_for_carrier = FALSE; if (!priv->carrier) { _LOGD (LOGD_DEVICE, "Cancel queued activation request as we have no carrier after timeout"); - _clear_queued_act_request (priv); + _clear_queued_act_request (priv, + NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); } else { gs_unref_object NMActRequest *queued_req = NULL; @@ -11844,7 +11846,8 @@ nm_device_disconnect_active_connection (NMActiveConnection *active, priv = NM_DEVICE_GET_PRIVATE (self); if (NM_ACTIVE_CONNECTION (priv->queued_act_request) == active) { - _clear_queued_act_request (priv); + _clear_queued_act_request (priv, + NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); return; } if (NM_ACTIVE_CONNECTION (priv->act_request.obj) == active) { @@ -11874,7 +11877,8 @@ nm_device_queue_activation (NMDevice *self, NMActRequest *req) } /* supersede any already-queued request */ - _clear_queued_act_request (priv); + _clear_queued_act_request (priv, + NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); priv->queued_act_request = g_object_ref (req); priv->queued_act_request_is_waiting_for_carrier = must_queue; @@ -14743,8 +14747,10 @@ _set_state_full (NMDevice *self, if (state <= NM_DEVICE_STATE_UNAVAILABLE) { if (available_connections_del_all (self)) _notify (self, PROP_AVAILABLE_CONNECTIONS); - if (old_state > NM_DEVICE_STATE_UNAVAILABLE) - _clear_queued_act_request (priv); + if (old_state > NM_DEVICE_STATE_UNAVAILABLE) { + _clear_queued_act_request (priv, + NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); + } } /* Update the available connections list when a device first becomes available */ @@ -16189,7 +16195,8 @@ dispose (GObject *object) if (nm_clear_g_source (&priv->carrier_wait_id)) nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE); - _clear_queued_act_request (priv); + _clear_queued_act_request (priv, + NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); nm_clear_g_source (&priv->device_link_changed_id); nm_clear_g_source (&priv->device_ip_link_changed_id); From 8f360197318078afb1e7504d677973239abdd9d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:25:42 +0100 Subject: [PATCH 07/21] device: pass active-connection's state-change reason to nm_device_disconnect_active_connection() No change in behavior, yet. --- src/devices/nm-device.c | 5 +++-- src/devices/nm-device.h | 3 ++- src/nm-manager.c | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index cf8de90bea..c5b3a181f3 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11826,7 +11826,8 @@ _carrier_wait_check_act_request_must_queue (NMDevice *self, NMActRequest *req) void nm_device_disconnect_active_connection (NMActiveConnection *active, - NMDeviceStateReason device_reason) + NMDeviceStateReason device_reason, + NMActiveConnectionStateReason active_reason) { NMDevice *self; NMDevicePrivate *priv; @@ -11838,7 +11839,7 @@ nm_device_disconnect_active_connection (NMActiveConnection *active, if (!self) { /* hm, no device? Just fail the active connection. */ nm_active_connection_set_state_fail (active, - NM_ACTIVE_CONNECTION_STATE_REASON_UNKNOWN, + active_reason, NULL); return; } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 1bd95a83bd..4851f36fee 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -770,7 +770,8 @@ void nm_device_queue_state (NMDevice *self, gboolean nm_device_get_firmware_missing (NMDevice *self); void nm_device_disconnect_active_connection (NMActiveConnection *active, - NMDeviceStateReason device_reason); + NMDeviceStateReason device_reason, + NMActiveConnectionStateReason active_reason); void nm_device_queue_activation (NMDevice *device, NMActRequest *req); diff --git a/src/nm-manager.c b/src/nm-manager.c index d335cf9df2..33595a06d4 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4550,7 +4550,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * nm_device_disconnect_active_connection ( all_ac_arr ? all_ac_arr->pdata[i] : ac, - NM_DEVICE_STATE_REASON_NEW_ACTIVATION); + NM_DEVICE_STATE_REASON_NEW_ACTIVATION, + NM_ACTIVE_CONNECTION_STATE_REASON_UNKNOWN); } } } From 71a090c92095369015259494cae1fa3948795fb8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:28:06 +0100 Subject: [PATCH 08/21] device: use correct active-connection's state-change reason in nm_device_disconnect_active_connection() It just makes more sense, to let the caller decide on the reason. --- src/devices/nm-device.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c5b3a181f3..6adf821cc4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11847,8 +11847,7 @@ nm_device_disconnect_active_connection (NMActiveConnection *active, priv = NM_DEVICE_GET_PRIVATE (self); if (NM_ACTIVE_CONNECTION (priv->queued_act_request) == active) { - _clear_queued_act_request (priv, - NM_ACTIVE_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); + _clear_queued_act_request (priv, active_reason); return; } if (NM_ACTIVE_CONNECTION (priv->act_request.obj) == active) { From e1b0451d688231eec4c00a1fb92c5be2fbaae3a9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:32:37 +0100 Subject: [PATCH 09/21] device: always disconnect in nm_device_disconnect_active_connection() Previously, if @active referenced a device but was not currently queued or the current activation request, nothing was done. Now, in such a case still call nm_active_connection_set_state_fail(). Note that nm_active_connection_set_state_fail() has no effects on active-connections that are already in disconnected state (which we would expect by such an active connection). Likely there is no visible change here, but it feels more correct to ensure the active connection is always failed. --- src/devices/nm-device.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6adf821cc4..f47331f230 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11835,13 +11835,9 @@ nm_device_disconnect_active_connection (NMActiveConnection *active, g_return_if_fail (NM_IS_ACTIVE_CONNECTION (active)); self = nm_active_connection_get_device (active); - if (!self) { /* hm, no device? Just fail the active connection. */ - nm_active_connection_set_state_fail (active, - active_reason, - NULL); - return; + goto do_fail; } priv = NM_DEVICE_GET_PRIVATE (self); @@ -11850,15 +11846,25 @@ nm_device_disconnect_active_connection (NMActiveConnection *active, _clear_queued_act_request (priv, active_reason); return; } + if (NM_ACTIVE_CONNECTION (priv->act_request.obj) == active) { if (priv->state < NM_DEVICE_STATE_DEACTIVATING) { nm_device_state_changed (self, NM_DEVICE_STATE_DEACTIVATING, device_reason); } else { - /* it's going down already... */ + /* @active is the current ac of @self, but it's going down already. + * Nothing to do. */ } + return; } + + /* the active connection references this device, but it's neither the + * queued_act_request nor the current act_request. Just set it to fail... */ +do_fail: + nm_active_connection_set_state_fail (active, + active_reason, + NULL); } void From a0a36d55a1e69ca26595144a4f5f65ca7040f056 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:38:38 +0100 Subject: [PATCH 10/21] manager: fail early from nm_manager_deactivate_connection() Drop the @success variable, and just return on error right away. --- src/nm-manager.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 33595a06d4..648cc611b0 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -5408,31 +5408,26 @@ nm_manager_deactivate_connection (NMManager *manager, NMDeviceStateReason reason, GError **error) { - gboolean success = FALSE; - if (NM_IS_VPN_CONNECTION (active)) { NMActiveConnectionStateReason vpn_reason = NM_ACTIVE_CONNECTION_STATE_REASON_USER_DISCONNECTED; if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CONNECTION_REMOVED) vpn_reason = NM_ACTIVE_CONNECTION_STATE_REASON_CONNECTION_REMOVED; - if (nm_vpn_connection_deactivate (NM_VPN_CONNECTION (active), vpn_reason, FALSE)) - success = TRUE; - else + if (!nm_vpn_connection_deactivate (NM_VPN_CONNECTION (active), vpn_reason, FALSE)) { g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_NOT_ACTIVE, "The VPN connection was not active."); + return FALSE; + } } else { g_assert (NM_IS_ACT_REQUEST (active)); nm_device_state_changed (nm_active_connection_get_device (active), NM_DEVICE_STATE_DEACTIVATING, reason); - success = TRUE; } - if (success) - _notify (manager, PROP_ACTIVE_CONNECTIONS); - - return success; + _notify (manager, PROP_ACTIVE_CONNECTIONS); + return TRUE; } static void From 023ebd8eebb7bb705097d970f710d76f26fd356b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:41:14 +0100 Subject: [PATCH 11/21] manager: use nm_device_disconnect_active_connection() in nm_manager_deactivate_connection() We should not blindly change the device's state. Instead, call nm_device_disconnect_active_connection() which will figure out whether the device state needs to change. Note that it is very possible that the active-connection instance is still queued, not yet queued, or already disconnected. nm_device_disconnect_active_connection() does the right thing in all cases. --- src/nm-manager.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 648cc611b0..4c9b11a524 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -5420,10 +5420,10 @@ nm_manager_deactivate_connection (NMManager *manager, return FALSE; } } else { - g_assert (NM_IS_ACT_REQUEST (active)); - nm_device_state_changed (nm_active_connection_get_device (active), - NM_DEVICE_STATE_DEACTIVATING, - reason); + nm_assert (NM_IS_ACT_REQUEST (active)); + nm_device_disconnect_active_connection (active, + reason, + NM_ACTIVE_CONNECTION_STATE_REASON_UNKNOWN); } _notify (manager, PROP_ACTIVE_CONNECTIONS); From 9e8c3d2ebf2efd61658e69ef8c3db3df3b0b8f0a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 15:31:59 +0100 Subject: [PATCH 12/21] keep-alive: let NMKeepAlive instance access the owner object that it is keeping alive NMKeepAlive is a full API independent of NMActiveConnection. For good reasons: - it moves complexity away from nm-active-connection.c - in the future, we can use NMKeepAlive also for NMSettingsConnection As such, the user should also directly interact with NMKeepAlive, instead of going through NMActiveConnection. For that to work, it must be possible to get the owner of the NMKeepAlive instance, which is kept alive. --- src/nm-active-connection.c | 3 ++ src/nm-keep-alive.c | 60 ++++++++++++++++++++++++++++++++++++++ src/nm-keep-alive.h | 6 ++++ 3 files changed, 69 insertions(+) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 59f7622c77..559d05cf13 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -1447,6 +1447,7 @@ nm_active_connection_init (NMActiveConnection *self) priv->version_id = _version_id_new (); priv->keep_alive = nm_keep_alive_new (); + _nm_keep_alive_set_owner (priv->keep_alive, G_OBJECT (self)); g_signal_connect_object (priv->keep_alive, "notify::" NM_KEEP_ALIVE_ALIVE, (GCallback) keep_alive_alive_changed, self, @@ -1532,6 +1533,8 @@ finalize (GObject *object) NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); nm_dbus_track_obj_path_set (&priv->settings_connection, NULL, FALSE); + + _nm_keep_alive_set_owner (priv->keep_alive, NULL); g_clear_object (&priv->keep_alive); G_OBJECT_CLASS (nm_active_connection_parent_class)->finalize (object); diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 0b3b7fabd7..4af6c38ddb 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -34,6 +34,8 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMKeepAlive, ); typedef struct { + GObject *owner; + NMSettingsConnection *connection; GDBusConnection *dbus_connection; char *dbus_client; @@ -398,6 +400,62 @@ get_property (GObject *object, /*****************************************************************************/ +/** + * nm_keep_alive_get_owner: + * @self: the #NMKeepAlive + * + * Returns: the owner instance associated with this @self. This commonly + * is set to be the target instance, which @self guards for being alive. + * Returns a gpointer, but of course it's some GObject instance. */ +gpointer /* GObject * */ +nm_keep_alive_get_owner (NMKeepAlive *self) +{ + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + nm_assert (!priv->owner || G_IS_OBJECT (priv->owner)); + + return priv->owner; +} + +/** + * _nm_keep_alive_set_owner: + * @self: the #NMKeepAlive + * @owner: the owner to set or unset. + * + * Sets or unsets the owner instance. Think of the owner the target + * instance that is guarded by @self. It's the responsibility of the + * owner to set and properly unset this pointer. As the owner also + * controls the lifetime of the NMKeepAlive instance. + * + * This API is not to be called by everybody, but only the owner of + * @self. + */ +void +_nm_keep_alive_set_owner (NMKeepAlive *self, + GObject *owner) +{ + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + nm_assert (!owner || G_IS_OBJECT (owner)); + + /* it's bad style to reset the owner object. You are supposed to + * set it once, and clear it once. That's it. */ + nm_assert (!owner || !priv->owner); + + /* optimally, we would take a reference to @owner. But the + * owner already owns a refrence to the keep-alive, so we cannot + * just own a reference back. + * + * We could register a weak-pointer here. But instead, declare that + * owner is required to set itself as owner when creating the + * keep-alive instance, and unset itself when it lets go of the + * keep-alive instance (at latest, when the owner itself gets destroyed). + */ + priv->owner = owner; +} + +/*****************************************************************************/ + static void nm_keep_alive_init (NMKeepAlive *self) { @@ -419,6 +477,8 @@ dispose (GObject *object) { NMKeepAlive *self = NM_KEEP_ALIVE (object); + nm_assert (!NM_KEEP_ALIVE_GET_PRIVATE (self)->owner); + /* disarm also happens to free all resources. */ nm_keep_alive_disarm (self); } diff --git a/src/nm-keep-alive.h b/src/nm-keep-alive.h index 3a7cd66858..72133224cd 100644 --- a/src/nm-keep-alive.h +++ b/src/nm-keep-alive.h @@ -53,4 +53,10 @@ void nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, GDBusConnection *connection, const char *client_address); +gpointer /* GObject * */ nm_keep_alive_get_owner (NMKeepAlive *self); + +/* _nm_keep_alive_set_owner() is reserved for the owner to set/unset itself. */ +void _nm_keep_alive_set_owner (NMKeepAlive *self, + GObject *owner); + #endif /* __NETWORKMANAGER_KEEP_ALIVE_H__ */ From f95a5263667a03aaf67635d28532c242d3025bd9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 14:51:22 +0100 Subject: [PATCH 13/21] keep-alive: use NMKeepAlive API directly instead of via NMActiveConnection NMKeepAlive is a proper GObject type, with a specific API that on the one end allows to configure watches/bindings, and on the other end exposes and is-alive property and the owner instance. That's great, as NMActiveConnection is not concerned with either end (moving complexity away from "nm-active-connection.c") and as we later can reuse NMKeepAlive with NMSettingsConnection. However, we don't need to wrap this API by NMActiveConnection. Doing so means, we need to expose all the watch/bind functions also as part of NMActiveConnection API. The only ugliness here is, that NMPolicy subscribes to property changed signal of the keep alive instance, which would fail horribly if NMActiveConnection ever decides to swap the keep alive instance (in which case NMPolicy would have to disconnect the signal, and possibly reconnect it to another NMKeepAlive instance). We avoid that by just not doing that and documenting it. --- src/nm-active-connection.c | 63 +++++++++++++------------------------- src/nm-active-connection.h | 7 +---- src/nm-manager.c | 10 ++++-- src/nm-policy.c | 47 ++++++++++++++++++---------- 4 files changed, 62 insertions(+), 65 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 559d05cf13..6d504e0e4d 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -105,7 +105,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMActiveConnection, PROP_INT_MASTER_READY, PROP_INT_ACTIVATION_TYPE, PROP_INT_ACTIVATION_REASON, - PROP_INT_KEEP_ALIVE, ); enum { @@ -176,14 +175,6 @@ NM_UTILS_FLAGS2STR_DEFINE_STATIC (_state_flags_to_string, NMActivationStateFlags /*****************************************************************************/ -static void -keep_alive_alive_changed (NMActiveConnection *ac, - GParamSpec *pspec, - NMKeepAlive *keep_alive) -{ - _notify (ac, PROP_INT_KEEP_ALIVE); -} - static void _settings_connection_updated (NMSettingsConnection *sett_conn, gboolean by_user, @@ -921,12 +912,30 @@ nm_active_connection_get_activation_reason (NMActiveConnection *self) return NM_ACTIVE_CONNECTION_GET_PRIVATE (self)->activation_reason; } -gboolean +/*****************************************************************************/ + +/** + * nm_active_connection_get_keep_alive: + * @self: the #NMActiveConnection instance + * + * Gives the #NMKeepAlive instance of the active connection. Note that + * @self is guaranteed not to swap the keep-alive instance, so it is + * in particular safe to assume that the keep-alive instance is alive + * as long as @self, and that nm_active_connection_get_keep_alive() + * will return always the same instance. + * + * In particular this means, that it is safe and encouraged, that you + * register to the notify:alive property changed signal of the returned + * instance. + * + * Returns: the #NMKeepAlive instance. + */ +NMKeepAlive * nm_active_connection_get_keep_alive (NMActiveConnection *self) { - NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (self), NULL); - return nm_keep_alive_is_alive (priv->keep_alive); + return NM_ACTIVE_CONNECTION_GET_PRIVATE (self)->keep_alive; } /*****************************************************************************/ @@ -1020,23 +1029,6 @@ nm_active_connection_set_parent (NMActiveConnection *self, NMActiveConnection *p g_object_weak_ref ((GObject *) priv->parent, parent_destroyed, self); } -/** - * nm_active_connection_bind_dbus_client: - * @self: the #NMActiveConnection - * @dbus_client: The dbus client to watch. - * - * Binds the lifetime of this active connection to the given dbus client. If - * the dbus client disappears, then the connection will be disconnected. - */ -void -nm_active_connection_bind_dbus_client (NMActiveConnection *self, GDBusConnection *dbus_con, const char *dbus_client) -{ - NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); - - nm_keep_alive_set_dbus_client_watch (priv->keep_alive, dbus_con, dbus_client); - nm_keep_alive_arm (priv->keep_alive); -} - /*****************************************************************************/ static void @@ -1335,9 +1327,6 @@ get_property (GObject *object, guint prop_id, case PROP_INT_MASTER_READY: g_value_set_boolean (value, priv->master_ready); break; - case PROP_INT_KEEP_ALIVE: - g_value_set_boolean (value, nm_keep_alive_is_alive (priv->keep_alive)); - break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1446,12 +1435,9 @@ nm_active_connection_init (NMActiveConnection *self) priv->activation_type = NM_ACTIVATION_TYPE_MANAGED; priv->version_id = _version_id_new (); + /* the keep-alive instance must never change. Callers rely on that. */ priv->keep_alive = nm_keep_alive_new (); _nm_keep_alive_set_owner (priv->keep_alive, G_OBJECT (self)); - g_signal_connect_object (priv->keep_alive, "notify::" NM_KEEP_ALIVE_ALIVE, - (GCallback) keep_alive_alive_changed, - self, - G_CONNECT_SWAPPED); } static void @@ -1746,11 +1732,6 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class) G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); - obj_properties[PROP_INT_KEEP_ALIVE] = - g_param_spec_boolean (NM_ACTIVE_CONNECTION_INT_KEEP_ALIVE, "", "", - TRUE, G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS); - g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); signals[DEVICE_CHANGED] = diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index 5b2e421507..2b5a6b59ee 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -59,7 +59,6 @@ #define NM_ACTIVE_CONNECTION_INT_MASTER_READY "int-master-ready" #define NM_ACTIVE_CONNECTION_INT_ACTIVATION_TYPE "int-activation-type" #define NM_ACTIVE_CONNECTION_INT_ACTIVATION_REASON "int-activation-reason" -#define NM_ACTIVE_CONNECTION_INT_KEEP_ALIVE "int-keep-alive" /* Signals */ #define NM_ACTIVE_CONNECTION_STATE_CHANGED "state-changed" @@ -186,12 +185,8 @@ NMActivationType nm_active_connection_get_activation_type (NMActiveConnection *s NMActivationReason nm_active_connection_get_activation_reason (NMActiveConnection *self); -gboolean nm_active_connection_get_keep_alive (NMActiveConnection *self); +NMKeepAlive *nm_active_connection_get_keep_alive (NMActiveConnection *self); void nm_active_connection_clear_secrets (NMActiveConnection *self); -void nm_active_connection_bind_dbus_client (NMActiveConnection *self, - GDBusConnection *dbus_con, - const char *dbus_name); - #endif /* __NETWORKMANAGER_ACTIVE_CONNECTION_H__ */ diff --git a/src/nm-manager.c b/src/nm-manager.c index 4c9b11a524..b05fbb443a 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -39,6 +39,7 @@ #include "platform/nm-platform.h" #include "platform/nmp-object.h" #include "nm-hostname-manager.h" +#include "nm-keep-alive.h" #include "nm-rfkill-manager.h" #include "dhcp/nm-dhcp-manager.h" #include "settings/nm-settings.h" @@ -5378,8 +5379,13 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj, if (!active) goto error; - if (bind_dbus_client) - nm_active_connection_bind_dbus_client (active, dbus_connection, sender); + if (bind_dbus_client) { + NMKeepAlive *keep_alive; + + keep_alive = nm_active_connection_get_keep_alive (active); + nm_keep_alive_set_dbus_client_watch (keep_alive, dbus_connection, sender); + nm_keep_alive_arm (keep_alive); + } nm_active_connection_authorize (active, incompl_conn, diff --git a/src/nm-policy.c b/src/nm-policy.c index 4e47b8c902..e4facf2614 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -30,6 +30,7 @@ #include "NetworkManagerUtils.h" #include "nm-act-request.h" +#include "nm-keep-alive.h" #include "devices/nm-device.h" #include "nm-setting-ip4-config.h" #include "nm-setting-connection.h" @@ -2183,26 +2184,36 @@ active_connection_state_changed (NMActiveConnection *active, } static void -active_connection_keep_alive_changed (NMActiveConnection *ac, +active_connection_keep_alive_changed (NMKeepAlive *keep_alive, GParamSpec *pspec, NMPolicy *self) { - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); + NMPolicyPrivate *priv; + NMActiveConnection *ac; GError *error = NULL; - if (nm_active_connection_get_keep_alive (ac)) + nm_assert (NM_IS_POLICY (self)); + nm_assert (NM_IS_KEEP_ALIVE (keep_alive)); + nm_assert (NM_IS_ACTIVE_CONNECTION (nm_keep_alive_get_owner (keep_alive))); + + if (nm_keep_alive_is_alive (keep_alive)) return; - if (nm_active_connection_get_state (ac) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { - if (!nm_manager_deactivate_connection (priv->manager, - ac, - NM_DEVICE_STATE_REASON_CONNECTION_REMOVED, - &error)) { - _LOGW (LOGD_DEVICE, "connection '%s' is no longer kept alive, but error deactivating it: %s", - nm_active_connection_get_settings_connection_id (ac), - error->message); - g_clear_error (&error); - } + ac = nm_keep_alive_get_owner (keep_alive); + + if (nm_active_connection_get_state (ac) > NM_ACTIVE_CONNECTION_STATE_ACTIVATED) + return; + + priv = NM_POLICY_GET_PRIVATE (self); + + if (!nm_manager_deactivate_connection (priv->manager, + ac, + NM_DEVICE_STATE_REASON_CONNECTION_REMOVED, + &error)) { + _LOGW (LOGD_DEVICE, "connection '%s' is no longer kept alive, but error deactivating it: %s", + nm_active_connection_get_settings_connection_id (ac), + error->message); + g_clear_error (&error); } } @@ -2213,6 +2224,7 @@ active_connection_added (NMManager *manager, { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF (priv); + NMKeepAlive *keep_alive; if (NM_IS_VPN_CONNECTION (active)) { g_signal_connect (active, NM_VPN_CONNECTION_INTERNAL_STATE_CHANGED, @@ -2223,13 +2235,16 @@ active_connection_added (NMManager *manager, self); } + keep_alive = nm_active_connection_get_keep_alive (active); + g_signal_connect (active, "notify::" NM_ACTIVE_CONNECTION_STATE, G_CALLBACK (active_connection_state_changed), self); - g_signal_connect (active, "notify::" NM_ACTIVE_CONNECTION_INT_KEEP_ALIVE, + g_signal_connect (keep_alive, + "notify::" NM_KEEP_ALIVE_ALIVE, G_CALLBACK (active_connection_keep_alive_changed), self); - active_connection_keep_alive_changed (active, NULL, self); + active_connection_keep_alive_changed (keep_alive, NULL, self); } static void @@ -2249,7 +2264,7 @@ active_connection_removed (NMManager *manager, g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); - g_signal_handlers_disconnect_by_func (active, + g_signal_handlers_disconnect_by_func (nm_active_connection_get_keep_alive (active), active_connection_keep_alive_changed, self); } From c9354cb47771263b0762e0400de1b894fa480cd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 08:24:55 +0100 Subject: [PATCH 14/21] keep-alive: drop unused nm_keep_alive_set_forced() set-forced is currently unused, so drop it. NMKeepAlive in principle determines the alive-status based on multiple aspects, that in combination render the instance alive or dead. These aspects cooperate in a particular way. By default, a keep-alive instance should be alive. If there are conditions enabled that further determine the alive-state, then these conditions cooperate in a particular way. As it was, the force-flag would just overrule them all. But is that useful? The nm_keep_alive_set_forced() API also means that only one user caller can have control over the flag. Independent callers cannot cooperate on setting the flag, because there is no reference-counting or registered handles. At least today, it's unclear whether this flag really should overrule all other conditions and how this flag would actually be used. Drop it for now. --- src/nm-keep-alive.c | 17 ----------------- src/nm-keep-alive.h | 3 --- 2 files changed, 20 deletions(-) diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 4af6c38ddb..c519ceb756 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -46,7 +46,6 @@ typedef struct { bool armed:1; bool disarmed:1; - bool forced:1; bool alive:1; bool dbus_client_confirmed:1; } NMKeepAlivePrivate; @@ -88,9 +87,6 @@ _is_alive (NMKeepAlive *self) return TRUE; } - if (priv->forced) - return TRUE; - if ( priv->connection && NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) @@ -129,19 +125,6 @@ nm_keep_alive_is_alive (NMKeepAlive *self) /*****************************************************************************/ -void -nm_keep_alive_set_forced (NMKeepAlive *self, gboolean forced) -{ - NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); - - if (priv->forced != (!!forced)) { - priv->forced = forced; - _notify_alive (self); - } -} - -/*****************************************************************************/ - static void connection_flags_changed (NMSettingsConnection *connection, NMKeepAlive *self) diff --git a/src/nm-keep-alive.h b/src/nm-keep-alive.h index 72133224cd..160b2adb58 100644 --- a/src/nm-keep-alive.h +++ b/src/nm-keep-alive.h @@ -43,9 +43,6 @@ gboolean nm_keep_alive_is_alive (NMKeepAlive *self); void nm_keep_alive_arm (NMKeepAlive *self); void nm_keep_alive_disarm (NMKeepAlive *self); -void nm_keep_alive_set_forced (NMKeepAlive *self, - gboolean forced); - void nm_keep_alive_set_settings_connection_watch_visible (NMKeepAlive *self, NMSettingsConnection *connection); From 936c6d0b0a9fd43cf8431f0a271dc07b317ce7d6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 10:47:51 +0100 Subject: [PATCH 15/21] core: add nm_manager_for_each_active_connection_safe() and nm_manager_for_each_device_safe() Analog to c_list_for_each_safe(), which allows to delete the current instance while iterating. Note that modifying the list any other way is unsafe. --- src/nm-manager.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/nm-manager.h b/src/nm-manager.h index 06ca633de8..4b159f1ebd 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -97,6 +97,19 @@ const CList * nm_manager_get_active_connections (NMManager *manager); }); \ iter = c_list_entry (iter->active_connections_lst.next, NMActiveConnection, active_connections_lst)) +#define nm_manager_for_each_active_connection_safe(manager, iter, tmp_list, iter_safe) \ + for (tmp_list = nm_manager_get_active_connections (manager), \ + iter_safe = tmp_list->next; \ + ({ \ + if (iter_safe != tmp_list) { \ + iter = c_list_entry (iter_safe, NMActiveConnection, active_connections_lst); \ + iter_safe = iter_safe->next; \ + } else \ + iter = NULL; \ + (iter != NULL); \ + }); \ + ) + NMSettingsConnection **nm_manager_get_activatable_connections (NMManager *manager, gboolean for_auto_activation, gboolean sort, @@ -121,6 +134,19 @@ const CList * nm_manager_get_devices (NMManager *manager); }); \ iter = c_list_entry (iter->devices_lst.next, NMDevice, devices_lst)) +#define nm_manager_for_each_device_safe(manager, iter, tmp_list, iter_safe) \ + for (tmp_list = nm_manager_get_devices (manager), \ + iter_safe = tmp_list->next; \ + ({ \ + if (iter_safe != tmp_list) { \ + iter = c_list_entry (iter_safe, NMDevice, devices_lst); \ + iter_safe = iter_safe->next; \ + } else \ + iter = NULL; \ + (iter != NULL); \ + }); \ + ) + NMDevice * nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex); NMDevice * nm_manager_get_device_by_path (NMManager *manager, From 83d12313486d133ecf17d8471de0335f6cdcd825 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 10:51:21 +0100 Subject: [PATCH 16/21] core: in NMPolicy's _deactivate_if_active() safely iterate over active connections It's not clear that calling nm_manager_deactivate_connection() does not remove the active-connection entirely from the list. Just to be sure, use nm_manager_for_each_active_connection_safe() which allows deleting the current entry while iterating (all other modifications to the list are not allowed). --- src/nm-policy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index e4facf2614..00eaf4a94b 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2404,12 +2404,12 @@ _deactivate_if_active (NMPolicy *self, NMSettingsConnection *connection) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); NMActiveConnection *ac; - const CList *tmp_list; + const CList *tmp_list, *tmp_safe; GError *error = NULL; nm_assert (NM_IS_SETTINGS_CONNECTION (connection)); - nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + nm_manager_for_each_active_connection_safe (priv->manager, ac, tmp_list, tmp_safe) { if ( nm_active_connection_get_settings_connection (ac) == connection && (nm_active_connection_get_state (ac) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED)) { From 8e2d69f83b0e92974f1704e8dd6c547818bea022 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 12:52:31 +0100 Subject: [PATCH 17/21] keep-alive: log alive/dead status of keep-alive instance --- src/nm-keep-alive.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index c519ceb756..052113bf5b 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -114,6 +114,7 @@ _notify_alive (NMKeepAlive *self) if (priv->alive == _is_alive (self)) return; priv->alive = !priv->alive; + _LOGD ("instance is now %s", priv->alive ? "alive" : "dead"); _notify (self, PROP_ALIVE); } From f59db9bb446dee3672411eb9779e4f7fb1a1843c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 12:41:57 +0100 Subject: [PATCH 18/21] keep-alive: default keep-alive instance to alive NMKeepAlive supports conditions (watches, bindings) that determine whether the instance signals alive state. Currently, there are two conditions: watch a VISIBLE state of a connection and watch whether a D-Bus client is registered. Anyway, if none of these conditions are enabled, then the default should be that the instance is alive. If one or more conditions are enabled, then they cooprate in a particular way, depending on whether the condition is enabled and whether it is satisfied. Previously, the code couldn't differenciate whether a D-Bus watch was not enabled or whether the D-Bus client already disconnected. It would just check whether priv->dbus_client was set, but that alone was not sufficient to differentiate. That means, we couldn't determine whether the keep-alive instance is alive (by default, as no D-Bus watch is enabled) or whether the keep-alive instance is dead (because the D-Bus client disconnected). Fix that, by tracking explicitly whether to watch based on a priv->dbus_client_watching flag. Note, that problems by this were avoided earlier, by only arming the instance when enabling a condition. But I think the concept of arming the keep-alive instance merely means that the instance is ready to fire events. It should not mean that the user only arms the instance after registering a condition. Also, without this, we couldn't unbind the NMKeepAlive instance from all conditions, without making it dead right away. Unbinding/disabling all conditions should render the instance alive, not dead. --- src/nm-keep-alive.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index 052113bf5b..cb495baac3 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -48,6 +48,7 @@ typedef struct { bool alive:1; bool dbus_client_confirmed:1; + bool dbus_client_watching:1; } NMKeepAlivePrivate; struct _NMKeepAlive { @@ -87,18 +88,31 @@ _is_alive (NMKeepAlive *self) return TRUE; } - if ( priv->connection - && NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), - NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) - return TRUE; + if (priv->dbus_client_watching) { + if (_is_alive_dbus_client (self)) { + /* no matter what, the keep-alive is alive, because there is a D-Bus client + * still around keeping it alive. */ + return TRUE; + } + /* the D-Bus client is gone. The only other binding (below) for the connection's + * visibility cannot keep the instance alive. + * + * As such, a D-Bus client watch is authorative and overrules other conditions (that + * we have so far). */ + return FALSE; + } - /* Perform this check as last. We want to confirm whether the dbus-client - * is alive lazyly, so if we already decided above that the keep-alive - * is good, we don't rely on the outcome of this check. */ - if (_is_alive_dbus_client (self)) - return TRUE; + if (priv->connection) { + if (!NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), + NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) { + /* the connection is invisible (and apparently has no D-Bus client + * watch above). The instance is dead. */ + return FALSE; + } + } - return FALSE; + /* by default, the instance is alive. */ + return TRUE; } static void @@ -287,13 +301,16 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, { NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + if (priv->disarmed) + return; + cleanup_dbus_watch (self); - if ( client_address - && !priv->disarmed) { + if (client_address) { _LOGD ("Registering dbus client watch for keep alive"); priv->dbus_client = g_strdup (client_address); + priv->dbus_client_watching = TRUE; priv->dbus_client_confirmed = FALSE; priv->dbus_connection = g_object_ref (connection); priv->subscription_id = g_dbus_connection_signal_subscribe (connection, @@ -306,7 +323,8 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self, name_owner_changed_cb, self, NULL); - } + } else + priv->dbus_client_watching = FALSE; _notify_alive (self); } From a4bdb161eb50ab4e8f4f0336b9aef531a85923d4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 20 Nov 2018 16:01:32 +0100 Subject: [PATCH 19/21] device: arm keep-alive instance when queuing active-connection for activation Now that the keep-alive instance defaults to ALIVE by default, we can always arm it when starting to activate the active-connection. The keep-alive instance may have been armed earlier already: for example, when binding its lifetime to a D-Bus name or when watching the connection's visible state. However, at the moment when we queue the active-connection for activation, we also want to make sure that the keep-alive instance is armed. It is nicer for consistancy reasons. Note, that nm_keep_alive_arm() has no effect if nm_keep_alive_disarm() was called earlier already. Also note, that NMActiveConnection will disarm the keep-alive instance, when changing to a state greater than ACTIVATED. So, all works together nicely. Also, no longer arm the keep-alive instance in the constructor of NMActiveConnection. It would essentially mean, that the instances is aremd very early. Also, as alternative point of interest, arm the keep-alive instance when registering the signal handler in "nm-policy.c". --- src/devices/nm-device.c | 18 +++++++++++++++++- src/nm-active-connection.c | 4 +--- src/nm-policy.c | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f47331f230..94a491544b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -67,6 +67,7 @@ #include "settings/nm-settings.h" #include "nm-setting-ethtool.h" #include "nm-auth-utils.h" +#include "nm-keep-alive.h" #include "nm-netns.h" #include "nm-dispatcher.h" #include "nm-config.h" @@ -11870,9 +11871,24 @@ do_fail: void nm_device_queue_activation (NMDevice *self, NMActRequest *req) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDevicePrivate *priv; gboolean must_queue; + g_return_if_fail (NM_IS_DEVICE (self)); + g_return_if_fail (NM_IS_ACT_REQUEST (req)); + + nm_keep_alive_arm (nm_active_connection_get_keep_alive (NM_ACTIVE_CONNECTION (req))); + + if (nm_active_connection_get_state (NM_ACTIVE_CONNECTION (req)) >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) { + /* it's already deactivating. Nothing to do. */ + nm_assert (NM_IN_SET (nm_active_connection_get_device (NM_ACTIVE_CONNECTION (req)), NULL, self)); + return; + } + + nm_assert (self == nm_active_connection_get_device (NM_ACTIVE_CONNECTION (req))); + + priv = NM_DEVICE_GET_PRIVATE (self); + must_queue = _carrier_wait_check_act_request_must_queue (self, req); if ( !priv->act_request.obj diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 6d504e0e4d..96f164638d 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -1472,10 +1472,8 @@ constructed (GObject *object) if (NM_IN_SET ((NMActivationReason) priv->activation_reason, NM_ACTIVATION_REASON_AUTOCONNECT, - NM_ACTIVATION_REASON_AUTOCONNECT_SLAVES)) { + NM_ACTIVATION_REASON_AUTOCONNECT_SLAVES)) nm_keep_alive_set_settings_connection_watch_visible (priv->keep_alive, priv->settings_connection.obj); - nm_keep_alive_arm (priv->keep_alive); - } } static void diff --git a/src/nm-policy.c b/src/nm-policy.c index 00eaf4a94b..ab9571f6df 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2237,6 +2237,8 @@ active_connection_added (NMManager *manager, keep_alive = nm_active_connection_get_keep_alive (active); + nm_keep_alive_arm (keep_alive); + g_signal_connect (active, "notify::" NM_ACTIVE_CONNECTION_STATE, G_CALLBACK (active_connection_state_changed), self); From b4d3334853b3b6bd28836c85c6290e4518b6f96d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Nov 2018 13:17:07 +0100 Subject: [PATCH 20/21] keep-alive: only declare keep-alive dead when connection becomes invisible A user can always manually activate his/her own profile, even if the profile is currently is invisible. -- it could be invisible, because the profile has "connection.permissions" set but the user has no active session. Now, if the user should be able to activate such a profile, then it cannot fail simply because being invisible all along. Instead, the alive check must only fail, if a connection becomes invisible that was visible in the time since it is monitored. --- src/nm-keep-alive.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c index cb495baac3..e601483b96 100644 --- a/src/nm-keep-alive.c +++ b/src/nm-keep-alive.c @@ -49,6 +49,7 @@ typedef struct { bool alive:1; bool dbus_client_confirmed:1; bool dbus_client_watching:1; + bool connection_was_visible:1; } NMKeepAlivePrivate; struct _NMKeepAlive { @@ -102,13 +103,17 @@ _is_alive (NMKeepAlive *self) return FALSE; } - if (priv->connection) { - if (!NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), - NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) { - /* the connection is invisible (and apparently has no D-Bus client - * watch above). The instance is dead. */ - return FALSE; - } + if ( priv->connection + && priv->connection_was_visible + && !NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), + NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) { + /* note that we only declare the keep-alive as dead due to invisible + * connection, if + * (1) we monitor a connection, obviously + * (2) the connection was visible earlier and is no longer. It was + * was invisible all the time, it does not suffice. + */ + return FALSE; } /* by default, the instance is alive. */ @@ -144,6 +149,24 @@ static void connection_flags_changed (NMSettingsConnection *connection, NMKeepAlive *self) { + NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self); + + if ( !priv->connection_was_visible + && NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), + NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) { + /* the profile was never visible but now it becomes visible. + * Remember that. + * + * Before this happens (that is, if the device was invisible all along), + * the keep alive instance is considered alive (w.r.t. watching the connection). + * + * The reason is to allow a user to manually activate an invisible profile and keep + * it alive. At least, as long until the user logs out the first time (which is the + * first time, the profiles changes from visible to invisible). + * + * Yes, that is odd. How to improve? */ + priv->connection_was_visible = TRUE; + } _notify_alive (self); } @@ -168,6 +191,8 @@ _set_settings_connection_watch_visible (NMKeepAlive *self, if ( connection && !priv->disarmed) { priv->connection = g_object_ref (connection); + priv->connection_was_visible = NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection), + NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE); g_signal_connect (priv->connection, NM_SETTINGS_CONNECTION_FLAGS_CHANGED, G_CALLBACK (connection_flags_changed), From b635b4d4194514baf6bf5b32f1116210c8822668 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Nov 2018 13:30:16 +0100 Subject: [PATCH 21/21] core: improve and fix keeping connection active based on "connection.permissions" By setting "connection.permissions", a profile is restricted to a particular user. That means for example, that another user cannot see, modify, delete, activate or deactivate the profile. It also means, that the profile will only autoconnect when the user is logged in (has a session). Note that root is always able to activate the profile. Likewise, the user is also allowed to manually activate the own profile, even if no session currently exists (which can easily happen with `sudo`). When the user logs out (the session goes away), we want do disconnect the profile, however there are conflicting goals here: 1) if the profile was activate by root user, then logging out the user should not disconnect the profile. The patch fixes that by not binding the activation to the connection, if the activation is done by the root user. 2) if the profile was activated by the owner when it had no session, then it should stay alive until the user logs in (once) and logs out again. This is already handled by the previous commit. Yes, this point is odd. If you first do $ sudo -u $OTHER_USER nmcli connection up $PROFILE the profile activates despite not having a session. If you then $ ssh guest@localhost nmcli device you'll still see the profile active. However, the moment the SSH session ends, a session closes and the profile disconnects. It's unclear, how to solve that any better. I think, a user who cares about this, should not activate the profile without having a session in the first place. There are quite some special cases, in particular with internal activations. In those cases we need to decide whether to bind the activation to the profile's visibility. Also, expose the "bind" setting in the D-Bus API. Note, that in the future this flag may be modified via D-Bus API. Like we may also add related API that allows to tweak the lifetime of the activation. Also, I think we broke handling of connection visiblity with 37e8c53eeed "core: Introduce helper class to track connection keep alive". This should be fixed now too, with improved behavior. Fixes: 37e8c53eeed579fe34a68819cd12f3295d581394 https://bugzilla.redhat.com/show_bug.cgi?id=1530977 --- libnm-core/nm-dbus-interface.h | 19 ++++--- src/devices/nm-device.c | 19 ++++++- src/devices/nm-device.h | 1 + src/nm-act-request.c | 3 ++ src/nm-act-request.h | 1 + src/nm-active-connection.c | 45 ++++++++++------ src/nm-active-connection.h | 7 +++ src/nm-checkpoint.c | 6 +++ src/nm-manager.c | 93 ++++++++++++++++++++++++++++++++-- src/nm-manager.h | 1 + src/nm-policy.c | 12 ++++- src/vpn/nm-vpn-connection.c | 2 + src/vpn/nm-vpn-connection.h | 1 + 13 files changed, 181 insertions(+), 29 deletions(-) diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h index 28683c1dd4..aac4d7eadd 100644 --- a/libnm-core/nm-dbus-interface.h +++ b/libnm-core/nm-dbus-interface.h @@ -960,20 +960,25 @@ typedef enum { /*< flags >*/ * @NM_ACTIVATION_STATE_FLAG_IP6_READY: IPv6 setting is completed. * @NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES: The master has any slave devices attached. * This only makes sense if the device is a master. + * @NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY: the lifetime + * of the activation is bound to the visilibity of the connection profile, + * which in turn depends on "connection.permissions" and whether a session + * for the user exists. Since: 1.16 * * Flags describing the current activation state. * * Since: 1.10 **/ typedef enum { /*< flags >*/ - NM_ACTIVATION_STATE_FLAG_NONE = 0, + NM_ACTIVATION_STATE_FLAG_NONE = 0, - NM_ACTIVATION_STATE_FLAG_IS_MASTER = (1LL << 0), - NM_ACTIVATION_STATE_FLAG_IS_SLAVE = (1LL << 1), - NM_ACTIVATION_STATE_FLAG_LAYER2_READY = (1LL << 2), - NM_ACTIVATION_STATE_FLAG_IP4_READY = (1LL << 3), - NM_ACTIVATION_STATE_FLAG_IP6_READY = (1LL << 4), - NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES = (1LL << 5), + NM_ACTIVATION_STATE_FLAG_IS_MASTER = (1LL << 0), + NM_ACTIVATION_STATE_FLAG_IS_SLAVE = (1LL << 1), + NM_ACTIVATION_STATE_FLAG_LAYER2_READY = (1LL << 2), + NM_ACTIVATION_STATE_FLAG_IP4_READY = (1LL << 3), + NM_ACTIVATION_STATE_FLAG_IP6_READY = (1LL << 4), + NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES = (1LL << 5), + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY = (1LL << 6), } NMActivationStateFlags; /** diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 94a491544b..0f27ae3b01 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -2384,10 +2384,27 @@ nm_device_get_act_request (NMDevice *self) return NM_DEVICE_GET_PRIVATE (self)->act_request.obj; } +NMActivationStateFlags +nm_device_get_activation_state_flags (NMDevice *self) +{ + NMActRequest *ac; + + g_return_val_if_fail (NM_IS_DEVICE (self), NM_ACTIVATION_STATE_FLAG_NONE); + + ac = NM_DEVICE_GET_PRIVATE (self)->act_request.obj; + if (!ac) + return NM_ACTIVATION_STATE_FLAG_NONE; + return nm_active_connection_get_state_flags (NM_ACTIVE_CONNECTION (ac)); +} + NMSettingsConnection * nm_device_get_settings_connection (NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDevicePrivate *priv; + + g_return_val_if_fail (NM_IS_DEVICE (self), NULL); + + priv = NM_DEVICE_GET_PRIVATE (self); return priv->act_request.obj ? nm_act_request_get_settings_connection (priv->act_request.obj) : NULL; } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 4851f36fee..cd07e24630 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -543,6 +543,7 @@ NMConnection * nm_device_get_settings_connection_get_connection (NMDevice *self NMConnection * nm_device_get_applied_connection (NMDevice *dev); gboolean nm_device_has_unmodified_applied_connection (NMDevice *self, NMSettingCompareFlags compare_flags); +NMActivationStateFlags nm_device_get_activation_state_flags (NMDevice *self); gpointer /* (NMSetting *) */ nm_device_get_applied_setting (NMDevice *dev, GType setting_type); diff --git a/src/nm-act-request.c b/src/nm-act-request.c index e4c65e59ea..cd81696490 100644 --- a/src/nm-act-request.c +++ b/src/nm-act-request.c @@ -539,6 +539,7 @@ nm_act_request_init (NMActRequest *req) * @subject: the #NMAuthSubject representing the requestor of the activation * @activation_type: the #NMActivationType * @activation_reason: the reason for activation + * @initial_state_flags: the initial state flags. * @device: the device/interface to configure according to @connection * * Creates a new device-based activation request. If an applied connection is @@ -553,6 +554,7 @@ nm_act_request_new (NMSettingsConnection *settings_connection, NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, NMDevice *device) { g_return_val_if_fail (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection), NULL); @@ -567,6 +569,7 @@ nm_act_request_new (NMSettingsConnection *settings_connection, NM_ACTIVE_CONNECTION_INT_SUBJECT, subject, NM_ACTIVE_CONNECTION_INT_ACTIVATION_TYPE, (int) activation_type, NM_ACTIVE_CONNECTION_INT_ACTIVATION_REASON, (int) activation_reason, + NM_ACTIVE_CONNECTION_STATE_FLAGS, (guint) initial_state_flags, NULL); } diff --git a/src/nm-act-request.h b/src/nm-act-request.h index a8f09271a1..af2b749055 100644 --- a/src/nm-act-request.h +++ b/src/nm-act-request.h @@ -42,6 +42,7 @@ NMActRequest *nm_act_request_new (NMSettingsConnection *settings_connec NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, NMDevice *device); NMSettingsConnection *nm_act_request_get_settings_connection (NMActRequest *req); diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 96f164638d..966a274351 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -163,14 +163,18 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_state_to_string, NMActiveConnectionState, ); #define state_to_string(state) NM_UTILS_LOOKUP_STR (_state_to_string, state) +/* the maximum required buffer size for _state_flags_to_string(). */ +#define _NM_ACTIVATION_STATE_FLAG_TO_STRING_BUFSIZE (255) + NM_UTILS_FLAGS2STR_DEFINE_STATIC (_state_flags_to_string, NMActivationStateFlags, - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_NONE, "none"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IS_MASTER, "is-master"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IS_SLAVE, "is-slave"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_LAYER2_READY, "layer2-ready"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IP4_READY, "ip4-ready"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IP6_READY, "ip6-ready"), - NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES, "master-has-slaves"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_NONE, "none"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IS_MASTER, "is-master"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IS_SLAVE, "is-slave"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_LAYER2_READY, "layer2-ready"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IP4_READY, "ip4-ready"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_IP6_READY, "ip6-ready"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES, "master-has-slaves"), + NM_UTILS_FLAGS2STR (NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY, "lifetime-bound-to-profile-visibility"), ); /*****************************************************************************/ @@ -361,14 +365,20 @@ nm_active_connection_set_state_flags_full (NMActiveConnection *self, f = (priv->state_flags & ~mask) | (state_flags & mask); if (f != priv->state_flags) { - char buf1[G_N_ELEMENTS (_nm_utils_to_string_buffer)]; - char buf2[G_N_ELEMENTS (_nm_utils_to_string_buffer)]; + char buf1[_NM_ACTIVATION_STATE_FLAG_TO_STRING_BUFSIZE]; + char buf2[_NM_ACTIVATION_STATE_FLAG_TO_STRING_BUFSIZE]; _LOGD ("set state-flags %s (was %s)", _state_flags_to_string (f, buf1, sizeof (buf1)), _state_flags_to_string (priv->state_flags, buf2, sizeof (buf2))); priv->state_flags = f; _notify (self, PROP_STATE_FLAGS); + + nm_keep_alive_set_settings_connection_watch_visible (priv->keep_alive, + NM_FLAGS_HAS (priv->state_flags, + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY) + ? priv->settings_connection.obj + : NULL); } } @@ -1384,6 +1394,12 @@ set_property (GObject *object, guint prop_id, g_return_if_reached (); _set_activation_type (self, (NMActivationType) i); break; + case PROP_STATE_FLAGS: + /* construct-only */ + priv->state_flags = g_value_get_uint (value); + nm_assert ((guint) priv->state_flags == g_value_get_uint (value)); + nm_assert (!NM_FLAGS_ANY (priv->state_flags, ~NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY)); + break; case PROP_INT_ACTIVATION_REASON: /* construct-only */ i = g_value_get_int (value); @@ -1467,13 +1483,12 @@ constructed (GObject *object) g_steal_pointer (&priv->applied_connection)); } + if (NM_FLAGS_HAS (priv->state_flags, + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY)) + nm_keep_alive_set_settings_connection_watch_visible (priv->keep_alive, priv->settings_connection.obj); + g_return_if_fail (priv->subject); g_return_if_fail (priv->activation_reason != NM_ACTIVATION_REASON_UNSET); - - if (NM_IN_SET ((NMActivationReason) priv->activation_reason, - NM_ACTIVATION_REASON_AUTOCONNECT, - NM_ACTIVATION_REASON_AUTOCONNECT_SLAVES)) - nm_keep_alive_set_settings_connection_watch_visible (priv->keep_alive, priv->settings_connection.obj); } static void @@ -1625,7 +1640,7 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class) obj_properties[PROP_STATE_FLAGS] = g_param_spec_uint (NM_ACTIVE_CONNECTION_STATE_FLAGS, "", "", 0, G_MAXUINT32, NM_ACTIVATION_STATE_FLAG_NONE, - G_PARAM_READABLE | + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); obj_properties[PROP_DEFAULT] = diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index 2b5a6b59ee..1d4ce29e4e 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -163,6 +163,13 @@ nm_active_connection_set_state_flags (NMActiveConnection *self, nm_active_connection_set_state_flags_full (self, state_flags, state_flags); } +static inline void +nm_active_connection_set_state_flags_clear (NMActiveConnection *self, + NMActivationStateFlags state_flags) +{ + nm_active_connection_set_state_flags_full (self, NM_ACTIVATION_STATE_FLAG_NONE, state_flags); +} + NMDevice * nm_active_connection_get_device (NMActiveConnection *self); gboolean nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device); diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index b0cf1f5117..5489ed4956 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -46,6 +46,7 @@ typedef struct { guint64 ac_version_id; NMDeviceState state; bool realized:1; + bool activation_lifetime_bound_to_profile_visiblity:1; NMUnmanFlagOp unmanaged_explicit; NMActivationReason activation_reason; } DeviceCheckpoint; @@ -335,6 +336,9 @@ activate: subject, NM_ACTIVATION_TYPE_MANAGED, dev_checkpoint->activation_reason, + dev_checkpoint->activation_lifetime_bound_to_profile_visiblity + ? NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY + : NM_ACTIVATION_STATE_FLAG_NONE, &local_error)) { _LOGW ("rollback: reactivation of connection %s/%s failed: %s", nm_settings_connection_get_id (connection), @@ -439,6 +443,8 @@ device_checkpoint_create (NMDevice *device) dev_checkpoint->settings_connection = nm_simple_connection_new_clone (nm_settings_connection_get_connection (settings_connection)); dev_checkpoint->ac_version_id = nm_active_connection_version_id_get (NM_ACTIVE_CONNECTION (act_request)); dev_checkpoint->activation_reason = nm_active_connection_get_activation_reason (NM_ACTIVE_CONNECTION (act_request)); + dev_checkpoint->activation_lifetime_bound_to_profile_visiblity = NM_FLAGS_HAS (nm_active_connection_get_state_flags (NM_ACTIVE_CONNECTION (act_request)), + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY); } return dev_checkpoint; diff --git a/src/nm-manager.c b/src/nm-manager.c index b05fbb443a..8bd5a3acc0 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -320,6 +320,7 @@ static NMActiveConnection *_new_active_connection (NMManager *self, NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, GError **error); static void policy_activating_ac_changed (GObject *object, GParamSpec *pspec, gpointer user_data); @@ -2698,6 +2699,18 @@ recheck_assume_connection (NMManager *self, GError *error = NULL; subject = nm_auth_subject_new_internal (); + + /* Note: the lifetime of the activation connection is always bound to the profiles visiblity + * via NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY. + * + * This only makes a difference, if the profile actually has "connection.permissions" + * set to limit visibility (which is not the case for externally managed, generated profiles). + * + * If we assume a previously active connection whose lifetime was unbound, we now bind it + * after restart. That is not correct, and can mean that the profile becomes subject to + * deactivation after restart (if the user logs out). + * + * This should be improved, but it's unclear how. */ active = _new_active_connection (self, FALSE, sett_conn, @@ -2708,6 +2721,7 @@ recheck_assume_connection (NMManager *self, subject, generated ? NM_ACTIVATION_TYPE_EXTERNAL : NM_ACTIVATION_TYPE_ASSUME, generated ? NM_ACTIVATION_REASON_EXTERNAL : NM_ACTIVATION_REASON_ASSUME, + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY, &error); if (!active) { @@ -3892,11 +3906,16 @@ ensure_master_active_connection (NMManager *self, GError **error) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMActiveConnection *ac; NMActiveConnection *master_ac = NULL; NMDeviceState master_state; + gboolean bind_lifetime_to_profile_visibility; - g_assert (connection); - g_assert (master_connection || master_device); + g_return_val_if_fail (connection, NULL); + g_return_val_if_fail (master_connection || master_device, FALSE); + + bind_lifetime_to_profile_visibility = NM_FLAGS_HAS (nm_device_get_activation_state_flags (device), + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY); /* If the master device isn't activated then we need to activate it using * compatible connection. If it's already activating we can just proceed. @@ -3921,8 +3940,16 @@ ensure_master_active_connection (NMManager *self, if ( (master_state == NM_DEVICE_STATE_ACTIVATED) || nm_device_is_activating (master_device)) { /* Device already using master_connection */ - g_assert (device_connection); - return NM_ACTIVE_CONNECTION (nm_device_get_act_request (master_device)); + ac = NM_ACTIVE_CONNECTION (nm_device_get_act_request (master_device)); + g_return_val_if_fail (device_connection, ac); + + if (!bind_lifetime_to_profile_visibility) { + /* unbind the lifetime. */ + nm_active_connection_set_state_flags_clear (ac, + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY); + } + + return ac; } /* If the device is disconnected, find a compatible connection and @@ -3959,6 +3986,9 @@ ensure_master_active_connection (NMManager *self, subject, NM_ACTIVATION_TYPE_MANAGED, activation_reason, + bind_lifetime_to_profile_visibility + ? NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY + : NM_ACTIVATION_STATE_FLAG_NONE, error); return master_ac; } @@ -4007,6 +4037,9 @@ ensure_master_active_connection (NMManager *self, subject, NM_ACTIVATION_TYPE_MANAGED, activation_reason, + bind_lifetime_to_profile_visibility + ? NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY + : NM_ACTIVATION_STATE_FLAG_NONE, error); return master_ac; } @@ -4168,6 +4201,7 @@ autoconnect_slaves (NMManager *self, master_device)) { gs_free SlaveConnectionInfo *slaves = NULL; guint i, n_slaves = 0; + gboolean bind_lifetime_to_profile_visibility; slaves = find_slaves (self, master_connection, master_device, &n_slaves); if (n_slaves > 1) { @@ -4182,6 +4216,10 @@ autoconnect_slaves (NMManager *self, GINT_TO_POINTER (!nm_streq0 (value, "index"))); } + bind_lifetime_to_profile_visibility = n_slaves > 0 + && NM_FLAGS_HAS (nm_device_get_activation_state_flags (master_device), + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY); + for (i = 0; i < n_slaves; i++) { SlaveConnectionInfo *slave = &slaves[i]; const char *uuid; @@ -4236,6 +4274,9 @@ autoconnect_slaves (NMManager *self, subject, NM_ACTIVATION_TYPE_MANAGED, NM_ACTIVATION_REASON_AUTOCONNECT_SLAVES, + bind_lifetime_to_profile_visibility + ? NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY + : NM_ACTIVATION_STATE_FLAG_NONE, &local_err); if (local_err) { _LOGW (LOGD_CORE, "Slave connection activation failed: %s", local_err->message); @@ -4296,6 +4337,40 @@ unmanaged_to_disconnected (NMDevice *device) } } +static NMActivationStateFlags +_activation_bind_lifetime_to_profile_visibility (NMAuthSubject *subject) +{ + if ( nm_auth_subject_is_internal (subject) + || nm_auth_subject_get_unix_process_uid (subject) == 0) { + /* internal requests and requests from root are always unbound. */ + return NM_ACTIVATION_STATE_FLAG_NONE; + } + + /* if the activation was not done by internal decision nor root, there + * are the following cases: + * + * - the connection has "connection.permissions" unset and the profile + * is not restricted to a user and commonly always visible. It does + * not hurt to bind the lifetime, because we expect the profile to be + * visible at the moment. If the profile changes (while still being active), + * we want to pick-up changes to the visibility and possibly disconnect. + * + * - the connection has "connection.permissions" set, and the current user + * is the owner: + * + * - Usually, we would expect that the profile is visible at the moment, + * and of course we want to bind the lifetime. The moment the user + * logs out, the connection becomes invisible and disconnects. + * + * - the profile at this time could already be invisible (e.g. if the + * user didn't ceate a proper session (sudo) and manually activates + * an invisible profile. In this case, we still want to bind the + * lifetime, and it will disconnect after the user logs in and logs + * out again. NMKeepAlive takes care of that. + */ + return NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY; +} + /* The parent connection is ready; we can proceed realizing the device and * progressing the device to disconencted state. */ @@ -4425,6 +4500,8 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * subject, NM_ACTIVATION_TYPE_MANAGED, nm_active_connection_get_activation_reason (active), + nm_active_connection_get_state_flags (active) + & NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY, error); if (!parent_ac) { g_prefix_error (error, "%s failed to activate parent: ", nm_device_get_iface (device)); @@ -4625,6 +4702,7 @@ _new_active_connection (NMManager *self, NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, GError **error) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); @@ -4696,6 +4774,7 @@ _new_active_connection (NMManager *self, parent_device, nm_dbus_object_get_path (NM_DBUS_OBJECT (parent)), activation_reason, + initial_state_flags, subject); } @@ -4705,6 +4784,7 @@ _new_active_connection (NMManager *self, subject, activation_type, activation_reason, + initial_state_flags, device); } @@ -4768,6 +4848,7 @@ fail: * @activation_type: whether to assume the connection. That is, take over gracefully, * non-destructible. * @activation_reason: the reason for activation + * @initial_state_flags: the inital state flags for the activation. * @error: return location for an error * * Begins a new internally-initiated activation of @sett_conn on @device. @@ -4789,6 +4870,7 @@ nm_manager_activate_connection (NMManager *self, NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, GError **error) { NMManagerPrivate *priv; @@ -4842,6 +4924,7 @@ nm_manager_activate_connection (NMManager *self, subject, activation_type, activation_reason, + initial_state_flags, error); if (!active) return NULL; @@ -5101,6 +5184,7 @@ impl_manager_activate_connection (NMDBusObject *obj, subject, NM_ACTIVATION_TYPE_MANAGED, NM_ACTIVATION_REASON_USER_REQUEST, + _activation_bind_lifetime_to_profile_visibility (subject), &error); if (!active) goto error; @@ -5375,6 +5459,7 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj, subject, NM_ACTIVATION_TYPE_MANAGED, NM_ACTIVATION_REASON_USER_REQUEST, + _activation_bind_lifetime_to_profile_visibility (subject), &error); if (!active) goto error; diff --git a/src/nm-manager.h b/src/nm-manager.h index 4b159f1ebd..ecb4b0172d 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -175,6 +175,7 @@ NMActiveConnection *nm_manager_activate_connection (NMManager *manager, NMAuthSubject *subject, NMActivationType activation_type, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, GError **error); gboolean nm_manager_deactivate_connection (NMManager *manager, diff --git a/src/nm-policy.c b/src/nm-policy.c index ab9571f6df..b9e26f604e 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1284,6 +1284,7 @@ auto_activate_device (NMPolicy *self, subject, NM_ACTIVATION_TYPE_MANAGED, NM_ACTIVATION_REASON_AUTOCONNECT, + NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY, &error); if (!ac) { _LOGI (LOGD_DEVICE, "connection '%s' auto-activation failed: %s", @@ -1674,9 +1675,14 @@ activate_secondary_connections (NMPolicy *self, GError *error = NULL; guint32 i; gboolean success = TRUE; + NMActivationStateFlags initial_state_flags; s_con = nm_connection_get_setting_connection (connection); - nm_assert (s_con); + nm_assert (NM_IS_SETTING_CONNECTION (s_con)); + + /* we propagate the activation's state flags. */ + initial_state_flags = nm_device_get_activation_state_flags (device) + & NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY; for (i = 0; i < nm_setting_connection_get_num_secondaries (s_con); i++) { NMSettingsConnection *sett_conn; @@ -1700,7 +1706,6 @@ activate_secondary_connections (NMPolicy *self, } req = nm_device_get_act_request (device); - g_assert (req); _LOGD (LOGD_DEVICE, "activating secondary connection '%s (%s)' for base connection '%s (%s)'", nm_settings_connection_get_id (sett_conn), sec_uuid, @@ -1713,6 +1718,7 @@ activate_secondary_connections (NMPolicy *self, nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (req)), NM_ACTIVATION_TYPE_MANAGED, nm_active_connection_get_activation_reason (NM_ACTIVE_CONNECTION (req)), + initial_state_flags, &error); if (ac) secondary_ac_list = g_slist_append (secondary_ac_list, g_object_ref (ac)); @@ -2162,6 +2168,8 @@ vpn_connection_retry_after_failure (NMVpnConnection *vpn, NMPolicy *self) nm_active_connection_get_subject (ac), NM_ACTIVATION_TYPE_MANAGED, nm_active_connection_get_activation_reason (ac), + ( nm_active_connection_get_state_flags (ac) + & NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY), &error)) { _LOGW (LOGD_DEVICE, "VPN '%s' reconnect failed: %s", nm_settings_connection_get_id (connection), diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index 7f606c2554..d4f9a5d54b 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -851,6 +851,7 @@ nm_vpn_connection_new (NMSettingsConnection *settings_connection, NMDevice *parent_device, const char *specific_object, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, NMAuthSubject *subject) { g_return_val_if_fail (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection), NULL); @@ -864,6 +865,7 @@ nm_vpn_connection_new (NMSettingsConnection *settings_connection, NM_ACTIVE_CONNECTION_INT_SUBJECT, subject, NM_ACTIVE_CONNECTION_INT_ACTIVATION_REASON, activation_reason, NM_ACTIVE_CONNECTION_VPN, TRUE, + NM_ACTIVE_CONNECTION_STATE_FLAGS, (guint) initial_state_flags, NULL); } diff --git a/src/vpn/nm-vpn-connection.h b/src/vpn/nm-vpn-connection.h index e409cc314f..5482fb0d8b 100644 --- a/src/vpn/nm-vpn-connection.h +++ b/src/vpn/nm-vpn-connection.h @@ -53,6 +53,7 @@ NMVpnConnection * nm_vpn_connection_new (NMSettingsConnection *settings_connecti NMDevice *parent_device, const char *specific_object, NMActivationReason activation_reason, + NMActivationStateFlags initial_state_flags, NMAuthSubject *subject); void nm_vpn_connection_activate (NMVpnConnection *self,