From 744e35e1d2ec569bf8131a397c7ea8ac7a881c1b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 16 Dec 2014 15:05:35 +0100 Subject: [PATCH 1/2] Revert "team: start teamd when ensuring team connection else teamdctl_connect() fails" We don't want to start a teamd instance when there's an externally added team interface. We just don't want to try to the daemon if it's not there (addressed by a later commit). This reverts commit a78386b6d1a3d62063aa039d4ee9eba77c04a284. Conflicts: src/devices/team/nm-device-team.c --- src/devices/team/nm-device-team.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 16f1fcc2a5..2e5424ccc2 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -51,8 +51,6 @@ G_DEFINE_TYPE (NMDeviceTeam, nm_device_team, NM_TYPE_DEVICE) #define NM_DEVICE_TEAM_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_DEVICE_TEAM, NMDeviceTeamPrivate)) -static gboolean teamd_start (NMDevice *device, NMSettingTeam *s_team); - typedef struct { struct teamdctl *tdc; GPid teamd_pid; @@ -178,8 +176,7 @@ update_connection (NMDevice *device, NMConnection *connection) } g_object_set (G_OBJECT (s_team), NM_SETTING_TEAM_CONFIG, NULL, NULL); - teamd_start (device, s_team); - if (NM_DEVICE_TEAM_GET_PRIVATE (device)->teamd_pid > 0 && ensure_teamd_connection (device)) { + if (ensure_teamd_connection (device)) { const char *config = NULL; int err; @@ -426,8 +423,9 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team) priv->tdc || priv->teamd_timeout) { - /* Just return if teamd_start() was already called */ - return TRUE; + /* FIXME g_assert that this never hits. For now, be more reluctant, and try to recover. */ + g_warn_if_reached (); + teamd_cleanup (device, FALSE); } teamd_binary = nm_utils_find_helper ("teamd", NULL, NULL); From 03a5a85d6cdca30f8dfd4dfe8d152524627f9d90 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 16 Dec 2014 17:10:10 +0100 Subject: [PATCH 2/2] team: get configuration only when teamd appears on bus for externally added interfaces teamd first adds the link and only then listens on the bus therefore we race with it. Let's watch for the bus presence even for the teamd devices we didn't add for all their lifetime and recheck for assumed connections as we see them. --- src/devices/team/nm-device-team.c | 65 +++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 2e5424ccc2..2933fee976 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -169,6 +169,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); if (!s_team) { s_team = (NMSettingTeam *) nm_setting_team_new (); @@ -176,7 +177,7 @@ update_connection (NMDevice *device, NMConnection *connection) } g_object_set (G_OBJECT (s_team), NM_SETTING_TEAM_CONFIG, NULL, NULL); - if (ensure_teamd_connection (device)) { + if (priv->tdc) { const char *config = NULL; int err; @@ -273,11 +274,6 @@ teamd_cleanup (NMDevice *device, gboolean device_state_failed) { NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (device); - if (priv->teamd_dbus_watch) { - g_bus_unwatch_name (priv->teamd_dbus_watch); - priv->teamd_dbus_watch = 0; - } - if (priv->teamd_process_watch) { g_source_remove (priv->teamd_process_watch); priv->teamd_process_watch = 0; @@ -333,6 +329,7 @@ teamd_dbus_appeared (GDBusConnection *connection, _LOGI (LOGD_TEAM, "teamd appeared on D-Bus"); teamd_timeout_remove (device); + nm_device_queue_recheck_assume (device); success = ensure_teamd_connection (device); if (nm_device_get_state (device) == NM_DEVICE_STATE_PREPARE) { @@ -355,7 +352,7 @@ teamd_dbus_vanished (GDBusConnection *connection, g_return_if_fail (priv->teamd_dbus_watch); - if (priv->teamd_timeout) { + if (!priv->teamd_pid) { /* 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, until the * startup timeout is over. @@ -363,7 +360,7 @@ teamd_dbus_vanished (GDBusConnection *connection, * Note that g_bus_watch_name is guaranteed to alternate vanished/appeared signals, * so we won't hit this condition again (because the next signal is either 'appeared' * or 'timeout'). */ - _LOGD (LOGD_TEAM, "teamd vanished from D-Bus (ignored)"); + _LOGD (LOGD_TEAM, "teamd not on D-Bus (ignored)"); return; } @@ -403,6 +400,25 @@ teamd_child_setup (gpointer user_data G_GNUC_UNUSED) nm_unblock_posix_signals (NULL); } +static void +nm_device_team_watch_dbus (NMDeviceTeam *self) +{ + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); + const char *iface = nm_device_get_ip_iface (NM_DEVICE (self)); + char *tmp_str = NULL; + + /* Register D-Bus name watcher */ + tmp_str = g_strdup_printf ("org.libteam.teamd.%s", iface); + priv->teamd_dbus_watch = g_bus_watch_name (G_BUS_TYPE_SYSTEM, + tmp_str, + G_BUS_NAME_WATCHER_FLAGS_NONE, + teamd_dbus_appeared, + teamd_dbus_vanished, + NM_DEVICE (self), + NULL); + g_free (tmp_str); +} + static gboolean teamd_start (NMDevice *device, NMSettingTeam *s_team) { @@ -417,8 +433,7 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team) gboolean ret; int status; - if (priv->teamd_dbus_watch || - priv->teamd_process_watch || + if (priv->teamd_process_watch || priv->teamd_pid > 0 || priv->tdc || priv->teamd_timeout) @@ -477,17 +492,6 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team) /* Start a timeout for teamd to appear at D-Bus */ priv->teamd_timeout = g_timeout_add_seconds (5, teamd_timeout_cb, device); - /* Register D-Bus name watcher */ - tmp_str = g_strdup_printf ("org.libteam.teamd.%s", iface); - priv->teamd_dbus_watch = g_bus_watch_name (G_BUS_TYPE_SYSTEM, - tmp_str, - G_BUS_NAME_WATCHER_FLAGS_NONE, - teamd_dbus_appeared, - teamd_dbus_vanished, - device, - NULL); - g_free (tmp_str); - ret = g_spawn_async ("/", (char **) argv->pdata, NULL, G_SPAWN_DO_NOT_REAP_CHILD, &teamd_child_setup, NULL, &priv->teamd_pid, &error); g_ptr_array_free (argv, TRUE); @@ -691,6 +695,16 @@ nm_device_team_init (NMDeviceTeam * self) { } +static void +constructed (GObject *object) +{ + NMDeviceTeam *self = NM_DEVICE_TEAM (object); + + G_OBJECT_CLASS (nm_device_team_parent_class)->constructed (object); + + nm_device_team_watch_dbus (self); +} + static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) @@ -728,6 +742,14 @@ set_property (GObject *object, guint prop_id, static void dispose (GObject *object) { + NMDeviceTeam *self = NM_DEVICE_TEAM (object); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); + + if (priv->teamd_dbus_watch) { + g_bus_unwatch_name (priv->teamd_dbus_watch); + priv->teamd_dbus_watch = 0; + } + teamd_cleanup (NM_DEVICE (object), FALSE); G_OBJECT_CLASS (nm_device_team_parent_class)->dispose (object); @@ -744,6 +766,7 @@ nm_device_team_class_init (NMDeviceTeamClass *klass) parent_class->connection_type = NM_SETTING_TEAM_SETTING_NAME; /* virtual methods */ + object_class->constructed = constructed; object_class->get_property = get_property; object_class->set_property = set_property; object_class->dispose = dispose;