[libvirt] [PATCHv3] Add forwarder attribute to <dns /> element.
Eric Blake
eblake at redhat.com
Tue Sep 17 23:36:17 UTC 2013
On 09/13/2013 10:31 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
s/resolv /resolve /
> domain names from domain 0, when domain option is used.
>
> Signed-off-by: Diego Woitasen <diego.woitasen at vhgroup.net>
> ---
> docs/formatnetwork.html.in | 8 ++++
> docs/schemas/network.rng | 5 +++
> src/conf/network_conf.c | 43 ++++++++++++++++++++--
> src/conf/network_conf.h | 2 +
> src/network/bridge_driver.c | 8 ++++
> .../nat-network-dns-forwarders.conf | 16 ++++++++
> .../nat-network-dns-forwarders.xml | 12 ++++++
> tests/networkxml2conftest.c | 1 +
> 8 files changed, 92 insertions(+), 3 deletions(-)
> create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf
> create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml
Yay for docs and tests alongside the feature.
Failed 'make syntax-check'
trailing_blank
src/conf/network_conf.c:2301: if (!(def->forwardPlainNames ||
def->forwarders || def->nhosts ||
tests/networkxml2confdata/nat-network-dns-forwarders.xml:6: <dns>
maint.mk: found trailing blank(s)
> Currently supported sub-elements of <code><dns></code> are:
> <dl>
> + <dt><code>forwarder</code></dt>
> + <dd>A <code>dns</code> element can have 0 or more <code>forwarder</code> elements.
> + Each forwarder element defines an IP address to be used as forwarder
> + in DNS server configuration. The addr attribute is required and defines the
> + IP address of every forwarder. <span class="since">Since N/A</span>
s,N/A,1.1.3,
Even if we don't know the next release number, it's always safe to do
current release + 1; if we decide the next release will be renumbered to
1.2.0, we do a search-and-replace at that time; it's easier to replace
actual numbers than to search for N/A.
Also, long lines (it doesn't hurt to wrap to keep within 80 columns);
although you were just copying other long lines in the area.
> +++ b/docs/schemas/network.rng
> @@ -217,6 +217,11 @@
> </attribute>
> </optional>
> <zeroOrMore>
> + <element name="forwarder">
> + <attribute name="addr"><ref name="ipAddr"/></attribute>
> + </element>
> + </zeroOrMore>
> + <zeroOrMore>
> <element name="txt">
Pre-existing, but this is is missing an <interleave>. We prefer
allowing the user to do either:
<dns>
<txt ...>
<forwarder ...>
</dns>
or
<dns>
<forwarder ...>
<txt ...>
</dns>
since there is no logical connection that requires them to come in a
particular order. And while fixing that, I also noticed that the
pre-existing indentation is screwy.
> @@ -1037,8 +1042,9 @@ virNetworkDNSDefParseXML(const char *networkName,
> xmlNodePtr *hostNodes = NULL;
> xmlNodePtr *srvNodes = NULL;
> xmlNodePtr *txtNodes = NULL;
> + xmlNodePtr *fwdNodes = NULL;
> char *forwardPlainNames = NULL;
> - int nhosts, nsrvs, ntxts;
> + int nfwds, nhosts, nsrvs, ntxts;
Unusual ordering; it would be nice to always have fwds after txt,
instead of sometimes before.
> @@ -2282,6 +2314,11 @@ virNetworkDNSDefFormat(virBufferPtr buf,
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 2);
>
> + for (i = 0; i < def->nfwds; i++) {
> + virBufferAsprintf(buf, "<forwarders addr='%s' />\n",
No space before /> in generated output. Also, this outputs
<forwarders>, but the parser and RNG expect <forwarder>. Did you test this?
> + def->forwarders[i]);
Indentation is off.
> +++ 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; i < network->def->dns.nfwds; i++) {
Spaces around '=' in C code.
> +++ b/tests/networkxml2confdata/nat-network-dns-forwarders.xml
> @@ -0,0 +1,12 @@
> +<network>
> + <name>default</name>
> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid>
> + <forward dev='eth0' mode='nat'/>
> + <bridge name='virbr0' stp='on' delay='0' />
No space before />
> + <dns>
> + <forwarder addr='8.8.8.8' />
> + <forwarder addr='8.8.4.4' />
> + </dns>
> + <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);
>
Ah, I see why you missed some of this problems - you didn't enhance
networkxml2xmltest.c. [Ugh - why are we repeating files verbatim between
networkxml2confdata, networkxml2xmlin, and networkxml2xmlout? We should
fix that up in a separate patch to copy how qemu xml tests have been
streamlined to assume out matches in if no out file exists, and to use
confdata for input tests for both xml2xml and xml2conf].
ACK with the following squashed in, so I will push shortly.
diff --git c/docs/formatnetwork.html.in i/docs/formatnetwork.html.in
index feb6895..3d57ac1 100644
--- c/docs/formatnetwork.html.in
+++ i/docs/formatnetwork.html.in
@@ -688,10 +688,12 @@
Currently supported sub-elements of <code><dns></code> are:
<dl>
<dt><code>forwarder</code></dt>
- <dd>A <code>dns</code> element can have 0 or more
<code>forwarder</code> elements.
- Each forwarder element defines an IP address to be used as
forwarder
- in DNS server configuration. The addr attribute is required
and defines the
- IP address of every forwarder. <span class="since">Since
N/A</span>
+ <dd>A <code>dns</code> element can have 0 or
+ more <code>forwarder</code> elements. Each forwarder
+ element defines an IP address to be used as forwarder in
+ DNS server configuration. The addr attribute is required
+ and defines the IP address of every
+ forwarder. <span class="since">Since 1.1.3</span>
</dd>
<dt><code>txt</code></dt>
<dd>A <code>dns</code> element can have 0 or more
<code>txt</code> elements.
diff --git c/docs/schemas/network.rng i/docs/schemas/network.rng
index 4cc3af8..0e7da89 100644
--- c/docs/schemas/network.rng
+++ i/docs/schemas/network.rng
@@ -242,6 +242,7 @@
</choice>
</attribute>
</optional>
+ <interleave>
<zeroOrMore>
<element name="forwarder">
<attribute name="addr"><ref name="ipAddr"/></attribute>
@@ -256,13 +257,21 @@
<zeroOrMore>
<element name="srv">
<attribute name="service"><text/></attribute>
- <attribute name="protocol"><ref
name="protocol"/></attribute>
+ <attribute name="protocol">
+ <ref name="protocol"/>
+ </attribute>
<optional>
<attribute name="domain"><ref
name="dnsName"/></attribute>
<attribute name="target"><text/></attribute>
- <attribute name="port"><ref
name="unsignedShort"/></attribute>
- <attribute name="priority"><ref
name="unsignedShort"/></attribute>
- <attribute name="weight"><ref
name="unsignedShort"/></attribute>
+ <attribute name="port">
+ <ref name="unsignedShort"/>
+ </attribute>
+ <attribute name="priority">
+ <ref name="unsignedShort"/>
+ </attribute>
+ <attribute name="weight">
+ <ref name="unsignedShort"/>
+ </attribute>
</optional>
</element>
</zeroOrMore>
@@ -274,6 +283,7 @@
</oneOrMore>
</element>
</zeroOrMore>
+ </interleave>
</element>
</optional>
<optional>
diff --git c/src/conf/network_conf.c i/src/conf/network_conf.c
index 0ba843b..6968e25 100644
--- c/src/conf/network_conf.c
+++ i/src/conf/network_conf.c
@@ -1044,7 +1044,7 @@ virNetworkDNSDefParseXML(const char *networkName,
xmlNodePtr *txtNodes = NULL;
xmlNodePtr *fwdNodes = NULL;
char *forwardPlainNames = NULL;
- int nfwds, nhosts, nsrvs, ntxts;
+ int nhosts, nsrvs, ntxts, nfwds;
size_t i;
int ret = -1;
xmlNodePtr save = ctxt->node;
@@ -2315,7 +2315,7 @@ virNetworkDNSDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, 2);
for (i = 0; i < def->nfwds; i++) {
- virBufferAsprintf(buf, "<forwarders addr='%s' />\n",
+ virBufferAsprintf(buf, "<forwarder addr='%s'/>\n",
def->forwarders[i]);
}
diff --git c/src/network/bridge_driver.c i/src/network/bridge_driver.c
index a2cfb35..8787bdb 100644
--- c/src/network/bridge_driver.c
+++ i/src/network/bridge_driver.c
@@ -710,7 +710,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
if (network->def->dns.forwarders) {
virBufferAddLit(&configbuf, "no-resolv\n");
- for (i=0; i < network->def->dns.nfwds; i++) {
+ for (i = 0; i < network->def->dns.nfwds; i++) {
virBufferAsprintf(&configbuf, "server=%s\n",
network->def->dns.forwarders[i]);
}
diff --git c/tests/networkxml2confdata/nat-network-dns-forwarders.xml
i/tests/networkxml2confdata/nat-network-dns-forwarders.xml
index eebec97..8fab78e 100644
--- c/tests/networkxml2confdata/nat-network-dns-forwarders.xml
+++ i/tests/networkxml2confdata/nat-network-dns-forwarders.xml
@@ -2,10 +2,10 @@
<name>default</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid>
<forward dev='eth0' mode='nat'/>
- <bridge name='virbr0' stp='on' delay='0' />
+ <bridge name='virbr0' stp='on' delay='0'/>
<dns>
- <forwarder addr='8.8.8.8' />
- <forwarder addr='8.8.4.4' />
+ <forwarder addr='8.8.8.8'/>
+ <forwarder addr='8.8.4.4'/>
</dns>
<ip address='192.168.122.1' netmask='255.255.255.0'>
</ip>
diff --git c/tests/networkxml2xmlin/nat-network-dns-forwarders.xml
i/tests/networkxml2xmlin/nat-network-dns-forwarders.xml
new file mode 100644
index 0000000..eebec97
--- /dev/null
+++ i/tests/networkxml2xmlin/nat-network-dns-forwarders.xml
@@ -0,0 +1,12 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid>
+ <forward dev='eth0' mode='nat'/>
+ <bridge name='virbr0' stp='on' delay='0' />
+ <dns>
+ <forwarder addr='8.8.8.8' />
+ <forwarder addr='8.8.4.4' />
+ </dns>
+ <ip address='192.168.122.1' netmask='255.255.255.0'>
+ </ip>
+</network>
diff --git c/tests/networkxml2xmlout/nat-network-dns-forwarders.xml
i/tests/networkxml2xmlout/nat-network-dns-forwarders.xml
new file mode 100644
index 0000000..930a42a
--- /dev/null
+++ i/tests/networkxml2xmlout/nat-network-dns-forwarders.xml
@@ -0,0 +1,14 @@
+<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>
+ <forwarder addr='8.8.8.8'/>
+ <forwarder addr='8.8.4.4'/>
+ </dns>
+ <ip address='192.168.122.1' netmask='255.255.255.0'>
+ </ip>
+</network>
diff --git c/tests/networkxml2xmltest.c i/tests/networkxml2xmltest.c
index d04039d..c4fca08 100644
--- c/tests/networkxml2xmltest.c
+++ i/tests/networkxml2xmltest.c
@@ -108,6 +108,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-forwarders");
DO_TEST("nat-network-forward-nat-address");
DO_TEST("8021Qbh-net");
DO_TEST("direct-net");
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130917/75f01dc2/attachment-0001.sig>
More information about the libvir-list
mailing list