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(a)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.