On Tue, Aug 09, 2011 at 05:50:55PM +0200, Michal Novotny wrote:
Hi,
this is the patch to the bridge driver and libvirt XML
file to include support for the SRV records in the DNS.
The syntax is based on DNSMasq man page and tests for
both xml2xml and xml2argv were added as well.
Signed-off-by: Michal Novotny <minovotn(a)redhat.com>
---
docs/formatnetwork.html.in | 12 ++
docs/schemas/network.rng | 30 +++++
src/conf/network_conf.c | 125 ++++++++++++++++++++
src/conf/network_conf.h | 14 ++
src/network/bridge_driver.c | 18 +++
.../nat-network-dns-srv-record.argv | 1 +
.../nat-network-dns-srv-record.xml | 26 ++++
tests/networkxml2argvtest.c | 1 +
.../nat-network-dns-srv-record.xml | 26 ++++
.../nat-network-dns-srv-record.xml | 26 ++++
tests/networkxml2xmltest.c | 1 +
12 files changed, 281 insertions(+), 1 deletions(-)
create mode 100644 tests/networkxml2argvdata/nat-network-dns-srv-record.argv
create mode 100644 tests/networkxml2argvdata/nat-network-dns-srv-record.xml
create mode 100644 tests/networkxml2xmlin/nat-network-dns-srv-record.xml
create mode 100644 tests/networkxml2xmlout/nat-network-dns-srv-record.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index ddfa77c..4f748c4 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -148,6 +148,7 @@
<mac address='00:16:3E:5D:C7:9E'/>
<dns>
<txt name="example" value="example value"
/>
+ <srv service="name" protocol="tcp"
domain="test-domain-name" target="." port="1024"
priority="10" weight="10"/>
</dns>
<ip address="192.168.122.1"
netmask="255.255.255.0">
<dhcp>
@@ -196,6 +197,17 @@
<span class="since">Since 0.9.3</span>
</dd>
</dl>
+ <dl>
+ <dt><code>srv</code></dt>
+ <dd>The <code>dns</code> element can have also 0 or more
<code>srv</code>
+ record elements. Each srv record element defines a DNS SRV record
+ and has 2 mandatory and 4 optional attributes. The mandatory attributes
+ are service name and protocol (tcp, udp) and you can define optional
+ target, port, priority and weight arguments. The body of the element
+ have to be set to some data and therefore it's a pair tag.
+ <span class="since">Since 0.9.5</span>
+ </dd>
+ </dl>
</dd>
<dt><code>ip</code></dt>
<dd>The <code>address</code> attribute defines an IPv4 address
in
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 1c44471..413698a 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -138,6 +138,22 @@
</element>
</zeroOrMore>
<zeroOrMore>
+ <element name="srv">
+ <attribute
name="service"><text/></attribute>
+ <attribute name="protocol">
+ <choice>
+ <value>tcp</value>
+ <value>udp</value>
+ </choice>
+ </attribute>
+ <attribute
name="domain"><text/></attribute>
+ <attribute
name="target"><text/></attribute>
+ <attribute name="port"><ref
name="port-for-user"/></attribute>
+ <attribute name="priority"><ref
name="short-int"/></attribute>
+ <attribute name="weight"><ref
name="short-int"/></attribute>
+ </element>
+ </zeroOrMore>
+ <zeroOrMore>
<element name="host">
<attribute name="ip"><ref
name="ipv4Addr"/></attribute>
<oneOrMore>
@@ -206,6 +222,20 @@
</element>
</define>
+ <define name="port-for-user">
+ <data type="integer">
+ <param name="minInclusive">1024</param>
+ <param name="maxInclusive">65535</param>
+ </data>
+ </define>
+
+ <define name="short-int">
+ <data type="integer">
+ <param name="minInclusive">0</param>
+ <param name="maxInclusive">127</param>
+ </data>
+ </define>
+
<define name='addr-family'>
<data type='string'>
<param name="pattern">(ipv4)|(ipv6)</param>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b11c482..120b149 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,14 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def)
}
VIR_FREE(def->hosts);
}
+ if (def->nsrvrecords) {
^^^^ please remove the tab above before commit, are you sure you ran
"make syntax-check" ?
+ while (def->nsrvrecords--) {
+ VIR_FREE(def->srvrecords[def->nsrvrecords].domain);
+ VIR_FREE(def->srvrecords[def->nsrvrecords].service);
+ VIR_FREE(def->srvrecords[def->nsrvrecords].protocol);
+ VIR_FREE(def->srvrecords[def->nsrvrecords].target);
+ }
+ }
VIR_FREE(def);
}
}
@@ -552,6 +560,107 @@ error:
}
static int
+virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def,
+ xmlNodePtr cur)
+{
+ char *domain = NULL;
+ char *service = NULL;
Somehow I guess Clang or other tools will complain about the extra
initialization here, but IMHO that's fine. An option would be to move
the def->srvrecords allocation before the fetch of service attribute.
+ char *protocol = NULL;
+ char *target = NULL;
+ char *portString = NULL;
+ char *priorityString = NULL;
+ char *weightString = NULL;
+ int port;
+ int priority;
+ int weight;
+ int ret = 0;
+
+ if (!(service = virXMLPropString(cur, "service"))) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ "%s", _("Missing required service attribute
in dns srv record"));
+ goto error;
+ }
+ if (!(protocol = virXMLPropString(cur, "protocol"))) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ _("Missing required protocol attribute in dns srv
record '%s'"), service);
+ goto error;
+ }
+
+ target = virXMLPropString(cur, "target");
+ domain = virXMLPropString(cur, "domain");
+ portString = virXMLPropString(cur, "port");
+ priorityString = virXMLPropString(cur, "priority");
+ weightString = virXMLPropString(cur, "weight");
+
+ if (VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + 1) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+
+ if (portString &&
+ virStrToLong_i(portString, NULL, 10, &port) < 0) {
+ virNetworkReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse 'port' attribute"));
+ goto error;
+ }
+
+ if (priorityString &&
+ virStrToLong_i(priorityString, NULL, 10, &priority) < 0) {
+ virNetworkReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse 'priority' attribute"));
+ goto error;
+ }
+
+ if (weightString &&
+ virStrToLong_i(weightString, NULL, 10, &weight) < 0) {
+ virNetworkReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse 'weight' attribute"));
+ goto error;
+ }
Hum, using virXPathInt() using the XPath expressions "@port",
"@priority" and "@weight" would probably allow to simplify all this
quite a bit, but you would have to extend virNetworkDNSDefParseXML()
to carry the xpath context from virNetworkDefParseXML() and
update the node for it.
+ def->srvrecords[def->nsrvrecords].domain = domain;
+ def->srvrecords[def->nsrvrecords].service = service;
+ def->srvrecords[def->nsrvrecords].protocol = protocol;
+ def->srvrecords[def->nsrvrecords].target = target;
+ def->srvrecords[def->nsrvrecords].port = port;
+ def->srvrecords[def->nsrvrecords].priority = priority;
+ def->srvrecords[def->nsrvrecords].weight = weight;
Hum, the range values, service and target should all be checked
you need at least the set of minimal check you provided in the rng
to be done in the C parsing.
+ def->nsrvrecords++;
+
+ goto cleanup;
+
+error:
+ if (domain)
+ VIR_FREE(domain);
+ if (service)
+ VIR_FREE(service);
+ if (protocol)
+ VIR_FREE(protocol);
+ if (target)
+ VIR_FREE(target);
+
+ ret = 1;
+
+cleanup:
+ if (portString)
+ VIR_FREE(portString);
+ if (priorityString)
+ VIR_FREE(priorityString);
+ if (weightString)
+ VIR_FREE(weightString);
+
+ domain = NULL;
+ service = NULL;
+ protocol = NULL;
+ target = NULL;
+ portString = NULL;
+ priorityString = NULL;
+ weightString = NULL;
+
+ return ret;
+}
+
+static int
virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
xmlNodePtr node)
{
@@ -598,6 +707,11 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
name = NULL;
value = NULL;
} else if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "srv")) {
+ ret = virNetworkDNSSrvDefParseXML(def, cur);
+ if (ret < 0)
+ goto error;
+ } else if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "host")) {
ret = virNetworkDNSHostsDefParseXML(def, cur);
if (ret < 0)
@@ -1147,6 +1261,17 @@ virNetworkDNSDefFormat(virBufferPtr buf,
def->txtrecords[i].value);
}
+ for (i = 0 ; i < def->nsrvrecords ; i++) {
+ virBufferAsprintf(buf, " <srv service='%s'
protocol='%s' domain='%s' target='%s' port='%d'
priority='%d' weight='%d' />\n",
+ def->srvrecords[i].service,
+ def->srvrecords[i].protocol,
+ def->srvrecords[i].domain,
+ def->srvrecords[i].target,
+ def->srvrecords[i].port,
+ def->srvrecords[i].priority,
+ def->srvrecords[i].weight);
+ }
+
Hum, if attributes are optional is that a good idea to serialize back
the default value when saving the XML, I would think the answer is no.
But that requires to use special values when parsing like -1 or NULL
and check them here. In any case it seems this code would fail and
print for example target="(null)" or crash if target wasn't specified in
the input XML.
if (def->nhosts) {
int ii, j;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 869085e..08e08b2 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -67,6 +67,18 @@ struct _virNetworkDNSTxtRecordsDef {
char *value;
};
+typedef struct _virNetworkDNSSrvRecordsDef virNetworkDNSSrvRecordsDef;
+typedef virNetworkDNSSrvRecordsDef *virNetworkDNSSrvRecordsDefPtr;
+struct _virNetworkDNSSrvRecordsDef {
+ char *domain;
+ char *service;
+ char *protocol;
+ char *target;
+ int port;
+ int priority;
+ int weight;
+};
+
struct _virNetworkDNSHostsDef {
virSocketAddr ip;
int nnames;
@@ -80,6 +92,8 @@ struct _virNetworkDNSDef {
virNetworkDNSTxtRecordsDefPtr txtrecords;
unsigned int nhosts;
virNetworkDNSHostsDefPtr hosts;
+ unsigned int nsrvrecords;
+ virNetworkDNSSrvRecordsDefPtr srvrecords;
};
typedef struct _virNetworkDNSDef *virNetworkDNSDefPtr;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c90db63..e7f3b26 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -559,6 +559,24 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
virCommandAddArgPair(cmd, "--txt-record", record);
VIR_FREE(record);
}
+
+ for (i = 0; i < dns->nsrvrecords; i++) {
+ char *record = NULL;
+ if (virAsprintf(&record, "%s.%s.%s,%s,%d,%d,%d",
+ dns->srvrecords[i].service,
+ dns->srvrecords[i].protocol,
+ dns->srvrecords[i].domain,
+ dns->srvrecords[i].target,
+ dns->srvrecords[i].port,
+ dns->srvrecords[i].priority,
+ dns->srvrecords[i].weight) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
okay, but this fully need to be checked for default values...
+ virCommandAddArgPair(cmd, "--srv-host",
record);
+ VIR_FREE(record);
+ }
}
/*
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
new file mode 100644
index 0000000..2ea9809
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= --except-interface lo
--srv-host=name.tcp.test-domain-name,.,1024,10,10 --listen-address 192.168.122.1
--listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address
2001:db8:ac10:fd01::1 --listen-address 10.24.10.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
\ No newline at end of file
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.xml
b/tests/networkxml2argvdata/nat-network-dns-srv-record.xml
new file mode 100644
index 0000000..4be85b5
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.xml
@@ -0,0 +1,26 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+ <forward dev='eth1' mode='nat'>
+ <interface dev='eth1'/>
+ </forward>
+ <bridge name='virbr0' stp='on' delay='0' />
+ <dns>
+ <srv service='name' protocol='tcp'
domain='test-domain-name' target='.' port='1024'
priority='10' weight='10' />
need to add more SRV records to test for optional attributes handling
+ </dns>
+ <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>
+ </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/networkxml2argvtest.c b/tests/networkxml2argvtest.c
index 4a11d6f..a9da613 100644
--- a/tests/networkxml2argvtest.c
+++ b/tests/networkxml2argvtest.c
@@ -120,6 +120,7 @@ mymain(void)
DO_TEST("netboot-network");
DO_TEST("netboot-proxy-network");
DO_TEST("nat-network-dns-txt-record");
+ DO_TEST("nat-network-dns-srv-record");
DO_TEST("nat-network-dns-hosts");
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
diff --git a/tests/networkxml2xmlin/nat-network-dns-srv-record.xml
b/tests/networkxml2xmlin/nat-network-dns-srv-record.xml
new file mode 100644
index 0000000..4be85b5
--- /dev/null
+++ b/tests/networkxml2xmlin/nat-network-dns-srv-record.xml
@@ -0,0 +1,26 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+ <forward dev='eth1' mode='nat'>
+ <interface dev='eth1'/>
+ </forward>
+ <bridge name='virbr0' stp='on' delay='0' />
+ <dns>
+ <srv service='name' protocol='tcp'
domain='test-domain-name' target='.' port='1024'
priority='10' weight='10' />
+ </dns>
+ <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>
+ </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-srv-record.xml
b/tests/networkxml2xmlout/nat-network-dns-srv-record.xml
new file mode 100644
index 0000000..4be85b5
--- /dev/null
+++ b/tests/networkxml2xmlout/nat-network-dns-srv-record.xml
@@ -0,0 +1,26 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+ <forward dev='eth1' mode='nat'>
+ <interface dev='eth1'/>
+ </forward>
+ <bridge name='virbr0' stp='on' delay='0' />
+ <dns>
+ <srv service='name' protocol='tcp'
domain='test-domain-name' target='.' port='1024'
priority='10' weight='10' />
+ </dns>
+ <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>
+ </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 5cdbedb..8ee8e0e 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-srv-record");
DO_TEST("nat-network-dns-hosts");
DO_TEST("8021Qbh-net");
DO_TEST("direct-net");
I like the principle and directions of the patch but there is some
needed checks and cleanup before pushing it :-)
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/