[libvirt] [RFC PATCH] network: add an option to disable dnsmasq's bind-dynamic

[Michal Privoznik]

On 06.05.2015 15:29, Cédric Bosdonnat wrote:
> When building vlans on top of veth networks, dnsmasq doesn't catch
> DNS requests on the vlans interfaces. Allowing to disable the
> bind-dynamic helps this use case.
> ---
> 
>  src/conf/network_conf.c     | 12 ++++++++++++
>  src/conf/network_conf.h     |  1 +
>  src/network/bridge_driver.c |  3 ++-
>  3 files changed, 15 insertions(+), 1 deletion(-)

I know this is patch just to demonstrate the idea, so I will not point
out obvious (e.g. missing RNG schema and documentation).

> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index f4a9df0..63e26e1 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1987,6 +1987,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>      xmlNodePtr forwardNode = NULL;
>      char *ipv6nogwStr = NULL;
>      char *trustGuestRxFilters = NULL;
> +    char *binddynamicStr = NULL;
>      xmlNodePtr save = ctxt->node;
>      xmlNodePtr bandwidthNode = NULL;
>      xmlNodePtr vlanNode;
> @@ -2049,6 +2050,16 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>          VIR_FREE(trustGuestRxFilters);
>      }
>  
> +    /* Default for binddynamic is on */
> +    def->binddynamic = true;
> +    binddynamicStr = virXPathString("string(./@binddynamic)", ctxt);
> +    if (binddynamicStr) {
> +        if (STRNEQ(binddynamicStr, "no")) {

s/STRNEQ/STREQ/ or even better virTristateSwitchTypeFromString().

Moreover, I'm curious if we can come up with not so dnsmasq specific
attribute name. But that's just cosmetics.

> +            def->binddynamic = false;
> +        }
> +        VIR_FREE(binddynamicStr);
> +    }
> +
>      /* Parse network domain information */
>      def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
>      tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt);
> @@ -2326,6 +2337,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>      VIR_FREE(ipNodes);
>      VIR_FREE(portGroupNodes);
>      VIR_FREE(ipv6nogwStr);
> +    VIR_FREE(binddynamicStr);
>      VIR_FREE(trustGuestRxFilters);
>      ctxt->node = save;
>      return NULL;
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index f69d999..163581e 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -253,6 +253,7 @@ struct _virNetworkDef {
>      virNetDevBandwidthPtr bandwidth;
>      virNetDevVlan vlan;
>      int trustGuestRxFilters; /* enum virTristateBool */
> +    bool binddynamic; /* to force off bind_dynamic option of dnsmasq */
>  };
>  
>  typedef struct _virNetworkObj virNetworkObj;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index d195085..5dddc4b 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -988,7 +988,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
>      /* dnsmasq will *always* listen on localhost unless told otherwise */
>      virBufferAddLit(&configbuf, "except-interface=lo\n");
>  
> -    if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) {
> +    if (network->def->binddynamic &&
> +        dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) {

I think the logic should be slightly different, when specifically
requested in XML but not provided by dnsmasq an error must be thrown.

>          /* using --bind-dynamic with only --interface (no
>           * --listen-address) prevents dnsmasq from responding to dns
>           * queries that arrive on some interface other than our bridge
> 

Since this is not the first request I see to disable dynamic bind I
think it's really needed. I'm too lazy to dig out the other requests
from history (maybe it was a bugzilla I saw, or an IRC chat, or here on
the list, ...).

So, ACK to the idea.

Michal