From a7bab4015ebf12714f6e7ea6303cb46df6465058 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 27 Sep 2013 14:40:18 -0500 Subject: [PATCH] core: have ActiveConnection track device state instead of subclasses Both NMActRequest and NMVPNConnection need to track their device's state, so instead of both subclasses having to do so, consolidate that code into the superclass. --- src/nm-activation-request.c | 48 ++++----------------- src/nm-active-connection.c | 62 +++++++++++++++++++++++++-- src/nm-active-connection.h | 9 +++- src/nm-policy.c | 12 +++--- src/vpn-manager/nm-vpn-connection.c | 65 +++++++++-------------------- src/vpn-manager/nm-vpn-connection.h | 1 - 6 files changed, 101 insertions(+), 96 deletions(-) diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c index 61d7782d44..922af93a08 100644 --- a/src/nm-activation-request.c +++ b/src/nm-activation-request.c @@ -51,7 +51,6 @@ typedef struct { } ShareRule; typedef struct { - guint device_state_id; GSList *secrets_calls; gboolean shared; GSList *share_rules; @@ -286,15 +285,15 @@ nm_act_request_add_share_rule (NMActRequest *req, /********************************************************************/ static void -device_state_changed (NMDevice *device, GParamSpec *pspec, NMActRequest *self) +device_state_changed (NMActiveConnection *active, + NMDevice *device, + NMDeviceState new_state, + NMDeviceState old_state) { - NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (self); NMActiveConnectionState ac_state = NM_ACTIVE_CONNECTION_STATE_UNKNOWN; - g_return_if_fail (device == nm_active_connection_get_device (NM_ACTIVE_CONNECTION (self))); - /* Set NMActiveConnection state based on the device's state */ - switch (nm_device_get_state (device)) { + switch (new_state) { case NM_DEVICE_STATE_PREPARE: case NM_DEVICE_STATE_CONFIG: case NM_DEVICE_STATE_NEED_AUTH: @@ -314,12 +313,6 @@ device_state_changed (NMDevice *device, GParamSpec *pspec, NMActRequest *self) case NM_DEVICE_STATE_UNMANAGED: case NM_DEVICE_STATE_UNAVAILABLE: ac_state = NM_ACTIVE_CONNECTION_STATE_DEACTIVATED; - - /* No longer need to pay attention to device state */ - if (device && priv->device_state_id) { - g_signal_handler_disconnect (device, priv->device_state_id); - priv->device_state_id = 0; - } break; default: break; @@ -327,11 +320,11 @@ device_state_changed (NMDevice *device, GParamSpec *pspec, NMActRequest *self) if ( ac_state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED || ac_state == NM_ACTIVE_CONNECTION_STATE_UNKNOWN) { - nm_active_connection_set_default (NM_ACTIVE_CONNECTION (self), FALSE); - nm_active_connection_set_default6 (NM_ACTIVE_CONNECTION (self), FALSE); + nm_active_connection_set_default (active, FALSE); + nm_active_connection_set_default6 (active, FALSE); } - nm_active_connection_set_state (NM_ACTIVE_CONNECTION (self), ac_state); + nm_active_connection_set_state (active, ac_state); } static void @@ -394,36 +387,13 @@ nm_act_request_init (NMActRequest *req) { } -static void -constructed (GObject *object) -{ - NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (object); - NMDevice *device; - - G_OBJECT_CLASS (nm_act_request_parent_class)->constructed (object); - - device = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (object)); - g_assert (device); - priv->device_state_id = g_signal_connect (device, - "notify::" NM_DEVICE_STATE, - G_CALLBACK (device_state_changed), - NM_ACT_REQUEST (object)); -} - static void dispose (GObject *object) { NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (object); NMConnection *connection; - NMDevice *device; GSList *iter; - device = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (object)); - if (device && priv->device_state_id) { - g_signal_handler_disconnect (device, priv->device_state_id); - priv->device_state_id = 0; - } - /* Clear any share rules */ if (priv->share_rules) { nm_act_request_set_shared (NM_ACT_REQUEST (object), FALSE); @@ -453,8 +423,8 @@ nm_act_request_class_init (NMActRequestClass *req_class) g_type_class_add_private (req_class, sizeof (NMActRequestPrivate)); /* virtual methods */ - object_class->constructed = constructed; object_class->dispose = dispose; active_class->master_failed = master_failed; + active_class->device_state_changed = device_state_changed; } diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 3de997b9a9..168d92df33 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -44,6 +44,7 @@ typedef struct { char *path; char *specific_object; NMDevice *device; + guint32 device_state_id; gboolean is_default; gboolean is_default6; @@ -119,8 +120,10 @@ nm_active_connection_set_state (NMActiveConnection *self, } if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { - /* Device is no longer relevant when deactivated */ - g_clear_object (&priv->device); + /* Device is no longer relevant when deactivated; emit property change + * notification so clients re-read the value, which will be NULL due to + * conditions in get_property(). + */ g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES); } } @@ -278,6 +281,45 @@ nm_active_connection_get_device (NMActiveConnection *self) return NM_ACTIVE_CONNECTION_GET_PRIVATE (self)->device; } +static void +device_state_changed (NMDevice *device, + NMDeviceState new_state, + NMDeviceState old_state, + NMDeviceStateReason reason, + gpointer user_data) +{ + NMActiveConnection *self = NM_ACTIVE_CONNECTION (user_data); + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + + if (old_state < NM_DEVICE_STATE_DISCONNECTED) + return; + + if (old_state > NM_DEVICE_STATE_DISCONNECTED) { + /* Ignore disconnects if this ActiveConnection has not yet started + * activating. This is caused by activating a device when it's + * already activated, which causes a deactivating of the device before + * activating the new connection. + */ + if (new_state == NM_DEVICE_STATE_DISCONNECTED && + old_state > NM_DEVICE_STATE_DISCONNECTED && + priv->state == NM_ACTIVE_CONNECTION_STATE_UNKNOWN) { + return; + } + + /* If the device used to be active, but now is disconnected/failed, we + * no longer care about its state. + */ + if (new_state <= NM_DEVICE_STATE_DISCONNECTED || new_state == NM_DEVICE_STATE_FAILED) { + g_signal_handler_disconnect (device, priv->device_state_id); + priv->device_state_id = 0; + } + } + + /* Let subclasses handle the state change */ + if (NM_ACTIVE_CONNECTION_GET_CLASS (self)->device_state_changed) + NM_ACTIVE_CONNECTION_GET_CLASS (self)->device_state_changed (self, device, new_state, old_state); +} + NMActiveConnection * nm_active_connection_get_master (NMActiveConnection *self) { @@ -523,12 +565,18 @@ set_property (GObject *object, guint prop_id, priv->connection = g_value_dup_object (value); break; case PROP_INT_DEVICE: - g_warn_if_fail (priv->device == NULL); + g_return_if_fail (priv->device == NULL); priv->device = g_value_dup_object (value); if (priv->device && priv->master) { master_device = nm_active_connection_get_device (priv->master); g_warn_if_fail (priv->device != master_device); } + if (priv->device) { + priv->device_state_id = g_signal_connect (priv->device, + "state-changed", + G_CALLBACK (device_state_changed), + NM_ACTIVE_CONNECTION (object)); + } break; case PROP_INT_SUBJECT: priv->subject = g_value_dup_object (value); @@ -579,7 +627,7 @@ get_property (GObject *object, guint prop_id, break; case PROP_DEVICES: devices = g_ptr_array_sized_new (1); - if (priv->device) + if (priv->device && priv->state < NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) g_ptr_array_add (devices, g_strdup (nm_device_get_path (priv->device))); g_value_take_boxed (value, devices); break; @@ -628,6 +676,12 @@ dispose (GObject *object) priv->specific_object = NULL; g_clear_object (&priv->connection); + + if (priv->device_state_id) { + g_assert (priv->device); + g_signal_handler_disconnect (priv->device, priv->device_state_id); + priv->device_state_id = 0; + } g_clear_object (&priv->device); if (priv->master) { diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index 835266b3f1..acb2f4dd52 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -58,7 +58,14 @@ typedef struct { typedef struct { GObjectClass parent; - void (*master_failed) (NMActiveConnection *connection); + /* re-emits device state changes as a convenience for subclasses for + * device states >= DISCONNECTED. + */ + void (*device_state_changed) (NMActiveConnection *connection, + NMDevice *device, + NMDeviceState new_state, + NMDeviceState old_state); + void (*master_failed) (NMActiveConnection *connection); } NMActiveConnectionClass; GType nm_active_connection_get_type (void); diff --git a/src/nm-policy.c b/src/nm-policy.c index 9f480032e2..96a85208ad 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -669,7 +669,7 @@ update_ip4_routing (NMPolicy *policy, gboolean force_update) gw_addr = nm_ip4_config_get_gateway (ip4_config); if (vpn) { - NMDevice *parent = nm_vpn_connection_get_parent_device (vpn); + NMDevice *parent = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); int parent_ifindex = nm_device_get_ip_ifindex (parent); NMIP4Config *parent_ip4 = nm_device_get_ip4_config (parent); guint32 parent_mss = parent_ip4 ? nm_ip4_config_get_mss (parent_ip4) : 0; @@ -683,7 +683,7 @@ update_ip4_routing (NMPolicy *policy, gboolean force_update) } } - default_device = nm_vpn_connection_get_parent_device (vpn); + default_device = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); } else { int mss = nm_ip4_config_get_mss (ip4_config); @@ -856,7 +856,7 @@ update_ip6_routing (NMPolicy *policy, gboolean force_update) gw_addr = &in6addr_any; if (vpn) { - NMDevice *parent = nm_vpn_connection_get_parent_device (vpn); + NMDevice *parent = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); int parent_ifindex = nm_device_get_ip_ifindex (parent); NMIP6Config *parent_ip6 = nm_device_get_ip6_config (parent); guint32 parent_mss = parent_ip6 ? nm_ip6_config_get_mss (parent_ip6) : 0; @@ -873,7 +873,7 @@ update_ip6_routing (NMPolicy *policy, gboolean force_update) } } - default_device6 = nm_vpn_connection_get_parent_device (vpn); + default_device6 = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); } else { int mss = nm_ip6_config_get_mss (ip6_config); @@ -1136,7 +1136,7 @@ process_secondaries (NMPolicy *policy, ac_path = nm_active_connection_get_path (active); if (NM_IS_VPN_CONNECTION (active)) - device = nm_vpn_connection_get_parent_device (NM_VPN_CONNECTION (active)); + device = nm_active_connection_get_device (active); for (iter = priv->pending_secondaries; iter; iter = g_slist_next (iter)) { PendingSecondaryData *secondary_data = (PendingSecondaryData *) iter->data; @@ -1740,7 +1740,7 @@ vpn_connection_deactivated (NMPolicy *policy, NMVPNConnection *vpn) nm_dns_manager_begin_updates (mgr, __func__); ip_iface = nm_vpn_connection_get_ip_iface (vpn); - parent = nm_vpn_connection_get_parent_device (vpn); + parent = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); ip4_config = nm_vpn_connection_get_ip4_config (vpn); if (ip4_config) { diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index ec66732d2f..6962e301d0 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -71,9 +71,6 @@ typedef struct { SecretsReq secrets_idx; char *username; - NMDevice *parent_dev; - gulong device_monitor; - NMVPNConnectionState vpn_state; NMVPNConnectionStateReason failure_reason; DBusGProxy *proxy; @@ -163,7 +160,7 @@ call_plugin_disconnect (NMVPNConnection *self) } static void -vpn_cleanup (NMVPNConnection *connection) +vpn_cleanup (NMVPNConnection *connection, NMDevice *parent_dev) { NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (connection); @@ -173,8 +170,8 @@ vpn_cleanup (NMVPNConnection *connection) nm_platform_address_flush (priv->ip_ifindex); } - nm_device_set_vpn4_config (priv->parent_dev, NULL); - nm_device_set_vpn6_config (priv->parent_dev, NULL); + nm_device_set_vpn4_config (parent_dev, NULL); + nm_device_set_vpn6_config (parent_dev, NULL); g_free (priv->banner); priv->banner = NULL; @@ -197,6 +194,7 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, { NMVPNConnectionPrivate *priv; NMVPNConnectionState old_vpn_state; + NMDevice *parent_dev = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (connection)); g_return_if_fail (NM_IS_VPN_CONNECTION (connection)); @@ -242,7 +240,7 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, /* Let dispatcher scripts know we're up and running */ nm_dispatcher_call_vpn (DISPATCHER_ACTION_VPN_UP, priv->connection, - priv->parent_dev, + parent_dev, priv->ip_iface, priv->ip4_config, priv->ip6_config, @@ -255,7 +253,7 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, /* Let dispatcher scripts know we're about to go down */ nm_dispatcher_call_vpn (DISPATCHER_ACTION_VPN_DOWN, priv->connection, - priv->parent_dev, + parent_dev, priv->ip_iface, NULL, NULL, @@ -265,7 +263,7 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, /* Tear down and clean up the connection */ call_plugin_disconnect (connection); - vpn_cleanup (connection); + vpn_cleanup (connection, parent_dev); /* Fall through */ default: priv->secrets_idx = SECRETS_REQ_SYSTEM; @@ -276,20 +274,17 @@ nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, } static void -device_state_changed (NMDevice *device, +device_state_changed (NMActiveConnection *active, + NMDevice *device, NMDeviceState new_state, - NMDeviceState old_state, - NMDeviceStateReason reason, - gpointer user_data) + NMDeviceState old_state) { - NMVPNConnection *connection = NM_VPN_CONNECTION (user_data); - if (new_state <= NM_DEVICE_STATE_DISCONNECTED) { - nm_vpn_connection_set_vpn_state (connection, + nm_vpn_connection_set_vpn_state (NM_VPN_CONNECTION (active), NM_VPN_CONNECTION_STATE_DISCONNECTED, NM_VPN_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); } else if (new_state == NM_DEVICE_STATE_FAILED) { - nm_vpn_connection_set_vpn_state (connection, + nm_vpn_connection_set_vpn_state (NM_VPN_CONNECTION (active), NM_VPN_CONNECTION_STATE_FAILED, NM_VPN_CONNECTION_STATE_REASON_DEVICE_DISCONNECTED); } @@ -655,6 +650,7 @@ static gboolean nm_vpn_connection_apply_config (NMVPNConnection *connection) { NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (connection); + NMDevice *parent_dev = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (connection)); nm_platform_link_set_up (priv->ip_ifindex); @@ -671,9 +667,9 @@ nm_vpn_connection_apply_config (NMVPNConnection *connection) /* Add any explicit route to the VPN gateway through the parent device */ if (priv->ip4_external_gw) - add_ip4_vpn_gateway_route (priv->parent_dev, priv->ip4_external_gw); + add_ip4_vpn_gateway_route (parent_dev, priv->ip4_external_gw); if (priv->ip6_external_gw) - add_ip6_vpn_gateway_route (priv->parent_dev, priv->ip6_external_gw); + add_ip6_vpn_gateway_route (parent_dev, priv->ip6_external_gw); nm_log_info (LOGD_VPN, "VPN connection '%s' (IP Config Get) complete.", nm_connection_get_id (priv->connection)); @@ -1400,14 +1396,6 @@ nm_vpn_connection_get_ip_ifindex (NMVPNConnection *connection) return NM_VPN_CONNECTION_GET_PRIVATE (connection)->ip_ifindex; } -NMDevice * -nm_vpn_connection_get_parent_device (NMVPNConnection *connection) -{ - g_return_val_if_fail (NM_IS_VPN_CONNECTION (connection), NULL); - - return NM_VPN_CONNECTION_GET_PRIVATE (connection)->parent_dev; -} - guint32 nm_vpn_connection_get_ip4_internal_gateway (NMVPNConnection *connection) { @@ -1677,23 +1665,12 @@ nm_vpn_connection_init (NMVPNConnection *self) static void constructed (GObject *object) { - NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (object); NMConnection *connection; - NMDevice *device; G_OBJECT_CLASS (nm_vpn_connection_parent_class)->constructed (object); connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (object)); - priv->connection = g_object_ref (connection); - - device = (NMDevice *) nm_active_connection_get_device (NM_ACTIVE_CONNECTION (object)); - g_assert (device); - - priv->parent_dev = g_object_ref (device); - - priv->device_monitor = g_signal_connect (device, "state-changed", - G_CALLBACK (device_state_changed), - object); + NM_VPN_CONNECTION_GET_PRIVATE (object)->connection = g_object_ref (connection); } static void @@ -1715,11 +1692,6 @@ dispose (GObject *object) if (priv->ip6_external_gw) g_free (priv->ip6_external_gw); - if (priv->device_monitor) - g_signal_handler_disconnect (priv->parent_dev, priv->device_monitor); - - g_clear_object (&priv->parent_dev); - if (priv->ip4_config) g_object_unref (priv->ip4_config); if (priv->ip6_config) @@ -1758,6 +1730,7 @@ get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) { NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (object); + NMDevice *parent_dev; switch (prop_id) { case PROP_VPN_STATE: @@ -1767,7 +1740,8 @@ get_property (GObject *object, guint prop_id, g_value_set_string (value, priv->banner ? priv->banner : ""); break; case PROP_MASTER: - g_value_set_boxed (value, nm_device_get_path (priv->parent_dev)); + parent_dev = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (object)); + g_value_set_boxed (value, parent_dev ? nm_device_get_path (parent_dev) : "/"); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -1789,6 +1763,7 @@ nm_vpn_connection_class_init (NMVPNConnectionClass *connection_class) object_class->dispose = dispose; object_class->finalize = finalize; active_class->master_failed = master_failed; + active_class->device_state_changed = device_state_changed; g_object_class_override_property (object_class, PROP_MASTER, NM_ACTIVE_CONNECTION_MASTER); diff --git a/src/vpn-manager/nm-vpn-connection.h b/src/vpn-manager/nm-vpn-connection.h index d0554faee2..b98dcbe629 100644 --- a/src/vpn-manager/nm-vpn-connection.h +++ b/src/vpn-manager/nm-vpn-connection.h @@ -82,7 +82,6 @@ NMIP4Config * nm_vpn_connection_get_ip4_config (NMVPNConnection *connect NMIP6Config * nm_vpn_connection_get_ip6_config (NMVPNConnection *connection); const char * nm_vpn_connection_get_ip_iface (NMVPNConnection *connection); int nm_vpn_connection_get_ip_ifindex (NMVPNConnection *connection); -NMDevice * nm_vpn_connection_get_parent_device (NMVPNConnection *connection); guint32 nm_vpn_connection_get_ip4_internal_gateway (NMVPNConnection *connection); struct in6_addr * nm_vpn_connection_get_ip6_internal_gateway (NMVPNConnection *connection);