From b74457ac82a360feb57c226f7f8a31a2d159ba17 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 7 Sep 2015 10:48:48 +0200
Subject: [PATCH 1/3] config/test: add a test for nm_config_reload() with
different signals
(cherry picked from commit e6c64af8be89374ed318ed1a395ead7cd00a6753)
(cherry picked from commit ce7687e49900a72dd2cdd9ee7595004e4f3cf4e6)
---
src/tests/config/test-config.c | 82 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c
index 8b8028e..a5975c1 100644
--- a/src/tests/config/test-config.c
+++ b/src/tests/config/test-config.c
@@ -396,6 +396,86 @@ test_config_confdir_parse_error (void)
g_clear_error (&error);
}
+/*****************************************************************************/
+
+static void
+_test_signal_config_changed_cb (NMConfig *config,
+ NMConfigData *config_data,
+ NMConfigChangeFlags changes,
+ NMConfigData *old_data,
+ gpointer user_data)
+{
+ const NMConfigChangeFlags *expected = user_data;
+
+ g_assert (changes);
+ g_assert_cmpint (changes, ==, *expected);
+ g_assert (NM_IS_CONFIG (config));
+ g_assert (NM_IS_CONFIG_DATA (config_data));
+
+ g_assert (config_data == old_data);
+ g_assert (config_data == nm_config_get_data (config));
+}
+
+static void
+_test_signal_config_changed_cb2 (NMConfig *config,
+ NMConfigData *config_data,
+ NMConfigChangeFlags changes,
+ NMConfigData *old_data,
+ gpointer user_data)
+{
+ const NMConfigChangeFlags *expected = user_data;
+
+ g_assert (changes);
+ g_assert_cmpint (changes, ==, *expected);
+}
+
+static void
+test_config_signal (void)
+{
+ gs_unref_object NMConfig *config = NULL;
+ NMConfigChangeFlags expected;
+ gs_unref_object NMConfigData *config_data_orig = NULL;
+
+ config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", NULL);
+
+ config_data_orig = g_object_ref (nm_config_get_data_orig (config));
+
+ g_signal_connect (G_OBJECT (config),
+ NM_CONFIG_SIGNAL_CONFIG_CHANGED,
+ G_CALLBACK (_test_signal_config_changed_cb),
+ &expected);
+
+ expected = NM_CONFIG_CHANGE_SIGUSR1;
+ g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: signal SIGUSR1");
+ nm_config_reload (config, SIGUSR1);
+
+ expected = NM_CONFIG_CHANGE_SIGUSR2;
+ g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: signal SIGUSR2");
+ nm_config_reload (config, SIGUSR2);
+
+ expected = NM_CONFIG_CHANGE_SIGHUP;
+ g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: signal SIGHUP (no changes from disk)*");
+ nm_config_reload (config, SIGHUP);
+
+
+ /* test with subscribing two signals... */
+ g_signal_connect (G_OBJECT (config),
+ NM_CONFIG_SIGNAL_CONFIG_CHANGED,
+ G_CALLBACK (_test_signal_config_changed_cb2),
+ &expected);
+ expected = NM_CONFIG_CHANGE_SIGUSR2;
+ g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE, "*config: signal SIGUSR2");
+ nm_config_reload (config, SIGUSR2);
+ g_signal_handlers_disconnect_by_func (config, _test_signal_config_changed_cb2, &expected);
+
+
+ g_signal_handlers_disconnect_by_func (config, _test_signal_config_changed_cb, &expected);
+
+ g_assert (config_data_orig == nm_config_get_data (config));
+}
+
+/*****************************************************************************/
+
NMTST_DEFINE ();
int
@@ -419,6 +499,8 @@ main (int argc, char **argv)
g_test_add_func ("/config/confdir", test_config_confdir);
g_test_add_func ("/config/confdir-parse-error", test_config_confdir_parse_error);
+ g_test_add_func ("/config/signal", test_config_signal);
+
/* This one has to come last, because it leaves its values in
* nm-config.c's global variables, and there's no way to reset
* those to NULL.
--
2.4.3
From ac6ed03af5be72b3fe0e6796804129e02043e18e Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 8 Sep 2015 21:39:40 +0200
Subject: [PATCH 2/3] config: workaround invocation of "config-changed" signal
There seems to be a bug in glib/ffi that hits on s390x/ppc64 architecture.
It causes @changes in nm-dns-manager.c:config_changed_cb() to be NONE,
although it is clearly set (see the related bug rh #1260577 for glib).
Workaround this, by making the argument type a plain guint.
Note that the ill behavior is caught by test_config_signal() in
"src/tests/config/test-config.c".
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1062301
(cherry picked from commit e7d66f1df61ebdc7652ba34a4e1baddcc86b9e26)
(cherry picked from commit 9aecabe29f39350c5e31a61af4d49062108dfc89)
---
src/nm-config.c | 12 +++++++++++-
src/tests/config/test-config.c | 5 ++++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/nm-config.c b/src/nm-config.c
index 86b3d92..011e57f 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -1214,7 +1214,17 @@ nm_config_class_init (NMConfigClass *config_class)
G_SIGNAL_RUN_FIRST,
G_STRUCT_OFFSET (NMConfigClass, config_changed),
NULL, NULL, NULL,
- G_TYPE_NONE, 3, NM_TYPE_CONFIG_DATA, NM_TYPE_CONFIG_CHANGE_FLAGS, NM_TYPE_CONFIG_DATA);
+ G_TYPE_NONE,
+ 3,
+ NM_TYPE_CONFIG_DATA,
+ /* Use plain guint type for changes argument. This avoids
+ * glib/ffi bug https://bugzilla.redhat.com/show_bug.cgi?id=1260577 */
+ /* NM_TYPE_CONFIG_CHANGE_FLAGS, */
+ G_TYPE_UINT,
+ NM_TYPE_CONFIG_DATA);
+
+ G_STATIC_ASSERT_EXPR (sizeof (guint) == sizeof (NMConfigChangeFlags));
+ G_STATIC_ASSERT_EXPR (((gint64) ((NMConfigChangeFlags) -1)) > ((gint64) 0));
}
static void
diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c
index a5975c1..b973e60 100644
--- a/src/tests/config/test-config.c
+++ b/src/tests/config/test-config.c
@@ -458,7 +458,10 @@ test_config_signal (void)
nm_config_reload (config, SIGHUP);
- /* test with subscribing two signals... */
+ /* test with subscribing two signals...
+ *
+ * This test exposes glib bug https://bugzilla.redhat.com/show_bug.cgi?id=1260577
+ * for which we however have a workaround in 'nm-config.c' */
g_signal_connect (G_OBJECT (config),
NM_CONFIG_SIGNAL_CONFIG_CHANGED,
G_CALLBACK (_test_signal_config_changed_cb2),
--
2.4.3
From 8bb4c36cdc6033a2216fbe3e8fbac23c9b3bc5f6 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 9 Sep 2015 12:22:16 +0200
Subject: [PATCH 3/3] platform/test: add test for invoking platform signals
There seems to be an issue with glib/ffi that causes failures
to pass enum-typed arguments to signals (related bug rh#1260577).
Add a test for platform signals which, beside NM_CONFIG_SIGNAL_CONFIG_CHANGED,
is the only place where we use enum-typed arguments for signals.
Strangely, this test doesn't cause the failure, so it's unclear why
the workaround was necessary for "config-changed" signal (commit
e7d66f1df61ebdc7652ba34a4e1baddcc86b9e26).
(cherry picked from commit 52cd5ee6120b643aa143cbd439f81a0c02c8313e)
(cherry picked from commit d7970c58eb10e9d59c855d61017299a1a811ee2a)
---
src/platform/tests/test-link.c | 47 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c
index ce371b7..f9645d8 100644
--- a/src/platform/tests/test-link.c
+++ b/src/platform/tests/test-link.c
@@ -118,6 +118,41 @@ software_add (NMLinkType link_type, const char *name)
}
static void
+test_link_changed_signal_cb (NMPlatform *platform,
+ NMPObjectType obj_type,
+ int ifindex,
+ const NMPlatformIP4Route *route,
+ NMPlatformSignalChangeType change_type,
+ NMPlatformReason reason,
+ gboolean *p_test_link_changed_signal_arg)
+{
+ /* test invocation of platform signals with multiple listeners
+ * connected to the signal. Platform signals have enum-typed
+ * arguments and there seem to be an issue with invoking such
+ * signals on s390x and ppc64 archs.
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1260577
+ *
+ * As the test shows, the failure is not reproducible for
+ * platform signals.
+ */
+ g_assert (NM_IS_PLATFORM (platform));
+ g_assert (platform == NM_PLATFORM_GET);
+
+ g_assert (ifindex > 0);
+ g_assert (route);
+
+ g_assert_cmpint (obj_type, ==, NMP_OBJECT_TYPE_LINK);
+
+ g_assert_cmpint ((gint64) change_type, !=, (gint64) 0);
+ g_assert_cmpint (change_type, !=, NM_PLATFORM_SIGNAL_NONE);
+
+ g_assert_cmpint ((gint64) reason, !=, (gint64) 0);
+ g_assert_cmpint (reason, !=, NM_PLATFORM_REASON_NONE);
+
+ *p_test_link_changed_signal_arg = TRUE;
+}
+
+static void
test_slave (int master, int type, SignalData *master_changed)
{
int ifindex;
@@ -125,6 +160,8 @@ test_slave (int master, int type, SignalData *master_changed)
SignalData *link_changed, *link_removed;
char *value;
NMLinkType link_type = nm_platform_link_get_type (NM_PLATFORM_GET, master);
+ gboolean test_link_changed_signal_arg1;
+ gboolean test_link_changed_signal_arg2;
g_assert (NM_IN_SET (link_type, NM_LINK_TYPE_TEAM, NM_LINK_TYPE_BOND, NM_LINK_TYPE_BRIDGE));
@@ -158,11 +195,21 @@ test_slave (int master, int type, SignalData *master_changed)
else
g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex));
+ test_link_changed_signal_arg1 = FALSE;
+ test_link_changed_signal_arg2 = FALSE;
+ g_signal_connect (NM_PLATFORM_GET, NM_PLATFORM_SIGNAL_LINK_CHANGED, G_CALLBACK (test_link_changed_signal_cb), &test_link_changed_signal_arg1);
+ g_signal_connect (NM_PLATFORM_GET, NM_PLATFORM_SIGNAL_LINK_CHANGED, G_CALLBACK (test_link_changed_signal_cb), &test_link_changed_signal_arg2);
+
/* Set master up */
g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, master, NULL));
g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, master));
accept_signals (master_changed, 1, 2);
+ g_signal_handlers_disconnect_by_func (NM_PLATFORM_GET, G_CALLBACK (test_link_changed_signal_cb), &test_link_changed_signal_arg1);
+ g_signal_handlers_disconnect_by_func (NM_PLATFORM_GET, G_CALLBACK (test_link_changed_signal_cb), &test_link_changed_signal_arg2);
+ g_assert (test_link_changed_signal_arg1);
+ g_assert (test_link_changed_signal_arg2);
+
/* Master with a disconnected slave is disconnected
*
* For some reason, bonding and teaming slaves are automatically set up. We
--
2.4.3