Re: [RFC PATCH] use configuration file for dnsmasq on shared connections

[Dan Williams <dcbw redhat com>]

On Sat, 2012-09-01 at 16:51 +0200, hvoigt hvoigt net wrote:
> Previously no configuration was read at all. This lead to the situation
> that the dnsmasq instance listening on the shared interface would not
> resolve dns queries.
> ---
> Hi,
> 
> when I configured my system to share the wired connection on Linux  
> Mint 13 I found that everything was working except the dns  
> configuration via dhcp. This patch solved the problem by loading the  
> same configuration for the dnsmasq bound to the shared interface.

What specific configuration was not getting picked up?  NetworkManager
configures dnsmasq automatically to forward DNS queries to the
nameservers that are already listed in /etc/resolv.conf, so I'd expect
things to work automatically.  Did you have any other dnsmasq processes
running on your system, besides the one that NM launched?

Dan

> This is basically a demonstration patch since I have never contributed  
> anything to NetworkManager. I do not know much about its internal  
> design. For example: The config file seems to be generated in  
> nm-dns-dnsmasq.c. To make sure that the config file is already  
> generated in the new place we probably want to call something there as  
> well.
> 
> Maybe someone could give me some hints and I will try to adopt my patch.
> 
> Cheers Heiko
> 
> P.S.: Please reply-to-all since I am not subscribed to the list.
> 
>   src/dns-manager/nm-dns-dnsmasq.c         |   16 +++++++---------
>   src/dns-manager/nm-dns-dnsmasq.h         |    3 +++
>   src/dnsmasq-manager/Makefile.am          |    3 ++-
>   src/dnsmasq-manager/nm-dnsmasq-manager.c |   13 ++++---------
>   4 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/src/dns-manager/nm-dns-dnsmasq.c  
> b/src/dns-manager/nm-dns-dnsmasq.c
> index 9090e26..531e446 100644
> --- a/src/dns-manager/nm-dns-dnsmasq.c
> +++ b/src/dns-manager/nm-dns-dnsmasq.c
> @@ -40,8 +40,6 @@ G_DEFINE_TYPE (NMDnsDnsmasq, nm_dns_dnsmasq,  
> NM_TYPE_DNS_PLUGIN)
>   #define NM_DNS_DNSMASQ_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE  
> ((o), NM_TYPE_DNS_DNSMASQ, NMDnsDnsmasqPrivate))
> 
>   #define PIDFILE LOCALSTATEDIR "/run/nm-dns-dnsmasq.pid"
> -#define CONFFILE LOCALSTATEDIR "/run/nm-dns-dnsmasq.conf"
> -#define CONFDIR SYSCONFDIR "/NetworkManager/dnsmasq.d"
> 
>   typedef struct {
>   	guint32 foo;
> @@ -287,15 +285,15 @@ update (NMDnsPlugin *plugin,
>   	}
> 
>   	/* Write out the config file */
> -	if (!g_file_set_contents (CONFFILE, conf->str, -1, &error)) {
> +	if (!g_file_set_contents (DNSMASQ_CONFFILE, conf->str, -1, &error)) {
>   		nm_log_warn (LOGD_DNS, "Failed to write dnsmasq config file %s: (%d) %s",
> -		             CONFFILE,
> +		             DNSMASQ_CONFFILE,
>   		             error ? error->code : -1,
>   		             error && error->message ? error->message : "(unknown)");
>   		g_clear_error (&error);
>   		goto out;
>   	}
> -	ignored = chmod (CONFFILE, 0644);
> +	ignored = chmod (DNSMASQ_CONFFILE, 0644);
> 
>   	nm_log_dbg (LOGD_DNS, "dnsmasq local caching DNS configuration:");
>   	nm_log_dbg (LOGD_DNS, "%s", conf->str);
> @@ -307,10 +305,10 @@ update (NMDnsPlugin *plugin,
>   	argv[4] = "--bind-interfaces";
>   	argv[5] = "--pid-file=" PIDFILE;
>   	argv[6] = "--listen-address=127.0.0.1"; /* Should work for both 4 and 6 */
> -	argv[7] = "--conf-file=" CONFFILE;
> +	argv[7] = "--conf-file=" DNSMASQ_CONFFILE;
>   	argv[8] = "--cache-size=400";
>   	argv[9] = "--proxy-dnssec"; /* Allow DNSSEC to pass through */
> -	argv[10] = "--conf-dir=" CONFDIR;
> +	argv[10] = "--conf-dir=" DNSMASQ_CONFDIR;
>   	argv[11] = NULL;
> 
>   	/* And finally spawn dnsmasq */
> @@ -363,7 +361,7 @@ child_quit (NMDnsPlugin *plugin, gint status)
>   	} else {
>   		nm_log_warn (LOGD_DNS, "dnsmasq died from an unknown cause");
>   	}
> -	unlink (CONFFILE);
> +	unlink (DNSMASQ_CONFFILE);
> 
>   	if (failed)
>   		g_signal_emit_by_name (self, NM_DNS_PLUGIN_FAILED);
> @@ -405,7 +403,7 @@ nm_dns_dnsmasq_init (NMDnsDnsmasq *self)
>   static void
>   dispose (GObject *object)
>   {
> -	unlink (CONFFILE);
> +	unlink (DNSMASQ_CONFFILE);
> 
>   	G_OBJECT_CLASS (nm_dns_dnsmasq_parent_class)->dispose (object);
>   }
> diff --git a/src/dns-manager/nm-dns-dnsmasq.h  
> b/src/dns-manager/nm-dns-dnsmasq.h
> index 01ec579..662edff 100644
> --- a/src/dns-manager/nm-dns-dnsmasq.h
> +++ b/src/dns-manager/nm-dns-dnsmasq.h
> @@ -31,6 +31,9 @@
>   #define NM_IS_DNS_DNSMASQ_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE  
> ((klass), NM_TYPE_DNS_DNSMASQ))
>   #define NM_DNS_DNSMASQ_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS  
> ((obj), NM_TYPE_DNS_DNSMASQ, NMDnsDnsmasqClass))
> 
> +#define DNSMASQ_CONFFILE LOCALSTATEDIR "/run/nm-dns-dnsmasq.conf"
> +#define DNSMASQ_CONFDIR SYSCONFDIR "/NetworkManager/dnsmasq.d"
> +
>   typedef struct {
>   	NMDnsPlugin parent;
>   } NMDnsDnsmasq;
> diff --git a/src/dnsmasq-manager/Makefile.am b/src/dnsmasq-manager/Makefile.am
> index 8b7dd68..6e4044f 100644
> --- a/src/dnsmasq-manager/Makefile.am
> +++ b/src/dnsmasq-manager/Makefile.am
> @@ -13,7 +13,8 @@ libdnsmasq_manager_la_SOURCES = \
> 
>   libdnsmasq_manager_la_CPPFLAGS = \
>   	$(GLIB_CFLAGS) \
> -	-DLOCALSTATEDIR=\"$(localstatedir)\"
> +	-DLOCALSTATEDIR=\"$(localstatedir)\" \
> +	-DSYSCONFDIR=\"$(sysconfdir)\"
> 
>   libdnsmasq_manager_la_LIBADD = \
>   	$(top_builddir)/src/logging/libnm-logging.la \
> diff --git a/src/dnsmasq-manager/nm-dnsmasq-manager.c  
> b/src/dnsmasq-manager/nm-dnsmasq-manager.c
> index 0b3b629..f0a19aa 100644
> --- a/src/dnsmasq-manager/nm-dnsmasq-manager.c
> +++ b/src/dnsmasq-manager/nm-dnsmasq-manager.c
> @@ -27,9 +27,10 @@
>   #include <arpa/inet.h>
>   #include <stdlib.h>
> 
> +#include "../dns-manager/nm-dns-dnsmasq.h"
>   #include "nm-dnsmasq-manager.h"
>   #include "nm-logging.h"
>   #include "nm-glib-compat.h"
>   #include "nm-posix-signals.h"
> 
>   typedef struct {
> @@ -275,14 +276,8 @@ create_dm_cmd_line (const char *iface,
>   		nm_cmd_line_add_string (cmd, "--log-queries");
>   	}
> 
> -	/* dnsmasq may read from it's default config file location, which if that
> -	 * location is a valid config file, it will combine with the options here
> -	 * and cause undesirable side-effects.  Like sending bogus IP addresses
> -	 * as the gateway or whatever.  So tell dnsmasq not to use any config file
> -	 * at all.
> -	 */
> -	nm_cmd_line_add_string (cmd, "--conf-file");
> -
> +	nm_cmd_line_add_string (cmd, "--conf-file=" DNSMASQ_CONFFILE);
> +	nm_cmd_line_add_string (cmd, "--conf-dir=" DNSMASQ_CONFDIR);
>   	nm_cmd_line_add_string (cmd, "--no-hosts");
>   	nm_cmd_line_add_string (cmd, "--keep-in-foreground");
>   	nm_cmd_line_add_string (cmd, "--bind-interfaces");