From 969a08cff1196185cc630a1898793b80b3d94da6 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 23 Aug 2016 16:19:42 +0200

Subject: [PATCH 1/2] ifcfg-rh: make out_unhandled argument non-optional

Depending on the connection we are about to read,

we would assert that the user provided a @out_unhandled

argument.

That means, the user must always provide a valid @out_unhandled

pointer, because he cannot know beforehand how the reading

of the ifcfg file goes.

(cherry picked from commit 50d7ac4af3f6908a09a857d4127e196b2df37c27)

(cherry picked from commit 6de181247f2f999b8ebec75e21bdd07f1c1f18bb)

---

 src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c |  4 +++-

 src/settings/plugins/ifcfg-rh/reader.c              |  9 ++-------

 src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 10 ++++++----

 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c

index 82f2059..184e95b 100644

--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c

+++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c

@@ -395,7 +395,9 @@ commit_changes (NMSettingsConnection *connection,

 	 */

 	filename = nm_settings_connection_get_filename (connection);

 	if (filename) {

-		reread = connection_from_file (filename, NULL, NULL, NULL);

+		gs_free char *unhandled = NULL;

+

+		reread = connection_from_file (filename, &unhandled, NULL, NULL);

 		if (reread) {

 			same = nm_connection_compare (NM_CONNECTION (connection),

 			                              reread,

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c

index af5d291..e3d8ee1 100644

--- a/src/settings/plugins/ifcfg-rh/reader.c

+++ b/src/settings/plugins/ifcfg-rh/reader.c

@@ -4848,7 +4848,7 @@ create_unhandled_connection (const char *filename, shvarFile *ifcfg,

 	NMSetting *s_con;

 	char *value;

-	g_assert (out_spec != NULL);

+	nm_assert (out_spec && !*out_spec);

 	connection = nm_simple_connection_new ();

@@ -4963,8 +4963,7 @@ connection_from_file_full (const char *filename,

 	const char *ifcfg_name = NULL;

 	g_return_val_if_fail (filename != NULL, NULL);

-	if (out_unhandled)

-		g_return_val_if_fail (*out_unhandled == NULL, NULL);

+	g_return_val_if_fail (out_unhandled && !*out_unhandled, NULL);

 	/* Non-NULL only for unit tests; normally use /etc/sysconfig/network */

 	if (!network_file)

@@ -4982,8 +4981,6 @@ connection_from_file_full (const char *filename,

 		return NULL;

 	if (!svGetValueBoolean (parsed, "NM_CONTROLLED", TRUE)) {

-		g_assert (out_unhandled != NULL);

-

 		connection = create_unhandled_connection (filename, parsed, "unmanaged", out_unhandled);

 		if (!connection)

 			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,

@@ -5136,8 +5133,6 @@ connection_from_file_full (const char *filename,

 	else if (!strcasecmp (type, TYPE_BRIDGE))

 		connection = bridge_connection_from_ifcfg (filename, parsed, error);

 	else {

-		g_assert (out_unhandled != NULL);

-

 		connection = create_unhandled_connection (filename, parsed, "unrecognized", out_unhandled);

 		if (!connection)

 			PARSE_WARNING ("connection type was unrecognized but device was not uniquely identified; device may be managed");

diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

index b73f2f5..3a67a83 100644

--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

@@ -67,11 +67,14 @@ _connection_from_file (const char *filename,

 {

 	NMConnection *connection;

 	GError *error = NULL;

+	char *unhandled_fallback = NULL;

 	g_assert (!out_unhandled || !*out_unhandled);

-	connection = connection_from_file_test (filename, network_file, test_type, out_unhandled, &error);

+	connection = connection_from_file_test (filename, network_file, test_type,

+	                                        out_unhandled ?: &unhandled_fallback, &error);

 	g_assert_no_error (error);

+	g_assert (!unhandled_fallback);

 	if (out_unhandled && *out_unhandled)

 		nmtst_assert_connection_verifies (connection);

@@ -89,13 +92,12 @@ _connection_from_file_fail (const char *filename,

 	NMConnection *connection;

 	GError *local = NULL;

 	char *unhandled = NULL;

-	char **p_unhandled = (nmtst_get_rand_int () % 2) ? &unhandled : NULL;

-	connection = connection_from_file_test (filename, network_file, test_type, p_unhandled, &local);

+	connection = connection_from_file_test (filename, network_file, test_type, &unhandled, &local);

 	g_assert (!connection);

 	g_assert (local);

-	g_assert (!p_unhandled || !*p_unhandled);

+	g_assert (!unhandled);

 	g_propagate_error (error, local);

 }

-- 

2.7.4

From eb446ad72bfdac1f5f93dcc2714cb222b0a18dc5 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 23 Aug 2016 14:36:09 +0200

Subject: [PATCH 2/2] ifcfg-rh: accept TEAM connections also without DEVICETYPE

 setting

Allow omitting DEVICETYPE=Team or DEVICETYPE=TeamPort and accept

team connections based on the presence of TEAM_CONFIG/TEAM_MASTER

alone.

Also, check first for a team slave before checking for bond

slave. That is what initscripts do and matters if somebody wrongly

sets MASTER and TEAM_MASTER.

libteam:     https://github.com/jpirko/libteam/commit/20d45a1e026c27f4ec37383d4bb2c446a2744c02

initscripts: https://git.fedorahosted.org/cgit/initscripts.git/commit/?id=3235be4a3da91bc91c698b318935240dbdf81aac

https://bugzilla.redhat.com/show_bug.cgi?id=1367180

(cherry picked from commit 114eb5b9634353731ed50b44299e650958bff596)

(cherry picked from commit a82b7c8d5e5753ebdf92b285cfc3ed74208d8901)

---

 src/settings/plugins/ifcfg-rh/reader.c             | 48 +++++++++++++---------

 .../ifcfg-rh/tests/network-scripts/Makefile.am     |  6 ++-

 .../tests/network-scripts/ifcfg-test-team-master   |  6 ---

 .../tests/network-scripts/ifcfg-test-team-master-1 |  6 +++

 .../tests/network-scripts/ifcfg-test-team-master-2 |  5 +++

 .../tests/network-scripts/ifcfg-test-team-port     |  5 ---

 .../tests/network-scripts/ifcfg-test-team-port-1   |  5 +++

 .../tests/network-scripts/ifcfg-test-team-port-2   |  4 ++

 .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c         | 18 ++++----

 9 files changed, 62 insertions(+), 41 deletions(-)

 delete mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master

 create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1

 create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2

 delete mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port

 create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-1

 create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-2

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c

index e3d8ee1..d1ea597 100644

--- a/src/settings/plugins/ifcfg-rh/reader.c

+++ b/src/settings/plugins/ifcfg-rh/reader.c

@@ -1621,26 +1621,29 @@ check_if_bond_slave (shvarFile *ifcfg,

 	 */

 }

-static void

+static gboolean

 check_if_team_slave (shvarFile *ifcfg,

                      NMSettingConnection *s_con)

 {

-	char *value;

+	gs_free char *value = NULL;

-	value = svGetValue (ifcfg, "DEVICETYPE", FALSE);

-	if (!value)

-		return;

-	if (strcasecmp (value, TYPE_TEAM_PORT)) {

-		g_free (value);

-		return;

-	}

-	g_free (value);

 	value = svGetValue (ifcfg, "TEAM_MASTER", FALSE);

 	if (!value)

-		return;

+		return FALSE;

 	g_object_set (s_con, NM_SETTING_CONNECTION_MASTER, value, NULL);

 	g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_TEAM_SETTING_NAME, NULL);

-	g_free (value);

+	return TRUE;

+}

+

+static void

+check_if_slave (shvarFile *ifcfg,

+                NMSettingConnection *s_con)

+{

+	g_return_if_fail (NM_IS_SETTING_CONNECTION (s_con));

+

+	if (check_if_team_slave (ifcfg, s_con))

+		return;

+	check_if_bond_slave (ifcfg, s_con);

 }

 typedef struct {

@@ -3948,8 +3951,7 @@ wired_connection_from_ifcfg (const char *file,

 		g_object_unref (connection);

 		return NULL;

 	}

-	check_if_bond_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

-	check_if_team_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

+	check_if_slave (ifcfg, (NMSettingConnection *) con_setting);

 	nm_connection_add_setting (connection, con_setting);

 	wired_setting = make_wired_setting (ifcfg, file, &s_8021x, error);

@@ -4099,8 +4101,7 @@ infiniband_connection_from_ifcfg (const char *file,

 		g_object_unref (connection);

 		return NULL;

 	}

-	check_if_bond_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

-	check_if_team_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

+	check_if_slave (ifcfg, (NMSettingConnection *) con_setting);

 	nm_connection_add_setting (connection, con_setting);

 	infiniband_setting = make_infiniband_setting (ifcfg, file, error);

@@ -4599,8 +4600,6 @@ is_bond_device (const char *name, shvarFile *parsed)

 	if (svGetValueBoolean (parsed, "BONDING_MASTER", FALSE))

 		return TRUE;

-	

-	/* XXX: Check for "bond[\d]+"? */

 	return FALSE;

 }

@@ -4816,8 +4815,7 @@ vlan_connection_from_ifcfg (const char *file,

 		g_object_unref (connection);

 		return NULL;

 	}

-	check_if_bond_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

-	check_if_team_slave (ifcfg, NM_SETTING_CONNECTION (con_setting));

+	check_if_slave (ifcfg, (NMSettingConnection *) con_setting);

 	nm_connection_add_setting (connection, con_setting);

 	vlan_setting = make_vlan_setting (ifcfg, file, error);

@@ -5008,6 +5006,16 @@ connection_from_file_full (const char *filename,

 			type = g_strdup (TYPE_ETHERNET);

 		g_free (devtype);

 	}

+	if (!type) {

+		gs_free char *t = NULL;

+

+		/* Team and TeamPort types are also accepted by the mere

+		 * presense of TEAM_CONFIG/TEAM_MASTER. They don't require

+		 * DEVICETYPE. */

+		t = svGetValue (parsed, "TEAM_CONFIG", FALSE);

+		if (t)

+			type = g_strdup (TYPE_TEAM);

+	}

 	if (!type)

 		type = svGetValue (parsed, "TYPE", FALSE);

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am

index fc759e6..7531ec0 100644

--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am

+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am

@@ -132,8 +132,10 @@ EXTRA_DIST = \

 	ifcfg-test-dcb-pgpct-not-100 \

 	ifcfg-test-fcoe-fabric \

 	ifcfg-test-fcoe-vn2vn \

-	ifcfg-test-team-master \

-	ifcfg-test-team-port \

+	ifcfg-test-team-master-1 \

+	ifcfg-test-team-master-2 \

+	ifcfg-test-team-port-1 \

+	ifcfg-test-team-port-2 \

 	ifcfg-test-team-port-empty-config \

 	ifcfg-test-vlan-trailing-spaces \

 	ifcfg-test-dns-options \

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master

deleted file mode 100644

index 7edc736..0000000

--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master

+++ /dev/null

@@ -1,6 +0,0 @@

-DEVICE=team0

-ONBOOT=no

-DEVICETYPE=Team

-BOOTPROTO=dhcp

-TEAM_CONFIG="{ \"device\": \"team0\", \"link_watch\": { \"name\": \"ethtool\" } }"

-

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1

new file mode 100644

index 0000000..7edc736

--- /dev/null

+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1

@@ -0,0 +1,6 @@

+DEVICE=team0

+ONBOOT=no

+DEVICETYPE=Team

+BOOTPROTO=dhcp

+TEAM_CONFIG="{ \"device\": \"team0\", \"link_watch\": { \"name\": \"ethtool\" } }"

+

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2

new file mode 100644

index 0000000..d01e37c

--- /dev/null

+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2

@@ -0,0 +1,5 @@

+DEVICE=team0

+ONBOOT=no

+BOOTPROTO=dhcp

+TEAM_CONFIG="{ \"device\": \"team0\", \"link_watch\": { \"name\": \"ethtool\" } }"

+

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port

deleted file mode 100644

index 966bec6..0000000

--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port

+++ /dev/null

@@ -1,5 +0,0 @@

-TYPE=Ethernet

-TEAM_PORT_CONFIG="{ \"p4p1\": { \"prio\": -10, \"sticky\": true } }"

-DEVICE=p4p1

-TEAM_MASTER=team0

-DEVICETYPE=TeamPort

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-1 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-1

new file mode 100644

index 0000000..966bec6

--- /dev/null

+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-1

@@ -0,0 +1,5 @@

+TYPE=Ethernet

+TEAM_PORT_CONFIG="{ \"p4p1\": { \"prio\": -10, \"sticky\": true } }"

+DEVICE=p4p1

+TEAM_MASTER=team0

+DEVICETYPE=TeamPort

diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-2 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-2

new file mode 100644

index 0000000..992510e

--- /dev/null

+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-port-2

@@ -0,0 +1,4 @@

+TYPE=Ethernet

+TEAM_PORT_CONFIG="{ \"p4p1\": { \"prio\": -10, \"sticky\": true } }"

+DEVICE=p4p1

+TEAM_MASTER=team0

diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

index 3a67a83..3e3f7ca 100644

--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

@@ -8430,15 +8430,15 @@ test_write_fcoe_mode (gconstpointer user_data)

 }

 static void

-test_read_team_master (void)

+test_read_team_master (gconstpointer user_data)

 {

+	const char *const PATH_NAME = user_data;

 	NMConnection *connection;

 	NMSettingConnection *s_con;

 	NMSettingTeam *s_team;

 	const char *expected_config = "{ \"device\": \"team0\", \"link_watch\": { \"name\": \"ethtool\" } }";

-	connection = _connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master",

-	                                    NULL, TYPE_ETHERNET, NULL);

+	connection = _connection_from_file (PATH_NAME, NULL, TYPE_ETHERNET, NULL);

 	g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "team0");

@@ -8546,15 +8546,15 @@ test_write_team_master (void)

 }

 static void

-test_read_team_port (void)

+test_read_team_port (gconstpointer user_data)

 {

+	const char *const PATH_NAME = user_data;

 	NMConnection *connection;

 	NMSettingConnection *s_con;

 	NMSettingTeamPort *s_team_port;

 	const char *expected_config = "{ \"p4p1\": { \"prio\": -10, \"sticky\": true } }";

-	connection = _connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-port",

-	                                    NULL, TYPE_ETHERNET, NULL);

+	connection = _connection_from_file (PATH_NAME, NULL, TYPE_ETHERNET, NULL);

 	s_con = nm_connection_get_setting_connection (connection);

 	g_assert (s_con);

@@ -9048,9 +9048,11 @@ int main (int argc, char **argv)

 	g_test_add_func (TPATH "bridge/write-component", test_write_bridge_component);

 	g_test_add_func (TPATH "bridge/read-missing-stp", test_read_bridge_missing_stp);

-	g_test_add_func (TPATH "team/read-master", test_read_team_master);

+	g_test_add_data_func (TPATH "team/read-master-1", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master-1", test_read_team_master);

+	g_test_add_data_func (TPATH "team/read-master-2", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-master-2", test_read_team_master);

 	g_test_add_func (TPATH "team/write-master", test_write_team_master);

-	g_test_add_func (TPATH "team/read-port", test_read_team_port);

+	g_test_add_data_func (TPATH "team/read-port-1", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-port-1", test_read_team_port);

+	g_test_add_data_func (TPATH "team/read-port-2", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-team-port-2", test_read_team_port);

 	g_test_add_func (TPATH "team/write-port", test_write_team_port);

 	g_test_add_func (TPATH "team/read-port-empty-config", test_read_team_port_empty_config);

-- 

2.7.4