[libvirt] [PATCH] Add forwarders attribute to <dns /> element.

Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used. Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d54f2aa..99f12d2 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -175,6 +175,12 @@ virNetworkDNSSrvDefClear(virNetworkDNSSrvDefPtr def) static void virNetworkDNSDefClear(virNetworkDNSDefPtr def) { + if (def->forwarders) { + size_t i; + for (i = 0; def->forwarders[i] != NULL; i++) + VIR_FREE(def->forwarders[i]); + VIR_FREE(def->forwarders); + } if (def->txts) { while (def->ntxts) virNetworkDNSTxtDefClear(&def->txts[--def->ntxts]); @@ -1038,6 +1044,7 @@ virNetworkDNSDefParseXML(const char *networkName, xmlNodePtr *srvNodes = NULL; xmlNodePtr *txtNodes = NULL; char *forwardPlainNames = NULL; + char *forwarders = NULL; int nhosts, nsrvs, ntxts; size_t i; int ret = -1; @@ -1058,6 +1065,20 @@ virNetworkDNSDefParseXML(const char *networkName, } } + forwarders = virXPathString("string(./@forwarders)", ctxt); + if (forwarders) { + def->forwarders = virStringSplit(forwarders, ",", 0); + for (i = 0; def->forwarders[i] != NULL; i++) { + if (virSocketAddrParse(NULL, def->forwarders[i], AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid forwarder IP address '%s' " + "in network '%s'"), + def->forwarders[i], networkName); + goto cleanup; + } + } + } + nhosts = virXPathNodeSet("./host", ctxt, &hostNodes); if (nhosts < 0) { virReportError(VIR_ERR_XML_ERROR, @@ -1120,6 +1141,7 @@ virNetworkDNSDefParseXML(const char *networkName, ret = 0; cleanup: + VIR_FREE(forwarders); VIR_FREE(forwardPlainNames); VIR_FREE(hostNodes); VIR_FREE(srvNodes); @@ -2267,12 +2289,22 @@ virNetworkDNSDefFormat(virBufferPtr buf, int result = 0; size_t i, j; - if (!(def->forwardPlainNames || def->nhosts || def->nsrvs || def->ntxts)) + if (!(def->forwardPlainNames || def->forwarders || def->nhosts || + def->nsrvs || def->ntxts)) goto out; virBufferAddLit(buf, "<dns"); if (def->forwardPlainNames) { virBufferAddLit(buf, " forwardPlainNames='yes'"); + if (!(def->forwarders || def->nhosts || def->nsrvs || def->ntxts)) { + virBufferAddLit(buf, "/>\n"); + goto out; + } + } + + if (def->forwarders) { + char *forwarders = virStringJoin((const char **)def->forwarders, ","); + virBufferAsprintf(buf, " forwarders='%s'", forwarders); if (!(def->nhosts || def->nsrvs || def->ntxts)) { virBufferAddLit(buf, "/>\n"); goto out; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index c28bfae..3bedc2f 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -122,6 +122,7 @@ struct _virNetworkDNSDef { virNetworkDNSHostDefPtr hosts; size_t nsrvs; virNetworkDNSSrvDefPtr srvs; + char **forwarders; }; typedef struct _virNetworkIpDef virNetworkIpDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3a8be90..c167254 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -708,6 +708,14 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (!network->def->dns.forwardPlainNames) virBufferAddLit(&configbuf, "domain-needed\n"); + if (network->def->dns.forwarders) { + virBufferAddLit(&configbuf, "no-resolv\n"); + for (i=0; network->def->dns.forwarders[i] != NULL; i++) { + virBufferAsprintf(&configbuf, "server=%s\n", + network->def->dns.forwarders[i]); + } + } + if (network->def->domain) { virBufferAsprintf(&configbuf, "domain=%s\n" diff --git a/tests/networkxml2confdata/nat-network-dns-forwarders.conf b/tests/networkxml2confdata/nat-network-dns-forwarders.conf new file mode 100644 index 0000000..ea6a54a --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-forwarders.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-needed +no-resolv +server=8.8.8.8 +local=// +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-forwarders.xml b/tests/networkxml2confdata/nat-network-dns-forwarders.xml new file mode 100644 index 0000000..7955cc7 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-forwarders.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 forwarders='8.8.8.8'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 5825af3..ad50e88 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -145,6 +145,7 @@ mymain(void) DO_TEST("nat-network-dns-srv-record", full); DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); + DO_TEST("nat-network-dns-forwarders", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 1.8.1.2

On Mon, Sep 02, 2013 at 08:50:30AM -0300, Diego Woitasen wrote:
Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
@@ -1058,6 +1065,20 @@ virNetworkDNSDefParseXML(const char *networkName, } }
+ forwarders = virXPathString("string(./@forwarders)", ctxt); + if (forwarders) { + def->forwarders = virStringSplit(forwarders, ",", 0);
It is frowned upon to create XML where one attribute encodes multiple separate values. For example, makes it impossible for apps to extract a single value using xpath queies. If we need to support multiple addresses for forwarders, then this suggests we need a child element underneath <dns> eg perhaps <dns> <forwarder addr='8.8.8.8'/> <forwarder addr='1.2.3.4'/> <forwarder addr='234:223::4242'/> </dns> Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Sep 2, 2013 at 8:54 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Mon, Sep 02, 2013 at 08:50:30AM -0300, Diego Woitasen wrote:
Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
@@ -1058,6 +1065,20 @@ virNetworkDNSDefParseXML(const char *networkName, } }
+ forwarders = virXPathString("string(./@forwarders)", ctxt); + if (forwarders) { + def->forwarders = virStringSplit(forwarders, ",", 0);
It is frowned upon to create XML where one attribute encodes multiple separate values. For example, makes it impossible for apps to extract a single value using xpath queies.
If we need to support multiple addresses for forwarders, then this suggests we need a child element underneath <dns> eg perhaps
<dns> <forwarder addr='8.8.8.8'/> <forwarder addr='1.2.3.4'/> <forwarder addr='234:223::4242'/> </dns>
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Ok, I'll fix it. What do you think about the feature? -- Diego Woitasen VHGroup - Lead Linux and open source solutions architect www.vhgroup.net

On Mon, Sep 02, 2013 at 09:27:59AM -0300, Diego Woitasen wrote:
On Mon, Sep 2, 2013 at 8:54 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Mon, Sep 02, 2013 at 08:50:30AM -0300, Diego Woitasen wrote:
Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
@@ -1058,6 +1065,20 @@ virNetworkDNSDefParseXML(const char *networkName, } }
+ forwarders = virXPathString("string(./@forwarders)", ctxt); + if (forwarders) { + def->forwarders = virStringSplit(forwarders, ",", 0);
It is frowned upon to create XML where one attribute encodes multiple separate values. For example, makes it impossible for apps to extract a single value using xpath queies.
If we need to support multiple addresses for forwarders, then this suggests we need a child element underneath <dns> eg perhaps
<dns> <forwarder addr='8.8.8.8'/> <forwarder addr='1.2.3.4'/> <forwarder addr='234:223::4242'/> </dns>
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Ok, I'll fix it.
What do you think about the feature?
The feature proposal sounds fine to me. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/02/2013 07:50 AM, Diego Woitasen wrote:
Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
You also need to add documentation describing the new element (I agree with danpb's suggestion) to docs/formatnetwork.html.in, and expand the grammar in docs/schemas/network.rng.

Yes, sorry. I've update that yesterday. Resent the patch but forgot to add the v2 suffix to the mail subject :( I'll resend it later this week with the v2 suffix if I don't receive feedback. May be it was discarded by that reason. Regards, Diego On Tue, Sep 3, 2013 at 7:15 AM, Laine Stump <laine@laine.org> wrote:
On 09/02/2013 07:50 AM, Diego Woitasen wrote:
Useful to set custom forwarders instead of using the contents of /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> --- src/conf/network_conf.c | 34 +++++++++++++++++++++- src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 8 +++++ .../nat-network-dns-forwarders.conf | 15 ++++++++++ .../nat-network-dns-forwarders.xml | 9 ++++++ tests/networkxml2conftest.c | 1 + 6 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
You also need to add documentation describing the new element (I agree with danpb's suggestion) to docs/formatnetwork.html.in, and expand the grammar in docs/schemas/network.rng.
-- Diego Woitasen VHGroup - Lead Linux and open source solutions architect www.vhgroup.net

On 09/03/2013 07:21 AM, Diego Woitasen wrote:
Yes, sorry. I've update that yesterday. Resent the patch but forgot to add the v2 suffix to the mail subject :(
I'll resend it later this week with the v2 suffix if I don't receive feedback. May be it was discarded by that reason.
Nah, I was just catching up on 5 days of vacation mail, and didn't notice the newer version of the patch until after I'd already replied to the first. It's of course better to put "Patchv2" on the V2 of a patch, but not necessary to resend just for that reason.
participants (3)
-
Daniel P. Berrange
-
Diego Woitasen
-
Laine Stump