[libvirt] [PATCH] add a nodnsmasq option for networks
Some people want to define a libvirt network but have dns served by another daemon. Libvirt used to support that, but hasn't for several years. Two long-open bugs on this are https://bugzilla.redhat.com/show_bug.cgi?id=636115 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/584862 I'm not certain whether we really want to support this, as another option is to simply create the bridge with external tools and tell libvirt vms to use that bridge. However, if we do want to support it, this patch provides that support. This patch allows an admin to set <nodnsmasq>true</nodnsmasq> in a bridge definition to avoid having libvirt start a dnsmasq. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/conf/network_conf.c | 15 +++++++++++++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) Index: libvirt-1.2.16/src/conf/network_conf.c =================================================================== --- libvirt-1.2.16.orig/src/conf/network_conf.c +++ libvirt-1.2.16/src/conf/network_conf.c @@ -1985,6 +1985,7 @@ virNetworkDefParseXML(xmlXPathContextPtr xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; char *ipv6nogwStr = NULL; + char *noDnsmasqStr = NULL; char *trustGuestRxFilters = NULL; xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; @@ -2018,6 +2019,20 @@ virNetworkDefParseXML(xmlXPathContextPtr def->uuid_specified = true; } + /* check if user requested no dnsmasq */ + noDnsmasqStr = virXPathString("string(./nodnsmasq[1])", ctxt); + if (noDnsmasqStr) { + if (STREQ(noDnsmasqStr, "yes") || STREQ(noDnsmasqStr, "true")) { + def->nodnsmasq = true; + } else if (STRNEQ(noDnsmasqStr, "no") && STRNEQ(noDnsmasqStr, "false")) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid nodnsmasq setting '%s' in network '%s'"), + noDnsmasqStr, def->name); + goto error; + } + VIR_FREE(noDnsmasqStr); + } + /* check if definitions with no IPv6 gateway addresses is to * allow guest-to-guest communications. */ @@ -2660,6 +2675,9 @@ virNetworkDefFormatBuf(virBufferPtr buf, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + if (def->nodnsmasq) + virBufferAsprintf(buf, "<nodnsmasq>true</nodnsmasq>\n"); + if (def->forward.type != VIR_NETWORK_FORWARD_NONE) { const char *dev = NULL; if (!def->forward.npfs) Index: libvirt-1.2.16/src/conf/network_conf.h =================================================================== --- libvirt-1.2.16.orig/src/conf/network_conf.h +++ libvirt-1.2.16/src/conf/network_conf.h @@ -231,6 +231,7 @@ struct _virNetworkDef { bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ bool mac_specified; + bool nodnsmasq; /* specified if ip6tables rules added * when no ipv6 gateway addresses specified. Index: libvirt-1.2.16/src/network/bridge_driver.c =================================================================== --- libvirt-1.2.16.orig/src/network/bridge_driver.c +++ libvirt-1.2.16/src/network/bridge_driver.c @@ -2146,7 +2146,7 @@ networkStartNetworkVirtual(virNetworkDri /* start dnsmasq if there are any IP addresses (v4 or v6) */ - if ((v4present || v6present) && + if ((v4present || v6present) && !network->def->nodnsmasq && networkStartDhcpDaemon(driver, network) < 0) goto err3;
On 12/02/2015 02:50 PM, Serge Hallyn wrote:
Some people want to define a libvirt network but have dns served by another daemon. Libvirt used to support that, but hasn't for several years. Two long-open bugs on this are
https://bugzilla.redhat.com/show_bug.cgi?id=636115 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/584862
I'm not certain whether we really want to support this, as another option is to simply create the bridge with external tools and tell libvirt vms to use that bridge. However, if we do want to support it, this patch provides that support.
This patch allows an admin to set <nodnsmasq>true</nodnsmasq> in a bridge definition to avoid having libvirt start a dnsmasq.
Actually someone asked about making an option for this a long time ago, and I made a suggestion on how to do it, but neither I nor the person who asked ever got around to doing it. First, about your proposal: I think that allowing a user to completely disable everything done by dnsmasq (i.e. DHCP and DNS services) is something reasonable for us to support. The option that you've defined gets the job done, but it is specific to a backend implementation that uses dnsmasq, and we want to avoid that in case someone in the future makes a different implementation that uses something else for dhcp and/or dns When there is no <dhcp> element in a network, the dhcp part of dnsmasq already isn't enabled (so dnsmasq isn't listening on the dhcp port), but the dns server is always enabled and listening on port 53. It makes sense that we should be able to disable the DNS server regardless of whether or not we are using dnsmasq's dhcp server. If both are disabled, then dnsmasq simply wouldn't be started (after it's not listening on any ports, so there's no point :-). There is already a <dns> element in a network definition, so the logical place for the option that enables/disables DNS would be as an attribute to that element: <dns enable='no'/> So if there is no enable attribute (or if it is set to "yes"), DNS is turned on. If enable='no', then it is turned off. If dhcp is enabled for the network, then dnsmasq would still be started up for that, but would get the proper options so it didn't listen for DNS requests. A couple notes: * For backward compatibility, we have to have the default be that DNS is enabled. * yes/no is much more commonly used in libvirt's XML than true/false, so that is the preferred setting (there is an enum for that, virTristateBool, so you can use virTristateBoolType(To|From)String when parsing (be sure to reject if the return value of *FromString() is <= 0, so nobody can enter "enable='default'").
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- src/conf/network_conf.c | 15 +++++++++++++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)
Index: libvirt-1.2.16/src/conf/network_conf.c =================================================================== --- libvirt-1.2.16.orig/src/conf/network_conf.c +++ libvirt-1.2.16/src/conf/network_conf.c @@ -1985,6 +1985,7 @@ virNetworkDefParseXML(xmlXPathContextPtr xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; char *ipv6nogwStr = NULL; + char *noDnsmasqStr = NULL; char *trustGuestRxFilters = NULL; xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; @@ -2018,6 +2019,20 @@ virNetworkDefParseXML(xmlXPathContextPtr def->uuid_specified = true; }
+ /* check if user requested no dnsmasq */ + noDnsmasqStr = virXPathString("string(./nodnsmasq[1])", ctxt); + if (noDnsmasqStr) { + if (STREQ(noDnsmasqStr, "yes") || STREQ(noDnsmasqStr, "true")) { + def->nodnsmasq = true; + } else if (STRNEQ(noDnsmasqStr, "no") && STRNEQ(noDnsmasqStr, "false")) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid nodnsmasq setting '%s' in network '%s'"), + noDnsmasqStr, def->name); + goto error; + } + VIR_FREE(noDnsmasqStr); + } + /* check if definitions with no IPv6 gateway addresses is to * allow guest-to-guest communications. */ @@ -2660,6 +2675,9 @@ virNetworkDefFormatBuf(virBufferPtr buf, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr);
+ if (def->nodnsmasq) + virBufferAsprintf(buf, "<nodnsmasq>true</nodnsmasq>\n"); + if (def->forward.type != VIR_NETWORK_FORWARD_NONE) { const char *dev = NULL; if (!def->forward.npfs) Index: libvirt-1.2.16/src/conf/network_conf.h =================================================================== --- libvirt-1.2.16.orig/src/conf/network_conf.h +++ libvirt-1.2.16/src/conf/network_conf.h @@ -231,6 +231,7 @@ struct _virNetworkDef { bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ bool mac_specified; + bool nodnsmasq;
/* specified if ip6tables rules added * when no ipv6 gateway addresses specified. Index: libvirt-1.2.16/src/network/bridge_driver.c =================================================================== --- libvirt-1.2.16.orig/src/network/bridge_driver.c +++ libvirt-1.2.16/src/network/bridge_driver.c @@ -2146,7 +2146,7 @@ networkStartNetworkVirtual(virNetworkDri
/* start dnsmasq if there are any IP addresses (v4 or v6) */ - if ((v4present || v6present) && + if ((v4present || v6present) && !network->def->nodnsmasq && networkStartDhcpDaemon(driver, network) < 0) goto err3;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Quoting Laine Stump (laine@laine.org):
On 12/02/2015 02:50 PM, Serge Hallyn wrote:
Some people want to define a libvirt network but have dns served by another daemon. Libvirt used to support that, but hasn't for several years. Two long-open bugs on this are
https://bugzilla.redhat.com/show_bug.cgi?id=636115 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/584862
I'm not certain whether we really want to support this, as another option is to simply create the bridge with external tools and tell libvirt vms to use that bridge. However, if we do want to support it, this patch provides that support.
This patch allows an admin to set <nodnsmasq>true</nodnsmasq> in a bridge definition to avoid having libvirt start a dnsmasq.
Actually someone asked about making an option for this a long time ago, and I made a suggestion on how to do it, but neither I nor the person who asked ever got around to doing it.
First, about your proposal: I think that allowing a user to completely disable everything done by dnsmasq (i.e. DHCP and DNS services) is something reasonable for us to support. The option that you've defined gets the job done, but it is specific to a backend implementation that uses dnsmasq, and we want to avoid that in case someone in the future makes a different implementation that uses something else for dhcp and/or dns
When there is no <dhcp> element in a network, the dhcp part of dnsmasq already isn't enabled (so dnsmasq isn't listening on the dhcp port), but the dns server is always enabled and listening on port 53. It makes sense that we should be able to disable the DNS server regardless of whether or not we are using dnsmasq's dhcp server. If both are disabled, then dnsmasq simply wouldn't be started (after it's not listening on any ports, so there's no point :-).
There is already a <dns> element in a network definition, so the logical place for the option that enables/disables DNS would be as an attribute to that element:
<dns enable='no'/>
Yup, that does make more sense. I can aim to send a patch along those lines, but I'm particularly un-familiar with the xml parsing code so if someone else wants to do it i won't feel my toes being stepped on :)
So if there is no enable attribute (or if it is set to "yes"), DNS is turned on. If enable='no', then it is turned off. If dhcp is enabled for the network, then dnsmasq would still be started up for that, but would get the proper options so it didn't listen for DNS requests.
A couple notes:
* For backward compatibility, we have to have the default be that DNS is enabled. * yes/no is much more commonly used in libvirt's XML than true/false, so that is the preferred setting (there is an enum for that, virTristateBool, so you can use virTristateBoolType(To|From)String when parsing (be sure to reject if the return value of *FromString() is <= 0, so nobody can enter "enable='default'").
Ok, thanks -serge
participants (2)
-
Laine Stump -
Serge Hallyn