[libvirt] [PATCH] bridge_driver: use conffile for dnsmasq if it exists
By default dnsmasq is spawned with option --conf-file="" which disables reading of global configuration file -- this is fine for most situations. This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before. Patch should be applied after my earlier one. --- src/network/bridge_driver.c | 26 +++++++++++++++++++------- 1 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f2857b4..702ec95 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -391,6 +391,7 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, static int networkBuildDnsmasqArgv(virNetworkObjPtr network, const char *pidfile, + const char *conffile, virCommandPtr cmd) { int r, ret = -1; int nbleases = 0; @@ -428,8 +429,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, virCommandAddArgPair(cmd, "--pid-file", pidfile); - /* *no* conf file */ - virCommandAddArgList(cmd, "--conf-file=", "", NULL); + /* if conf file exists use it */ + if (virFileExists(conffile)) + virCommandAddArgPair(cmd, "--conf-file", conffile); + else + virCommandAddArgList(cmd, "--conf-file=", "", NULL); /* * XXX does not actually work, due to some kind of @@ -528,35 +532,41 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) virCommandPtr cmd = NULL; char *pidfile = NULL; int ret = -1, err; + char *conffile = NULL; network->dnsmasqPid = -1; if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot start dhcp daemon without IPv4 address for server")); - goto cleanup2; + goto cleanup3; } if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_PID_DIR); - goto cleanup2; + goto cleanup3; } if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_STATE_DIR); - goto cleanup2; + goto cleanup3; } if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) { virReportOOMError(); + goto cleanup3; + } + + if (virAsprintf(&conffile, DNSMASQ_STATE_DIR "/%s-dnsmasq.conf", network->def->name) < 0) { + virReportOOMError(); goto cleanup2; } cmd = virCommandNew(DNSMASQ); - if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) { + if (networkBuildDnsmasqArgv(network, pidfile, conffile, cmd) < 0) { goto cleanup1; } @@ -577,9 +587,11 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) ret = 0; cleanup1: - VIR_FREE(pidfile); virCommandFree(cmd); + VIR_FREE(conffile); cleanup2: + VIR_FREE(pidfile); +cleanup3: return ret; }
On 12/21/2010 03:40 PM, Paweł Krześniak wrote:
By default dnsmasq is spawned with option --conf-file="" which disables reading of global configuration file -- this is fine for most situations.
In fact, the libvirt policy is that it is essential to NOT allow the use of global configuration files - if it is worth changing, it is worth calling out directly in the XML directly. Why? Because if you run your guest today, then someone edits the global config file, and you run your guest tomorrow, you have no explainable reason logged in libvirt's generated qemu/dnsmasq/... command line that explains the difference in behavior, if those differences are hidden inside a global config file of an external tool. Furthermore, from the security perspective, sVirt requires that separate domains cannot share resources, and that should include common host config files.
This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before.
You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/12/22 Eric Blake <eblake@redhat.com>:
On 12/21/2010 03:40 PM, Paweł Krześniak wrote:
This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before.
You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file.
I want to create isolated environment for guests - they will be connected to one bridge and will use private DNS data. No single packet from this isolated network can reach external network - this means no 53/udp traffic to resolvers defined in host's /etc/resolv.conf. I'm using following dnsmasq parameters to achive this: no-hosts, no-resolv, addn-hosts, server. It will be nice if one could set log-queries and local-ttl also. With my patch I'm creating config file for this network and dnsmasq runs with --conf-file=/path/to/my/network_name-dnsmasq.conf option. This difference *is* visible on processes list. -- Pawel
On 12/22/2010 04:09 AM, Paweł Krześniak wrote:
On 12/21/2010 03:40 PM, Paweł Krześniak wrote:
This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before. You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file. I want to create isolated environment for guests - they will be connected to one bridge and will use private DNS data. No single
2010/12/22 Eric Blake<eblake@redhat.com>: packet from this isolated network can reach external network - this means no 53/udp traffic to resolvers defined in host's /etc/resolv.conf.
I'm using following dnsmasq parameters to achive this: no-hosts, no-resolv, addn-hosts, server. It will be nice if one could set log-queries and local-ttl also.
With my patch I'm creating config file for this network and dnsmasq runs with --conf-file=/path/to/my/network_name-dnsmasq.conf option. This difference *is* visible on processes list.
The following BZ is related to this discussion: https://bugzilla.redhat.com/show_bug.cgi?id=637055 I agree with Eric that this should be discussed with Dan Berrange and/or Daniel Veillard before comitting anything to the tree (definitely it needs doing in some manner, though).
On Wed, Dec 22, 2010 at 16:50, Laine Stump <laine@laine.org> wrote:
I agree with Eric that this should be discussed with Dan Berrange and/or Daniel Veillard before comitting anything to the tree (definitely it needs doing in some manner, though).
Another idea came to my mind: Following logic behind <emulator> tag for domain definition, we could introduce XML tag in network definition which would override default location of dnsmasq (and radvd). For example: <network> <name>xxxnet</name> <dnsmasq>/usr/local/bin/my-dnsmasq</dnsmasq> <radvd>/usr/local/bin/my-radvd</radvd> ... </network> What do you think? Merry Christmas, -- Pawel
On 12/23/2010 06:50 AM, Paweł Krześniak wrote:
On Wed, Dec 22, 2010 at 16:50, Laine Stump <laine@laine.org> wrote:
I agree with Eric that this should be discussed with Dan Berrange and/or Daniel Veillard before comitting anything to the tree (definitely it needs doing in some manner, though).
Another idea came to my mind: Following logic behind <emulator> tag for domain definition, we could introduce XML tag in network definition which would override default location of dnsmasq (and radvd).
For example: <network> <name>xxxnet</name> <dnsmasq>/usr/local/bin/my-dnsmasq</dnsmasq> <radvd>/usr/local/bin/my-radvd</radvd>
Rather than tying the element name to our current choice of program, can we come up with something more generic? <network> <name>xxxnet</name> <ipv4routing>/usr/local/bin/my-dnsmasq</ipv4routing> <ipv6routing>/usr/local/bin/my-radvd</ipv6routing> </network> But the idea of allowing the XML to specify an override for the program to run seems like it might be worthwhile. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/12/23 Eric Blake <eblake@redhat.com>:
Rather than tying the element name to our current choice of program, can we come up with something more generic?
<network> <name>xxxnet</name> <ipv4routing>/usr/local/bin/my-dnsmasq</ipv4routing> <ipv6routing>/usr/local/bin/my-radvd</ipv6routing> </network>
But the idea of allowing the XML to specify an override for the program to run seems like it might be worthwhile.
I don't think <ipv[46]routing> are the best names, because it goes rather about tuning settings for DNS/DHCP (or NDP for ipv6) or starting additional services per started network and not setting routing. Temporary ignoring the naming I prepared following patch: https://www.redhat.com/archives/libvir-list/2010-December/msg01052.html What do you think? -- Pawel
On Thu, Dec 23, 2010 at 09:53:57AM -0700, Eric Blake wrote:
On 12/23/2010 06:50 AM, Paweł Krześniak wrote:
On Wed, Dec 22, 2010 at 16:50, Laine Stump <laine@laine.org> wrote:
I agree with Eric that this should be discussed with Dan Berrange and/or Daniel Veillard before comitting anything to the tree (definitely it needs doing in some manner, though).
Another idea came to my mind: Following logic behind <emulator> tag for domain definition, we could introduce XML tag in network definition which would override default location of dnsmasq (and radvd).
For example: <network> <name>xxxnet</name> <dnsmasq>/usr/local/bin/my-dnsmasq</dnsmasq> <radvd>/usr/local/bin/my-radvd</radvd>
Rather than tying the element name to our current choice of program, can we come up with something more generic?
<network> <name>xxxnet</name> <ipv4routing>/usr/local/bin/my-dnsmasq</ipv4routing> <ipv6routing>/usr/local/bin/my-radvd</ipv6routing> </network>
But the idea of allowing the XML to specify an override for the program to run seems like it might be worthwhile.
We don't want to directly expose the fact that we're running a process for these services in the XML, because in the future there's a good chance the way we run dnsmasq will change, such that we don't run it per network, instead having one shared instance. Daniel
On Wed, Dec 22, 2010 at 10:09:54AM +0100, Paweł Krześniak wrote:
2010/12/22 Eric Blake <eblake@redhat.com>:
On 12/21/2010 03:40 PM, Paweł Krześniak wrote:
This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before.
You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file.
I want to create isolated environment for guests - they will be connected to one bridge and will use private DNS data. No single packet from this isolated network can reach external network - this means no 53/udp traffic to resolvers defined in host's /etc/resolv.conf.
This sounds like a useful feature for libvirt to directly support, rather than requiring a hack in a external config file. eg, Some XML element to indicate whether to enable DNS proxying or not. Regards, Daniel
2011/1/4 Daniel P. Berrange <berrange@redhat.com>:
On Wed, Dec 22, 2010 at 10:09:54AM +0100, Paweł Krześniak wrote:
I want to create isolated environment for guests - they will be connected to one bridge and will use private DNS data. No single packet from this isolated network can reach external network - this means no 53/udp traffic to resolvers defined in host's /etc/resolv.conf.
This sounds like a useful feature for libvirt to directly support, rather than requiring a hack in a external config file. eg, Some XML element to indicate whether to enable DNS proxying or not.
yes, but it's only the first part of this useful feature. next parts are dnsmasq options like --addn-hosts --no-hosts --server --log-queries --local-ttl (and probably some more..) OK. I'll be happy with following *hack*: diff --git a/configure.ac b/configure.ac index acd30d9..7107661 100644 --- a/configure.ac +++ b/configure.ac @@ -134,7 +134,7 @@ dnl We will hard-code paths to these programs unless we cannot dnl detect them, in which case we'll search for the program dnl along the $PATH at runtime and fail if it's not there. AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq], - [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) + [/usr/local/sbin:/sbin:/usr/sbin:$PATH]) AC_PATH_PROG([RADVD], [radvd], [radvd], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([BRCTL], [brctl], [brctl], -- Pawel
On Tue, Dec 21, 2010 at 09:47:10PM -0700, Eric Blake wrote:
On 12/21/2010 03:40 PM, Paweł Krześniak wrote:
By default dnsmasq is spawned with option --conf-file="" which disables reading of global configuration file -- this is fine for most situations.
In fact, the libvirt policy is that it is essential to NOT allow the use of global configuration files - if it is worth changing, it is worth calling out directly in the XML directly. Why? Because if you run your guest today, then someone edits the global config file, and you run your guest tomorrow, you have no explainable reason logged in libvirt's generated qemu/dnsmasq/... command line that explains the difference in behavior, if those differences are hidden inside a global config file of an external tool. Furthermore, from the security perspective, sVirt requires that separate domains cannot share resources, and that should include common host config files.
This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named "<network_name>-dnsmasq.conf" in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before.
You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file.
Correct. dnsmasq is the current linux specific implementation detail. The fact that we use dnsmasq is not a guarenteed part of the public API/ABI and as such dnsmasq specific bits should not be part of the XML. If there are further configuration params that are required we need to model them in the XML formally. NB, there was a discussion with the dnsmsaq maintainer a few months back now about changing the dnsmasq architecture such that we only need one dnsmasq process. We would be hard pressed to do such a change if we allowed arbitrary dnsmasq config files to be passed in the XML because we cannot see what the implications would be. So, NACK to this patch Regards, Daniel
2011/1/4 Daniel P. Berrange <berrange@redhat.com>:
NB, there was a discussion with the dnsmsaq maintainer a few months back now about changing the dnsmasq architecture such that we only need one dnsmasq process.
is this consistent with Laine's arguments about radvd? read last paragraph of this: https://www.redhat.com/archives/libvir-list/2010-December/msg00912.html -- Pawel
On Wed, Jan 05, 2011 at 09:54:53PM +0100, Paweł Krześniak wrote:
2011/1/4 Daniel P. Berrange <berrange@redhat.com>:
NB, there was a discussion with the dnsmsaq maintainer a few months back now about changing the dnsmasq architecture such that we only need one dnsmasq process.
is this consistent with Laine's arguments about radvd? read last paragraph of this: https://www.redhat.com/archives/libvir-list/2010-December/msg00912.html
What Laine writes is simply our current thinking about radvd. Since usage of radvd is an internal implementation detail, we are free to change our mind about this at any time in the future. Likewise for dnsmasq. Exposing the functionality you propose in the XML restricts our flexibility in this respect Daniel
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump -
Paweł Krześniak