From 969a08cff1196185cc630a1898793b80b3d94da6 Mon Sep 17 00:00:00 2001 From: Thomas Haller 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 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