From e8c58c957ba130575c3293c8b93ffe4cf97b3865 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 19 Jun 2013 16:18:29 -0500 Subject: [PATCH] platform: specify link-added signal as asynchronous With the move of udev logic into the Linux platform class, the link-added signals are asynchronous, that is they are not emitted during the call to nm_platform_*_add(), but after that call has returned. The Fake implementation still emitted them synchronously, which broke the testcases. Convert the Fake implementation to emit link-added signals asynchronously and update the testcases to handle this. --- src/platform/nm-fake-platform.c | 42 ++++++++++++++++++++++++-- src/platform/nm-platform.c | 3 +- src/platform/tests/test-common.c | 3 +- src/platform/tests/test-common.h | 6 ++-- src/platform/tests/test-link.c | 51 ++++++++++++++++++++++---------- 5 files changed, 83 insertions(+), 22 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 61e0070043..95757aa1cd 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -35,6 +35,8 @@ typedef struct { GArray *ip6_addresses; GArray *ip4_routes; GArray *ip6_routes; + + GSList *link_added_ids; } NMFakePlatformPrivate; typedef struct { @@ -160,18 +162,49 @@ link_get_all (NMPlatform *platform) return links; } +typedef struct { + NMPlatform *platform; + int ifindex; + guint id; +} LinkAddedInfo; + +static gboolean +link_added_emit (gpointer user_data) +{ + LinkAddedInfo *info = user_data; + NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (info->platform); + NMFakePlatformLink *device; + + priv->link_added_ids = g_slist_remove (priv->link_added_ids, GUINT_TO_POINTER (info->id)); + + device = link_get (info->platform, info->ifindex); + g_assert (device); + g_signal_emit_by_name (info->platform, NM_PLATFORM_LINK_ADDED, info->ifindex, &(device->link)); + return FALSE; +} + static gboolean link_add (NMPlatform *platform, const char *name, NMLinkType type) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (platform); NMFakePlatformLink device; + LinkAddedInfo *info; link_init (&device, priv->links->len, type, name); g_array_append_val (priv->links, device); - if (device.link.ifindex) - g_signal_emit_by_name (platform, NM_PLATFORM_LINK_ADDED, device.link.ifindex, &device.link); + if (device.link.ifindex) { + /* Platform requires LINK_ADDED signal emission from an idle handler */ + info = g_new0 (LinkAddedInfo, 1); + info->platform = platform; + info->ifindex = device.link.ifindex; + info->id = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, + link_added_emit, + info, + g_free); + priv->link_added_ids = g_slist_prepend (priv->link_added_ids, GUINT_TO_POINTER (info->id)); + } return TRUE; } @@ -989,6 +1022,11 @@ nm_fake_platform_finalize (GObject *object) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE (object); int i; + GSList *iter; + + for (iter = priv->link_added_ids; iter; iter = iter->next) + g_source_remove (GPOINTER_TO_UINT (iter->data)); + g_slist_free (priv->link_added_ids); g_hash_table_unref (priv->options); for (i = 0; i < priv->links->len; i++) { diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 1d6a580f38..5d25141408 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -320,7 +320,8 @@ nm_platform_link_get_all (void) * @type: Interface type * * Add a software interface. Sets platform->error to NM_PLATFORM_ERROR_EXISTS - * if interface is already already exists. + * if interface is already already exists. Any link-added signal will be + * emitted from an idle handler and not within this function. */ static gboolean nm_platform_link_add (const char *name, NMLinkType type) diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index 0f4492e134..b4d415b2dc 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -1,7 +1,7 @@ #include "test-common.h" SignalData * -add_signal_full (const char *name, GCallback callback, int ifindex) +add_signal_full (const char *name, GCallback callback, int ifindex, const char *ifname) { SignalData *data = g_new0 (SignalData, 1); @@ -9,6 +9,7 @@ add_signal_full (const char *name, GCallback callback, int ifindex) data->received = FALSE; data->handler_id = g_signal_connect (nm_platform_get (), name, callback, data); data->ifindex = ifindex; + data->ifname = ifname; g_assert (data->handler_id >= 0); diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h index 46657d482b..b6ce50f9dc 100644 --- a/src/platform/tests/test-common.h +++ b/src/platform/tests/test-common.h @@ -18,10 +18,12 @@ typedef struct { gboolean received; GMainLoop *loop; int ifindex; + const char *ifname; } SignalData; -SignalData *add_signal_full (const char *name, GCallback callback, int ifindex); -#define add_signal(name, callback) add_signal_full (name, (GCallback) callback, 0) +SignalData *add_signal_full (const char *name, GCallback callback, int ifindex, const char *ifname); +#define add_signal(name, callback) add_signal_full (name, (GCallback) callback, 0, NULL) +#define add_signal_ifname(name, callback, ifname) add_signal_full (name, (GCallback) callback, 0, ifname) void accept_signal (SignalData *data); void wait_signal (SignalData *data); void free_signal (SignalData *data); diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 6392103cdf..b0afe5e6e5 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -27,6 +27,8 @@ link_callback (NMPlatform *platform, int ifindex, NMPlatformLink *received, Sign if (data->ifindex && data->ifindex != received->ifindex) return; + if (data->ifname && g_strcmp0 (data->ifname, nm_platform_link_get_name (ifindex)) != 0) + return; if (data->loop) g_main_loop_quit (data->loop); @@ -128,7 +130,7 @@ test_loopback (void) } static int -virtual_add (NMLinkType link_type, const char *name, SignalData *link_added, SignalData *link_changed) +virtual_add (NMLinkType link_type, const char *name) { switch (link_type) { case NM_LINK_TYPE_DUMMY: @@ -150,33 +152,44 @@ virtual_add (NMLinkType link_type, const char *name, SignalData *link_added, Sig } case NM_LINK_TYPE_TEAM: return nm_platform_team_add (name); - case NM_LINK_TYPE_VLAN: - /* Don't call link_callback for the bridge interface */ - if (nm_platform_bridge_add (PARENT_NAME)) - wait_signal (link_added); + case NM_LINK_TYPE_VLAN: { + SignalData *parent_added; + SignalData *parent_changed; + /* Don't call link_callback for the bridge interface */ + parent_added = add_signal_ifname ("link-added", link_callback, PARENT_NAME); + if (nm_platform_bridge_add (PARENT_NAME)) + wait_signal (parent_added); + free_signal (parent_added); + + parent_changed = add_signal_ifname ("link-changed", link_callback, PARENT_NAME); g_assert (nm_platform_link_set_up (nm_platform_link_get_ifindex (PARENT_NAME))); - accept_signal (link_changed); + accept_signal (parent_changed); + free_signal (parent_changed); return nm_platform_vlan_add (name, nm_platform_link_get_ifindex (PARENT_NAME), VLAN_ID, 0); + } default: g_error ("Link type %d unhandled.", link_type); } } static void -test_slave (int master, int type, SignalData *link_added, SignalData *master_changed, SignalData *link_removed) +test_slave (int master, int type, SignalData *master_changed, SignalData *link_removed) { int ifindex; - SignalData *link_changed = add_signal ("link-changed", link_callback); + SignalData *link_changed = add_signal_ifname ("link-changed", link_callback, SLAVE_NAME); + SignalData *link_added; char *value; - g_assert (virtual_add (type, SLAVE_NAME, link_added, link_changed)); + link_added = add_signal_ifname ("link-added", link_callback, SLAVE_NAME); + g_assert (virtual_add (type, SLAVE_NAME)); ifindex = nm_platform_link_get_ifindex (SLAVE_NAME); g_assert (ifindex > 0); wait_signal (link_added); + free_signal (link_added); /* Set the slave up to see whether master's IFF_LOWER_UP is set correctly. * @@ -270,12 +283,13 @@ test_virtual (NMLinkType link_type, const char *link_typename) char *value; int vlan_parent, vlan_id; - SignalData *link_added = add_signal ("link-added", link_callback); - SignalData *link_changed = add_signal ("link-changed", link_callback); + SignalData *link_added; + SignalData *link_changed; SignalData *link_removed = add_signal ("link-removed", link_callback); /* Add */ - g_assert (virtual_add (link_type, DEVICE_NAME, link_added, link_changed)); + link_added = add_signal_ifname ("link-added", link_callback, DEVICE_NAME); + g_assert (virtual_add (link_type, DEVICE_NAME)); no_error (); g_assert (nm_platform_link_exists (DEVICE_NAME)); ifindex = nm_platform_link_get_ifindex (DEVICE_NAME); @@ -291,10 +305,12 @@ test_virtual (NMLinkType link_type, const char *link_typename) wait_signal (link_added); /* Add again */ - g_assert (!virtual_add (link_type, DEVICE_NAME, link_added, link_changed)); + g_assert (!virtual_add (link_type, DEVICE_NAME)); error (NM_PLATFORM_ERROR_EXISTS); /* Set ARP/NOARP */ + link_changed = add_signal ("link-changed", link_callback); + link_changed->ifindex = ifindex; g_assert (nm_platform_link_uses_arp (ifindex)); g_assert (nm_platform_link_set_noarp (ifindex)); g_assert (!nm_platform_link_uses_arp (ifindex)); @@ -332,7 +348,7 @@ test_virtual (NMLinkType link_type, const char *link_typename) case NM_LINK_TYPE_BOND: case NM_LINK_TYPE_TEAM: link_changed->ifindex = ifindex; - test_slave (ifindex, NM_LINK_TYPE_DUMMY, link_added, link_changed, link_removed); + test_slave (ifindex, NM_LINK_TYPE_DUMMY, link_changed, link_removed); link_changed->ifindex = 0; break; default: @@ -392,7 +408,7 @@ test_vlan () static void test_internal (void) { - SignalData *link_added = add_signal (NM_PLATFORM_LINK_ADDED, link_callback); + SignalData *link_added; SignalData *link_changed = add_signal (NM_PLATFORM_LINK_CHANGED, link_callback); SignalData *link_removed = add_signal (NM_PLATFORM_LINK_REMOVED, link_callback); const char mac[6] = { 0x00, 0xff, 0x11, 0xee, 0x22, 0xdd }; @@ -406,6 +422,7 @@ test_internal (void) error (NM_PLATFORM_ERROR_NOT_FOUND); /* Add device */ + link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME); g_assert (nm_platform_dummy_add (DEVICE_NAME)); no_error (); wait_signal (link_added); @@ -471,6 +488,7 @@ test_internal (void) error (NM_PLATFORM_ERROR_NOT_FOUND); /* Add back */ + g_assert_cmpstr (link_added->ifname, ==, DEVICE_NAME); g_assert (nm_platform_dummy_add (DEVICE_NAME)); no_error (); wait_signal (link_added); @@ -492,11 +510,12 @@ test_internal (void) static void test_external (void) { - SignalData *link_added = add_signal (NM_PLATFORM_LINK_ADDED, link_callback); + SignalData *link_added; SignalData *link_changed = add_signal (NM_PLATFORM_LINK_CHANGED, link_callback); SignalData *link_removed = add_signal (NM_PLATFORM_LINK_REMOVED, link_callback); int ifindex; + link_added = add_signal_ifname (NM_PLATFORM_LINK_ADDED, link_callback, DEVICE_NAME); run_command ("ip link add %s type %s", DEVICE_NAME, "dummy"); wait_signal (link_added); g_assert (nm_platform_link_exists (DEVICE_NAME));