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

@@ -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_default (active, FALSE);
-		nm_active_connection_set_default6 (NM_ACTIVE_CONNECTION (self), 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;
 }
 

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 */
+		/* Device is no longer relevant when deactivated; emit property change
-		g_clear_object (&priv->device);
+		 * 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) {

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

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) {

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_vpn4_config (parent_dev, NULL);
-	nm_device_set_vpn6_config (priv->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,
+                      NMDeviceState old_state)
-                      NMDeviceStateReason reason,
-                      gpointer user_data)
 {
-	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);
+	NM_VPN_CONNECTION_GET_PRIVATE (object)->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);
 }
 
 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);
 

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