[libvirt] [PATCH v2 5/5] Network: Add support for DNS hosts definition to the network XML
Laine Stump
laine at laine.org
Wed Apr 27 17:57:53 UTC 2011
On 04/01/2011 06:45 AM, Michal Novotny wrote:
> Make: passed
> Make check: passed
> Make syntax-check: passed
>
> Hi,
> this is the patch to add support for DNS hosts definition in the
> network XML description to generate the the hosts file. This patch
> uses the addnhosts* APIs implemented to the src/util/dnsmasq.c by
> part 2 of this patch series.
>
> Also, tests for the XML to XML definition and command-line
> regression tests has been added.
>
> Michal
Same comments about the commit message - remove the testing comments,
salutation, signature; add in a short example of the XML.
> Signed-off-by: Michal Novotny<minovotn at redhat.com>
> ---
> docs/formatnetwork.html.in | 7 +
> docs/schemas/network.rng | 8 ++
> src/conf/network_conf.c | 128 ++++++++++++++++++--
> src/conf/network_conf.h | 9 ++
> src/network/bridge_driver.c | 20 ++-
> .../networkxml2argvdata/nat-network-dns-hosts.argv | 1 +
> .../networkxml2argvdata/nat-network-dns-hosts.xml | 19 +++
> tests/networkxml2argvtest.c | 1 +
> tests/networkxml2xmlin/nat-network-dns-hosts.xml | 27 ++++
> tests/networkxml2xmlout/nat-network-dns-hosts.xml | 27 ++++
> tests/networkxml2xmltest.c | 1 +
> 11 files changed, 234 insertions(+), 14 deletions(-)
> create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.argv
> create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.xml
> create mode 100644 tests/networkxml2xmlin/nat-network-dns-hosts.xml
> create mode 100644 tests/networkxml2xmlout/nat-network-dns-hosts.xml
>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 5211ed2..5d18ed9 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -222,6 +222,13 @@
> separated by commas.
> <span class="since">Since 0.9.1</span>
> </dd>
> +<dt><code>host</code></dt>
> +<dd>The<code>host</code> element is the definition of DNS hosts to be passed
> + to the DNS service. The IP address is identified by the<code>ip</code> attribute
> + and the names for the IP addresses are identified in the<code>hostname</code>
> + subelements of the<code>host</code> element.
> +<span class="since">Since 0.9.1</span>
> +</dd>
> </dl>
>
> <h2><a name="examples">Example configuration</a></h2>
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index e27dace..05066a5 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -146,6 +146,14 @@
> <attribute name="value"><text/></attribute>
> </element>
> </zeroOrMore>
> +<zeroOrMore>
> +<element name="host">
> +<attribute name="ip"><ref name="ipv4-addr"/></attribute>
> +<oneOrMore>
> +<element name="hostname"><text/></element>
I think it would be better to simply call it "name" rather than
"hostname" (since "host" is already implied by its parent being "<host>")
> +</oneOrMore>
> +</element>
> +</zeroOrMore>
Also, this will move up a level along with the rest of <dns>.
> </element>
> </optional>
> </element>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index b7427d0..1f88649 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -435,6 +435,53 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
> }
>
> static int
> +virNetworkDNSHostsDefParseXML(virNetworkIpDefPtr def,
(will be virNetworkDefPtr def...)
> + xmlNodePtr node,
> + virSocketAddr ip)
> +{
> + xmlNodePtr cur;
> + int result = -1;
> +
> + if (def->dns->hosts == NULL) {
> + if (VIR_ALLOC(def->dns->hosts)< 0)
> + goto oom_error;
> + def->dns->nhosts = 0;
> + }
> +
> + cur = node->children;
> + while (cur != NULL) {
> + if (cur->type == XML_ELEMENT_NODE&&
> + xmlStrEqual(cur->name, BAD_CAST "hostname")) {
Again, s/hostname/name/
> + if (cur->children != NULL) {
> + char *hostname;
> +
> + hostname = strdup((char *)cur->children->content);
> +
> + if (VIR_REALLOC_N(def->dns->hosts, def->dns->nhosts + 1)< 0) {
> + VIR_FREE(hostname);
> + result = -1;
> + goto oom_error;
> + }
> +
> + def->dns->hosts[def->dns->nhosts].name = strdup(hostname);
Rather than strduping hostname, then freeing it, just assign the
original directly into the struct.
> + def->dns->hosts[def->dns->nhosts].ip = ip;
> + def->dns->nhosts++;
> +
> + VIR_FREE(hostname);
> + }
> + }
> +
> + cur = cur->next;
> + }
> +
> + return 0;
> +
> +oom_error:
> + virReportOOMError();
> + return result;
> +}
> +
> +static int
> virNetworkDNSDefParseXML(virNetworkIpDefPtr def,
> xmlNodePtr node)
> {
> @@ -476,6 +523,27 @@ virNetworkDNSDefParseXML(virNetworkIpDefPtr def,
>
> VIR_FREE(name);
> VIR_FREE(value);
> + } else if (cur->type == XML_ELEMENT_NODE&&
> + xmlStrEqual(cur->name, BAD_CAST "host")) {
> + char *ip;
> + virSocketAddr inaddr;
> + memset(&inaddr, 0, sizeof(inaddr));
> +
> + if (!(ip = virXMLPropString(cur, "ip"))) {
> + cur = cur->next;
> + continue;
> + }
> + if ((ip == NULL) ||
> + (virSocketParseAddr(ip,&inaddr, AF_UNSPEC)< 0)) {
> + virNetworkReportError(VIR_ERR_XML_DETAIL,
> + _("Missing IP address in DNS host definition"));
"Missing/incorrect". Also, since def will end up being virNetworkDefPtr,
you can add the name of the network, as well as the string that was
given for ip. That will help in finding the source of the error.
> + VIR_FREE(ip);
> + goto error;
> + }
> + VIR_FREE(ip);
> + result = virNetworkDNSHostsDefParseXML(def, cur, inaddr);
> + if (result)
> + goto error;
> }
>
> cur = cur->next;
> @@ -485,6 +553,7 @@ virNetworkDNSDefParseXML(virNetworkIpDefPtr def,
>
> oom_error:
> virReportOOMError();
> +error:
> return result;
> }
>
> @@ -888,17 +957,60 @@ virNetworkIpDefFormat(virBufferPtr buf,
>
> virBufferAddLit(buf, "</dhcp>\n");
> }
> - if ((def->dns != NULL)&& (def->dns->ntxtrecords)) {
> - int ii;
> -
> + if (def->dns != NULL) {
> virBufferAddLit(buf, "<dns>\n");
> - for (ii = 0 ; ii< def->dns->ntxtrecords ; ii++) {
> - virBufferVSprintf(buf, "<txt-record name='%s' value='%s' />\n",
> - def->dns->txtrecords[ii].name,
> - def->dns->txtrecords[ii].value);
> +
> + if (def->dns->ntxtrecords) {
This if() should have been in the patch for <txt>. That way this patch
wouldn't be polluted with diffs unrelated to the new feature introduced
in this patch.
> + int ii;
> +
> + for (ii = 0 ; ii< def->dns->ntxtrecords; ii++) {
> + virBufferVSprintf(buf, "<txt-record name='%s' value='%s' />\n",
> + def->dns->txtrecords[ii].name,
> + def->dns->txtrecords[ii].value);
> + }
> + }
> + if (def->dns->nhosts) {
> + int ii, j;
> + char **iplist = NULL;
> + int iplist_size = 0;
> + bool in_list;
> +
> + if (VIR_ALLOC(iplist)< 0)
> + goto error;
> +
> + for (ii = 0 ; ii< def->dns->nhosts; ii++) {
> + char *ip = virSocketFormatAddr(&def->dns->hosts[ii].ip);
> + in_list = false;
> + for (j = 0; j< iplist_size; j++)
> + if (STREQ(iplist[j], ip))
> + in_list = true;
> +
> + if (!in_list) {
> + virBufferVSprintf(buf, "<host ip='%s'>\n", ip);
> +
> + for (j = 0 ; j< def->dns->nhosts; j++) {
> + char *thisip = virSocketFormatAddr(&def->dns->hosts[j].ip);
> + if (STREQ(ip, thisip))
> + virBufferVSprintf(buf, "<hostname>%s</hostname>\n",
> + def->dns->hosts[j].name);
> + }
> + virBufferVSprintf(buf, "</host>\n");
> +
> + if (VIR_REALLOC_N(iplist, iplist_size + 1)< 0)
> + goto error;
> +
> + iplist[iplist_size] = strdup(ip);
> + iplist_size++;
> + }
> + }
> +
> + for (j = 0; j< iplist_size; j++)
> + VIR_FREE(iplist[j]);
> + VIR_FREE(iplist);
It would make more sense if virNetworkDNSHostsDef->name was char** and
had an array of names, as you did in PATCH 4/5. That would make the
object a more direct representation of the XML data, and eliminate all
of this complicated code dealing with "iplist" and nested loops.
> }
> +
> virBufferAddLit(buf, "</dns>\n");
> - }
> + }
>
> virBufferAddLit(buf, "</ip>\n");
>
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 5f47595..305ef0f 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -64,9 +64,18 @@ struct _virNetworkDNSTxtRecordsDef {
> char *value;
> };
>
> +struct virNetworkDNSHostsDef {
> + virSocketAddr ip;
> + char *name;
Again - make name char** as you did in PATCH 4/5.
> +} virNetworkDNSHostsDef;
> +
> +typedef struct virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr;
> +
> struct virNetworkDNSDef {
> unsigned int ntxtrecords;
> + unsigned int nhosts;
> virNetworkDNSTxtRecordsDefPtr txtrecords;
> + virNetworkDNSHostsDefPtr hosts;
> } virNetworkDNSDef;
>
> typedef struct virNetworkDNSDef *virNetworkDNSDefPtr;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 4ad3143..99a61b0 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -434,13 +434,20 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
This function is going to also need a virNetworkDefPtr arg if dns moves
up a level.
> goto cleanup;
> }
>
> - if (! force&& virFileExists(dctx->hostsfile->path))
> - return 0;
> + if (!(! force&& virFileExists(dctx->hostsfile->path))) {
That is much easier to understand if written as:
if (force || !virFileExists(dctx->hostsfile->path)
> + for (i = 0; i< ipdef->nhosts; i++) {
> + virNetworkDHCPHostDefPtr host =&(ipdef->hosts[i]);
> + if ((host->mac)&& VIR_SOCKET_HAS_ADDR(&host->ip))
> + dnsmasqAddDhcpHost(dctx, host->mac,&host->ip, host->name);
> + }
> + }
>
> - for (i = 0; i< ipdef->nhosts; i++) {
> - virNetworkDHCPHostDefPtr host =&(ipdef->hosts[i]);
> - if ((host->mac)&& VIR_SOCKET_HAS_ADDR(&host->ip))
> - dnsmasqAddDhcpHost(dctx, host->mac,&host->ip, host->name);
> + if (ipdef->dns) {
> + for (i = 0; i< ipdef->dns->nhosts; i++) {
> + virNetworkDNSHostsDefPtr host =&(ipdef->dns->hosts[i]);
> + if (VIR_SOCKET_HAS_ADDR(&host->ip))
> + dnsmasqAddHost(dctx,&host->ip, host->name);
If you change the virNetworkDNSHostDefPtr to contain char **name instead
of char *name, and dnsmasqAddHost to accept char** rather than char*,
this can remain pretty much unchanged.
> + }
> }
>
> if (dnsmasqSave(dctx)< 0)
> @@ -589,6 +596,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
> if (dctx->hostsfile->nhosts)
> virCommandAddArgPair(cmd, "--dhcp-hostsfile",
> dctx->hostsfile->path);
> + VIR_DEBUG("ADDN HOSTS: %d => %p", dctx->addnhostsfile->nhosts, ipdef->dns);
Did you mean to leave this DEBUG in?
> if (dctx->addnhostsfile->nhosts)
> virCommandAddArgPair(cmd, "--addn-hosts",
> dctx->addnhostsfile->path);
> diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
> new file mode 100644
> index 0000000..99dc724
> --- /dev/null
> +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
> @@ -0,0 +1 @@
> +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --except-interface lo --listen-address 192.168.122.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
> diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.xml b/tests/networkxml2argvdata/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..35ee151
> --- /dev/null
> +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.xml
> @@ -0,0 +1,19 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +<forward dev='eth1' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<ip address='192.168.122.1' netmask='255.255.255.0'>
> +<dhcp>
> +<range start='192.168.122.2' end='192.168.122.254' />
> +<host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10' />
> +<host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11' />
> +</dhcp>
> +<dns>
> +<host ip='192.168.122.1'>
> +<hostname>host</hostname>
> +<hostname>gateway</hostname>
> +</host>
> +</dns>
> +</ip>
> +</network>
> diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c
> index e3a8bb4..ce09206 100644
> --- a/tests/networkxml2argvtest.c
> +++ b/tests/networkxml2argvtest.c
> @@ -112,6 +112,7 @@ mymain(int argc, char **argv)
> DO_TEST("netboot-network");
> DO_TEST("netboot-proxy-network");
> DO_TEST("nat-network-dns-txt-record");
> + DO_TEST("nat-network-dns-hosts");
>
> return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
> }
> diff --git a/tests/networkxml2xmlin/nat-network-dns-hosts.xml b/tests/networkxml2xmlin/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..fe545cf
> --- /dev/null
> +++ b/tests/networkxml2xmlin/nat-network-dns-hosts.xml
> @@ -0,0 +1,27 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +<forward dev='eth1' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<ip address='192.168.122.1' netmask='255.255.255.0'>
> +<dhcp>
> +<range start='192.168.122.2' end='192.168.122.254' />
> +<host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10' />
> +<host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11' />
> +</dhcp>
> +<dns>
> +<host ip='192.168.122.1'>
> +<hostname>host</hostname>
> +<hostname>gateway</hostname>
> +</host>
> +</dns>
> +</ip>
> +<ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'>
> +</ip>
> +<ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'>
> +</ip>
> +<ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'>
> +</ip>
> +<ip family='ipv4' address='10.24.10.1'>
> +</ip>
> +</network>
> diff --git a/tests/networkxml2xmlout/nat-network-dns-hosts.xml b/tests/networkxml2xmlout/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..fe545cf
> --- /dev/null
> +++ b/tests/networkxml2xmlout/nat-network-dns-hosts.xml
> @@ -0,0 +1,27 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +<forward dev='eth1' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<ip address='192.168.122.1' netmask='255.255.255.0'>
> +<dhcp>
> +<range start='192.168.122.2' end='192.168.122.254' />
> +<host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10' />
> +<host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11' />
> +</dhcp>
> +<dns>
> +<host ip='192.168.122.1'>
> +<hostname>host</hostname>
> +<hostname>gateway</hostname>
> +</host>
> +</dns>
> +</ip>
> +<ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'>
> +</ip>
> +<ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'>
> +</ip>
> +<ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'>
> +</ip>
> +<ip family='ipv4' address='10.24.10.1'>
> +</ip>
> +</network>
> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
> index beb00ef..f5c5715 100644
> --- a/tests/networkxml2xmltest.c
> +++ b/tests/networkxml2xmltest.c
> @@ -91,6 +91,7 @@ mymain(int argc, char **argv)
> DO_TEST("netboot-network");
> DO_TEST("netboot-proxy-network");
> DO_TEST("nat-network-dns-txt-record");
> + DO_TEST("nat-network-dns-hosts");
>
> return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
> }
More information about the libvir-list
mailing list