ifcfg-rh: read/write VLAN GVRP flags with GVRP= ifcfg file variable

initscripts uses GVRP variable for the flag (since 2011):
https://git.fedorahosted.org/cgit/initscripts.git/commit/?id=f662d4777625cd3bedea19cccabea7741a8b45c9

But continue reading "GVRP" from VLAN_FLAG= if GVRP= is missing.

libnm-core/nm-setting-vlan.c

@@ -762,8 +762,8 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
 	 **/
 	/* ---ifcfg-rh---
 	 * property: flags
-	 * variable: VLAN_FLAGS, REORDER_HDR
+	 * variable: REORDER_HDR, GVRP, VLAN_FLAGS
-	 * values: "GVRP", "LOOSE_BINDING" for VLAN_FLAGS; 0 or 1 for REORDER_HDR
+	 * values: "yes or "no" for REORDER_HDR and GVRP; "LOOSE_BINDING" for VLAN_FLAGS
 	 * description: VLAN flags.
 	 * ---end---
 	 */

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

@@ -4575,6 +4575,7 @@ make_vlan_setting (shvarFile *ifcfg,
 	char *end = NULL;
 	gint vlan_id = -1;
 	guint32 vlan_flags = 0;
+	gint gvrp;
 
 	value = svGetValue (ifcfg, "VLAN_ID", FALSE);
 	if (value) {
@@ -4652,9 +4653,14 @@ make_vlan_setting (shvarFile *ifcfg,
 	if (svGetValueBoolean (ifcfg, "REORDER_HDR", FALSE))
 		vlan_flags |= NM_VLAN_FLAG_REORDER_HEADERS;
 
+	gvrp = svGetValueBoolean (ifcfg, "GVRP", -1);
+	if (gvrp > 0)
+		vlan_flags |= NM_VLAN_FLAG_GVRP;
+
 	value = svGetValue (ifcfg, "VLAN_FLAGS", FALSE);
 	if (value) {
-		if (g_strstr_len (value, -1, "GVRP"))
+		/* Prefer GVRP variable; only take VLAN_FLAG=GVRP when GVRP is not specified */
+		if (g_strstr_len (value, -1, "GVRP") && gvrp == -1)
 			vlan_flags |= NM_VLAN_FLAG_GVRP;
 		if (g_strstr_len (value, -1, "LOOSE_BINDING"))
 			vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING;

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

@@ -102,6 +102,8 @@ EXTRA_DIST = \
 	ifcfg-test-vlan-only-device \
 	ifcfg-test-vlan-physdev \
 	ifcfg-test-vlan-reorder-hdr-1 \
+	ifcfg-test-vlan-flags-1 \
+	ifcfg-test-vlan-flags-2 \
 	ifcfg-test-wifi-wep-no-keys \
 	ifcfg-test-permissions \
 	ifcfg-test-wifi-wep-agent-keys \

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-1

@@ -0,0 +1,14 @@
+VLAN=yes
+TYPE=Vlan
+DEVICE=super-vlan
+VLAN_ID=44
+PHYSDEV=eth9
+REORDER_HDR=yes
+GVRP=no
+VLAN_FLAGS="GVRP LOOSE_BINDING"
+VLAN_INGRESS_PRIORITY_MAP=0:1,2:5
+VLAN_EGRESS_PRIORITY_MAP=12:3,14:7,3:1
+ONBOOT=yes
+BOOTPROTO=static
+IPADDR=192.168.43.149
+NETMASK=255.255.255.0

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-2

@@ -0,0 +1,11 @@
+VLAN=yes
+TYPE=Vlan
+DEVICE=super-vlan
+VLAN_ID=44
+PHYSDEV=eth9
+REORDER_HDR=no
+VLAN_FLAGS="GVRP LOOSE_BINDING"
+ONBOOT=yes
+BOOTPROTO=static
+IPADDR=192.168.43.149
+NETMASK=255.255.255.0

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

@@ -11130,6 +11130,62 @@ test_read_vlan_reorder_hdr_1 (void)
 	g_object_unref (connection);
 }
 
+static void
+test_read_vlan_flags_1 (void)
+{
+	NMConnection *connection;
+	GError *error = NULL;
+	NMSettingVlan *s_vlan;
+
+	connection = connection_from_file_test (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-flags-1",
+	                                        NULL,
+	                                        TYPE_ETHERNET,
+	                                        NULL,
+	                                        &error);
+	g_assert_no_error (error);
+	g_assert (connection != NULL);
+
+	g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "super-vlan");
+
+	s_vlan = nm_connection_get_setting_vlan (connection);
+	g_assert (s_vlan);
+
+	g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9");
+	g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 44);
+	/* reorder_hdr and loose_binding */
+	g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 5);
+
+	g_object_unref (connection);
+}
+
+static void
+test_read_vlan_flags_2 (void)
+{
+	NMConnection *connection;
+	GError *error = NULL;
+	NMSettingVlan *s_vlan;
+
+	connection = connection_from_file_test (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-flags-2",
+	                                        NULL,
+	                                        TYPE_ETHERNET,
+	                                        NULL,
+	                                        &error);
+	g_assert_no_error (error);
+	g_assert (connection != NULL);
+
+	g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "super-vlan");
+
+	s_vlan = nm_connection_get_setting_vlan (connection);
+	g_assert (s_vlan);
+
+	g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9");
+	g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 44);
+	/* gvrp and loose_binding */
+	g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 6);
+
+	g_object_unref (connection);
+}
+
 static void
 test_write_vlan (void)
 {
@@ -11157,6 +11213,54 @@ test_write_vlan (void)
 	g_object_unref (connection);
 }
 
+static void
+test_write_vlan_flags (void)
+{
+	NMConnection *connection, *reread;
+	char *written = NULL;
+	GError *error = NULL;
+	gboolean success = FALSE;
+
+	connection = connection_from_file_test (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-flags-2",
+	                                        NULL,
+	                                        TYPE_VLAN,
+	                                        NULL,
+	                                        &error);
+	g_assert (connection != NULL);
+
+	success = writer_new_connection (connection,
+	                                 TEST_SCRATCH_DIR "/network-scripts/",
+	                                 &written,
+	                                 &error);
+	g_assert (success);
+
+	/* reread will be normalized, so we must normalize connection too. */
+	nm_connection_normalize (connection, NULL, NULL, NULL);
+
+	/* re-read the connection for comparison */
+	reread = connection_from_file_test (written,
+	                                    NULL,
+	                                    TYPE_ETHERNET,
+	                                    NULL,
+	                                    &error);
+
+	unlink (written);
+	g_free (written);
+
+	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);
+}
+
 static void
 test_write_vlan_only_vlanid (void)
 {
@@ -12964,6 +13068,8 @@ int main (int argc, char **argv)
 	test_read_wifi_wep_agent_keys ();
 	test_read_infiniband ();
 	test_read_vlan_interface ();
+	g_test_add_func (TPATH "vlan/read-flags-1", test_read_vlan_flags_1);
+	g_test_add_func (TPATH "vlan/read-flags-2", test_read_vlan_flags_2);
 	test_read_vlan_only_vlan_id ();
 	test_read_vlan_only_device ();
 	g_test_add_func (TPATH "vlan/physdev", test_read_vlan_physdev);
@@ -13046,6 +13152,7 @@ int main (int argc, char **argv)
 	test_write_wifi_wep_agent_keys ();
 	test_write_infiniband ();
 	test_write_vlan ();
+	g_test_add_func (TPATH "vlan/write-flags", test_write_vlan_flags);
 	test_write_vlan_only_vlanid ();
 	g_test_add_func (TPATH "vlan/write-vlan-reorder-hdr", test_write_vlan_reorder_hdr);
 	test_write_ethernet_missing_ipv6 ();

src/settings/plugins/ifcfg-rh/writer.c

@@ -1264,13 +1264,10 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wired,
 	else
 		svSetValue (ifcfg, "REORDER_HDR", "no", FALSE);
 
+	svSetValue (ifcfg, "GVRP", vlan_flags & NM_VLAN_FLAG_GVRP ? "yes" : "no", 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);
 
 	tmp = vlan_priority_maplist_to_stringlist (s_vlan, NM_VLAN_INGRESS_MAP);