Re: [libvirt] [PATCH] network: add an option to make dns public

On 06/01/2015 07:54 AM, Cédric Bosdonnat wrote: > In some use cases we don't want the virtual network's DNS to only > listen to the vnet interface. Adding a publiclyAccessible attribute > to the dns element in the configuration allows the DNS to listen to > all interfaces. > > It simply disables the bind-dynamic option of dnsmasq for the network. > --- > docs/formatnetwork.html.in | 11 +++++++++++ > docs/schemas/network.rng | 15 ++++++++++----- > src/conf/network_conf.c | 6 ++++++ > src/conf/network_conf.h | 1 + > src/network/bridge_driver.c | 4 +++- > tests/networkxml2confdata/nat-network-dns-hosts.conf | 1 - > tests/networkxml2confdata/nat-network-dns-hosts.xml | 2 +- > 7 files changed, 32 insertions(+), 8 deletions(-) > > diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in > index 6abed8f..8e43658 100644 > --- a/docs/formatnetwork.html.in > +++ b/docs/formatnetwork.html.in > @@ -851,6 +851,17 @@ > DNS server. > </p> > > + <p> > + The dns element > + can have an optional <code>publiclyAccessible</code> > + attribute <span class="since">Since 1.2.17</span>. > + If <code>publiclyAccessible</code> is "yes", then the DNS server > + will handle requests for all interfaces. > + If <code>publiclyAccessible</code> is not set or "no", the DNS > + server will only handle requests for the interface of the virtual > + network. > + </p> > + > Currently supported sub-elements of <code>&lt;dns&gt;</code> are: > <dl> > <dt><code>forwarder</code></dt> > diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng > index 4edb6eb..f989625 100644 > --- a/docs/schemas/network.rng > +++ b/docs/schemas/network.rng > @@ -244,12 +244,17 @@ > and other features in the <dns> element --> > <optional> > <element name="dns"> > - <optional> > - <attribute name="forwardPlainNames"> > - <ref name="virYesNo"/> > - </attribute> > - </optional> > <interleave> > + <optional> > + <attribute name="forwardPlainNames"> > + <ref name="virYesNo"/> > + </attribute> > + </optional> > + <optional> > + <attribute name="publiclyAccessible"> > + <ref name="virYesNo"/> > + </attribute> > + </optional> Moving the attributes inside the <interleave> had me looking through other .rng's... I'm no expert, but had thought they really only mattered for <element>'s

> <zeroOrMore> > <element name="forwarder"> > <attribute name="addr"><ref name="ipAddr"/></attribute> > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > index f4a9df0..99bac6d 100644 > --- a/src/conf/network_conf.c > +++ b/src/conf/network_conf.c > @@ -1309,9 +1309,14 @@ virNetworkDNSDefParseXML(const char *networkName, > size_t i; > int ret = -1; > xmlNodePtr save = ctxt->node; > + char *publiclyAccessible = NULL; > > ctxt->node = node; > > + publiclyAccessible = virXPathString("string(./@publiclyAccessible)", ctxt); > + if (publiclyAccessible) > + def->publiclyAccessible = virTristateBoolTypeFromString(publiclyAccessible); > + > forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt); > if (forwardPlainNames) { > def->forwardPlainNames = virTristateBoolTypeFromString(forwardPlainNames); > @@ -1410,6 +1415,7 @@ virNetworkDNSDefParseXML(const char *networkName, > > ret = 0; > cleanup: > + VIR_FREE(publiclyAccessible); > VIR_FREE(forwardPlainNames); > VIR_FREE(fwdNodes); > VIR_FREE(hostNodes); > diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h > index f69d999..f555b6b 100644 > --- a/src/conf/network_conf.h > +++ b/src/conf/network_conf.h > @@ -136,6 +136,7 @@ struct _virNetworkDNSDef { > virNetworkDNSSrvDefPtr srvs; > size_t nfwds; > char **forwarders; > + int publiclyAccessible; /* enum virTristateBool */ > }; > > typedef struct _virNetworkIpDef virNetworkIpDef; > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index d195085..c39b1a5 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -996,8 +996,10 @@ networkDnsmasqConfContents(virNetworkObjPtr network, > * other than one of the virtual guests connected directly to > * this network). This was added in response to CVE 2012-3411. > */ > + if (network->def->dns.publiclyAccessible != VIR_TRISTATE_BOOL_YES) > + virBufferAddLit(&configbuf, > + "bind-dynamic\n"); > virBufferAsprintf(&configbuf, > - "bind-dynamic\n" > "interface=%s\n", > network->def->bridge); > } else { > diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.conf b/tests/networkxml2confdata/nat-network-dns-hosts.conf > index 021316f..759a9e9 100644 > --- a/tests/networkxml2confdata/nat-network-dns-hosts.conf > +++ b/tests/networkxml2confdata/nat-network-dns-hosts.conf > @@ -10,6 +10,5 @@ expand-hosts > domain-needed > local=// > except-interface=lo > -bind-dynamic > interface=virbr0 > addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts > diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.xml b/tests/networkxml2confdata/nat-network-dns-hosts.xml > index 9add456..969dfa5 100644 > --- a/tests/networkxml2confdata/nat-network-dns-hosts.xml > +++ b/tests/networkxml2confdata/nat-network-dns-hosts.xml > @@ -4,7 +4,7 @@ > <forward dev='eth0' mode='nat'/> > <bridge name='virbr0' stp='on' delay='0'/> > <domain name="example.com"/> > - <dns forwardPlainNames='no'> > + <dns forwardPlainNames='no' publiclyAccessible='yes'> > <host ip='192.168.122.1'> > <hostname>host</hostname> > <hostname>gateway</hostname> > Rather than change an existing test, a new test or two should be created... One that specifically states 'yes' and possibly one that has 'no' keeping the existing one with nothing provided to be sure that works as well. I don't mind doing that for you, but also I figure by bumping this perhaps Laine will take a look too since he usually responds to most of the network related patches anyway... It seems fine to me though.