[PATCH] support for domain-search, request for comments/testing



Hi everyone,

a few days ago I received a contribution in form of a patch [1], which
adds domain-search support to NetworkManager (0.6).
I'll be quoting the relevant part here:

> ISC DHCP 3.1 provides support for the domain-search option. This is the
> correct way for a DHCP server to provide a domain search list. Wedging
> multiple domains into the domain-name option is a horrible hack.
> 
> It seems, however, that that is what NetworkManager expects, and so how
> it generates an /etc/resolv.conf. This behaviour should be changed in
> light of dhclient 3.1, and should do something like this:
> 
> if the domain-search option is present in the data received via DHCP,
> use it in favour of the domain-name option for setting the search
> directive in /etc/resolv.conf
> 
> if the domain-name option is present in the data received via DHCP, use
> it to set the domain directive in /etc/resolv.conf
> 
> if only the domain-name option is present in the data received via DHCP
> (and domain-search is not), for backwards compatibility, set the search
> directive in /etc/resolv.conf
> 
> It appears, from my testing, that NetworkManager interacts with
> resolvconf, so the above only applies when resolvconf is not installed.
> I've already filed #460609 to have resolvconf update its behaviour.
> 
> This bug is important for consistent treatment of /etc/resolv.conf when
> DHCP and the domain-search option is in use.

I updated the patch to cleanly apply against HEAD of
NETWORKMANAGER_0_6_0_RELEASE and attached it to this email.

It's an interesting feature worth looking at and I'd like to see it in
0.6.6 if somehow possible. The patch author, Bas Zoetekouw, said that he
successfully tested the patch with an old version of dhcpd and a new
one, which supports this domain-search option. I currently don't have
access to a dhcp3-server with support for domain-search, so I can't test
it myself. Thus this request for testing.
I'd also like to know, if this patch has a chance of making into 0.6.6,
and if not, if you plan to release a 0.6.6.1 or 0.6.7 and what you think
of the patch in general.

Cheers,
Michael






[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=465158
-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
Index: NetworkManager-0.6/src/dhcp-manager/nm-dhcp-manager.c
===================================================================
--- NetworkManager-0.6.orig/src/dhcp-manager/nm-dhcp-manager.c	2008-03-02 09:25:16.000000000 +0100
+++ NetworkManager-0.6/src/dhcp-manager/nm-dhcp-manager.c	2008-03-02 09:26:33.000000000 +0100
@@ -472,6 +472,7 @@
 	guint32		num_ip4_nis_servers = 0;
 	char *		hostname = NULL;
 	char *		domain_names = NULL;
+	char *		domain_searches = NULL;
 	char *		nis_domain = NULL;
 	guint32 *		ip4_nis_servers = NULL;
 	struct in_addr	temp_addr;
@@ -520,6 +521,7 @@
 	get_ip4_string (manager, dev, "host_name", &hostname, TRUE);
 	get_ip4_uint32s (manager, dev, "domain_name_servers", &ip4_nameservers, &num_ip4_nameservers, FALSE);
 	get_ip4_string (manager, dev, "domain_name", &domain_names, TRUE);
+	get_ip4_string (manager, dev, "domain_search", &domain_searches, TRUE);
 	get_ip4_string (manager, dev, "nis_domain", &nis_domain, TRUE);
 	get_ip4_uint32s (manager, dev, "nis_servers", &ip4_nis_servers, &num_ip4_nis_servers, TRUE);
 
@@ -557,14 +559,27 @@
 
 	if (domain_names)
 	{
-		char **searches = g_strsplit (domain_names, " ", 0);
+		char **domains = g_strsplit (domain_names, " ", 0);
 		char **s;
 
-		for (s = searches; *s; s++)
+		for (s = domains; *s; s++)
 		{
 			nm_info ("  domain name '%s'", *s);
 			nm_ip4_config_add_domain (ip4_config, *s);
 		}
+		g_strfreev (domains);
+	}
+
+	if (domain_searches)
+	{
+		char **searches = g_strsplit (domain_searches, " ", 0);
+		char **s;
+
+		for (s = searches; *s; s++)
+		{
+			nm_info ("  domain search '%s'", *s);
+			nm_ip4_config_add_search (ip4_config, *s);
+		}
 		g_strfreev (searches);
 	}
 
Index: NetworkManager-0.6/src/named-manager/nm-named-manager.c
===================================================================
--- NetworkManager-0.6.orig/src/named-manager/nm-named-manager.c	2008-03-02 09:25:16.000000000 +0100
+++ NetworkManager-0.6/src/named-manager/nm-named-manager.c	2008-03-02 09:26:33.000000000 +0100
@@ -319,7 +319,7 @@
 static char *
 compute_searches (NMNamedManager *mgr, NMIP4Config *config)
 {
-	int i, num_searches;
+	int i, num_searches, num_domains;
 	GString *str = NULL;
 
 	g_return_val_if_fail (mgr != NULL, g_strdup (""));
@@ -328,14 +328,54 @@
 	if (!config)
 		return g_strdup ("");
 
-	num_searches = nm_ip4_config_get_num_domains (config);
-	for (i = 0; i < num_searches; i++)
-	{
-		if (!str)
-			str = g_string_new ("search");
+	num_searches = nm_ip4_config_get_num_searches (config);
+	num_domains  = nm_ip4_config_get_num_domains  (config);
+
+	/* ISC DHCP 3.1 provides support for the domain-search option. This is the
+	 * correct way for a DHCP server to provide a domain search list. Wedging
+	 * multiple domains into the domain-name option is a horrible hack.
+	 *
+	 * So, we handle it like this (as proposed by Andrew Pollock at
+	 * http://bugs.debian.org/465158):
+	 *
+	 * - if the domain-search option is present in the data received via DHCP,
+	 *   use it in favour of the domain-name option for setting the search
+	 *   directive in /etc/resolv.conf
+	 *
+	 * - if the domain-name option is present in the data received via DHCP, use
+	 *   it to set the domain directive in /etc/resolv.conf
+	 *   (this is handled in compute_domain() below)
+	 *
+	 * - if only the domain-name option is present in the data received via DHCP
+	 *   (and domain-search is not), for backwards compatibility, set the search
+	 *   directive in /etc/resolv.conf to the specified domain names
+	 */
 
-		g_string_append_c (str, ' ');
-		g_string_append (str, nm_ip4_config_get_domain (config, i));		
+	if ( num_searches == 0 && num_domains > 0 )
+	{
+		/* old, pre-ISC-3.1 fallback:
+		 * use (possibly multiple entries from) domain-name for the search entry */
+		str = g_string_new ("search");
+		for (i = 0; i < num_domains; i++)
+		{
+			g_string_append_c (str, ' ');
+			g_string_append (str, nm_ip4_config_get_domain (config, i));
+		}
+	}
+	else if ( num_searches > 0 )
+	{
+		/* the "correct", post-ISC-3.1 situation: use searches just as received */
+		str = g_string_new ("search");
+		for (i = 0; i < num_searches; i++)
+		{
+			g_string_append_c (str, ' ');
+			g_string_append (str, nm_ip4_config_get_search (config, i));
+		}
+	}
+	else /* num_searches == 0 && num_domains == 0 */
+	{
+		/* no info available at all */
+		return g_strdup ("");
 	}
 
 	if (!str)
@@ -346,11 +386,41 @@
 	return g_string_free (str, FALSE);
 }
 
+static char *
+compute_domain (NMNamedManager *mgr, NMIP4Config *config)
+{
+	int num_domains;
+	GString *str = NULL;
+
+	g_return_val_if_fail (mgr != NULL, g_strdup (""));
+
+	/* config can be NULL */
+	if (!config)
+		return g_strdup ("");
+
+	num_domains = nm_ip4_config_get_num_domains (config);
+
+	/* warn the user that he should change his config
+	 * (see comments above in compute_search() ) */
+	if ( num_domains > 1 )
+		nm_warning ("DHCP server sent multiple entries in domain-name; this is obsolete behaviour; please fix or update your DHCP server");
+
+	if (num_domains == 0)
+		return g_strdup ("");
+
+	str = g_string_new ("domain ");
+	g_string_append (str, nm_ip4_config_get_domain (config, 0));
+	g_string_append_c (str, '\n');
+
+	return g_string_free (str, FALSE);
+}
+
 static gboolean
 rewrite_resolv_conf (NMNamedManager *mgr, NMIP4Config *config, GError **error)
 {
 	const char *	tmp_resolv_conf = RESOLV_CONF ".tmp";
 	char *		searches = NULL;
+	char *		domain   = NULL;
 	FILE *		f;
 	NMIP4Config *ns_config = config;
 
@@ -388,23 +458,27 @@
 	}
 	g_return_val_if_fail (ns_config != NULL, FALSE);
 
+	domain   = compute_domain   (mgr, ns_config);
 	searches = compute_searches (mgr, ns_config);
 
 	if (mgr->priv->use_named == TRUE) {
 		/* Using caching-nameserver & local DNS */
-		if (fprintf (f, "%s%s%s", "; Use a local caching nameserver controlled by NetworkManager\n\n", searches, "\nnameserver 127.0.0.1\n") < 0)
+		if (fprintf (f, "%s%s%s%s", "; Use a local caching nameserver controlled by NetworkManager\n\n", domain, searches, "\nnameserver 127.0.0.1\n") < 0)
 			goto lose;
 	} else {
 		/* Using glibc resolver */
 		char *nameservers = compute_nameservers (mgr, ns_config);
 
+		fprintf (f, "%s\n\n", domain);
 		fprintf (f, "%s\n\n", searches);
-		g_free (searches);
-
 		fprintf (f, "%s\n\n", nameservers);
+
 		g_free (nameservers);
 	}
 
+	g_free (domain);
+	g_free (searches);
+
 	if (fclose (f) < 0)
 		goto lose;
 
@@ -414,6 +488,7 @@
 	return TRUE;
 
 lose:
+	g_free (domain);
 	g_free (searches);
 	fclose (f);
 	g_set_error (error,
Index: NetworkManager-0.6/src/nm-ip4-config.c
===================================================================
--- NetworkManager-0.6.orig/src/nm-ip4-config.c	2008-03-02 09:25:16.000000000 +0100
+++ NetworkManager-0.6/src/nm-ip4-config.c	2008-03-02 09:26:33.000000000 +0100
@@ -46,6 +46,7 @@
 
 	GSList *	nameservers;
 	GSList *	domains;
+	GSList *	searches;
 
 	gchar *	hostname;
 	gchar *	nis_domain;
@@ -275,6 +276,17 @@
 	config->domains = g_slist_append (config->domains, g_strdup (domain));
 }
 
+void nm_ip4_config_add_search (NMIP4Config *config, const char *search)
+{
+	g_return_if_fail (config != NULL);
+	g_return_if_fail (search != NULL);
+
+	if (!strlen (search))
+		return;
+
+	config->searches = g_slist_append (config->searches, g_strdup (search));
+}
+
 void nm_ip4_config_set_hostname (NMIP4Config *config, const char *hostname)
 {
 	g_return_if_fail (config != NULL);
@@ -329,6 +341,25 @@
 	return (g_slist_length (config->domains));
 }
 
+const char *nm_ip4_config_get_search (NMIP4Config *config, guint i)
+{
+	const char *search;
+
+	g_return_val_if_fail (config != NULL, NULL);
+	g_return_val_if_fail (i < g_slist_length (config->searches), NULL);
+
+	if ((search = (const char *) g_slist_nth_data (config->searches, i)))
+		return search;
+	return NULL;
+}
+
+guint32 nm_ip4_config_get_num_searches (NMIP4Config *config)
+{
+	g_return_val_if_fail (config != NULL, 0);
+
+	return (g_slist_length (config->searches));
+}
+
 guint32 nm_ip4_config_get_mtu (NMIP4Config *config)
 {
 	g_return_val_if_fail (config != NULL, 0);
Index: NetworkManager-0.6/src/nm-ip4-config.h
===================================================================
--- NetworkManager-0.6.orig/src/nm-ip4-config.h	2008-03-02 09:25:16.000000000 +0100
+++ NetworkManager-0.6/src/nm-ip4-config.h	2008-03-02 09:26:33.000000000 +0100
@@ -68,6 +68,14 @@
 const char *	nm_ip4_config_get_domain			(NMIP4Config *config, guint i);
 guint32		nm_ip4_config_get_num_domains		(NMIP4Config *config);
 
+void			nm_ip4_config_add_search			(NMIP4Config *config, const char *search);
+const char *	nm_ip4_config_get_search			(NMIP4Config *config, guint i);
+guint32		nm_ip4_config_get_num_searches		(NMIP4Config *config);
+
+void			nm_ip4_config_add_domain			(NMIP4Config *config, const char *search);
+const char *	nm_ip4_config_get_domain			(NMIP4Config *config, guint i);
+guint32		nm_ip4_config_get_num_searches		(NMIP4Config *config);
+
 guint32		nm_ip4_config_get_mtu			(NMIP4Config *config);
 void			nm_ip4_config_set_mtu			(NMIP4Config *config, guint32 mtu);
 

Attachment: signature.asc
Description: OpenPGP digital signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]