Re: [PATCH] add a nodnsmasq option for networks

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

 



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@xxxxxxxxxx>
---
  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]