From 76ca47e6b3908f29a1fe13b02b555d4608ff081a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 7 Nov 2013 01:08:02 -0600 Subject: [PATCH] core: make assumed activations go through all the stages Assumed slave connections need to be added to their master devices, which didn't used to happen because the devices activating assumed connections jumped directly to stage3, bypassing all the master/slave handling stuff. Instead, make all assumed connections go through all activation stages, but make sure that things which touch the device don't get done for assumed connections. This requires moving the master/slave code out of the override-able class methods because we need to call the master/slave code for assumed connections, but we don't want to call the override-able class activation methods. --- src/devices/nm-device-bond.c | 19 ++-- src/devices/nm-device-bridge.c | 13 ++- src/devices/nm-device-team.c | 50 +++++----- src/devices/nm-device.c | 168 ++++++++++++++++----------------- src/devices/nm-device.h | 5 +- 5 files changed, 133 insertions(+), 122 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 30fa54384f..8d9939b096 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -413,20 +413,23 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason) } static gboolean -enslave_slave (NMDevice *device, NMDevice *slave, NMConnection *connection) +enslave_slave (NMDevice *device, + NMDevice *slave, + NMConnection *connection, + gboolean configure) { - gboolean success, no_firmware = FALSE; + gboolean success = TRUE, no_firmware = FALSE; const char *iface = nm_device_get_ip_iface (device); const char *slave_iface = nm_device_get_ip_iface (slave); nm_device_master_check_slave_physical_port (device, slave, LOGD_BOND); - nm_device_take_down (slave, TRUE); - - success = nm_platform_link_enslave (nm_device_get_ip_ifindex (device), - nm_device_get_ip_ifindex (slave)); - - nm_device_bring_up (slave, TRUE, &no_firmware); + if (configure) { + nm_device_take_down (slave, TRUE); + success = nm_platform_link_enslave (nm_device_get_ip_ifindex (device), + nm_device_get_ip_ifindex (slave)); + nm_device_bring_up (slave, TRUE, &no_firmware); + } if (success) { nm_log_info (LOGD_BOND, "(%s): enslaved bond slave %s", iface, slave_iface); diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 0f99502826..92cc9f2a77 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -376,12 +376,17 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *reason) } static gboolean -enslave_slave (NMDevice *device, NMDevice *slave, NMConnection *connection) +enslave_slave (NMDevice *device, + NMDevice *slave, + NMConnection *connection, + gboolean configure) { - if (!nm_platform_link_enslave (nm_device_get_ip_ifindex (device), nm_device_get_ip_ifindex (slave))) - return FALSE; + if (configure) { + if (!nm_platform_link_enslave (nm_device_get_ip_ifindex (device), nm_device_get_ip_ifindex (slave))) + return FALSE; - commit_slave_options (slave, nm_connection_get_setting_bridge_port (connection)); + commit_slave_options (slave, nm_connection_get_setting_bridge_port (connection)); + } nm_log_info (LOGD_BRIDGE, "(%s): attached bridge port %s", nm_device_get_ip_iface (device), diff --git a/src/devices/nm-device-team.c b/src/devices/nm-device-team.c index b52842ab18..8f119dd85f 100644 --- a/src/devices/nm-device-team.c +++ b/src/devices/nm-device-team.c @@ -638,48 +638,52 @@ deactivate (NMDevice *dev) } static gboolean -enslave_slave (NMDevice *device, NMDevice *slave, NMConnection *connection) +enslave_slave (NMDevice *device, + NMDevice *slave, + NMConnection *connection, + gboolean configure) { #if WITH_TEAMDCTL NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); #endif - gboolean success, no_firmware = FALSE; + gboolean success = TRUE, no_firmware = FALSE; const char *iface = nm_device_get_ip_iface (device); const char *slave_iface = nm_device_get_ip_iface (slave); NMSettingTeamPort *s_team_port; nm_device_master_check_slave_physical_port (device, slave, LOGD_TEAM); - nm_device_take_down (slave, TRUE); + if (configure) { + nm_device_take_down (slave, TRUE); - s_team_port = nm_connection_get_setting_team_port (connection); - if (s_team_port) { - const char *config = nm_setting_team_port_get_config (s_team_port); + s_team_port = nm_connection_get_setting_team_port (connection); + if (s_team_port) { + const char *config = nm_setting_team_port_get_config (s_team_port); - if (config) { + if (config) { #if WITH_TEAMDCTL - if (!priv->tdc) { - nm_log_warn (LOGD_TEAM, "(%s): enslaved team port %s config not changed, not connected to teamd", - iface, slave_iface); - } else { - int err; + if (!priv->tdc) { + nm_log_warn (LOGD_TEAM, "(%s): enslaved team port %s config not changed, not connected to teamd", + iface, slave_iface); + } else { + int err; - err = teamdctl_port_config_update_raw (priv->tdc, slave_iface, config); - if (err) { - nm_log_err (LOGD_TEAM, "(%s): failed to update config for port %s", iface, slave_iface); - return FALSE; + err = teamdctl_port_config_update_raw (priv->tdc, slave_iface, config); + if (err) { + nm_log_err (LOGD_TEAM, "(%s): failed to update config for port %s", iface, slave_iface); + return FALSE; + } } - } #else - nm_log_warn (LOGD_TEAM, "(%s): enslaved team port %s config not changed due to lack of Teamd control support", - iface, slave_iface); + nm_log_warn (LOGD_TEAM, "(%s): enslaved team port %s config not changed due to lack of Teamd control support", + iface, slave_iface); #endif + } } + success = nm_platform_link_enslave (nm_device_get_ip_ifindex (device), + nm_device_get_ip_ifindex (slave)); + nm_device_bring_up (slave, TRUE, &no_firmware); } - success = nm_platform_link_enslave (nm_device_get_ip_ifindex (device), - nm_device_get_ip_ifindex (slave)); - - nm_device_bring_up (slave, TRUE, &no_firmware); if (success) { nm_log_info (LOGD_TEAM, "(%s): enslaved team port %s", iface, slave_iface); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index eca1dfb3aa..2ed64104af 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -168,6 +168,7 @@ typedef struct { typedef struct { NMDevice *slave; gboolean enslaved; + gboolean configure; guint watch_id; } SlaveInfo; @@ -958,7 +959,7 @@ nm_device_enslave_slave (NMDevice *dev, NMDevice *slave, NMConnection *connectio return FALSE; g_warn_if_fail (info->enslaved == FALSE); - success = NM_DEVICE_GET_CLASS (dev)->enslave_slave (dev, slave, connection); + success = NM_DEVICE_GET_CLASS (dev)->enslave_slave (dev, slave, connection, info->configure); info->enslaved = success; nm_device_slave_notify_enslave (info->slave, success); @@ -1245,6 +1246,8 @@ slave_state_changed (NMDevice *slave, * nm_device_master_add_slave: * @dev: the master device * @slave: the slave device to enslave + * @configure: pass %TRUE if the slave should be configured by the master, or + * %FALSE if it is already configured outside NetworkManager * * If @dev 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. @@ -1252,7 +1255,7 @@ slave_state_changed (NMDevice *slave, * Returns: %TRUE on success, %FALSE on failure */ gboolean -nm_device_master_add_slave (NMDevice *dev, NMDevice *slave) +nm_device_master_add_slave (NMDevice *dev, NMDevice *slave, gboolean configure) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); SlaveInfo *info; @@ -1265,6 +1268,7 @@ nm_device_master_add_slave (NMDevice *dev, NMDevice *slave) if (!find_slave_info (dev, slave)) { info = g_malloc0 (sizeof (SlaveInfo)); info->slave = g_object_ref (slave); + info->configure = configure; info->watch_id = g_signal_connect (slave, "state-changed", G_CALLBACK (slave_state_changed), dev); priv->slaves = g_slist_prepend (priv->slaves, info); @@ -1977,7 +1981,9 @@ master_ready_cb (NMActiveConnection *active, master = nm_active_connection_get_master (active); priv->master = g_object_ref (nm_active_connection_get_device (master)); - nm_device_master_add_slave (priv->master, self); + nm_device_master_add_slave (priv->master, + self, + nm_active_connection_get_assumed (active) ? FALSE : TRUE); nm_log_dbg (LOGD_DEVICE, "(%s): master connection ready; master device %s", nm_device_get_iface (self), @@ -1994,10 +2000,46 @@ master_ready_cb (NMActiveConnection *active, static NMActStageReturn act_stage1_prepare (NMDevice *self, NMDeviceStateReason *reason) { + return NM_ACT_STAGE_RETURN_SUCCESS; +} + +/* + * nm_device_activate_stage1_device_prepare + * + * Prepare for device activation + * + */ +static gboolean +nm_device_activate_stage1_device_prepare (gpointer user_data) +{ + NMDevice *self = NM_DEVICE (user_data); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const char *iface; NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS; + NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request); + /* Clear the activation source ID now that this stage has run */ + activation_source_clear (self, FALSE, 0); + + priv->ip4_state = priv->ip6_state = IP_NONE; + + iface = nm_device_get_iface (self); + nm_log_info (LOGD_DEVICE, "Activation (%s) Stage 1 of 5 (Device Prepare) started...", iface); + nm_device_state_changed (self, NM_DEVICE_STATE_PREPARE, NM_DEVICE_STATE_REASON_NONE); + + /* Assumed connections were already set up outside NetworkManager */ + if (!nm_active_connection_get_assumed (active)) { + ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &reason); + if (ret == NM_ACT_STAGE_RETURN_POSTPONE) { + goto out; + } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); + goto out; + } + g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); + } + if (nm_active_connection_get_master (active)) { /* If the master connection is ready for slaves, attach ourselves */ if (nm_active_connection_get_master_ready (active)) @@ -2012,47 +2054,10 @@ act_stage1_prepare (NMDevice *self, NMDeviceStateReason *reason) "notify::" NM_ACTIVE_CONNECTION_INT_MASTER_READY, (GCallback) master_ready_cb, self); - ret = NM_ACT_STAGE_RETURN_POSTPONE; + /* Postpone */ } - } - - return ret; -} - -/* - * nm_device_activate_stage1_device_prepare - * - * Prepare for device activation - * - */ -static gboolean -nm_device_activate_stage1_device_prepare (gpointer user_data) -{ - NMDevice *self = NM_DEVICE (user_data); - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const char *iface; - NMActStageReturn ret; - NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; - - /* Clear the activation source ID now that this stage has run */ - activation_source_clear (self, FALSE, 0); - - priv->ip4_state = priv->ip6_state = IP_NONE; - - iface = nm_device_get_iface (self); - nm_log_info (LOGD_DEVICE, "Activation (%s) Stage 1 of 5 (Device Prepare) started...", iface); - nm_device_state_changed (self, NM_DEVICE_STATE_PREPARE, NM_DEVICE_STATE_REASON_NONE); - - ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &reason); - if (ret == NM_ACT_STAGE_RETURN_POSTPONE) { - goto out; - } else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); - goto out; - } - g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); - - nm_device_activate_schedule_stage2_device_config (self); + } else + nm_device_activate_schedule_stage2_device_config (self); out: nm_log_info (LOGD_DEVICE, "Activation (%s) Stage 1 of 5 (Device Prepare) complete.", iface); @@ -2085,17 +2090,6 @@ nm_device_activate_schedule_stage1_device_prepare (NMDevice *self) static NMActStageReturn act_stage2_config (NMDevice *dev, NMDeviceStateReason *reason) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); - GSList *iter; - - /* If we have slaves that aren't yet enslaved, do that now */ - for (iter = priv->slaves; iter; iter = g_slist_next (iter)) { - SlaveInfo *info = iter->data; - - if (nm_device_get_state (info->slave) == NM_DEVICE_STATE_IP_CONFIG) - nm_device_enslave_slave (dev, info->slave, nm_device_get_connection (info->slave)); - } - /* Nothing to do */ return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -2111,10 +2105,13 @@ static gboolean nm_device_activate_stage2_device_config (gpointer user_data) { NMDevice *self = NM_DEVICE (user_data); - const char * iface; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + const char *iface; NMActStageReturn ret; NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; gboolean no_firmware = FALSE; + NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request); + GSList *iter; /* Clear the activation source ID now that this stage has run */ activation_source_clear (self, FALSE, 0); @@ -2123,23 +2120,33 @@ nm_device_activate_stage2_device_config (gpointer user_data) nm_log_info (LOGD_DEVICE, "Activation (%s) Stage 2 of 5 (Device Configure) starting...", iface); nm_device_state_changed (self, NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_REASON_NONE); - if (!nm_device_bring_up (self, FALSE, &no_firmware)) { - if (no_firmware) - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_FIRMWARE_MISSING); - else - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_CONFIG_FAILED); - goto out; + /* Assumed connections were already set up outside NetworkManager */ + if (!nm_active_connection_get_assumed (active)) { + if (!nm_device_bring_up (self, FALSE, &no_firmware)) { + if (no_firmware) + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_FIRMWARE_MISSING); + else + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_CONFIG_FAILED); + goto out; + } + + ret = NM_DEVICE_GET_CLASS (self)->act_stage2_config (self, &reason); + if (ret == NM_ACT_STAGE_RETURN_POSTPONE) + goto out; + else if (ret == NM_ACT_STAGE_RETURN_FAILURE) { + nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); + goto out; + } + g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); } - ret = NM_DEVICE_GET_CLASS (self)->act_stage2_config (self, &reason); - if (ret == NM_ACT_STAGE_RETURN_POSTPONE) - goto out; - else if (ret == NM_ACT_STAGE_RETURN_FAILURE) - { - nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason); - goto out; + /* If we have slaves that aren't yet enslaved, do that now */ + for (iter = priv->slaves; iter; iter = g_slist_next (iter)) { + SlaveInfo *info = iter->data; + + if (nm_device_get_state (info->slave) == NM_DEVICE_STATE_IP_CONFIG) + nm_device_enslave_slave (self, info->slave, nm_device_get_connection (info->slave)); } - g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); nm_log_info (LOGD_DEVICE, "Activation (%s) Stage 2 of 5 (Device Configure) successful.", iface); @@ -4616,23 +4623,14 @@ nm_device_activate (NMDevice *self, NMActRequest *req) priv->act_request = g_object_ref (req); g_object_notify (G_OBJECT (self), NM_DEVICE_ACTIVE_CONNECTION); - if (priv->state_reason == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) { - /* If it's an assumed connection, let the device subclass short-circuit - * the normal connection process and just copy its IP configs from the - * interface. - */ - nm_device_state_changed (self, NM_DEVICE_STATE_IP_CONFIG, NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); - nm_device_activate_schedule_stage3_ip_config_start (self); - } else { - /* HACK: update the state a bit early to avoid a race between the - * scheduled stage1 handler and nm_policy_device_change_check() thinking - * that the activation request isn't deferred because the deferred bit - * gets cleared a bit too early, when the connection becomes valid. - */ - nm_device_state_changed (self, NM_DEVICE_STATE_PREPARE, NM_DEVICE_STATE_REASON_NONE); + /* HACK: update the state a bit early to avoid a race between the + * scheduled stage1 handler and nm_policy_device_change_check() thinking + * that the activation request isn't deferred because the deferred bit + * gets cleared a bit too early, when the connection becomes valid. + */ + nm_device_state_changed (self, NM_DEVICE_STATE_PREPARE, NM_DEVICE_STATE_REASON_NONE); - nm_device_activate_schedule_stage1_device_prepare (self); - } + nm_device_activate_schedule_stage1_device_prepare (self); } /* diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index f907aed299..69d81754ce 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -185,7 +185,8 @@ typedef struct { gboolean (* enslave_slave) (NMDevice *self, NMDevice *slave, - NMConnection *connection); + NMConnection *connection, + gboolean configure); gboolean (* release_slave) (NMDevice *self, NMDevice *slave); @@ -233,7 +234,7 @@ void nm_device_set_vpn6_config (NMDevice *dev, NMIP6Config *config) void nm_device_capture_initial_config (NMDevice *dev); /* Master */ -gboolean nm_device_master_add_slave (NMDevice *dev, NMDevice *slave); +gboolean nm_device_master_add_slave (NMDevice *dev, NMDevice *slave, gboolean configure); GSList * nm_device_master_get_slaves (NMDevice *dev); gboolean nm_device_is_master (NMDevice *dev);