diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index 22333bdad..eedd460f8 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -3744,13 +3744,27 @@ make_vlan_setting (shvarFile *ifcfg, NMSettingVlan *s_vlan = NULL; char *value = NULL; char *iface_name = NULL; - char *master = NULL; - char *p = NULL; - guint32 vlan_id = 0; + char *parent = NULL; + const char *p = NULL, *w; + gboolean has_numbers = FALSE; + gint vlan_id = -1; guint32 vlan_flags = 0; + value = svGetValue (ifcfg, "VLAN_ID", FALSE); + if (value) { + errno = 0; + vlan_id = (gint) g_ascii_strtoll (value, NULL, 10); + if (vlan_id < 0 || vlan_id > 4096 || errno) { + g_set_error (error, IFCFG_PLUGIN_ERROR, 0, "Invalid VLAN_ID '%s'", value); + g_free (value); + return NULL; + } + g_free (value); + } + + /* Need DEVICE if we don't have a separate VLAN_ID property */ iface_name = svGetValue (ifcfg, "DEVICE", FALSE); - if (!iface_name) { + if (!iface_name && vlan_id < 0) { g_set_error_literal (error, IFCFG_PLUGIN_ERROR, 0, "Missing DEVICE property; cannot determine VLAN ID."); return NULL; @@ -3758,47 +3772,65 @@ make_vlan_setting (shvarFile *ifcfg, s_vlan = NM_SETTING_VLAN (nm_setting_vlan_new ()); - g_object_set (s_vlan, NM_SETTING_VLAN_INTERFACE_NAME, iface_name, NULL); + if (iface_name) { + g_object_set (s_vlan, NM_SETTING_VLAN_INTERFACE_NAME, iface_name, NULL); - p = strchr (iface_name, '.'); - if (p) { - /* eth0.43; PHYSDEV is assumed from it */ - master = g_strndup (iface_name, p - iface_name); - p++; - } else { - /* format like vlan43; PHYSDEV or MASTER must be set */ - if (g_str_has_prefix (iface_name, "vlan")) - p = iface_name + 4; + p = strchr (iface_name, '.'); + if (p) { + /* eth0.43; PHYSDEV is assumed from it */ + parent = g_strndup (iface_name, p - iface_name); + p++; + } else { + /* format like vlan43; PHYSDEV or MASTER must be set */ + if (g_str_has_prefix (iface_name, "vlan")) + p = iface_name + 4; + } - master = svGetValue (ifcfg, "PHYSDEV", FALSE); - if (master == NULL) - master = svGetValue (ifcfg, "MASTER", FALSE); + w = p; + while (*w && !has_numbers) + has_numbers = g_ascii_isdigit (*w); + + /* Grab VLAN ID from interface name; this takes precedence over the + * separate VLAN_ID property for backwards compat. + */ + if (has_numbers) { + errno = 0; + vlan_id = (gint) g_ascii_strtoll (p, NULL, 10); + if (vlan_id < 0 || vlan_id > 4095 || errno) { + g_set_error (error, IFCFG_PLUGIN_ERROR, 0, + "Failed to determine VLAN ID from DEVICE '%s'", + iface_name); + goto error; + } + } } - if (master == NULL) { + if (vlan_id < 0) { g_set_error_literal (error, IFCFG_PLUGIN_ERROR, 0, - "Failed to determine VLAN master from DEVICE, PHYSDEV, or MASTER"); + "Failed to determine VLAN ID from DEVICE or VLAN_ID."); goto error; } + g_object_set (s_vlan, NM_SETTING_VLAN_ID, vlan_id, NULL); - vlan_id = g_ascii_strtoull (p, NULL, 10); - if (vlan_id >= 0 && vlan_id < 4096) - g_object_set (s_vlan, NM_SETTING_VLAN_ID, vlan_id, NULL); - else { - g_set_error (error, IFCFG_PLUGIN_ERROR, 0, - "Failed to determine VLAN ID from DEVICE '%s'", - iface_name); + if (!parent) + parent = svGetValue (ifcfg, "PHYSDEV", FALSE); + if (parent == NULL) { + g_set_error_literal (error, IFCFG_PLUGIN_ERROR, 0, + "Failed to determine VLAN parent from DEVICE or PHYSDEV"); goto error; } + g_object_set (s_vlan, NM_SETTING_VLAN_PARENT, parent, NULL); if (svTrueValue (ifcfg, "REORDER_HDR", FALSE)) vlan_flags |= NM_VLAN_FLAG_REORDER_HEADERS; value = svGetValue (ifcfg, "VLAN_FLAGS", FALSE); - if (g_strstr_len (value, -1, "GVRP")) - vlan_flags |= NM_VLAN_FLAG_GVRP; - if (g_strstr_len (value, -1, "LOOSE_BINDING")) - vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING; + if (value) { + if (g_strstr_len (value, -1, "GVRP")) + vlan_flags |= NM_VLAN_FLAG_GVRP; + if (g_strstr_len (value, -1, "LOOSE_BINDING")) + vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING; + } g_object_set (s_vlan, NM_SETTING_VLAN_FLAGS, vlan_flags, NULL); g_free (value); @@ -3807,11 +3839,11 @@ make_vlan_setting (shvarFile *ifcfg, parse_prio_map_list (s_vlan, ifcfg, "VLAN_EGRESS_PRIORITY_MAP", NM_VLAN_EGRESS_MAP); if (out_master) - *out_master = master; + *out_master = svGetValue (ifcfg, "MASTER", FALSE); return (NMSetting *) s_vlan; error: - g_free (master); + g_free (parent); g_free (iface_name); g_object_unref (s_vlan); return NULL; @@ -3858,12 +3890,7 @@ vlan_connection_from_ifcfg (const char *file, nm_connection_add_setting (connection, vlan_setting); /* Handle master interface or connection */ - if (master == NULL) { - g_set_error_literal (error, IFCFG_PLUGIN_ERROR, 0, - "No master interface or connection UUID specified."); - g_object_unref (connection); - return NULL; - } else { + if (master) { g_object_set (con_setting, NM_SETTING_CONNECTION_MASTER, master, NULL); g_object_set (con_setting, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_VLAN_SETTING_NAME, 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 7c55b6262..56fee91d0 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am @@ -79,6 +79,8 @@ EXTRA_DIST = \ ifcfg-test-bridge-main \ ifcfg-test-bridge-component \ ifcfg-test-vlan-interface \ + ifcfg-test-vlan-only-vlanid \ + ifcfg-test-vlan-only-device \ ifcfg-test-wifi-wep-no-keys \ ifcfg-test-permissions \ ifcfg-test-wifi-wep-agent-keys \ diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-device b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-device new file mode 100644 index 000000000..4ba06f28e --- /dev/null +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-device @@ -0,0 +1,4 @@ +VLAN=yes +TYPE=Vlan +DEVICE=eth0.9 + diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-vlanid b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-vlanid new file mode 100644 index 000000000..622d41ec7 --- /dev/null +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-only-vlanid @@ -0,0 +1,6 @@ +VLAN=yes +TYPE=Vlan +PHYSDEV=eth9 +VLAN_ID=43 +ONBOOT=yes + 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 c38570200..89a10cad3 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -11756,7 +11756,6 @@ test_read_vlan_interface (void) char *route6file = NULL; gboolean ignore_error = FALSE; GError *error = NULL; - NMSettingConnection *s_con; NMSettingVlan *s_vlan; guint32 from = 0, to = 0; @@ -11778,16 +11777,11 @@ test_read_vlan_interface (void) g_free (routefile); g_free (route6file); - s_con = nm_connection_get_setting_connection (connection); - g_assert (s_con); - - g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, "eth9"); - g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_VLAN_SETTING_NAME); - s_vlan = nm_connection_get_setting_vlan (connection); g_assert (s_vlan); g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, "vlan43"); + g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 43); g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_GVRP | NM_VLAN_FLAG_LOOSE_BINDING); @@ -11821,6 +11815,90 @@ test_read_vlan_interface (void) g_object_unref (connection); } +#define TEST_IFCFG_VLAN_ONLY_VLANID TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-only-vlanid" + +static void +test_read_vlan_only_vlan_id (void) +{ + NMConnection *connection; + char *unmanaged = NULL; + char *keyfile = NULL; + char *routefile = NULL; + char *route6file = NULL; + gboolean ignore_error = FALSE; + GError *error = NULL; + NMSettingVlan *s_vlan; + + connection = connection_from_file (TEST_IFCFG_VLAN_ONLY_VLANID, + NULL, + TYPE_ETHERNET, + NULL, + &unmanaged, + &keyfile, + &routefile, + &route6file, + &error, + &ignore_error); + g_assert_no_error (error); + g_assert (connection != NULL); + + g_free (unmanaged); + g_free (keyfile); + g_free (routefile); + g_free (route6file); + + s_vlan = nm_connection_get_setting_vlan (connection); + g_assert (s_vlan); + + g_assert (nm_setting_vlan_get_interface_name (s_vlan) == NULL); + g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); + g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 43); + + g_object_unref (connection); +} + +#define TEST_IFCFG_VLAN_ONLY_DEVICE TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-only-device" + +static void +test_read_vlan_only_device (void) +{ + NMConnection *connection; + char *unmanaged = NULL; + char *keyfile = NULL; + char *routefile = NULL; + char *route6file = NULL; + gboolean ignore_error = FALSE; + GError *error = NULL; + NMSettingVlan *s_vlan; + + connection = connection_from_file (TEST_IFCFG_VLAN_ONLY_DEVICE, + NULL, + TYPE_ETHERNET, + NULL, + &unmanaged, + &keyfile, + &routefile, + &route6file, + &error, + &ignore_error); + g_assert_no_error (error); + g_assert (connection != NULL); + + g_free (unmanaged); + g_free (keyfile); + g_free (routefile); + g_free (route6file); + + s_vlan = nm_connection_get_setting_vlan (connection); + g_assert (s_vlan); + + g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, "eth0.9"); + g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth0"); + g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 9); + + g_object_unref (connection); +} + static void test_write_vlan (void) { @@ -11861,6 +11939,79 @@ test_write_vlan (void) g_free (route6file); } +static void +test_write_vlan_only_vlanid (void) +{ + NMConnection *connection, *reread; + char *unmanaged = NULL; + char *keyfile = NULL; + char *routefile = NULL; + char *route6file = NULL; + char *written = NULL; + gboolean ignore_error = FALSE; + GError *error = NULL; + gboolean success = FALSE; + + connection = connection_from_file (TEST_IFCFG_VLAN_ONLY_VLANID, + NULL, + TYPE_VLAN, + NULL, + &unmanaged, + &keyfile, + &routefile, + &route6file, + &error, + &ignore_error); + g_assert_no_error (error); + g_assert (connection != NULL); + + g_free (unmanaged); + unmanaged = NULL; + g_free (keyfile); + keyfile = NULL; + g_free (routefile); + routefile = NULL; + g_free (route6file); + route6file = NULL; + + success = writer_new_connection (connection, + TEST_SCRATCH_DIR "/network-scripts/", + &written, + &error); + g_assert (success); + + /* re-read the connection for comparison */ + reread = connection_from_file (written, + NULL, + TYPE_ETHERNET, + NULL, + &unmanaged, + &keyfile, + &routefile, + &route6file, + &error, + &ignore_error); + unlink (written); + g_free (written); + g_free (unmanaged); + g_free (keyfile); + g_free (routefile); + g_free (route6file); + + g_assert_no_error (error); + g_assert (reread != NULL); + + success = nm_connection_verify (reread, &error); + g_assert_no_error (error); + g_assert (success); + + success = nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT); + g_assert (success); + + g_object_unref (connection); + g_object_unref (reread); +} + #define TEST_IFCFG_BOND_MAIN TEST_IFCFG_DIR"/network-scripts/ifcfg-test-bond-main" static void @@ -12541,6 +12692,9 @@ int main (int argc, char **argv) test_read_permissions (); test_read_wifi_wep_agent_keys (); test_read_infiniband (); + test_read_vlan_interface (); + test_read_vlan_only_vlan_id (); + test_read_vlan_only_device (); test_write_wired_static (); test_write_wired_static_ip6_only (); @@ -12605,6 +12759,8 @@ int main (int argc, char **argv) test_write_permissions (); test_write_wifi_wep_agent_keys (); test_write_infiniband (); + test_write_vlan (); + test_write_vlan_only_vlanid (); /* iSCSI / ibft */ test_read_ibft_dhcp (); diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index 9afdf50ee..06840ce43 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -1152,7 +1152,7 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) { NMSettingVlan *s_vlan; NMSettingConnection *s_con; - const char *master = NULL; + char *tmp; guint32 vlan_flags = 0; GString *text = NULL; @@ -1170,17 +1170,13 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) svSetValue (ifcfg, "VLAN", "yes", FALSE); svSetValue (ifcfg, "TYPE", TYPE_VLAN, FALSE); - - master = nm_setting_connection_get_master (s_con); - if (!master) { - g_set_error_literal (error, IFCFG_PLUGIN_ERROR, 0, - "Missing VLAN master interface name or connection UUID"); - return FALSE; - } - svSetValue (ifcfg, "PHYSDEV", master, FALSE); - svSetValue (ifcfg, "MASTER", master, FALSE); - svSetValue (ifcfg, "DEVICE", nm_setting_vlan_get_interface_name (s_vlan), FALSE); + svSetValue (ifcfg, "PHYSDEV", nm_setting_vlan_get_parent (s_vlan), FALSE); + svSetValue (ifcfg, "MASTER", nm_setting_connection_get_master (s_con), FALSE); + + tmp = g_strdup_printf ("%d", nm_setting_vlan_get_id (s_vlan)); + svSetValue (ifcfg, "VLAN_ID", tmp, FALSE); + g_free (tmp); vlan_flags = nm_setting_vlan_get_flags (s_vlan); if (vlan_flags & NM_VLAN_FLAG_REORDER_HEADERS) @@ -1188,27 +1184,24 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) else svSetValue (ifcfg, "REORDER_HDR", "0", FALSE); + svSetValue (ifcfg, "VLAN_FLAGS", NULL, FALSE); if (vlan_flags & NM_VLAN_FLAG_GVRP) { if (vlan_flags & NM_VLAN_FLAG_LOOSE_BINDING) svSetValue (ifcfg, "VLAN_FLAGS", "GVRP,LOOSE_BINDING", FALSE); else svSetValue (ifcfg, "VLAN_FLAGS", "GVRP", FALSE); - } else { - if (vlan_flags & NM_VLAN_FLAG_LOOSE_BINDING) - svSetValue (ifcfg, "VLAN_FLAGS", "LOOSE_BINDING", FALSE); - } + } else if (vlan_flags & NM_VLAN_FLAG_LOOSE_BINDING) + svSetValue (ifcfg, "VLAN_FLAGS", "LOOSE_BINDING", FALSE); text = vlan_priority_maplist_to_stringlist (s_vlan, NM_VLAN_INGRESS_MAP); - if (text != NULL) - svSetValue (ifcfg, "VLAN_INGRESS_PRIORITY_MAP", text->str, FALSE); - g_string_free (text, TRUE); - text = NULL; + svSetValue (ifcfg, "VLAN_INGRESS_PRIORITY_MAP", text ? text->str : NULL, FALSE); + if (text) + g_string_free (text, TRUE); text = vlan_priority_maplist_to_stringlist (s_vlan, NM_VLAN_EGRESS_MAP); - if (text != NULL) - svSetValue (ifcfg, "VLAN_EGRESS_PRIORITY_MAP", text->str, FALSE); - g_string_free (text, TRUE); - text = NULL; + svSetValue (ifcfg, "VLAN_EGRESS_PRIORITY_MAP", text ? text->str : NULL, FALSE); + if (text) + g_string_free (text, TRUE); return TRUE; }