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