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.
This commit is contained in:
Thomas Haller 2018-11-20 14:51:22 +01:00
parent 9e8c3d2ebf
commit f95a526366
4 changed files with 62 additions and 65 deletions

View file

@ -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] =

View file

@ -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__ */

View file

@ -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,

View file

@ -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);
}