[libvirt] [PATCH v5 5/5] Network: Add support for DNS hosts definition to the network XML
Laine Stump
laine at laine.org
Thu Jun 23 05:47:46 UTC 2011
On 06/14/2011 09:02 AM, Michal Novotny wrote:
> This commit introduces names definition for the DNS hosts file using
> the following syntax:
>
> <dns>
> <host ip="192.168.1.1">
> <name>alias1</name>
> <name>alias2</name>
> </host>
> </dns>
>
> Signed-off-by: Michal Novotny<minovotn at redhat.com>
> ---
> docs/formatnetwork.html.in | 9 +--
> docs/schemas/network.rng | 8 ++
> src/conf/network_conf.c | 122 +++++++++++++++++++++
> src/conf/network_conf.h | 9 ++
> src/network/bridge_driver.c | 24 +++--
> tests/networkxml2xmlin/nat-network-dns-hosts.xml | 14 +++
> tests/networkxml2xmlout/nat-network-dns-hosts.xml | 14 +++
> tests/networkxml2xmltest.c | 1 +
> 8 files changed, 185 insertions(+), 16 deletions(-)
> 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 a036545..f17cc63 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -224,15 +224,8 @@
> 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>
> +<span class="since">Since 0.9.3</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 c42382e..05f45e5 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -97,6 +97,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>
> +</oneOrMore>
> +</element>
> +</zeroOrMore>
> </element>
> </optional>
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 93e931f..edc9186 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -114,6 +114,13 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def)
> }
> VIR_FREE(def->txtrecords);
> }
> + if (def->nhosts) {
> + while (def->nhosts--) {
> + VIR_FREE(def->hosts[def->nhosts].name);
> + VIR_FREE(def->hosts[def->nhosts].ip);
hosts[x].ip isn't a virSocketAddrPtr, it's a virSocketAddr, so it
doesn't need to be freed (this doesn't even compile).
> + }
> + VIR_FREE(def->hosts);
> + }
> VIR_FREE(def);
> }
> }
> @@ -451,6 +458,53 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
> }
>
> static int
> +virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def,
> + xmlNodePtr node,
> + virSocketAddr ip)
> +{
> + xmlNodePtr cur;
> + int ret = -1;
> +
> + if (def->hosts == NULL) {
> + if (VIR_ALLOC(def->hosts)< 0) {
> + virReportOOMError();
> + goto out;
> + }
> + def->nhosts = 0;
> + }
> +
> + cur = node->children;
> + while (cur != NULL) {
> + if (cur->type == XML_ELEMENT_NODE&&
> + xmlStrEqual(cur->name, BAD_CAST "hostname")) {
> + if (cur->children != NULL) {
> + char *hostname;
> +
> + hostname = strdup((char *)cur->children->content);
> +
> + if (VIR_REALLOC_N(def->hosts, def->nhosts + 1)< 0) {
> + VIR_FREE(hostname);
> + ret = -1;
ret = -1 is unnecessary here - it was initialized to that value and
hasn't been modified.
> + virReportOOMError();
> + goto out;
> + }
> +
> + def->hosts[def->nhosts].name = hostname;
> + def->hosts[def->nhosts].ip = ip;
> + def->nhosts++;
> + }
> + }
> +
> + cur = cur->next;
> + }
> +
> + ret = 0;
> +
> +out:
libvirt prefers to use "error" for this label when it's always used for
error returns.
> + return ret;
> +}
> +
> +static int
> virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
> xmlNodePtr node)
> {
> @@ -494,6 +548,27 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
> def->txtrecords[def->ntxtrecords].name = name;
> def->txtrecords[def->ntxtrecords].value = value;
> def->ntxtrecords++;
> + } else if (cur->type == XML_ELEMENT_NODE&&
> + xmlStrEqual(cur->name, BAD_CAST "host")) {
> + char *ip;
> + virSocketAddr inaddr;
> + memset(&inaddr, 0, sizeof(inaddr));
In some other examples in this file (eg, the bootp "server" attribute),
the address is optional, so inaddr must be initialized. In this case,
though, the IP address is not optional, so it doesn't need to be
initialized (similar to dhcp static host ip's).
> +
> + if (!(ip = virXMLPropString(cur, "ip"))) {
> + cur = cur->next;
> + continue;
> + }
I think the above code shouldn't be there - the ip address is required,
not optional, and the code just below will error out if ip is NULL.
> + if ((ip == NULL) ||
> + (virSocketParseAddr(ip,&inaddr, AF_UNSPEC)< 0)) {
> + virNetworkReportError(VIR_ERR_XML_DETAIL,
> + _("Missing IP address in DNS host definition"));
> + VIR_FREE(ip);
> + goto error;
> + }
> + VIR_FREE(ip);
> + ret = virNetworkDNSHostsDefParseXML(def, cur, inaddr);
You know, if you're going to parse the rest of the host element in a
function, you really should parse the ip attribute in that function too,
rather than sticking it up here.
> + if (ret< 0)
> + goto error;
> }
>
> cur = cur->next;
> @@ -852,6 +927,53 @@ virNetworkDNSDefFormat(virBufferPtr buf,
> def->txtrecords[i].value);
> }
>
> + if (def->nhosts) {
> + int ii, j;
> + char **iplist = NULL;
> + int iplist_size = 0;
> + bool in_list;
> +
> + if (VIR_ALLOC(iplist)< 0) {
> + virReportOOMError();
> + result = -1;
> + goto out;
> + }
> +
> + for (ii = 0 ; ii< def->nhosts; ii++) {
> + char *ip = virSocketFormatAddr(&def->hosts[ii].ip);
> + in_list = false;
> +
> + for (j = 0; j< iplist_size; j++)
> + if (STREQ(iplist[j], ip))
> + in_list = true;
This function is unnecessarily complicated by the duplication of one
piece of data into multiple places (the ip address). I think it would be
better if the data structure exactly mimicked the XML - an array of
structs that each has one IP address and an array of hostnames.
> +
> + if (!in_list) {
> + virBufferAsprintf(buf, "<host ip='%s'>\n", ip);
> +
> + for (j = 0 ; j< def->nhosts; j++) {
> + char *thisip = virSocketFormatAddr(&def->hosts[j].ip);
> + if (STREQ(ip, thisip))
> + virBufferAsprintf(buf, "<hostname>%s</hostname>\n",
> + def->hosts[j].name);
> + }
> + virBufferAsprintf(buf, "</host>\n");
> +
> + if (VIR_REALLOC_N(iplist, iplist_size + 1)< 0) {
> + virReportOOMError();
> + result = -1;
> + goto out;
> + }
> +
> + iplist[iplist_size] = ip;
> + iplist_size++;
> + }
> + }
> +
> + for (j = 0; j< iplist_size; j++)
> + VIR_FREE(iplist[j]);
> + VIR_FREE(iplist);
> + }
> +
> virBufferAddLit(buf, "</dns>\n");
> out:
> return result;
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index d0dac02..50b3713 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;
> +} virNetworkDNSHostsDef;
> +
> +typedef struct virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr;
> +
> struct virNetworkDNSDef {
> unsigned int ntxtrecords;
> + unsigned int nhosts;
> virNetworkDNSTxtRecordsDefPtr txtrecords;
> + virNetworkDNSHostsDefPtr hosts;
The "nXXX" and "XXX" members are usually kept next to each other, rather
than grouping "nXXX", "nYYY", etc. together.
> } virNetworkDNSDef;
>
> typedef struct virNetworkDNSDef *virNetworkDNSDefPtr;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index f55f759..d528875 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -436,6 +436,7 @@ networkShutdown(void) {
>
> static dnsmasqContext*
> networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
> + virNetworkDNSDefPtr dnsdef,
> char *name,
> bool force)
> {
> @@ -448,13 +449,20 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
> goto cleanup;
> }
>
> - if (! force&& virFileExists(dctx->hostsfile->path))
> - return 0;
> + 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 (dnsdef) {
> + for (i = 0; i< dnsdef->nhosts; i++) {
> + virNetworkDNSHostsDefPtr host =&(dnsdef->hosts[i]);
> + if (VIR_SOCKET_HAS_ADDR(&host->ip))
> + dnsmasqAddHost(dctx,&host->ip, host->name);
> + }
> }
>
> if (dnsmasqSave(dctx)< 0)
> @@ -603,7 +611,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
> if (ipdef->nranges || ipdef->nhosts)
> virCommandAddArg(cmd, "--dhcp-no-override");
>
> - if ((dctx = networkSaveDnsmasqHostsfile(ipdef, network->def->name, false))) {
> + if ((dctx = networkSaveDnsmasqHostsfile(ipdef, network->def->dns, network->def->name, false))) {
> if (dctx->hostsfile->nhosts)
> virCommandAddArgPair(cmd, "--dhcp-hostsfile",
> dctx->hostsfile->path);
> @@ -2239,7 +2247,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
> }
> }
> if (ipv4def) {
> - dnsmasqContext* dctx = networkSaveDnsmasqHostsfile(ipv4def, network->def->name, true);
> + dnsmasqContext* dctx = networkSaveDnsmasqHostsfile(ipv4def, network->def->dns, network->def->name, true);
> if (dctx == NULL)
> goto cleanup;
> dnsmasqContextFree(dctx);
> diff --git a/tests/networkxml2xmlin/nat-network-dns-hosts.xml b/tests/networkxml2xmlin/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..9a83fed
> --- /dev/null
> +++ b/tests/networkxml2xmlin/nat-network-dns-hosts.xml
> @@ -0,0 +1,14 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid>
> +<forward dev='eth0' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<dns>
> +<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/networkxml2xmlout/nat-network-dns-hosts.xml b/tests/networkxml2xmlout/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..9a83fed
> --- /dev/null
> +++ b/tests/networkxml2xmlout/nat-network-dns-hosts.xml
> @@ -0,0 +1,14 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid>
> +<forward dev='eth0' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<dns>
> +<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/networkxml2xmltest.c b/tests/networkxml2xmltest.c
> index 2cc8e56..065166d 100644
> --- a/tests/networkxml2xmltest.c
> +++ b/tests/networkxml2xmltest.c
> @@ -87,6 +87,7 @@ mymain(void)
> 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);
> }
I'm attaching a patch to squash in that addresses all the issues I've
noted above *except* changing the hosts def to have one ip address and
multiple hostnames (and associated simplification of the code). Once you
squash in my patch, and fix the data representation I can ACK this patch
too.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-squash-into-5-5.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110623/07cc2866/attachment-0001.ksh>
More information about the libvir-list
mailing list