Re: [PATH] Support resolvconf

[Roy Marples <roy marples name>]

On Sun, 2008-08-24 at 01:01 +0200, Alexander Sack wrote:
> OK, attached an updated (simplified) patch that works for me out of
> the box. I kept all changes, except that i used the composite ip config to set
> all at once - using "NetworkManager" as name.
> 
> I did that because I think that trying to mimic the "original" way of
> using resolvconf to manage individual interface dns info doesnt give
> us much benefit, while it adds complexity.

That is true.
Also, my understanding of NM was wrong - the code I was looking at was
given the "best" config and interface whereas I thought it was for the
activated interface.


> For the domain/search tweakage: I feel unsure about this (though I kept
> this part of your patch untouched for now). This is quite a specific
> feature you want to implicitly entrench here. Maybe we can push
> that back until we know if and how we want to implement such a
> feature?

There's no real need to push it back.
NM currently folds domain into searches, so there's no need to actually
use domain at all.

Attached is a new patch, taking all the above into account.
Also makes this less intrusive than the first pass :)

Thanks

Roy

Index: src/named-manager/nm-named-manager.c
===================================================================
--- src/named-manager/nm-named-manager.c	(revision 4002)
+++ src/named-manager/nm-named-manager.c	(working copy)
@@ -23,6 +23,7 @@
 
 #include "config.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -38,6 +39,7 @@
 #include "nm-ip4-config.h"
 #include "nm-utils.h"
 #include "NetworkManagerSystem.h"
+#include "NetworkManagerUtils.h"
 
 #ifdef HAVE_SELINUX
 #include <selinux/selinux.h>
@@ -47,6 +49,8 @@
 #define RESOLV_CONF "/etc/resolv.conf"
 #endif
 
+#define ADDR_BUF_LEN 50
+
 G_DEFINE_TYPE(NMNamedManager, nm_named_manager, G_TYPE_OBJECT)
 
 #define NM_NAMED_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), \
@@ -115,6 +119,7 @@
 	}
 }
 
+
 #if defined(TARGET_SUSE)
 /**********************************/
 /* SUSE */
@@ -164,10 +169,9 @@
 }
 
 static gboolean
-update_resolv_conf (const char *iface,
-				const char *domain,
-				char **searches,
+dispatch_netconfig (char **searches,
 				char **nameservers,
+				const char *iface,
 				GError **error)
 {
 	gint fd;
@@ -195,67 +199,47 @@
 
 	return TRUE;
 }
+#endif
 
-#else
-/**********************************/
-/* Generic */
 
 static gboolean
-update_resolv_conf (const char *iface,
-				const char *domain,
-				char **searches,
-				char **nameservers,
-				GError **error)
+write_resolv_conf (FILE *f, char **searches, char **nameservers, GError **error)
 {
-	const char *tmp_resolv_conf = RESOLV_CONF ".tmp";
-	char *domain_str = NULL;
 	char *searches_str = NULL;
 	char *nameservers_str = NULL;
-	FILE *f;
+	int i;
+	gboolean retval = FALSE;
 
-	if ((f = fopen (tmp_resolv_conf, "w")) == NULL) {
+	if (fprintf (f, "%s","# Generated by NetworkManager\n") < 0) {
 		g_set_error (error,
 				   NM_NAMED_MANAGER_ERROR,
 				   NM_NAMED_MANAGER_ERROR_SYSTEM,
-				   "Could not open " RESOLV_CONF ": %s\n",
-				   g_strerror (errno));
-		return FALSE;
-	}
-
-	if (fprintf (f, "%s","# generated by NetworkManager, do not edit!\n\n") < 0) {
-		g_set_error (error,
-				   NM_NAMED_MANAGER_ERROR,
-				   NM_NAMED_MANAGER_ERROR_SYSTEM,
 				   "Could not write " RESOLV_CONF ": %s\n",
 				   g_strerror (errno));
-		fclose (f);
 		return FALSE;
 	}
 
-	if (domain)
-		domain_str = g_strconcat ("domain ", domain, "\n\n", NULL);
 
 	if (searches) {
 		char *tmp_str;
 
 		tmp_str = g_strjoinv (" ", searches);
-		searches_str = g_strconcat ("search ", tmp_str, "\n\n", NULL);
+		searches_str = g_strconcat ("search ", tmp_str, "\n", NULL);
 		g_free (tmp_str);
 	}
 
 	if (nameservers) {
 		GString *str;
 		int num;
-		int i;
 
 		str = g_string_new ("");
 		num = g_strv_length (nameservers);
 
 		for (i = 0; i < num; i++) {
 			if (i == 3) {
+				g_string_append (str, "# ");
+				g_string_append (str, _("NOTE: the libc resolver may not support more than 3 nameservers."));
 				g_string_append (str, "\n# ");
-				g_string_append (str, _("NOTE: the glibc resolver does not support more than 3 nameservers."));
-				g_string_append (str, "\n# ");
 				g_string_append (str, _("The nameservers listed below may not be recognized."));
 				g_string_append_c (str, '\n');
 			}
@@ -268,27 +252,102 @@
 		nameservers_str = g_string_free (str, FALSE);
 	}
 
-	if (fprintf (f, "%s%s%s\n",
-	             domain_str ? domain_str : "",
+	if (fprintf (f, "%s%s",
 	             searches_str ? searches_str : "",
-	             nameservers_str ? nameservers_str : "") < 0) {
+	             nameservers_str ? nameservers_str : "") != -1)
+		retval = TRUE;
+
+	g_free (searches_str);
+	g_free (nameservers_str);
+
+	return retval;
+}
+
+
+static gboolean
+dispatch_resolvconf (char **searches,
+				char **nameservers,
+				const char *iface,
+				GError **error)
+{
+	char **paths, **path, *tmp, *cmd, *resolvconf = NULL;
+	FILE *f;
+	gboolean retval = FALSE;
+
+	/* See if resolvconf is installed */
+	if ((tmp = getenv ("PATH"))) {
+		paths = g_strsplit (tmp, ":", 0);
+		for (path = paths; *path; path++) {
+			resolvconf = g_strconcat (*path, "/resolvconf", NULL);
+			if (g_file_test (resolvconf, G_FILE_TEST_IS_EXECUTABLE))
+				break;
+			g_free (resolvconf);
+			resolvconf = NULL;
+		}
+		g_free (paths);
+		if (resolvconf == NULL)
+			return FALSE;
+	}
+
+	if (searches || nameservers) {
+		cmd = g_strconcat (resolvconf, " -a ", "NetworkManager", NULL);
+		nm_info ("(%s): writing resolv.conf to %s", iface, resolvconf);
+		if ((f = popen (cmd, "w")) == NULL)
+			g_set_error (error,
+				     NM_NAMED_MANAGER_ERROR,
+				     NM_NAMED_MANAGER_ERROR_SYSTEM,
+				     "Could not write to %s: %s\n",
+				     resolvconf,
+				     g_strerror (errno));
+		else {
+			retval = write_resolv_conf (f, searches, nameservers, error);
+			pclose (f);
+		}
+	} else {
+		cmd = g_strconcat (resolvconf, " -d ", "NetworkManager", NULL);
+		nm_info ("(%s): removing resolv.conf from %s", iface, resolvconf);
+		if (nm_spawn_process (cmd) == 0)
+			retval = TRUE;
+	}
+
+	g_free (cmd);
+	g_free (resolvconf);
+
+	return retval;
+}
+
+static gboolean
+update_resolv_conf (char **searches,
+				char **nameservers,
+				const char *iface,
+				GError **error)
+{
+	const char *tmp_resolv_conf = RESOLV_CONF ".tmp";
+	char tmp_resolv_conf_realpath [PATH_MAX];
+	FILE *f;
+
+	if (!realpath(tmp_resolv_conf, tmp_resolv_conf_realpath))
+		strcpy(tmp_resolv_conf_realpath, tmp_resolv_conf);
+
+	if ((f = fopen (tmp_resolv_conf_realpath, "w")) == NULL) {
 		g_set_error (error,
 				   NM_NAMED_MANAGER_ERROR,
 				   NM_NAMED_MANAGER_ERROR_SYSTEM,
-				   "Could not write to " RESOLV_CONF ": %s\n",
+				   "Could not open %s: %s\n",
+				   tmp_resolv_conf_realpath,
 				   g_strerror (errno));
+		return FALSE;
 	}
 
-	g_free (domain_str);
-	g_free (searches_str);
-	g_free (nameservers_str);
+	write_resolv_conf (f, searches, nameservers, error);
 
 	if (fclose (f) < 0) {
 		if (*error == NULL) {
 			g_set_error (error,
 					   NM_NAMED_MANAGER_ERROR,
 					   NM_NAMED_MANAGER_ERROR_SYSTEM,
-					   "Could not close " RESOLV_CONF ": %s\n",
+					   "Could not close %s: %s\n",
+					   tmp_resolv_conf_realpath,
 					   g_strerror (errno));
 		}
 	}
@@ -305,25 +364,23 @@
 
 	return *error ? FALSE : TRUE;
 }
-#endif
 
-#define ADDR_BUF_LEN 50
 
 static gboolean
 rewrite_resolv_conf (NMNamedManager *mgr, const char *iface, GError **error)
 {
+
 	NMNamedManagerPrivate *priv;
 	NMIP4Config *composite;
 	GSList *iter;
 	GPtrArray *array;
-	const char *domain = NULL;
 	char **searches = NULL;
 	char **nameservers = NULL;
 	int num_domains;
 	int num_searches;
 	int num_nameservers;
 	int i;
-	gboolean success;
+	gboolean success = FALSE;
 
 	g_return_val_if_fail (error != NULL, FALSE);
 	g_return_val_if_fail (*error == NULL, FALSE);
@@ -348,48 +405,28 @@
 		merge_one_ip4_config (composite, 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
-	 */
+	/* Some DHCP servers like to put multiple search paths into the domain
+	 * option as the domain-search option wasn't around in the first RFC.
+	 * We should try and support these old servers as best we can. */
 
 	num_domains = nm_ip4_config_get_num_domains (composite);
 	num_searches = nm_ip4_config_get_num_searches (composite);
 
-	/* Domain */
-	if (num_domains > 0)
-		domain = nm_ip4_config_get_domain (composite, 0);
+	/* Searches */
+	if (num_searches > 0) {
+		array = g_ptr_array_sized_new (num_searches + 1);
+		for (i = 0; i < num_searches; i++)
+			g_ptr_array_add (array, g_strdup (nm_ip4_config_get_search (composite, i)));
 
-	/* Searches */
-	if ((num_searches == 0)  && (num_domains > 0)) {
+		g_ptr_array_add (array, NULL);
+		searches = (char **) g_ptr_array_free (array, FALSE);
+	} else if (num_domains > 0) {
 		array = g_ptr_array_sized_new (num_domains + 1);
 		for (i = 0; i < num_domains; i++)
 			g_ptr_array_add (array, g_strdup (nm_ip4_config_get_domain (composite, i)));
 
 		g_ptr_array_add (array, NULL);
 		searches = (char **) g_ptr_array_free (array, FALSE);
-	} else if (num_searches > 0) {
-		array = g_ptr_array_sized_new (num_searches + 1);
-		for (i = 0; i < num_searches; i++)
-			g_ptr_array_add (array, g_strdup (nm_ip4_config_get_search (composite, i)));
-
-		g_ptr_array_add (array, NULL);
-		searches = (char **) g_ptr_array_free (array, FALSE);
 	}
 
 	/* Name servers */
@@ -416,14 +453,22 @@
 		nameservers = (char **) g_ptr_array_free (array, FALSE);
 	}
 
-	success = update_resolv_conf (iface, domain, searches, nameservers, error);
+	success = dispatch_resolvconf (searches, nameservers, iface, error);
 
-	g_strfreev (searches);
-	g_strfreev (nameservers);
+#ifdef TARGET_SUSE
+	if (success == FALSE)
+		success = dispatch_netconfig (searches, nameservers, iface, error);
+#endif
 
+	if (success == FALSE)
+		success = update_resolv_conf (searches, nameservers, iface, error);
+
 	if (success)
 		nm_system_update_dns ();
 
+	g_strfreev (searches);
+	g_strfreev (nameservers);
+
 	return success;
 }