ifcfg-rh: - WIP - allow layer2-only configs with disabled IPv4 and IPv6dcbw/ifcfg-rh-layer2-only

Fails some testcases still; have to figure out whether we care about
the backwards compatibility in those cases.  The issue is when IPv6
*is* enabled, but there is no BOOTPROTO and no IPv4 addreses defined.
Previously, this meant that IPv4 was disabled, but with these
patches it means IPv4=auto, since minimal ifcfg files with only
the DEVICE and HWADDR keys used to be interpreted as default DHCP.

6 files changed, 91 insertions, 111 deletions

diff --git a/libnm-util/nm-setting-ip4-config.c b/libnm-util/nm-setting-ip4-config.c
index 196ff1d36c..6caf864a63 100644
--- a/libnm-util/nm-setting-ip4-config.c
+++ b/libnm-util/nm-setting-ip4-config.c
@@ -691,15 +691,6 @@ nm_setting_ip4_config_get_may_fail (NMSettingIP4Config *setting)
 	return NM_SETTING_IP4_CONFIG_GET_PRIVATE (setting)->may_fail;
 }
 
-static gint
-find_setting_by_name (gconstpointer a, gconstpointer b)
-{
-	NMSetting *setting = NM_SETTING (a);
-	const char *str = (const char *) b;
-
-	return strcmp (nm_setting_get_name (setting), str);
-}
-
 static gboolean
 verify (NMSetting *setting, GSList *all_settings, GError **error)
 {
@@ -768,24 +759,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
-	/* Disabled method is not allowed when IPv6 is set to 'ignore' */
-	if (!strcmp (priv->method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED)) {
-		GSList *list = g_slist_find_custom (all_settings, NM_SETTING_IP6_CONFIG_SETTING_NAME, find_setting_by_name);
-		if (list) {
-			NMSettingIP6Config *s_ip6 = g_slist_nth_data (list, 0);
-			if (   s_ip6
-			    && !g_strcmp0 (nm_setting_ip6_config_get_method (s_ip6), NM_SETTING_IP6_CONFIG_METHOD_IGNORE)) {
-				g_set_error (error,
-				             NM_SETTING_IP4_CONFIG_ERROR,
-				             NM_SETTING_IP4_CONFIG_ERROR_INVALID_PROPERTY,
-				             _("IPv4 method '%s' is not allowed when IPv6 method 'ignore' is set"),
-				             priv->method);
-				g_prefix_error (error, "%s.%s: ", NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_METHOD);
-				return FALSE;
-			}
-		}
-	}
-
 	if (priv->dhcp_client_id && !strlen (priv->dhcp_client_id)) {
 		g_set_error_literal (error,
 		                     NM_SETTING_IP4_CONFIG_ERROR,
diff --git a/libnm-util/nm-setting-ip6-config.c b/libnm-util/nm-setting-ip6-config.c
index 7127724629..950a70cfce 100644
--- a/libnm-util/nm-setting-ip6-config.c
+++ b/libnm-util/nm-setting-ip6-config.c
@@ -675,15 +675,6 @@ nm_setting_ip6_config_get_ip6_privacy (NMSettingIP6Config *setting)
 	return NM_SETTING_IP6_CONFIG_GET_PRIVATE (setting)->ip6_privacy;
 }
 
-static gint
-find_setting_by_name (gconstpointer a, gconstpointer b)
-{
-	NMSetting *setting = NM_SETTING (a);
-	const char *str = (const char *) b;
-
-	return strcmp (nm_setting_get_name (setting), str);
-}
-
 static gboolean
 verify (NMSetting *setting, GSList *all_settings, GError **error)
 {
@@ -752,24 +743,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
-	/* Method 'ignore' is not allowed when IPv4 is set to 'disabled' */
-	if (!strcmp (priv->method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE)) {
-		GSList *list = g_slist_find_custom (all_settings, NM_SETTING_IP4_CONFIG_SETTING_NAME, find_setting_by_name);
-		if (list) {
-			NMSettingIP4Config *s_ip4 = g_slist_nth_data (list, 0);
-			if (   s_ip4
-			    && !g_strcmp0 (nm_setting_ip4_config_get_method (s_ip4), NM_SETTING_IP4_CONFIG_METHOD_DISABLED)) {
-				g_set_error (error,
-				             NM_SETTING_IP6_CONFIG_ERROR,
-				             NM_SETTING_IP6_CONFIG_ERROR_INVALID_PROPERTY,
-				             _("IPv6 method '%s' is not allowed when IPv4 method 'disabled' is set"),
-				             priv->method);
-				g_prefix_error (error, "%s.%s: ", NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_SETTING_IP6_CONFIG_METHOD);
-				return FALSE;
-			}
-		}
-	}
-
 	if (priv->dhcp_hostname && !strlen (priv->dhcp_hostname)) {
 		g_set_error_literal (error,
 		                     NM_SETTING_IP6_CONFIG_ERROR,
diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
index b37bdb1506..7c7c9387e9 100644
--- a/src/settings/plugins/ifcfg-rh/reader.c
+++ b/src/settings/plugins/ifcfg-rh/reader.c
@@ -1213,12 +1213,41 @@ error:
 	return success;
 }
 
+#define IF_HAS_RETURN(i, k) \
+	tmp = svGetValue (i, k, FALSE); \
+	if (tmp) { \
+		g_free (tmp); \
+		return TRUE; \
+	}
+
+static gboolean
+has_ip4_details (shvarFile *ifcfg)
+{
+	char *tmp;
+
+	IF_HAS_RETURN (ifcfg, "IPADDR");
+	IF_HAS_RETURN (ifcfg, "PREFIX");
+	IF_HAS_RETURN (ifcfg, "NETMASK");
+
+	IF_HAS_RETURN (ifcfg, "IPADDR0");
+	IF_HAS_RETURN (ifcfg, "PREFIX0");
+	IF_HAS_RETURN (ifcfg, "NETMASK0");
+
+	IF_HAS_RETURN (ifcfg, "IPADDR1");
+	IF_HAS_RETURN (ifcfg, "PREFIX1");
+	IF_HAS_RETURN (ifcfg, "NETMASK1");
+
+	IF_HAS_RETURN (ifcfg, "IPADDR2");
+	IF_HAS_RETURN (ifcfg, "PREFIX2");
+	IF_HAS_RETURN (ifcfg, "NETMASK2");
+
+	return FALSE;
+}
 
 static NMSetting *
 make_ip4_setting (shvarFile *ifcfg,
                   const char *network_file,
                   const char *iscsiadm_path,
-                  gboolean can_disable_ip4,
                   GError **error)
 {
 	NMSettingIP4Config *s_ip4 = NULL;
@@ -1287,8 +1316,21 @@ make_ip4_setting (shvarFile *ifcfg,
 			              NM_SETTING_IP4_CONFIG_NEVER_DEFAULT, never_default,
 			              NULL);
 			return NM_SETTING (s_ip4);
-		} else if (!g_ascii_strcasecmp (value, "none") || !g_ascii_strcasecmp (value, "static")) {
-			/* Static IP */
+		} else if (!g_ascii_strcasecmp (value, "none")) {
+			if (!has_ip4_details (ifcfg)) {
+				/* BOOTPROTO=none and no IP addresses means disabled; if there
+				 * are IP addresses though, use manual/static for backwards
+				 * compatibility.
+				 */
+				method = NM_SETTING_IP4_CONFIG_METHOD_DISABLED;
+			}
+		} else if (!g_ascii_strcasecmp (value, "static")) {
+			if (!has_ip4_details (ifcfg)) {
+				g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
+					         "BOOTPROTO=static but no IP addresses given");
+				g_free (value);
+				goto done;
+			}
 		} else if (strlen (value)) {
 			g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
 			             "Unknown BOOTPROTO '%s'", value);
@@ -1297,55 +1339,16 @@ make_ip4_setting (shvarFile *ifcfg,
 		}
 		g_free (value);
 	} else {
-		char *tmp_ip4, *tmp_prefix, *tmp_netmask;
-		char *tmp_ip4_0, *tmp_prefix_0, *tmp_netmask_0;
-		char *tmp_ip4_1, *tmp_prefix_1, *tmp_netmask_1;
-		char *tmp_ip4_2, *tmp_prefix_2, *tmp_netmask_2;
-
-		/* If there is no BOOTPROTO, no IPADDR, no PREFIX, no NETMASK, but
-		 * valid IPv6 configuration, assume that IPv4 is disabled.  Otherwise,
-		 * if there is no IPv6 configuration, assume DHCP is to be used.
-		 * Happens with minimal ifcfg files like the following that anaconda
-		 * sometimes used to write out:
+		/* If there is no BOOTPROTO, no IPADDR, no PREFIX, no NETMASK, assume
+		 * assume that IPv4 should be set to DHCP to handle minimal ifcfg files
+		 * like the following that anaconda sometimes used to write out:
 		 *
 		 * DEVICE=eth0
 		 * HWADDR=11:22:33:44:55:66
-		 *
 		 */
-		tmp_ip4 = svGetValue (ifcfg, "IPADDR", FALSE);
-		tmp_prefix = svGetValue (ifcfg, "PREFIX", FALSE);
-		tmp_netmask = svGetValue (ifcfg, "NETMASK", FALSE);
-		tmp_ip4_0 = svGetValue (ifcfg, "IPADDR0", FALSE);
-		tmp_prefix_0 = svGetValue (ifcfg, "PREFIX0", FALSE);
-		tmp_netmask_0 = svGetValue (ifcfg, "NETMASK0", FALSE);
-		tmp_ip4_1 = svGetValue (ifcfg, "IPADDR1", FALSE);
-		tmp_prefix_1 = svGetValue (ifcfg, "PREFIX1", FALSE);
-		tmp_netmask_1 = svGetValue (ifcfg, "NETMASK1", FALSE);
-		tmp_ip4_2 = svGetValue (ifcfg, "IPADDR2", FALSE);
-		tmp_prefix_2 = svGetValue (ifcfg, "PREFIX2", FALSE);
-		tmp_netmask_2 = svGetValue (ifcfg, "NETMASK2", FALSE);
-		if (   !tmp_ip4   && !tmp_prefix   && !tmp_netmask
-		    && !tmp_ip4_0 && !tmp_prefix_0 && !tmp_netmask_0
-		    && !tmp_ip4_1 && !tmp_prefix_1 && !tmp_netmask_1
-		    && !tmp_ip4_2 && !tmp_prefix_2 && !tmp_netmask_2) {
-			if (can_disable_ip4)
-				/* Nope, no IPv4 */
-				method = NM_SETTING_IP4_CONFIG_METHOD_DISABLED;
-			else
-				method = NM_SETTING_IP4_CONFIG_METHOD_AUTO;
-		}
-		g_free (tmp_ip4);
-		g_free (tmp_prefix);
-		g_free (tmp_netmask);
-		g_free (tmp_ip4_0);
-		g_free (tmp_prefix_0);
-		g_free (tmp_netmask_0);
-		g_free (tmp_ip4_1);
-		g_free (tmp_prefix_1);
-		g_free (tmp_netmask_1);
-		g_free (tmp_ip4_2);
-		g_free (tmp_prefix_2);
-		g_free (tmp_netmask_2);
+
+		method = has_ip4_details (ifcfg) ? NM_SETTING_IP4_CONFIG_METHOD_MANUAL :
+					NM_SETTING_IP4_CONFIG_METHOD_AUTO;
 	}
 
 	g_object_set (s_ip4,
@@ -4331,7 +4334,6 @@ connection_from_file (const char *filename,
 	NMSetting *s_ip4, *s_ip6, *s_port;
 	const char *ifcfg_name = NULL;
 	gboolean nm_controlled = TRUE;
-	gboolean can_disable_ip4 = FALSE;
 	char *unmanaged = NULL;
 
 	g_return_val_if_fail (filename != NULL, NULL);
@@ -4485,16 +4487,10 @@ connection_from_file (const char *filename,
 	} else if (utils_ignore_ip_config (connection)) {
 		PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: ignoring IP6 configuration");
 		g_object_unref (s_ip6);
-	} else {
-		const char *method;
-
+	} else
 		nm_connection_add_setting (connection, s_ip6);
-		method = nm_setting_ip6_config_get_method (NM_SETTING_IP6_CONFIG (s_ip6));
-		if (method && strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE))
-			can_disable_ip4 = TRUE;
-	}
 
-	s_ip4 = make_ip4_setting (parsed, network_file, iscsiadm_path, can_disable_ip4, error);
+	s_ip4 = make_ip4_setting (parsed, network_file, iscsiadm_path, error);
 	if (!s_ip4) {
 		g_object_unref (connection);
 		connection = NULL;
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 df3ebb124c..95d88efebf 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
 	ifcfg-test-bridge-main \
 	ifcfg-test-bridge-component \
 	ifcfg-test-bridge-missing-stp \
+	ifcfg-test-bridge-l2 \
 	ifcfg-test-vlan-interface \
 	ifcfg-test-vlan-only-vlanid \
 	ifcfg-test-vlan-only-device \
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bridge-l2 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bridge-l2
new file mode 100644
index 0000000000..df9921d18e
--- /dev/null
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-bridge-l2
@@ -0,0 +1,4 @@
+DEVICE=br0
+ONBOOT=yes
+BOOTPROTO=none
+TYPE=Bridge
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 73ea3f87f2..817d9a2323 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -11349,6 +11349,38 @@ test_read_bridge_missing_stp (void)
 	g_object_unref (connection);
 }
 
+static void
+test_read_bridge_layer2_only (void)
+{
+	NMConnection *connection;
+	GError *error = NULL;
+	NMSettingBridge *s_bridge;
+	NMSettingIP4Config *s_ip4;
+	NMSettingIP6Config *s_ip6;
+
+	connection = connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-bridge-l2",
+	                                   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_bridge = nm_connection_get_setting_bridge (connection);
+	g_assert (s_bridge);
+
+	g_assert_cmpstr (nm_setting_bridge_get_interface_name (s_bridge), ==, "br0");
+
+	s_ip4 = nm_connection_get_setting_ip4_config (connection);
+	g_assert (s_ip4);
+	g_assert_cmpstr (nm_setting_ip4_config_get_method (s_ip4), ==, "disabled");
+
+	s_ip6 = nm_connection_get_setting_ip6_config (connection);
+	g_assert (s_ip6);
+	g_assert_cmpstr (nm_setting_ip6_config_get_method (s_ip6), ==, "ignore");
+
+	g_object_unref (connection);
+}
+
 #define TEST_IFCFG_VLAN_INTERFACE TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-interface"
 
 static void
@@ -12638,6 +12670,7 @@ int main (int argc, char **argv)
 	test_read_bridge_component ();
 	test_write_bridge_component ();
 	test_read_bridge_missing_stp ();
+	g_test_add_func (TPATH "bridge/layer2-only", test_read_bridge_layer2_only);
 
 	/* Stuff we expect to fail for now */
 	test_write_wired_pppoe ();