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

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. --- This patch is v2 for this one: https://www.redhat.com/archives/libvir-list/2015-June/msg00018.html Diff to v1: * Use bind-interface if public DNS is requested * Add more tests * Write out the public value in the format function * Fixed the rng * Renamed the attribute to public: shouldn't mislead users I tested this patch with several configurations of running networks. The only thing I noted though is that the user may need to adapt the system dnsmasq to avoid address:port conflicts... but hey, when one uses such a hacky feature of the libvirt network, he needs to take care of the rest ;) docs/formatnetwork.html.in | 9 +++++++ docs/schemas/network.rng | 5 ++++ src/conf/network_conf.c | 28 ++++++++++++++++++++-- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 3 ++- .../nat-network-dns-not-public.conf | 15 ++++++++++++ .../nat-network-dns-not-public.xml | 15 ++++++++++++ .../nat-network-dns-public.conf | 15 ++++++++++++ .../networkxml2confdata/nat-network-dns-public.xml | 15 ++++++++++++ tests/networkxml2xmlin/nat-network-dns-public.xml | 9 +++++++ tests/networkxml2xmlout/nat-network-dns-public.xml | 11 +++++++++ tests/networkxml2xmltest.c | 1 + 12 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-not-public.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-not-public.xml create mode 100644 tests/networkxml2confdata/nat-network-dns-public.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-public.xml create mode 100644 tests/networkxml2xmlin/nat-network-dns-public.xml create mode 100644 tests/networkxml2xmlout/nat-network-dns-public.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 6abed8f..0141d93 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -851,6 +851,15 @@ DNS server. </p> + <p> + The dns element + can have an optional <code>public</code> attribute + <span class="since">Since 1.2.17</span>. If <code>public</code> + is "yes", then the DNS server will handle requests for all interfaces. + If <code>public</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><dns></code> are: <dl> <dt><code>forwarder</code></dt> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4edb6eb..f70e3dc 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -249,6 +249,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="public"> + <ref name="virYesNo"/> + </attribute> + </optional> <interleave> <zeroOrMore> <element name="forwarder"> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 72006e9..e90c004 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1332,9 +1332,14 @@ virNetworkDNSDefParseXML(const char *networkName, size_t i; int ret = -1; xmlNodePtr save = ctxt->node; + char *public = NULL; ctxt->node = node; + public = virXPathString("string(./@public)", ctxt); + if (public) + def->public = virTristateBoolTypeFromString(public); + forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt); if (forwardPlainNames) { def->forwardPlainNames = virTristateBoolTypeFromString(forwardPlainNames); @@ -1433,6 +1438,7 @@ virNetworkDNSDefParseXML(const char *networkName, ret = 0; cleanup: + VIR_FREE(public); VIR_FREE(forwardPlainNames); VIR_FREE(fwdNodes); VIR_FREE(hostNodes); @@ -2416,7 +2422,7 @@ virNetworkDNSDefFormat(virBufferPtr buf, size_t i, j; if (!(def->forwardPlainNames || def->nfwds || def->nhosts || - def->nsrvs || def->ntxts)) + def->nsrvs || def->ntxts || def->public)) return 0; virBufferAddLit(buf, "<dns"); @@ -2431,7 +2437,25 @@ virNetworkDNSDefFormat(virBufferPtr buf, return -1; } virBufferAsprintf(buf, " forwardPlainNames='%s'", fwd); - if (!(def->nfwds || def->nhosts || def->nsrvs || def->ntxts)) { + if (!(def->nfwds || def->nhosts || def->nsrvs || def->ntxts || + def->public)) { + virBufferAddLit(buf, "/>\n"); + return 0; + } + } + + if (def->public) { + const char *public = virTristateBoolTypeToString(def->public); + + if (!public) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown public type %d in network"), + def->public); + return -1; + } + virBufferAsprintf(buf, " public='%s'", public); + if (!(def->nfwds || def->nhosts || def->nsrvs || def->ntxts || + def->forwardPlainNames)) { virBufferAddLit(buf, "/>\n"); return 0; } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 1cd5100..ae239b8 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 public; /* enum virTristateBool */ }; typedef struct _virNetworkIpDef virNetworkIpDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3d6721b..8b11b51 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -991,7 +991,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 (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC) && + network->def->dns.public != VIR_TRISTATE_BOOL_YES) { /* 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 diff --git a/tests/networkxml2confdata/nat-network-dns-not-public.conf b/tests/networkxml2confdata/nat-network-dns-not-public.conf new file mode 100644 index 0000000..021316f --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-not-public.conf @@ -0,0 +1,15 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +domain=example.com +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-not-public.xml b/tests/networkxml2confdata/nat-network-dns-not-public.xml new file mode 100644 index 0000000..cc78f34 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-not-public.xml @@ -0,0 +1,15 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0'/> + <domain name="example.com"/> + <dns forwardPlainNames='no' public='no'> + <host ip='192.168.122.1'> + <hostname>host</hostname> + <hostname>gateway</hostname> + </host> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2confdata/nat-network-dns-public.conf b/tests/networkxml2confdata/nat-network-dns-public.conf new file mode 100644 index 0000000..05cbf43 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-public.conf @@ -0,0 +1,15 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +domain=example.com +expand-hosts +domain-needed +local=// +except-interface=lo +bind-interfaces +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-public.xml b/tests/networkxml2confdata/nat-network-dns-public.xml new file mode 100644 index 0000000..9c59c7b --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-public.xml @@ -0,0 +1,15 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0'/> + <domain name="example.com"/> + <dns forwardPlainNames='no' public='yes'> + <host ip='192.168.122.1'> + <hostname>host</hostname> + <hostname>gateway</hostname> + </host> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2xmlin/nat-network-dns-public.xml b/tests/networkxml2xmlin/nat-network-dns-public.xml new file mode 100644 index 0000000..0765a83 --- /dev/null +++ b/tests/networkxml2xmlin/nat-network-dns-public.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <dns public='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2xmlout/nat-network-dns-public.xml b/tests/networkxml2xmlout/nat-network-dns-public.xml new file mode 100644 index 0000000..66caacd --- /dev/null +++ b/tests/networkxml2xmlout/nat-network-dns-public.xml @@ -0,0 +1,11 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'> + <interface dev='eth0'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <dns public='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 290336e..ba565bc 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -97,6 +97,7 @@ mymain(void) DO_TEST("nat-network-dns-srv-record-minimal"); DO_TEST("nat-network-dns-hosts"); DO_TEST("nat-network-dns-forward-plain"); + DO_TEST("nat-network-dns-public"); DO_TEST("nat-network-dns-forwarders"); DO_TEST("nat-network-forward-nat-address"); DO_TEST("8021Qbh-net"); -- 2.1.4

On Mon, Jul 20, 2015 at 11:29:15 +0200, 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.
Would you please elaborate on the use cases where this would be useful? Libvirt networks shouldn't really be used for configuring dnsmasq for other purposes than for virtual machines where it's desired that the instances are separated.
It simply disables the bind-dynamic option of dnsmasq for the network. ---
This patch is v2 for this one: https://www.redhat.com/archives/libvir-list/2015-June/msg00018.html
Diff to v1: * Use bind-interface if public DNS is requested * Add more tests * Write out the public value in the format function * Fixed the rng * Renamed the attribute to public: shouldn't mislead users
I tested this patch with several configurations of running networks. The only thing I noted though is that the user may need to adapt the system dnsmasq to avoid address:port conflicts... but hey, when one uses such a hacky feature of the libvirt network, he needs to take care of the rest ;)
This paragraph emphasises that it doesn't sound like a good thing to do. NACK unless you will persuade me with a good enough use case. Peter

On Mon, 2015-07-20 at 16:25 +0200, Peter Krempa wrote:
On Mon, Jul 20, 2015 at 11:29:15 +0200, 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.
Would you please elaborate on the use cases where this would be useful? Libvirt networks shouldn't really be used for configuring dnsmasq for other purposes than for virtual machines where it's desired that the instances are separated.
This has been detailed in the previous mail thread, see here: https://www.redhat.com/archives/libvir-list/2015-June/msg00781.html and here: https://www.redhat.com/archives/libvir-list/2015-June/msg00813.html The feature has been requested by people using libvirt as a testing infrastructure for cloud setups with vlans on top of the libvirt-defined network. Maybe I should describe the use case in the commit log to avoid the question being raised again and again. -- Cedric
It simply disables the bind-dynamic option of dnsmasq for the network. ---
This patch is v2 for this one: https://www.redhat.com/archives/libvir-list/2015-June/msg00018.html
Diff to v1: * Use bind-interface if public DNS is requested * Add more tests * Write out the public value in the format function * Fixed the rng * Renamed the attribute to public: shouldn't mislead users
I tested this patch with several configurations of running networks. The only thing I noted though is that the user may need to adapt the system dnsmasq to avoid address:port conflicts... but hey, when one uses such a hacky feature of the libvirt network, he needs to take care of the rest ;)
This paragraph emphasises that it doesn't sound like a good thing to do.
NACK unless you will persuade me with a good enough use case.
Peter

On Mon, Jul 20, 2015 at 17:42:05 +0200, Cedric Bosdonnat wrote:
On Mon, 2015-07-20 at 16:25 +0200, Peter Krempa wrote:
On Mon, Jul 20, 2015 at 11:29:15 +0200, 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.
Would you please elaborate on the use cases where this would be useful? Libvirt networks shouldn't really be used for configuring dnsmasq for other purposes than for virtual machines where it's desired that the instances are separated.
This has been detailed in the previous mail thread, see here: https://www.redhat.com/archives/libvir-list/2015-June/msg00781.html and here: https://www.redhat.com/archives/libvir-list/2015-June/msg00813.html
The feature has been requested by people using libvirt as a testing infrastructure for cloud setups with vlans on top of the libvirt-defined network. Maybe I should describe the use case in the commit log to avoid the question being raised again and again.
I've read the conversation now. In my opinion if users try to circumvent the config of a libvirt network they might as well as provide a full network config themselves rather than trying to abuse libvirt into setting it up partially and then hacking up the rest. As of this patch. The documentation in the XML is misleading since it states that after that "all interfaces" will be handled. With the "bind-interface" option that isn't entirely true, only interfaces that share the subnetwork are handled [1]. In general, the use case you've described seems rather hackish as you even state for yourself and I don't think we should encourage this since for some other desired configurations it might not work and adding more and more workarounds just isn't a good idea. Said this, I'm not going to object if somebody else from the libvirt team thinks that it actually might be worthwhile, so I'm not going to explicitly NACK it. You need to persuade somebody else though. Peter [1] http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q4/006525.html
participants (3)
-
Cedric Bosdonnat
-
Cédric Bosdonnat
-
Peter Krempa