Re: [PATH] Support resolvconf



On Sat, Aug 23, 2008 at 12:54:07AM +0100, Roy Marples wrote:
> Hi List!
> 
> Now that NM supports the SUSE netconfig command, I've taken this a step
> futher and patched in support for resolvconf [1].

Thanks for working on this ...

> 
> However, one big change is as follows - only VPN sets the domain entry,
> all other "domains" get put in the search entry.
> The domain and search entries are mutually exclusive - DHCP client have
> traditionally put the domain into the search. It's just easier and
> allows that nasty "search path in domain" hack.
> As such, openresolv can use this to it's advantage, treating
> search/nameserver combos as global and domain/nameserver combos as
> specific. This is handy for VPN. Consider
> 
> # eth0
> search foo.com
> nameserver 1.2.3.4
> 
> # VPN
> domain bar.org
> nameserver 7.7.7.7
> 
> Using this, openresolv can configure a local resolver (other than libc)
> to forward any queries for bar.org to 7.7.7.7 and any other queries to
> 1.2.3.4. This makes for a powerful setup :)
> 

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.


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?

(Attached the interdiff to your patch [1], and a new complete diff [2]
against current trunk.

 [1] - monolithic.patch (interdiff)
 [2] - resolv-conf-monolithic.patch
)
 - Alexander

=== modified file 'src/named-manager/nm-named-manager.c'
--- src/named-manager/nm-named-manager.c	2008-08-23 21:49:52 +0000
+++ src/named-manager/nm-named-manager.c	2008-08-23 22:40:44 +0000
@@ -18,16 +18,17 @@
  *  You should have received a copy of the GNU General Public License along
  *  with this program; if not, write to the Free Software Foundation, Inc.,
  *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  */
 
 #include "config.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <arpa/inet.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <glib.h>
@@ -386,18 +387,21 @@ dispatch_resolvconf (NMIP4Config *cfg, c
 	return retval;
 }
 
 static gboolean
 update_resolv_conf (NMIP4Config *cfg, const char *iface, GError **error)
 {
 	const char *tmp_resolv_conf = RESOLV_CONF ".tmp";
 	FILE *f;
+	char tmp_resolv_conf_realpath [PATH_MAX];
+	if (!realpath(tmp_resolv_conf, tmp_resolv_conf_realpath))
+		strcpy(tmp_resolv_conf_realpath, tmp_resolv_conf);
 
-	if ((f = fopen (tmp_resolv_conf, "w")) == NULL) {
+	if ((f = fopen (tmp_resolv_conf_realpath, "w")) == NULL) {
 		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;
 	}
 
@@ -435,23 +439,16 @@ rewrite_resolv_conf (NMNamedManager *mgr
 	GSList *iter;
 	gboolean success = FALSE;
 
 	g_return_val_if_fail (error != NULL, FALSE);
 	g_return_val_if_fail (*error == NULL, FALSE);
 
 	priv = NM_NAMED_MANAGER_GET_PRIVATE (mgr);
 
-	/* If resolvconf is available, use it.
-	 * resolvconf only needs what the interface has provided, so don't merge */
-	if (dispatch_resolvconf (priv->vpn_config, iface, TRUE, error))
-		return TRUE;
-	if (dispatch_resolvconf (priv->device_config, iface, FALSE, error))
-		return TRUE;
-
 	/* Construct the composite config from all the currently active IP4Configs */
 	composite = nm_ip4_config_new ();
 
 	if (priv->vpn_config)
 		merge_one_ip4_config (composite, priv->vpn_config);
 
 	if (priv->device_config)
 		merge_one_ip4_config (composite, priv->device_config);
@@ -464,16 +461,21 @@ rewrite_resolv_conf (NMNamedManager *mgr
 
 		merge_one_ip4_config (composite, config);
 	}
 
 #ifdef TARGET_SUSE
 	success = dispatch_netconfig (composite, iface, error);
 #endif
 
+	if (!success)  {
+		dispatch_resolvconf (NULL, iface, FALSE, error);
+		success = dispatch_resolvconf (composite, iface, FALSE, error);
+        }
+
 	if (success == FALSE)
 		success = update_resolv_conf (composite, iface, error);
 
 	if (success)
 		nm_system_update_dns ();
 
 	return success;
 }

=== modified file 'src/named-manager/nm-named-manager.c'
--- src/named-manager/nm-named-manager.c	2008-08-23 21:49:06 +0000
+++ src/named-manager/nm-named-manager.c	2008-08-23 22:38:17 +0000
@@ -18,40 +18,44 @@
  *  You should have received a copy of the GNU General Public License along
  *  with this program; if not, write to the Free Software Foundation, Inc.,
  *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  */
 
 #include "config.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <arpa/inet.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <glib.h>
 
 #include <glib/gi18n.h>
 
 #include "nm-named-manager.h"
 #include "nm-ip4-config.h"
 #include "nm-utils.h"
 #include "NetworkManagerSystem.h"
+#include "NetworkManagerUtils.h"
 
 #ifdef HAVE_SELINUX
 #include <selinux/selinux.h>
 #endif
 
 #ifndef RESOLV_CONF
 #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), \
                                          NM_TYPE_NAMED_MANAGER, \
                                          NMNamedManagerPrivate))
 
 
 struct NMNamedManagerPrivate {
@@ -110,16 +114,75 @@ merge_one_ip4_config (NMIP4Config *dst, 
 		/* If no search domains were specified, add the 'domain' list to
 		 * search domains.
 		 */
 		for (i = 0; i < num_domains; i++)
 			nm_ip4_config_add_search (dst, nm_ip4_config_get_domain (src, i));
 	}
 }
 
+static void
+get_resolv_conf(NMIP4Config *cfg, char ***searches, char ***nameservers)
+{
+	int num_domains;
+	int num_searches;
+	int num_nameservers;
+	int i;
+	GPtrArray *array;
+
+	/* 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 (cfg);
+	num_searches = nm_ip4_config_get_num_searches (cfg);
+
+	/* 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 (cfg, i)));
+
+		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 (cfg, i)));
+
+		g_ptr_array_add (array, NULL);
+		*searches = (char **) g_ptr_array_free (array, FALSE);
+	}
+
+	/* Name servers */
+	num_nameservers = nm_ip4_config_get_num_nameservers (cfg);
+	if (num_nameservers > 0) {
+		array = g_ptr_array_sized_new (num_nameservers + 1);
+		for (i = 0; i < num_nameservers; i++) {
+			struct in_addr addr;
+			char *buf;
+
+			addr.s_addr = nm_ip4_config_get_nameserver (cfg, i);
+			buf = g_malloc0 (ADDR_BUF_LEN);
+			if (!buf)
+				continue;
+
+			if (inet_ntop (AF_INET, &addr, buf, ADDR_BUF_LEN))
+				g_ptr_array_add (array, buf);
+			else
+				nm_warning ("%s: error converting IP4 address 0x%X",
+						  __func__, ntohl (addr.s_addr));
+		}
+
+		g_ptr_array_add (array, NULL);
+		*nameservers = (char **) g_ptr_array_free (array, FALSE);
+	}
+}
+
+
 #if defined(TARGET_SUSE)
 /**********************************/
 /* SUSE */
 
 static void
 netconfig_child_setup (gpointer user_data G_GNUC_UNUSED)
 {
 	pid_t pid = getpid ();
@@ -159,134 +222,195 @@ write_to_netconfig (gint fd, const char 
 	int x;
 
 	str = g_strdup_printf ("%s='%s'\n", key, value);
 	x = write (fd, str, strlen (str));
 	g_free (str);
 }
 
 static gboolean
-update_resolv_conf (const char *iface,
-				const char *domain,
-				char **searches,
-				char **nameservers,
-				GError **error)
+dispatch_netconfig (NMIP4Config *cfg, const char *iface, GError **error)
 {
 	gint fd;
 	char *str;
+	char **searches, **nameservers;
 
 	fd = run_netconfig (error);
 	if (fd < 0)
 		return FALSE;
 
+	get_resolv_conf (cfg, &searches, &nameservers);
+
 	write_to_netconfig (fd, "INTERFACE", iface);
 
 	if (searches) {
 		str = g_strjoinv (" ", searches);
 		write_to_netconfig (fd, "DNSDOMAIN", str);
 		g_free (str);
+		g_strfreev (searches);
 	}
 
 	if (nameservers) {
 		str = g_strjoinv (" ", nameservers);
 		write_to_netconfig (fd, "DNSSERVERS", str);
 		g_free (str);
+		g_strfreev (nameservers);
 	}
 
 	close (fd);
 
 	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, NMIP4Config *cfg, gboolean do_domain, GError **error)
 {
-	const char *tmp_resolv_conf = RESOLV_CONF ".tmp";
 	char *domain_str = NULL;
 	char *searches_str = NULL;
 	char *nameservers_str = NULL;
-	FILE *f;
-
-	if ((f = fopen (tmp_resolv_conf, "w")) == NULL) {
-		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;
-	}
+	const char *domain;
+	char **searches = NULL;
+	char **nameservers = NULL;
+	int i;
+	gboolean retval = FALSE;
 
-	if (fprintf (f, "%s","# generated by NetworkManager, do not edit!\n\n") < 0) {
+	if (fprintf (f, "%s","# generated by NetworkManager\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);
+	get_resolv_conf (cfg, &searches, &nameservers);
+
+	if (do_domain) {
+		domain = nm_ip4_config_get_domain (cfg, 0);
+		domain_str = g_strconcat ("domain ", domain, "\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, "\n# ");
-				g_string_append (str, _("NOTE: the glibc resolver does not support more than 3 nameservers."));
+				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, _("The nameservers listed below may not be recognized."));
 				g_string_append_c (str, '\n');
 			}
 
 			g_string_append (str, "nameserver ");
 			g_string_append (str, nameservers[i]);
 			g_string_append_c (str, '\n');
 		}
 
 		nameservers_str = g_string_free (str, FALSE);
 	}
 
-	if (fprintf (f, "%s%s%s\n",
-	             domain_str ? domain_str : "",
+	if (fprintf (f, "%s%s%s",
+		     domain_str ? domain_str : "",
 	             searches_str ? searches_str : "",
-	             nameservers_str ? nameservers_str : "") < 0) {
+	             nameservers_str ? nameservers_str : "") != -1)
+		retval = TRUE;
+
+	g_strfreev (searches);
+	g_strfreev (nameservers);
+	g_free (domain_str);
+	g_free (searches_str);
+	g_free (nameservers_str);
+
+	return retval;
+}
+
+
+static gboolean
+dispatch_resolvconf (NMIP4Config *cfg, const char *iface, gboolean do_domain, GError **error)
+{
+	char **paths, **path, *tmp, *cmd;
+	char *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;
+	}
+
+	cmd = g_strconcat (resolvconf, cfg ? " -a " : " -d ", iface, NULL);
+	if (cfg) {
+		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, cfg, do_domain, error);
+			pclose (f);
+		}
+	} else {
+		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 (NMIP4Config *cfg, const char *iface, GError **error)
+{
+	const char *tmp_resolv_conf = RESOLV_CONF ".tmp";
+	FILE *f;
+	char tmp_resolv_conf_realpath [PATH_MAX];
+	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 " RESOLV_CONF ": %s\n",
 				   g_strerror (errno));
+		return FALSE;
 	}
 
-	g_free (domain_str);
-	g_free (searches_str);
-	g_free (nameservers_str);
+	write_resolv_conf (f, cfg, FALSE, 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",
 					   g_strerror (errno));
@@ -300,35 +424,25 @@ update_resolv_conf (const char *iface,
 					   NM_NAMED_MANAGER_ERROR_SYSTEM,
 					   "Could not replace " RESOLV_CONF ": %s\n",
 					   g_strerror (errno));
 		}
 	}
 
 	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);
 
 	priv = NM_NAMED_MANAGER_GET_PRIVATE (mgr);
 
 	/* Construct the composite config from all the currently active IP4Configs */
 	composite = nm_ip4_config_new ();
@@ -343,88 +457,27 @@ rewrite_resolv_conf (NMNamedManager *mgr
 		NMIP4Config *config = NM_IP4_CONFIG (iter->data);
 
 		if ((config == priv->vpn_config) || (config == priv->device_config))
 			continue;
 
 		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
-	 */
-
-	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)  && (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 */
-	num_nameservers = nm_ip4_config_get_num_nameservers (composite);
-	if (num_nameservers > 0) {
-		array = g_ptr_array_sized_new (num_nameservers + 1);
-		for (i = 0; i < num_nameservers; i++) {
-			struct in_addr addr;
-			char *buf;
-
-			addr.s_addr = nm_ip4_config_get_nameserver (composite, i);
-			buf = g_malloc0 (ADDR_BUF_LEN);
-			if (!buf)
-				continue;
-
-			if (inet_ntop (AF_INET, &addr, buf, ADDR_BUF_LEN))
-				g_ptr_array_add (array, buf);
-			else
-				nm_warning ("%s: error converting IP4 address 0x%X",
-						  __func__, ntohl (addr.s_addr));
-		}
-
-		g_ptr_array_add (array, NULL);
-		nameservers = (char **) g_ptr_array_free (array, FALSE);
-	}
+#ifdef TARGET_SUSE
+	success = dispatch_netconfig (composite, iface, error);
+#endif
 
-	success = update_resolv_conf (iface, domain, searches, nameservers, error);
+	if (!success)  {
+		dispatch_resolvconf (NULL, iface, FALSE, error);
+		success = dispatch_resolvconf (composite, iface, FALSE, error);
+        }
 
-	g_strfreev (searches);
-	g_strfreev (nameservers);
+	if (success == FALSE)
+		success = update_resolv_conf (composite, iface, error);
 
 	if (success)
 		nm_system_update_dns ();
 
 	return success;
 }
 
 gboolean



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