ifcfg-rh: fix handling of VLAN parent PHYSDEV key

The initscripts do this:

    MATCH='^.+\.[0-9]{1,4}$'
    if [[ "${DEVICE}" =~ $MATCH ]]; then
	VID=$(echo "${DEVICE}" | LC_ALL=C sed 's/^.*\.\([0-9]\+\)/\1/')
	PHYSDEV=${DEVICE%.*}
    fi
    MATCH='^vlan[0-9]{1,4}?'
    if [[ "${DEVICE}" =~ $MATCH ]]; then
	VID=$(echo "${DEVICE}" | LC_ALL=C sed 's/^vlan0*//')
	# PHYSDEV should be set in ifcfg-vlan* file
	if test -z "$PHYSDEV"; then
		net_log $"PHYSDEV should be set for device ${DEVICE}"
		exit 1
	fi
    fi

which means that if the VLAN name starts with "vlan" then
PHYSDEV must be set, otherwise the parent interface cannot
be determined.

Since PHYSDEV, if set, reflects the explicit intentions of the
user instead of assuming the name from DEVICE, make PHYSDEV
take precedence over determining the parent interface from
heuristics.

4 files changed, 46 insertions, 4 deletions

diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index 0ea97c4987..3a1dc5ad05 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -4123,13 +4123,25 @@ make_vlan_setting (shvarFile *ifcfg,
 
 	s_vlan = NM_SETTING_VLAN (nm_setting_vlan_new ());
 
+	/* Parent interface from PHYSDEV takes precedence if it exists */
+	parent = svGetValue (ifcfg, "PHYSDEV", FALSE);
+
 	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 */
-			parent = g_strndup (iface_name, p - iface_name);
+			/* eth0.43; PHYSDEV is assumed from it if unknown */
+			if (!parent) {
+				parent = g_strndup (iface_name, p - iface_name);
+				if (g_str_has_prefix (parent, "vlan")) {
+					/* Like initscripts, if no PHYSDEV and we get an obviously
+					 * invalid parent interface from DEVICE, fail.
+					 */
+					g_free (parent);
+					parent = NULL;
+				}
+			}
 			p++;
 		} else {
 			/* format like vlan43; PHYSDEV or MASTER must be set */
@@ -4158,8 +4170,6 @@ make_vlan_setting (shvarFile *ifcfg,
 	}
 	g_object_set (s_vlan, NM_SETTING_VLAN_ID, vlan_id, NULL);
 
-	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");
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 28230d7d99..df3ebb124c 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
@@ -85,6 +85,7 @@ EXTRA_DIST = \
 	ifcfg-test-vlan-interface \
 	ifcfg-test-vlan-only-vlanid \
 	ifcfg-test-vlan-only-device \
+	ifcfg-test-vlan-physdev \
 	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-physdev b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-physdev
new file mode 100644
index 0000000000..446c2a627f
--- /dev/null
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-physdev
@@ -0,0 +1,6 @@
+VLAN=yes
+TYPE=Vlan
+DEVICE=vlan0.3
+PHYSDEV=eth0
+VLAN_ID=3
+
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 eed585a5ff..73ea3f87f2 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -11505,6 +11505,30 @@ test_read_vlan_only_device (void)
 }
 
 static void
+test_read_vlan_physdev (void)
+{
+	NMConnection *connection;
+	GError *error = NULL;
+	NMSettingVlan *s_vlan;
+
+	connection = connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-physdev",
+	                                   NULL, TYPE_ETHERNET, NULL, NULL,
+	                                   NULL, NULL, NULL, &error, NULL);
+	g_assert_no_error (error);
+	g_assert (connection);
+	g_assert (nm_connection_verify (connection, &error));
+
+	s_vlan = nm_connection_get_setting_vlan (connection);
+	g_assert (s_vlan);
+
+	g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, "vlan0.3");
+	g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth0");
+	g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 3);
+
+	g_object_unref (connection);
+}
+
+static void
 test_write_vlan (void)
 {
 	NMConnection *connection;
@@ -12515,6 +12539,7 @@ int main (int argc, char **argv)
 	test_read_vlan_interface ();
 	test_read_vlan_only_vlan_id ();
 	test_read_vlan_only_device ();
+	g_test_add_func (TPATH "vlan/physdev", test_read_vlan_physdev);
 
 	test_write_wired_static ();
 	test_write_wired_static_ip6_only ();