From f9404d36fdbcf91a7f6ec358ee6aef2b448428c7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 5 Dec 2015 16:03:54 +0100 Subject: [PATCH 1/3] wifi: fix supplicant_connection_timeout_cb() using settings-connection (gdb) bt #0 0x00007fc1c920681b in g_logv () at /lib64/libglib-2.0.so.0 #1 0x00007fc1c920698f in g_log () at /lib64/libglib-2.0.so.0 #2 0x00007fc1c9523237 in g_type_check_instance_cast () at /lib64/libgobject-2.0.so.0 #3 0x00007fc1bdef10ed in supplicant_connection_timeout_cb (user_data=0x561a52451600) at nm-device-wifi.c:2207 #4 0x00007fc1c9200893 in g_timeout_dispatch () at /lib64/libglib-2.0.so.0 #5 0x00007fc1c91ffe3a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0 #6 0x00007fc1c92001d0 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0 #7 0x00007fc1c92004f2 in g_main_loop_run () at /lib64/libglib-2.0.so.0 #8 0x0000561a511583f3 in main (argc=1, argv=0x7ffc033f1e28) at main.c:488 --- src/devices/wifi/nm-device-wifi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 72540efed4..8f3630c84a 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2204,7 +2204,7 @@ supplicant_connection_timeout_cb (gpointer user_data) * dialogs, just retry or fail, and if we never connect the user can * fix the password somewhere else. */ - if (nm_settings_connection_get_timestamp (NM_SETTINGS_CONNECTION (connection), ×tamp)) + if (nm_settings_connection_get_timestamp (nm_act_request_get_settings_connection (req), ×tamp)) new_secrets = !timestamp; if (handle_auth_or_fail (self, req, new_secrets) == NM_ACT_STAGE_RETURN_POSTPONE) From 9a8d9a0d855cfe2f82b171d48f8efbedc4f4d417 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Dec 2015 10:33:19 +0100 Subject: [PATCH 2/3] device: implement slave property in parent device class Instead of reimplementing the slave property in bond, bridge and team, just add the property to the parent class. It's not that the parent class would be agnostic to the master/slave implementation, all the slaves are known to the every device type implementation. Also, the derived class doesn't know the correct time when to invoke the notify-changed for the slaves property. E.g. it should be only invoked after nm_device_slave_notify_enslave() when other components also consider the slave as enslaved. Later this will be fixed so that the SLAVES property correspond to what other master/slave related properties say. --- src/devices/nm-device-bond.c | 53 ++----------------------------- src/devices/nm-device-bond.h | 2 -- src/devices/nm-device-bridge.c | 53 ++----------------------------- src/devices/nm-device-bridge.h | 2 -- src/devices/nm-device.c | 28 +++++++++++++++- src/devices/nm-device.h | 6 +++- src/devices/team/nm-device-team.c | 53 ++----------------------------- src/devices/team/nm-device-team.h | 2 -- 8 files changed, 38 insertions(+), 161 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index cd1e1053f5..62e9860ee9 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -46,13 +46,6 @@ typedef struct { int dummy; } NMDeviceBondPrivate; -enum { - PROP_0, - PROP_SLAVES, - - LAST_PROP -}; - /******************************************************************/ static NMDeviceCapabilities @@ -415,7 +408,7 @@ enslave_slave (NMDevice *device, } else _LOGI (LOGD_BOND, "bond slave %s was enslaved", slave_iface); - g_object_notify (G_OBJECT (device), NM_DEVICE_BOND_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); return TRUE; } @@ -444,7 +437,7 @@ release_slave (NMDevice *device, nm_device_get_ip_iface (slave)); } - g_object_notify (G_OBJECT (device), NM_DEVICE_BOND_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); if (configure) { /* Kernel bonding code "closes" the slave when releasing it, (which clears @@ -488,36 +481,6 @@ nm_device_bond_init (NMDeviceBond * self) { } -static void -get_property (GObject *object, guint prop_id, - GValue *value, GParamSpec *pspec) -{ - GSList *list; - - switch (prop_id) { - break; - case PROP_SLAVES: - list = nm_device_master_get_slaves (NM_DEVICE (object)); - nm_utils_g_value_set_object_path_array (value, list, NULL, NULL); - g_slist_free (list); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - -static void -set_property (GObject *object, guint prop_id, - const GValue *value, GParamSpec *pspec) -{ - switch (prop_id) { - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - static void nm_device_bond_class_init (NMDeviceBondClass *klass) { @@ -528,10 +491,6 @@ nm_device_bond_class_init (NMDeviceBondClass *klass) NM_DEVICE_CLASS_DECLARE_TYPES (klass, NM_SETTING_BOND_SETTING_NAME, NM_LINK_TYPE_BOND) - /* virtual methods */ - object_class->get_property = get_property; - object_class->set_property = set_property; - parent_class->get_generic_capabilities = get_generic_capabilities; parent_class->is_available = is_available; parent_class->check_connection_compatible = check_connection_compatible; @@ -547,14 +506,6 @@ nm_device_bond_class_init (NMDeviceBondClass *klass) parent_class->enslave_slave = enslave_slave; parent_class->release_slave = release_slave; - /* properties */ - g_object_class_install_property - (object_class, PROP_SLAVES, - g_param_spec_boxed (NM_DEVICE_BOND_SLAVES, "", "", - G_TYPE_STRV, - G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS)); - nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (klass), NMDBUS_TYPE_DEVICE_BOND_SKELETON, NULL); diff --git a/src/devices/nm-device-bond.h b/src/devices/nm-device-bond.h index 8165bb8062..9e86719e3c 100644 --- a/src/devices/nm-device-bond.h +++ b/src/devices/nm-device-bond.h @@ -32,8 +32,6 @@ G_BEGIN_DECLS #define NM_IS_DEVICE_BOND_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DEVICE_BOND)) #define NM_DEVICE_BOND_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DEVICE_BOND, NMDeviceBondClass)) -#define NM_DEVICE_BOND_SLAVES "slaves" - typedef NMDevice NMDeviceBond; typedef NMDeviceClass NMDeviceBondClass; diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index b8887f444b..4a20f46b4d 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -44,13 +44,6 @@ typedef struct { int dummy; } NMDeviceBridgePrivate; -enum { - PROP_0, - PROP_SLAVES, - - LAST_PROP -}; - /******************************************************************/ static NMDeviceCapabilities @@ -351,7 +344,7 @@ enslave_slave (NMDevice *device, nm_device_get_ip_iface (slave)); } - g_object_notify (G_OBJECT (device), NM_DEVICE_BRIDGE_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); return TRUE; } @@ -381,7 +374,7 @@ release_slave (NMDevice *device, nm_device_get_ip_iface (slave)); } - g_object_notify (G_OBJECT (device), NM_DEVICE_BRIDGE_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); } static gboolean @@ -436,36 +429,6 @@ nm_device_bridge_init (NMDeviceBridge * self) { } -static void -get_property (GObject *object, guint prop_id, - GValue *value, GParamSpec *pspec) -{ - GSList *list; - - switch (prop_id) { - break; - case PROP_SLAVES: - list = nm_device_master_get_slaves (NM_DEVICE (object)); - nm_utils_g_value_set_object_path_array (value, list, NULL, NULL); - g_slist_free (list); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - -static void -set_property (GObject *object, guint prop_id, - const GValue *value, GParamSpec *pspec) -{ - switch (prop_id) { - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - static void nm_device_bridge_class_init (NMDeviceBridgeClass *klass) { @@ -476,10 +439,6 @@ nm_device_bridge_class_init (NMDeviceBridgeClass *klass) NM_DEVICE_CLASS_DECLARE_TYPES (klass, NM_SETTING_BRIDGE_SETTING_NAME, NM_LINK_TYPE_BRIDGE) - /* virtual methods */ - object_class->get_property = get_property; - object_class->set_property = set_property; - parent_class->get_generic_capabilities = get_generic_capabilities; parent_class->is_available = is_available; parent_class->check_connection_compatible = check_connection_compatible; @@ -494,14 +453,6 @@ nm_device_bridge_class_init (NMDeviceBridgeClass *klass) parent_class->enslave_slave = enslave_slave; parent_class->release_slave = release_slave; - /* properties */ - g_object_class_install_property - (object_class, PROP_SLAVES, - g_param_spec_boxed (NM_DEVICE_BRIDGE_SLAVES, "", "", - G_TYPE_STRV, - G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS)); - nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (klass), NMDBUS_TYPE_DEVICE_BRIDGE_SKELETON, NULL); diff --git a/src/devices/nm-device-bridge.h b/src/devices/nm-device-bridge.h index 1d0bf7ca0c..4648267525 100644 --- a/src/devices/nm-device-bridge.h +++ b/src/devices/nm-device-bridge.h @@ -33,8 +33,6 @@ G_BEGIN_DECLS #define NM_IS_DEVICE_BRIDGE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DEVICE_BRIDGE)) #define NM_DEVICE_BRIDGE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DEVICE_BRIDGE, NMDeviceBridgeClass)) -#define NM_DEVICE_BRIDGE_SLAVES "slaves" - typedef NMDevice NMDeviceBridge; typedef NMDeviceClass NMDeviceBridgeClass; diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9ddec1d08a..02b12d27a7 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -130,6 +130,7 @@ enum { PROP_METERED, PROP_LLDP_NEIGHBORS, PROP_REAL, + PROP_SLAVES, LAST_PROP }; @@ -2187,7 +2188,7 @@ nm_device_master_add_slave (NMDevice *self, NMDevice *slave, gboolean configure) * * Returns: any slaves of which @self is the master. Caller owns returned list. */ -GSList * +static GSList * nm_device_master_get_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); @@ -10399,6 +10400,24 @@ get_property (GObject *object, guint prop_id, case PROP_REAL: g_value_set_boolean (value, priv->real); break; + case PROP_SLAVES: { + GSList *slave_iter; + char **slave_list; + guint i; + + slave_list = g_new (char *, g_slist_length (priv->slaves) + 1); + for (slave_iter = priv->slaves, i = 0; slave_iter; slave_iter = slave_iter->next) { + SlaveInfo *info = slave_iter->data; + const char *path; + + path = nm_exported_object_get_path ((NMExportedObject *) info->slave); + if (path) + slave_list[i++] = g_strdup (path); + } + slave_list[i] = NULL; + g_value_take_boxed (value, slave_list); + break; + } default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -10698,6 +10717,13 @@ nm_device_class_init (NMDeviceClass *klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property + (object_class, PROP_SLAVES, + g_param_spec_boxed (NM_DEVICE_SLAVES, "", "", + G_TYPE_STRV, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS)); + /* Signals */ signals[STATE_CHANGED] = g_signal_new ("state-changed", diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index e2eaa92196..5d8104b428 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -61,6 +61,11 @@ #define NM_DEVICE_LLDP_NEIGHBORS "lldp-neighbors" #define NM_DEVICE_REAL "real" +/* the "slaves" property is internal in the parent class, but exposed + * by the derived classes NMDeviceBond, NMDeviceBridge and NMDeviceTeam. + * It is thus important that the property name matches. */ +#define NM_DEVICE_SLAVES "slaves" /* partially internal */ + #define NM_DEVICE_TYPE_DESC "type-desc" /* Internal only */ #define NM_DEVICE_RFKILL_TYPE "rfkill-type" /* Internal only */ #define NM_DEVICE_IFINDEX "ifindex" /* Internal only */ @@ -387,7 +392,6 @@ void nm_device_replace_vpn6_config (NMDevice *dev, void nm_device_capture_initial_config (NMDevice *dev); /* Master */ -GSList * nm_device_master_get_slaves (NMDevice *dev); gboolean nm_device_is_master (NMDevice *dev); /* Slave */ diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 70bfc78198..e56a648f8d 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -54,13 +54,6 @@ typedef struct { guint teamd_dbus_watch; } NMDeviceTeamPrivate; -enum { - PROP_0, - PROP_SLAVES, - - LAST_PROP -}; - static gboolean teamd_start (NMDevice *device, NMSettingTeam *s_team); /******************************************************************/ @@ -644,7 +637,7 @@ enslave_slave (NMDevice *device, } else _LOGI (LOGD_TEAM, "team port %s was enslaved", slave_iface); - g_object_notify (G_OBJECT (device), NM_DEVICE_TEAM_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); return TRUE; } @@ -669,7 +662,7 @@ release_slave (NMDevice *device, } else _LOGI (LOGD_TEAM, "team port %s was released", nm_device_get_ip_iface (slave)); - g_object_notify (G_OBJECT (device), NM_DEVICE_TEAM_SLAVES); + g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); if (configure) { /* Kernel team code "closes" the port when releasing it, (which clears @@ -745,36 +738,6 @@ constructed (GObject *object) g_free (tmp_str); } -static void -get_property (GObject *object, guint prop_id, - GValue *value, GParamSpec *pspec) -{ - GSList *list; - - switch (prop_id) { - break; - case PROP_SLAVES: - list = nm_device_master_get_slaves (NM_DEVICE (object)); - nm_utils_g_value_set_object_path_array (value, list, NULL, NULL); - g_slist_free (list); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - -static void -set_property (GObject *object, guint prop_id, - const GValue *value, GParamSpec *pspec) -{ - switch (prop_id) { - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - static void dispose (GObject *object) { @@ -801,10 +764,7 @@ nm_device_team_class_init (NMDeviceTeamClass *klass) NM_DEVICE_CLASS_DECLARE_TYPES (klass, NM_SETTING_TEAM_SETTING_NAME, NM_LINK_TYPE_TEAM) - /* virtual methods */ object_class->constructed = constructed; - object_class->get_property = get_property; - object_class->set_property = set_property; object_class->dispose = dispose; parent_class->create_and_realize = create_and_realize; @@ -822,15 +782,6 @@ nm_device_team_class_init (NMDeviceTeamClass *klass) parent_class->enslave_slave = enslave_slave; parent_class->release_slave = release_slave; - /* properties */ - g_object_class_install_property - (object_class, PROP_SLAVES, - g_param_spec_boxed (NM_DEVICE_TEAM_SLAVES, "", "", - G_TYPE_STRV, - G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS)); - - nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (klass), NMDBUS_TYPE_DEVICE_TEAM_SKELETON, NULL); diff --git a/src/devices/team/nm-device-team.h b/src/devices/team/nm-device-team.h index 2436705712..0e81afa744 100644 --- a/src/devices/team/nm-device-team.h +++ b/src/devices/team/nm-device-team.h @@ -32,8 +32,6 @@ G_BEGIN_DECLS #define NM_IS_DEVICE_TEAM_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DEVICE_TEAM)) #define NM_DEVICE_TEAM_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DEVICE_TEAM, NMDeviceTeamClass)) -#define NM_DEVICE_TEAM_SLAVES "slaves" - typedef NMDevice NMDeviceTeam; typedef NMDeviceClass NMDeviceTeamClass; From f45f702a5ece358bf8cc075dc3a2d2f04f99b4d7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Dec 2015 10:53:16 +0100 Subject: [PATCH 3/3] device: cleanup handling master/slave relationships in NMDevice I found the handling of the master-device very confusing because it was unclear who sets priv->master, and when it should be set. Now: - Setting priv->master (in a slave) always goes together with adding the master to priv->slaves (in the master). Previously, this was done at separate places, so it was not clear if master and slave always agree on their relationship -- in fact, they did not. - There are now three basic functions which do the enslaving/releasing: (1) nm_device_master_add_slave() (2) nm_device_master_enslave_slave() (3) nm_device_master_release_one_slave() Step 3/release basically undoes the 1/add and 2/enslave steps. - completing the enslaving/releasing is now done by (1) nm_device_slave_notify_enslave() (2) nm_device_slave_notify_release() These functions also emit signals like NM_DEVICE_MASTER. - Derived classes no longer emit NM_DEVICE_SLAVES notification. Instead the notification is emited together with NM_DEVICE_MASTER, whenever a slaves changes state. Also, NM_DEVICE_SLAVES list now only exposes slaves that are actually @is_enslaved. --- src/devices/nm-device-bond.c | 11 +- src/devices/nm-device-bridge.c | 4 - src/devices/nm-device.c | 184 ++++++++++++++++++------------ src/devices/team/nm-device-team.c | 10 +- 4 files changed, 119 insertions(+), 90 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 62e9860ee9..6510065d55 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -408,7 +408,6 @@ enslave_slave (NMDevice *device, } else _LOGI (LOGD_BOND, "bond slave %s was enslaved", slave_iface); - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); return TRUE; } @@ -432,20 +431,16 @@ release_slave (NMDevice *device, _LOGW (LOGD_BOND, "failed to release bond slave %s", nm_device_get_ip_iface (slave)); } - } else { - _LOGI (LOGD_BOND, "bond slave %s was released", - nm_device_get_ip_iface (slave)); - } - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); - - if (configure) { /* Kernel bonding code "closes" the slave when releasing it, (which clears * IFF_UP), so we must bring it back up here to ensure carrier changes and * other state is noticed by the now-released slave. */ if (!nm_device_bring_up (slave, TRUE, &no_firmware)) _LOGW (LOGD_BOND, "released bond slave could not be brought up."); + } else { + _LOGI (LOGD_BOND, "bond slave %s was released", + nm_device_get_ip_iface (slave)); } } diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 4a20f46b4d..e43ce858b6 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -344,8 +344,6 @@ enslave_slave (NMDevice *device, nm_device_get_ip_iface (slave)); } - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); - return TRUE; } @@ -373,8 +371,6 @@ release_slave (NMDevice *device, _LOGI (LOGD_BRIDGE, "bridge port %s was detached", nm_device_get_ip_iface (slave)); } - - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); } static gboolean diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 02b12d27a7..a3e057c6d4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -379,7 +379,7 @@ static gboolean nm_device_set_ip6_config (NMDevice *self, gboolean routes_full_sync, NMDeviceStateReason *reason); -static gboolean nm_device_master_add_slave (NMDevice *self, NMDevice *slave, gboolean configure); +static void nm_device_master_add_slave (NMDevice *self, NMDevice *slave, gboolean configure); static void nm_device_slave_notify_enslave (NMDevice *self, gboolean success); static void nm_device_slave_notify_release (NMDevice *self, NMDeviceStateReason reason); @@ -1026,15 +1026,6 @@ find_slave_info (NMDevice *self, NMDevice *slave) return NULL; } -static void -free_slave_info (SlaveInfo *info) -{ - g_signal_handler_disconnect (info->slave, info->watch_id); - g_clear_object (&info->slave); - memset (info, 0, sizeof (*info)); - g_free (info); -} - /** * nm_device_master_enslave_slave: * @self: the master device @@ -1105,38 +1096,55 @@ nm_device_master_enslave_slave (NMDevice *self, NMDevice *slave, NMConnection *c * If @self is capable of enslaving other devices (ie it's a bridge, bond, team, * etc) then this function releases the previously enslaved @slave and/or * updates the state of @self and @slave to reflect its release. - * - * Returns: %TRUE on success, %FALSE on failure, if this device cannot enslave - * other devices, or if @slave was never enslaved. */ -static gboolean +static void nm_device_master_release_one_slave (NMDevice *self, NMDevice *slave, gboolean configure, NMDeviceStateReason reason) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDevicePrivate *priv; + NMDevicePrivate *slave_priv; SlaveInfo *info; - gboolean success = FALSE; + gs_unref_object NMDevice *self_free = NULL; - g_return_val_if_fail (slave != NULL, FALSE); - g_return_val_if_fail (NM_DEVICE_GET_CLASS (self)->release_slave != NULL, FALSE); + g_return_if_fail (NM_DEVICE (self)); + g_return_if_fail (NM_DEVICE (slave)); + g_return_if_fail (NM_DEVICE_GET_CLASS (self)->release_slave != NULL); info = find_slave_info (self, slave); - if (!info) - return FALSE; - priv->slaves = g_slist_remove (priv->slaves, info); + _LOGt (LOGD_CORE, "master: release one slave %p/%s%s", slave, nm_device_get_iface (slave), + !info ? " (not registered)" : ""); + + if (!info) + g_return_if_reached (); + + priv = NM_DEVICE_GET_PRIVATE (self); + slave_priv = NM_DEVICE_GET_PRIVATE (slave); + + g_return_if_fail (self == slave_priv->master); + nm_assert (slave == info->slave); + + /* first, let subclasses handle the release ... */ if (info->slave_is_enslaved) NM_DEVICE_GET_CLASS (self)->release_slave (self, slave, configure); - nm_device_slave_notify_release (info->slave, reason); + /* raise notifications about the release, including clearing is_enslaved. */ + nm_device_slave_notify_release (slave, reason); - free_slave_info (info); + /* keep both alive until the end of the function. + * Transfers ownership from slave_priv->master. */ + self_free = self; + + priv->slaves = g_slist_remove (priv->slaves, info); + slave_priv->master = NULL; + + g_signal_handler_disconnect (slave, info->watch_id); + g_object_unref (slave); + g_slice_free (SlaveInfo, info); /* Ensure the device's hardware address is up-to-date; it often changes * when slaves change. */ nm_device_update_hw_address (self); - - return success; } /** @@ -1381,24 +1389,30 @@ nm_device_set_carrier (NMDevice *self, gboolean carrier) } static void -device_recheck_slave_status (NMDevice *self, NMPlatformLink *plink) +device_recheck_slave_status (NMDevice *self, const NMPlatformLink *plink) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - g_return_if_fail (plink != NULL); + g_return_if_fail (plink); + + if (priv->master) { + if ( plink->master > 0 + && plink->master == nm_device_get_ifindex (priv->master)) { + /* call add-slave again. We expect @self already to be added to + * the master, but this also triggers a recheck-assume. */ + nm_device_master_add_slave (priv->master, self, FALSE); + return; + } - if (priv->is_enslaved && plink->master != nm_device_get_ifindex (priv->master)) nm_device_master_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); - - if (plink->master && !priv->is_enslaved) { + } + if (plink->master > 0) { NMDevice *master; master = nm_manager_get_device_by_ifindex (nm_manager_get (), plink->master); - if (master && NM_DEVICE_GET_CLASS (master)->enslave_slave) { - g_clear_object (&priv->master); - priv->master = g_object_ref (master); + if (master && NM_DEVICE_GET_CLASS (master)->enslave_slave) nm_device_master_add_slave (master, self, FALSE); - } else if (master) { + else if (master) { _LOGI (LOGD_DEVICE, "enslaved to non-master-type device %s; ignoring", nm_device_get_iface (master)); } else { @@ -2152,33 +2166,51 @@ slave_state_changed (NMDevice *slave, * * If @self is capable of enslaving other devices (ie it's a bridge, bond, team, * etc) then this function adds @slave to the slave list for later enslavement. - * - * Returns: %TRUE on success, %FALSE on failure */ -static gboolean +static void nm_device_master_add_slave (NMDevice *self, NMDevice *slave, gboolean configure) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDevicePrivate *priv; + NMDevicePrivate *slave_priv; SlaveInfo *info; - g_return_val_if_fail (self != NULL, FALSE); - g_return_val_if_fail (slave != NULL, FALSE); - g_return_val_if_fail (NM_DEVICE_GET_CLASS (self)->enslave_slave != NULL, FALSE); + g_return_if_fail (NM_IS_DEVICE (self)); + g_return_if_fail (NM_IS_DEVICE (slave)); + g_return_if_fail (NM_DEVICE_GET_CLASS (self)->enslave_slave != NULL); + + priv = NM_DEVICE_GET_PRIVATE (self); + slave_priv = NM_DEVICE_GET_PRIVATE (slave); + + info = find_slave_info (self, slave); + + _LOGt (LOGD_CORE, "master: add one slave %p/%s%s", slave, nm_device_get_iface (slave), + info ? " (already registered)" : ""); if (configure) - g_return_val_if_fail (nm_device_get_state (slave) >= NM_DEVICE_STATE_DISCONNECTED, FALSE); + g_return_if_fail (nm_device_get_state (slave) >= NM_DEVICE_STATE_DISCONNECTED); - if (!find_slave_info (self, slave)) { - info = g_malloc0 (sizeof (SlaveInfo)); + if (!info) { + g_return_if_fail (!slave_priv->master); + g_return_if_fail (!slave_priv->is_enslaved); + + info = g_slice_new0 (SlaveInfo); info->slave = g_object_ref (slave); info->configure = configure; info->watch_id = g_signal_connect (slave, "state-changed", G_CALLBACK (slave_state_changed), self); priv->slaves = g_slist_append (priv->slaves, info); - } - nm_device_queue_recheck_assume (self); + slave_priv->master = g_object_ref (self); - return TRUE; + /* no need to emit + * + * g_object_notify (G_OBJECT (slave), NM_DEVICE_MASTER); + * + * because slave_priv->is_enslaved is not true, thus the value + * didn't change yet. */ + } else + g_return_if_fail (slave_priv->master == slave); + + nm_device_queue_recheck_assume (self); } @@ -2314,10 +2346,11 @@ nm_device_get_master (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->is_enslaved) + if (priv->is_enslaved) { + g_return_val_if_fail (priv->master, NULL); return priv->master; - else - return NULL; + } + return NULL; } /** @@ -2335,7 +2368,7 @@ nm_device_slave_notify_enslave (NMDevice *self, gboolean success) NMConnection *connection = nm_device_get_applied_connection (self); gboolean activating = (priv->state == NM_DEVICE_STATE_IP_CONFIG); - g_assert (priv->master); + g_return_if_fail (priv->master); if (!priv->is_enslaved) { if (success) { @@ -2347,6 +2380,7 @@ nm_device_slave_notify_enslave (NMDevice *self, gboolean success) priv->is_enslaved = TRUE; g_object_notify (G_OBJECT (self), NM_DEVICE_MASTER); + g_object_notify (G_OBJECT (priv->master), NM_DEVICE_SLAVES); } else if (activating) { _LOGW (LOGD_DEVICE, "Activation: connection '%s' could not be enslaved", nm_connection_get_id (connection)); @@ -2379,6 +2413,8 @@ nm_device_slave_notify_release (NMDevice *self, NMDeviceStateReason reason) NMDeviceState new_state; const char *master_status; + g_return_if_fail (priv->master); + if ( priv->state > NM_DEVICE_STATE_DISCONNECTED && priv->state <= NM_DEVICE_STATE_ACTIVATED) { if (reason == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) { @@ -2397,14 +2433,13 @@ nm_device_slave_notify_release (NMDevice *self, NMDeviceStateReason reason) master_status); nm_device_queue_state (self, new_state, reason); - } else if (priv->master) - _LOGI (LOGD_DEVICE, "released from master %s", nm_device_get_iface (priv->master)); - else - _LOGD (LOGD_DEVICE, "released from master%s", priv->is_enslaved ? "" : " (was not enslaved)"); + } else + _LOGI (LOGD_DEVICE, "released from master device %s", nm_device_get_iface (priv->master)); if (priv->is_enslaved) { priv->is_enslaved = FALSE; g_object_notify (G_OBJECT (self), NM_DEVICE_MASTER); + g_object_notify (G_OBJECT (priv->master), NM_DEVICE_SLAVES); } } @@ -2431,9 +2466,12 @@ nm_device_get_enslaved (NMDevice *self) void nm_device_removed (NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDevicePrivate *priv; - if (priv->is_enslaved) { + g_return_if_fail (NM_IS_DEVICE (self)); + + priv = NM_DEVICE_GET_PRIVATE (self); + if (priv->master) { /* this is called when something externally messes with the slave or during shut-down. * Release the slave from master, but don't touch the device. */ nm_device_master_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); @@ -3003,7 +3041,8 @@ nm_device_queue_recheck_assume (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (nm_device_can_assume_connections (self) && !priv->recheck_assume_id) + if ( !priv->recheck_assume_id + && nm_device_can_assume_connections (self)) priv->recheck_assume_id = g_idle_add (nm_device_emit_recheck_assume, self); } @@ -3216,26 +3255,31 @@ master_ready (NMDevice *self, NMActiveConnection *active) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - NMActiveConnection *master; + NMActiveConnection *master_connection; + NMDevice *master; g_return_if_fail (priv->state == NM_DEVICE_STATE_PREPARE); g_return_if_fail (!priv->master_ready_handled); /* Notify a master device that it has a new slave */ g_return_if_fail (nm_active_connection_get_master_ready (active)); - master = nm_active_connection_get_master (active); + master_connection = nm_active_connection_get_master (active); priv->master_ready_handled = TRUE; nm_clear_g_signal_handler (active, &priv->master_ready_id); - priv->master = g_object_ref (nm_active_connection_get_device (master)); - nm_device_master_add_slave (priv->master, - self, - nm_active_connection_get_assumed (active) ? FALSE : TRUE); + master = nm_active_connection_get_device (master_connection); _LOGD (LOGD_DEVICE, "master connection ready; master device %s", nm_device_get_iface (priv->master)); + if (priv->master && priv->master != master) + nm_device_master_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); + + /* If the master didn't change, add-slave only rechecks whether to assume a connection. */ + nm_device_master_add_slave (master, + self, + nm_active_connection_get_assumed (active) ? FALSE : TRUE); } static void @@ -9053,11 +9097,9 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean nm_device_master_release_slaves (self); /* slave: mark no longer enslaved */ - if (nm_platform_link_get_master (NM_PLATFORM_GET, priv->ifindex) <= 0) { - g_clear_object (&priv->master); - priv->is_enslaved = FALSE; - g_object_notify (G_OBJECT (self), NM_DEVICE_MASTER); - } + if ( priv->master + && nm_platform_link_get_master (NM_PLATFORM_GET, priv->ifindex) <= 0) + nm_device_master_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); /* Take out any entries in the routing table and any IP address the device had. */ ifindex = nm_device_get_ip_ifindex (self); @@ -10274,7 +10316,7 @@ set_property (GObject *object, guint prop_id, static void get_property (GObject *object, guint prop_id, - GValue *value, GParamSpec *pspec) + GValue *value, GParamSpec *pspec) { NMDevice *self = NM_DEVICE (object); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); @@ -10410,6 +10452,8 @@ get_property (GObject *object, guint prop_id, SlaveInfo *info = slave_iter->data; const char *path; + if (!NM_DEVICE_GET_PRIVATE (info->slave)->is_enslaved) + continue; path = nm_exported_object_get_path ((NMExportedObject *) info->slave); if (path) slave_list[i++] = g_strdup (path); diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index e56a648f8d..9ab848dedf 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -637,8 +637,6 @@ enslave_slave (NMDevice *device, } else _LOGI (LOGD_TEAM, "team port %s was enslaved", slave_iface); - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); - return TRUE; } @@ -659,12 +657,7 @@ release_slave (NMDevice *device, _LOGI (LOGD_TEAM, "released team port %s", nm_device_get_ip_iface (slave)); else _LOGW (LOGD_TEAM, "failed to release team port %s", nm_device_get_ip_iface (slave)); - } else - _LOGI (LOGD_TEAM, "team port %s was released", nm_device_get_ip_iface (slave)); - g_object_notify (G_OBJECT (device), NM_DEVICE_SLAVES); - - if (configure) { /* Kernel team code "closes" the port when releasing it, (which clears * IFF_UP), so we must bring it back up here to ensure carrier changes and * other state is noticed by the now-released port. @@ -672,7 +665,8 @@ release_slave (NMDevice *device, if (!nm_device_bring_up (slave, TRUE, &no_firmware)) _LOGW (LOGD_TEAM, "released team port %s could not be brought up", nm_device_get_ip_iface (slave)); - } + } else + _LOGI (LOGD_TEAM, "team port %s was released", nm_device_get_ip_iface (slave)); } static gboolean