[libvirt] [PATCH] add a nodnsmasq option for networks

[Laine Stump]

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