ifcfg-rh: use different variables for IPv4 and IPv6 DNS options

Until now the ifcfg-rh plugin merged the values of 'ipv4.dns-options'
and 'ipv6.dns-options' and wrote the result to the RES_OPTIONS
variable. This is wrong because writing a connection and reading it
back gives a different connection compared to the original.

This behavior existed since when DNS options were introduced, but it
became more evident now that we reread the connection after write,
because after doing a:

 $ nmcli connection modify ethie ipv4.dns-options ndots:2

the connection has both ipv4.dns-options and ipv6.dns-options set. In
order to delete the option, an user has to delete it from both
settings:

 $ nmcli connection modify ethie ipv4.dns-options "" ipv6.dns-options ""

To improve this let's use different variables for IPv4 and IPv6. To
keep backwards compatibility IPv4 still uses RES_OPTIONS, while IPv6
uses a new IPV6_RES_OPTIONS variable.

https://bugzilla.redhat.com/show_bug.cgi?id=1517794
(cherry picked from commit 837978556080aaca0a1d715cbf0b410fbd8d1082)

7 files changed, 66 insertions, 91 deletions

diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c
index db3a12be41..26e7f5056c 100644
--- a/libnm-core/nm-setting-ip4-config.c
+++ b/libnm-core/nm-setting-ip4-config.c
@@ -689,6 +689,14 @@ nm_setting_ip4_config_class_init (NMSettingIP4ConfigClass *ip4_class)
 	 */
 
 	/* ---ifcfg-rh---
+	 * property: dns-options
+	 * variable: RES_OPTIONS(+)
+	 * description: List of DNS options to be added to /etc/resolv.conf
+	 * example: RES_OPTIONS=ndots:2 timeout:3
+	 * ---end---
+	 */
+
+	/* ---ifcfg-rh---
 	 * property: dns-priority
 	 * variable: IPV4_DNS_PRIORITY(+)
 	 * description: The priority for DNS servers of this connection. Lower values have higher priority.
diff --git a/libnm-core/nm-setting-ip6-config.c b/libnm-core/nm-setting-ip6-config.c
index cc6381f07a..858ddaf2ef 100644
--- a/libnm-core/nm-setting-ip6-config.c
+++ b/libnm-core/nm-setting-ip6-config.c
@@ -671,6 +671,14 @@ nm_setting_ip6_config_class_init (NMSettingIP6ConfigClass *ip6_class)
 	 * ---end---
 	 */
 
+	/* ---ifcfg-rh---
+	 * property: dns-options
+	 * variable: IPV6_RES_OPTIONS(+)
+	 * description: List of DNS options to be added to /etc/resolv.conf
+	 * example: IPV6_RES_OPTIONS=ndots:2 timeout:3
+	 * ---end---
+	 */
+
 	/**
 	 * NMSettingIP6Config:ip6-privacy:
 	 *
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 7952393953..c32f5775f7 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -1678,8 +1678,6 @@ make_ip6_setting (shvarFile *ifcfg,
 	char *value = NULL;
 	char *str_value;
 	char *route6_path = NULL;
-	gs_free char *dns_options_free = NULL;
-	const char *dns_options = NULL;
 	gboolean ipv6init, ipv6forwarding, dhcp6 = FALSE;
 	char *method = NM_SETTING_IP6_CONFIG_METHOD_MANUAL;
 	char *ipv6addr, *ipv6addr_secondaries;
@@ -1717,7 +1715,6 @@ make_ip6_setting (shvarFile *ifcfg,
 		value = svGetValueStr_cp (ifcfg, "DEVICE");
 		ipv6_defaultgw = svGetValueStr_cp (network_ifcfg, "IPV6_DEFAULTGW");
 		ipv6_defaultdev = svGetValueStr_cp (network_ifcfg, "IPV6_DEFAULTDEV");
-		dns_options = svGetValue (network_ifcfg, "RES_OPTIONS", &dns_options_free);
 
 		if (ipv6_defaultgw) {
 			default_dev = strchr (ipv6_defaultgw, '%');
@@ -1948,8 +1945,7 @@ make_ip6_setting (shvarFile *ifcfg,
 	}
 
 	/* DNS options */
-	parse_dns_options (s_ip6, svGetValue (ifcfg, "RES_OPTIONS", &value));
-	parse_dns_options (s_ip6, dns_options);
+	parse_dns_options (s_ip6, svGetValue (ifcfg, "IPV6_RES_OPTIONS", &value));
 	g_free (value);
 
 	/* DNS priority */
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index d5cbcd3555..8c3d14b7af 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -2112,6 +2112,28 @@ write_user_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
 	return TRUE;
 }
 
+static void
+write_res_options (shvarFile *ifcfg, NMSettingIPConfig *s_ip, const char *var)
+{
+	nm_auto_free_gstring GString *value = NULL;
+	guint i, num_options;
+
+	if (!nm_setting_ip_config_has_dns_options (s_ip)) {
+		svUnsetValue (ifcfg, var);
+		return;
+	}
+
+	value = g_string_new (NULL);
+	num_options = nm_setting_ip_config_get_num_dns_options (s_ip);
+	for (i = 0; i < num_options; i++) {
+		if (i > 0)
+			g_string_append_c (value, ' ');
+		g_string_append (value, nm_setting_ip_config_get_dns_option (s_ip, i));
+	}
+
+	svSetValue (ifcfg, var, value->str);
+}
+
 static gboolean
 write_ip4_setting (NMConnection *connection,
                    shvarFile *ifcfg,
@@ -2143,6 +2165,7 @@ write_ip4_setting (NMConnection *connection,
 		 * Some IPv4 setting related options are not cleared,
 		 * for no strong reason. */
 		svUnsetValue (ifcfg, "BOOTPROTO");
+		svUnsetValue (ifcfg, "RES_OPTIONS");
 		svUnsetAll (ifcfg, SV_KEY_TYPE_IP4_ADDRESS);
 		return TRUE;
 	}
@@ -2339,6 +2362,8 @@ write_ip4_setting (NMConnection *connection,
 	else
 		svUnsetValue (ifcfg, "IPV4_DNS_PRIORITY");
 
+	write_res_options (ifcfg, s_ip4, "RES_OPTIONS");
+
 	return TRUE;
 }
 
@@ -2480,6 +2505,7 @@ write_ip6_setting (NMConnection *connection,
 		svUnsetValue (ifcfg, "IPV6_FAILURE_FATAL");
 		svUnsetValue (ifcfg, "IPV6_ROUTE_METRIC");
 		svUnsetValue (ifcfg, "IPV6_ADDR_GEN_MODE");
+		svUnsetValue (ifcfg, "IPV6_RES_OPTIONS");
 		return TRUE;
 	}
 
@@ -2635,66 +2661,9 @@ write_ip6_setting (NMConnection *connection,
 	else
 		svUnsetValue (ifcfg, "IPV6_DNS_PRIORITY");
 
-	NM_SET_OUT (out_route6_content, write_route_file (s_ip6));
-
-	return TRUE;
-}
+	write_res_options (ifcfg, s_ip6, "IPV6_RES_OPTIONS");
 
-static void
-add_dns_option (GPtrArray *array, const char *option)
-{
-	if (_nm_utils_dns_option_find_idx (array, option) < 0)
-		g_ptr_array_add (array, (gpointer) option);
-}
-
-static gboolean
-write_res_options (NMConnection *connection, shvarFile *ifcfg, GError **error)
-{
-	NMSettingIPConfig *s_ip6;
-	NMSettingIPConfig *s_ip4;
-	const char *method;
-	int i, num_options;
-	gs_unref_ptrarray GPtrArray *array = NULL;
-	GString *value;
-
-	s_ip4 = nm_connection_get_setting_ip4_config (connection);
-
-	if (!s_ip4) {
-		/* slave-type: clear res-options */
-		svUnsetValue (ifcfg, "RES_OPTIONS");
-		return TRUE;
-	}
-
-	array = g_ptr_array_new ();
-
-	method = nm_setting_ip_config_get_method (s_ip4);
-	if (g_strcmp0 (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED)) {
-		num_options = nm_setting_ip_config_get_num_dns_options (s_ip4);
-		for (i = 0; i < num_options; i++)
-			add_dns_option (array, nm_setting_ip_config_get_dns_option (s_ip4, i));
-	}
-
-	s_ip6 = nm_connection_get_setting_ip6_config (connection);
-	method = nm_setting_ip_config_get_method (s_ip6);
-	if (g_strcmp0 (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE)) {
-		num_options = nm_setting_ip_config_get_num_dns_options (s_ip6);
-		for (i = 0; i < num_options; i++)
-			add_dns_option (array, nm_setting_ip_config_get_dns_option (s_ip6, i));
-	}
-
-	if (   array->len > 0
-	    || nm_setting_ip_config_has_dns_options (s_ip4)
-	    || nm_setting_ip_config_has_dns_options (s_ip6)) {
-		value = g_string_new (NULL);
-		for (i = 0; i < array->len; i++) {
-			if (i > 0)
-				g_string_append_c (value, ' ');
-			g_string_append (value, array->pdata[i]);
-		}
-		svSetValue (ifcfg, "RES_OPTIONS", value->str);
-		g_string_free (value, TRUE);
-	} else
-		svUnsetValue (ifcfg, "RES_OPTIONS");
+	NM_SET_OUT (out_route6_content, write_route_file (s_ip6));
 
 	return TRUE;
 }
@@ -2927,9 +2896,6 @@ do_write_construct (NMConnection *connection,
 	                        error))
 		return FALSE;
 
-	if (!write_res_options (connection, ifcfg, error))
-		return FALSE;
-
 	write_connection_setting (s_con, ifcfg);
 
 	NM_SET_OUT (out_ifcfg, g_steal_pointer (&ifcfg));
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dns-options b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dns-options
index 62e301e38c..cea2471a9a 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dns-options
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dns-options
@@ -8,7 +8,8 @@ NM_CONTROLLED=yes
 PEERDNS=yes
 DNS1=10.2.0.4
 DOMAIN="lorem.com ipsum.org dolor.edu"
-RES_OPTIONS="ndots:3 single-request-reopen  inet6"
+RES_OPTIONS="ndots:3  single-request-reopen"
+IPV6_RES_OPTIONS="inet6"
 IPV6INIT=yes
 IPV6_AUTOCONF=no
 IPV6ADDR="1001:abba::1234/56"
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static
index 6d49c01c1f..34acf9fe17 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static
@@ -19,3 +19,4 @@ IPV6ADDR_SECONDARIES="dead:beaf::2/56"
 DNS3=1:2:3:4::a
 DNS4=1:2:3:4::b
 RES_OPTIONS=
+IPV6_RES_OPTIONS=
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 7df0c4efad..07b121b261 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -2162,8 +2162,9 @@ test_read_dns_options (void)
 	NMSettingIPConfig *s_ip4, *s_ip6;
 	char *unmanaged = NULL;
 	const char *option;
-	const char *options[] = { "ndots:3", "single-request-reopen", "inet6" };
-	guint32 i, options_len = sizeof (options) / sizeof (options[0]);
+	const char *options4[] = { "ndots:3", "single-request-reopen" };
+	const char *options6[] = { "inet6" };
+	guint32 i, num;
 
 	connection = _connection_from_file (TEST_IFCFG_DIR "/network-scripts/ifcfg-test-dns-options",
 	                                    NULL, TYPE_ETHERNET, &unmanaged);
@@ -2175,18 +2176,20 @@ test_read_dns_options (void)
 	s_ip6 = nm_connection_get_setting_ip6_config (connection);
 	g_assert (s_ip6);
 
-	i = nm_setting_ip_config_get_num_dns_options (s_ip4);
-	g_assert_cmpint (i, ==, options_len);
+	num = nm_setting_ip_config_get_num_dns_options (s_ip4);
+	g_assert_cmpint (num, ==, G_N_ELEMENTS (options4));
 
-	i = nm_setting_ip_config_get_num_dns_options (s_ip6);
-	g_assert_cmpint (i, ==, options_len);
-
-	for (i = 0; i < options_len; i++) {
+	for (i = 0; i < num; i++) {
 		option = nm_setting_ip_config_get_dns_option (s_ip4, i);
-		g_assert_cmpstr (options[i], ==, option);
+		g_assert_cmpstr (options4[i], ==, option);
+	}
 
+	num = nm_setting_ip_config_get_num_dns_options (s_ip6);
+	g_assert_cmpint (num, ==, G_N_ELEMENTS (options6));
+
+	for (i = 0; i < num; i++) {
 		option = nm_setting_ip_config_get_dns_option (s_ip6, i);
-		g_assert_cmpstr (options[i], ==, option);
+		g_assert_cmpstr (options6[i], ==, option);
 	}
 
 	g_object_unref (connection);
@@ -2297,6 +2300,8 @@ test_write_dns_options (void)
 	nm_setting_ip_config_add_address (s_ip4, addr);
 	nm_ip_address_unref (addr);
 
+	nm_setting_ip_config_add_dns_option (s_ip4, "debug");
+
 	/* IP6 setting */
 	s_ip6 = (NMSettingIPConfig *) nm_setting_ip6_config_new ();
 	nm_connection_add_setting (connection, NM_SETTING (s_ip6));
@@ -2312,26 +2317,16 @@ test_write_dns_options (void)
 	nm_setting_ip_config_add_address (s_ip6, addr6);
 	nm_ip_address_unref (addr6);
 
-	nm_setting_ip_config_add_dns_option (s_ip4, "debug");
 	nm_setting_ip_config_add_dns_option (s_ip6, "timeout:3");
 
 	nmtst_assert_connection_verifies (connection);
 
-	_writer_new_connection_FIXME (connection,
-	                              TEST_SCRATCH_DIR "/network-scripts/",
-	                              &testfile);
+	_writer_new_connection (connection,
+	                        TEST_SCRATCH_DIR "/network-scripts/",
+	                        &testfile);
 
 	reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL);
 
-	/* RES_OPTIONS is copied to both IPv4 and IPv6 settings */
-	nm_setting_ip_config_clear_dns_options (s_ip4, TRUE);
-	nm_setting_ip_config_add_dns_option (s_ip4, "debug");
-	nm_setting_ip_config_add_dns_option (s_ip4, "timeout:3");
-
-	nm_setting_ip_config_clear_dns_options (s_ip6, TRUE);
-	nm_setting_ip_config_add_dns_option (s_ip6, "debug");
-	nm_setting_ip_config_add_dns_option (s_ip6, "timeout:3");
-
 	nmtst_assert_connection_equals (connection, TRUE, reread, FALSE);
 }