On 08/11/2011 09:22 AM, Michal Novotny wrote:
Hi,
this is the second version of my 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.
Differences between v1 and v2:
- A minor rewrite of integer parsing functionality
- Extend tests to test with both minimal and full set of attributes
- Check for service name length implemented
Signed-off-by: Michal Novotny<minovotn(a)redhat.com>
---
docs/formatnetwork.html.in | 12 ++
docs/schemas/network.rng | 26 ++++
src/conf/network_conf.c | 130 +++++++++++++++++++-
src/conf/network_conf.h | 16 +++
src/network/bridge_driver.c | 43 +++++++
.../nat-network-dns-srv-record-minimal.argv | 1 +
.../nat-network-dns-srv-record-minimal.xml | 26 ++++
.../nat-network-dns-srv-record.argv | 1 +
.../nat-network-dns-srv-record.xml | 26 ++++
tests/networkxml2argvtest.c | 2 +
.../nat-network-dns-srv-record-minimal.xml | 26 ++++
.../nat-network-dns-srv-record.xml | 26 ++++
.../nat-network-dns-srv-record-minimal.xml | 26 ++++
.../nat-network-dns-srv-record.xml | 26 ++++
tests/networkxml2xmltest.c | 2 +
16 files changed, 391 insertions(+), 2 deletions(-)
create mode 100644 tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
create mode 100644 tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.xml
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-minimal.xml
create mode 100644 tests/networkxml2xmlin/nat-network-dns-srv-record.xml
create mode 100644 tests/networkxml2xmlout/nat-network-dns-srv-record-minimal.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 99031d0..51b1581 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -342,6 +342,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>
@@ -390,6 +391,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<code>srv</code> record element defines a
DNS SRV record
+ and has 2 mandatory and 5 optional attributes. The mandatory attributes
+ are service name and protocol (tcp, udp) and the optional attributes are
+ target, port, priority, weight and domain as defined in DNS server SRV
+ RFC (RFC 2782).
A couple of points:
1) reading through the RFC, it calls the domain "name", not "domain".
Should we be following the (in my opinion flawed) naming convention of
the RFC, or the (more correct, but different from the RFC) naming
convention used by dnsmasq? I guess domain, so I'm not asking for a
change, just pointing out the discrepancy.
2) Note that domain is only optional if a domain was specified at the
toplevel of the network. We should pass the toplevel def->domain down
through virNetworkDNSDefParseXML to virNetworkDNSSrvDefParseXML, not to
insert it into the srv object, but just so that we can log an error if
neither the srv nor the toplevel NetworkDef has a domain set.
+<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..dae2799 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -138,6 +138,19 @@
</element>
</zeroOrMore>
<zeroOrMore>
+<element name="srv">
+<attribute name="service"><text/></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>
+</optional>
+</element>
+</zeroOrMore>
+<zeroOrMore>
<element name="host">
<attribute name="ip"><ref
name="ipv4Addr"/></attribute>
<oneOrMore>
@@ -206,6 +219,19 @@
</element>
</define>
+<define name='unsignedShort'>
+<data type='integer'>
+<param name="minInclusive">0</param>
+<param name="maxInclusive">65535</param>
+</data>
+</define>
basictypes.rng has a uint8range and uint24range. a "uint16range" would
be a logical addition, although the existing types allow hexadecimal as
well as decimal. I think it would be very good to 1) have "16" in the
name of the type, rather than "short" (which could mean different things
to different people), and 2) put the type definition in basictypes.rng
so that it could later be used by other rng's. What I don't have an
opinion about is whether it's okay to define "uint16range" that allows
hexadecimal and use that, or define some different type that only allows
decimal. Anybody have an opinion? (if there's no strong opinion, I'd say
use uint16range).
+
+<define name='protocol'>
+<data type='string'>
+<param name='pattern'>(tcp)|(udp)</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..517c4d6 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,15 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def)
}
VIR_FREE(def->hosts);
}
+ if (def->nsrvrecords) {
+ 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->srvrecords);
+ }
Move the VIR_FREE(def->srvrecords past that last }. This way if there is
ever a case where there's a failure somewhere between allocating the
first def->srvrecords and incrementing def->nsrvrecords to 1, it won't
be leaked (with the current code that situation wouldn't occur, but if
you follow my recommendation below for error checking the integer
values, that will change; I just pushed a similar change for txtrecords
and hosts).
VIR_FREE(def);
}
}
@@ -552,8 +561,99 @@ error:
}
static int
+virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def,
+ xmlNodePtr cur,
+ xmlXPathContextPtr ctxt)
+{
+ char *domain;
+ char *service;
+ char *protocol;
+ char *target;
+ int port;
+ int priority;
+ int weight;
+ int ret = 0;
+ char xpath[1024] = { 0 };
+
+ if (!(service = virXMLPropString(cur, "service"))) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ "%s", _("Missing required service attribute
in dns srv record"));
+ goto error;
+ }
+
+ if (strlen(service)> DNS_RECORD_LENGTH_SRV) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ "%s", _("Service name is too long, limit is
%d bytes"), DNS_RECORD_LENGTH_SRV);
The line is already very long, so please put DNS_RECORD_LENGTH_SRV); on
the next line.
+ goto error;
+ }
+
+ if (!(protocol = virXMLPropString(cur, "protocol"))) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ _("Missing required protocol attribute in dns srv
record '%s'"), service);
Same thing here with service);
+ goto error;
+ }
+
+ /* Check whether protocol value is the supported one */
+ if (STRNEQ(protocol, "tcp")&& (STRNEQ(protocol,
"udp"))) {
+ virNetworkReportError(VIR_ERR_XML_DETAIL,
+ _("Invalid protocol attribute value
'%s'"), protocol);
Also move protocol); to the next line.
+ goto error;
+ }
+
+ if (VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + 1)< 0) {
+ virReportOOMError();
+ goto error;
+ }
+
+ def->srvrecords[def->nsrvrecords].service = service;
+ def->srvrecords[def->nsrvrecords].protocol = protocol;
+ def->srvrecords[def->nsrvrecords].domain = NULL;
+ def->srvrecords[def->nsrvrecords].target = NULL;
+ def->srvrecords[def->nsrvrecords].port = 0;
+ def->srvrecords[def->nsrvrecords].priority = 0;
+ def->srvrecords[def->nsrvrecords].weight = 0;
def->srvrecords[def->nsrvrecords].port = -1;
def->srvrecords[def->nsrvrecords].priority = -1;
def->srvrecords[def->nsrvrecords].weight = -1;
You should set the default values for port, priority, and weight to -1,
though, since 0 is a valid value according to the RFC (although it
doesn't really make much sense).
+
+ /* Following attributes are optional but we had to make sure their NULL above */
s/their/they're/
Right, because VIR_REALLOC_N doesn't 0-fill the new memory (it doesn't
know how many there were before).
+ if ((target = virXMLPropString(cur,
"target"))&& (domain = virXMLPropString(cur, "domain"))) {
Why are the integer attributes only checked for if target and domain are
specified? I would recommend first looking for existence of *all* the
attributes, then checking for presence of required attributes based on
what's there (for example, target is optional unless port is specified,
in which case it is required. Likewise, port is required if priority was
specified, and so on)
+ snprintf(xpath, sizeof(xpath),
"string(//network/dns/srv[@service='%s']/@port)", service);
This usage seems flawed. It's acceptable to have multiple records for
the same service: "More than one SRV record for a given service/domain
is allowed, all that match are returned" (from the dnsmasq manpage). So
you might end up with a different node than you were expecting.
Instead, you could follow the example of other users of the virXPath API
by doing something like this (also incorporating a check for invalid
values):
xmlNodePtr save = ctxt->node;
ctxt->node = cur;
int r;
r = virXPathInt("string(./@port)");
if (r == -2) {
virNetworkReportError(VIR_ERR_XML_DETAIL,
_("Invalid protocol attribute value
'%s'"),
protocol);
goto error;
} else if (r == 0) {
def->srvrecords[def->nsrvrecords].port = port;
}
/* if r == -1 then there was no value so leave it set to the sentinal (-1) */
[do similar for priority and weight]
ctxt->node = save;
+ if (virXPathInt(xpath, ctxt,&port))
+ def->srvrecords[def->nsrvrecords].port = port;
+
+ snprintf(xpath, sizeof(xpath),
"string(//network/dns/srv[@service='%s']/@priority)", service);
+ if (virXPathInt(xpath, ctxt,&priority))
+ def->srvrecords[def->nsrvrecords].priority = priority;
+
+ snprintf(xpath, sizeof(xpath),
"string(//network/dns/srv[@service='%s']/@weight)", service);
+ if (virXPathInt(xpath, ctxt,&weight))
+ def->srvrecords[def->nsrvrecords].weight = weight;
+
+ def->srvrecords[def->nsrvrecords].domain = domain;
+ def->srvrecords[def->nsrvrecords].target = target;
+ def->srvrecords[def->nsrvrecords].port = port;
+ def->srvrecords[def->nsrvrecords].priority = priority;
+ def->srvrecords[def->nsrvrecords].weight = weight;
You already set port, priority, and weight into the object up above;
these must be left over from the previous version of the patch.
+ }
+
+ def->nsrvrecords++;
+
+ goto cleanup;
+
+error:
+ VIR_FREE(domain);
+ VIR_FREE(service);
+ VIR_FREE(protocol);
+ VIR_FREE(target);
+
+ ret = 1;
You should be setting ret = -1 on an error.
Actually it's more common in libvirt for ret to be initialized to -1
(assume failure), then set it to 0 just before passing the error or
cleanup label at the bottom of the function. Also, we tend to zero out
pointers when we're finished with them, then unconditionally calling
VIR_FREE() on all the local pointers for *any* exit, whether successful
or not, for example:
int ret = -1;
...
domain = virXMLPropString(cur, "domain");
...
if (some kind of error) {
/* log error */
goto cleanup;
}
def->srvrecords[def->nsrvrecords].domain = domain;
domain = NULL;
...
ret = 0;
cleanup:
VIR_FREE(domain);
...
return ret;
+
+cleanup:
+ return ret;
+}
+
+static int
virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
- xmlNodePtr node)
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
{
xmlNodePtr cur;
int ret = -1;
@@ -598,6 +698,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, ctxt);
+ 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)
@@ -888,7 +993,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
dnsNode = virXPathNode("./dns", ctxt);
if (dnsNode != NULL) {
- if (virNetworkDNSDefParseXML(&def->dns, dnsNode)< 0)
+ if (virNetworkDNSDefParseXML(&def->dns, dnsNode, ctxt)< 0)
goto error;
}
@@ -1147,6 +1252,27 @@ virNetworkDNSDefFormat(virBufferPtr buf,
def->txtrecords[i].value);
}
+ for (i = 0 ; i< def->nsrvrecords ; i++) {
+ if (def->srvrecords[i].service&& def->srvrecords[i].protocol) {
+ virBufferAsprintf(buf, "<srv service='%s'
protocol='%s' ",
+ def->srvrecords[i].service,
+ def->srvrecords[i].protocol);
+
+ if (def->srvrecords[i].domain)
+ virBufferAsprintf(buf, "domain='%s' ",
def->srvrecords[i].domain);
+ if (def->srvrecords[i].target)
+ virBufferAsprintf(buf, "target='%s' ",
def->srvrecords[i].target);
+ if (def->srvrecords[i].port)
+ virBufferAsprintf(buf, "port='%d' ",
def->srvrecords[i].port);
+ if (def->srvrecords[i].priority)
+ virBufferAsprintf(buf, "priority='%d' ",
def->srvrecords[i].priority);
+ if (def->srvrecords[i].weight)
+ virBufferAsprintf(buf, "weight='%d' ",
def->srvrecords[i].weight);
Since 0 is a valid value for these according to the RFC, you should make
their defaults -1 (see above where they're initialized), and check for
-1 here.
+
+ virBufferAsprintf(buf, "/>\n");
+ }
+ }
+
if (def->nhosts) {
int ii, j;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 869085e..5f05a3a 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -24,6 +24,8 @@
#ifndef __NETWORK_CONF_H__
# define __NETWORK_CONF_H__
+#define DNS_RECORD_LENGTH_SRV (512 - 30) /* Limit minus overhead as mentioned in
RFC-2782 */
+
# include<libxml/parser.h>
# include<libxml/tree.h>
# include<libxml/xpath.h>
@@ -67,6 +69,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 +94,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..f4d952f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -559,6 +559,49 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
virCommandAddArgPair(cmd, "--txt-record", record);
VIR_FREE(record);
}
+
+ for (i = 0; i< dns->nsrvrecords; i++) {
+ char *record = NULL;
+ char *recordPort = NULL;
+ char *recordPriority = NULL;
+ char *recordWeight = NULL;
+
+ if (dns->srvrecords[i].service&& dns->srvrecords[i].protocol)
{
+ if (dns->srvrecords[i].port) {
+ if (virAsprintf(&recordPort, "%d",
dns->srvrecords[i].port)< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ }
+ if (dns->srvrecords[i].priority) {
+ if (virAsprintf(&recordPriority, "%d",
dns->srvrecords[i].priority)< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ }
+ if (dns->srvrecords[i].weight) {
+ if (virAsprintf(&recordWeight, "%d",
dns->srvrecords[i].weight)< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ }
+
Why are you separately converting these values into a string with
virAsprintf(), and then inserting them into the final string using %s.
Why not just do %d directly?
+ if (virAsprintf(&record,
"%s.%s.%s,%s,%s,%s,%s",
+ dns->srvrecords[i].service,
+ dns->srvrecords[i].protocol,
+ dns->srvrecords[i].domain ?
dns->srvrecords[i].domain : "",
+ dns->srvrecords[i].target ?
dns->srvrecords[i].target : "",
+ recordPort ? recordPort
: "",
+ recordPriority ? recordPriority
: "",
+ recordWeight ? recordWeight
: "")< 0) {
Aside from that, you need to leave out unspecified parts. It may be
simplest to build the arg with a virBuffer. And don't forget that if the
domain was not given in the srv record, you need to
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ virCommandAddArgPair(cmd, "--srv-host", record);
+ VIR_FREE(record);
+ }
+ }
}
/*
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
new file mode 100644
index 0000000..174f751
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= --except-interface lo
--srv-host=name.tcp.,,,, --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-minimal.xml
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.xml
new file mode 100644
index 0000000..e9b7680
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.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' />
+</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/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' />
+</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..2dd9b7f 100644
--- a/tests/networkxml2argvtest.c
+++ b/tests/networkxml2argvtest.c
@@ -120,6 +120,8 @@ 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-srv-record-minimal");
DO_TEST("nat-network-dns-hosts");
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
diff --git a/tests/networkxml2xmlin/nat-network-dns-srv-record-minimal.xml
b/tests/networkxml2xmlin/nat-network-dns-srv-record-minimal.xml
new file mode 100644
index 0000000..e9b7680
--- /dev/null
+++ b/tests/networkxml2xmlin/nat-network-dns-srv-record-minimal.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' />
+</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/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-minimal.xml
b/tests/networkxml2xmlout/nat-network-dns-srv-record-minimal.xml
new file mode 100644
index 0000000..e9b7680
--- /dev/null
+++ b/tests/networkxml2xmlout/nat-network-dns-srv-record-minimal.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' />
+</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..b9f6546 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -87,6 +87,8 @@ 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-srv-record-minimal");
DO_TEST("nat-network-dns-hosts");
DO_TEST("8021Qbh-net");
DO_TEST("direct-net");