From 06ded430e8403c3995ebdc01403f7cc9da988682 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 30 Jun 2016 09:54:21 +0200 Subject: [PATCH 1/2] team: fix handling of D-Bus name watch events When a device is activated any existing teamd instance is killed. But since commit 28274495d61c ("device/team: always try to connect to teamd in update_connection()") the disappearing of the D-Bus name owner always triggers an automatic restart of the instance in teamd_dbus_vanished() if the name was previously owned. This new instance conflicts with the instance we're about to start. Instead, restore the previous behavior of restarting teamd only if there is an activation in progress and use @tdc as a flag. This also means that update_connection() should not modify the value of @tdc. Fixes: 28274495d61cb434272e0e2307dfd824ee78de8e --- src/devices/team/nm-device-team.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 0199cc36c9..76e957bcdb 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -55,7 +55,6 @@ typedef struct { guint teamd_process_watch; guint teamd_timeout; guint teamd_dbus_watch; - gboolean teamd_dbus_name_owned; char *config; } NMDeviceTeamPrivate; @@ -180,6 +179,7 @@ update_connection (NMDevice *device, NMConnection *connection) NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMSettingTeam *s_team = nm_connection_get_setting_team (connection); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); + struct teamdctl *tdc = priv->tdc; if (!s_team) { s_team = (NMSettingTeam *) nm_setting_team_new (); @@ -190,6 +190,13 @@ update_connection (NMDevice *device, NMConnection *connection) if (!priv->config && ensure_teamd_connection (device)) teamd_read_config (device); + /* Restore previous tdc state */ + if (priv->tdc && !tdc) { + teamdctl_disconnect (priv->tdc); + teamdctl_free (priv->tdc); + priv->tdc = NULL; + } + g_object_set (G_OBJECT (s_team), NM_SETTING_TEAM_CONFIG, priv->config, NULL); } @@ -321,7 +328,6 @@ teamd_dbus_appeared (GDBusConnection *connection, gboolean success; g_return_if_fail (priv->teamd_dbus_watch); - priv->teamd_dbus_name_owned = TRUE; _LOGI (LOGD_TEAM, "teamd appeared on D-Bus"); nm_device_queue_recheck_assume (device); @@ -384,7 +390,7 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, g_return_if_fail (priv->teamd_dbus_watch); - if (!priv->teamd_dbus_name_owned) { + if (!priv->tdc) { /* g_bus_watch_name will always raise an initial signal, to indicate whether the * name exists/not exists initially. Do not take this as a failure if it hadn't * previously appeared. @@ -393,8 +399,6 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, return; } - priv->teamd_dbus_name_owned = FALSE; - _LOGI (LOGD_TEAM, "teamd vanished from D-Bus"); teamd_cleanup (device, TRUE); From 5d4fc4c9ace2d5226ecf5205ea5384424c5a9835 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 30 Jun 2016 10:10:48 +0200 Subject: [PATCH 2/2] team: fail the connection if the teamd configuration can't be read If the read of teamd configuration failed (possibly due to a timeout), fail the connection immediately where possible instead of letting it continue and risk to block again at the next read. https://bugzilla.redhat.com/show_bug.cgi?id=1257237 --- src/devices/team/nm-device-team.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 76e957bcdb..6d2560493f 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -152,7 +152,7 @@ ensure_teamd_connection (NMDevice *device) return !!priv->tdc; } -static void +static gboolean teamd_read_config (NMDevice *device) { NMDeviceTeam *self = NM_DEVICE_TEAM (device); @@ -163,7 +163,7 @@ teamd_read_config (NMDevice *device) if (priv->tdc) { err = teamdctl_config_get_raw_direct (priv->tdc, &config); if (err) - _LOGI (LOGD_TEAM, "failed to read teamd config (err=%d)", err); + return FALSE; } if (!nm_streq0 (config, priv->config)) { @@ -171,6 +171,8 @@ teamd_read_config (NMDevice *device) priv->config = g_strdup (config); _notify (self, PROP_CONFIG); } + + return TRUE; } static void @@ -204,9 +206,9 @@ update_connection (NMDevice *device, NMConnection *connection) static gboolean master_update_slave_connection (NMDevice *self, - NMDevice *slave, - NMConnection *connection, - GError **error) + NMDevice *slave, + NMConnection *connection, + GError **error) { NMSettingTeamPort *s_port; char *port_config = NULL; @@ -310,7 +312,10 @@ teamd_timeout_cb (gpointer user_data) /* Read again the configuration after the timeout since it might * have changed. */ - teamd_read_config (device); + if (!teamd_read_config (device)) { + _LOGW (LOGD_TEAM, "failed to read teamd configuration"); + nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); + } } return G_SOURCE_REMOVE; @@ -370,10 +375,11 @@ teamd_dbus_appeared (GDBusConnection *connection, */ success = ensure_teamd_connection (device); if (nm_device_get_state (device) == NM_DEVICE_STATE_PREPARE) { - if (success) { - teamd_read_config (device); + if (success) + success = teamd_read_config (device); + if (success) nm_device_activate_schedule_stage2_device_config (device); - } else if (!nm_device_uses_assumed_connection (device)) + else if (!nm_device_uses_assumed_connection (device)) nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED); } }