[libvirt] [PATCH v4] Implement DNS SRV record into the bridge driver

Hi, this is the fourth version of my SRV record for DNSMasq patch rebased for the current codebase 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. This is basically un-reviewed version 3 of this patch rebased to the current codebase with changed version information to 0.9.9 as it's supposed to go in 0.9.9 or later because of current 0.9.8 features freeze. Also, the second part of this patch is fixing the networkxml2argv test to pass both checks, i.e. both unit tests and also syntax check. Please review, Michal Signed-off-by: Michal Novotny <minovotn@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 +++++++ tests/networkxml2argvdata/isolated-network.argv | 14 ++- .../networkxml2argvdata/nat-network-dns-hosts.argv | 12 ++- .../nat-network-dns-srv-record-minimal.argv | 16 +++ .../nat-network-dns-srv-record-minimal.xml | 26 ++++ .../nat-network-dns-srv-record.argv | 16 +++ .../nat-network-dns-srv-record.xml | 26 ++++ .../nat-network-dns-txt-record.argv | 19 ++- tests/networkxml2argvdata/nat-network.argv | 19 ++- tests/networkxml2argvdata/netboot-network.argv | 17 ++- .../networkxml2argvdata/netboot-proxy-network.argv | 15 ++- tests/networkxml2argvdata/routed-network.argv | 8 +- tests/networkxml2argvtest.c | 14 ++- .../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 ++++ 21 files changed, 501 insertions(+), 32 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 02302fa..79cb4ce 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -345,6 +345,7 @@ <domain name="example.com"/> <dns> <txt name="example" value="example value" /> + <srv service="name" protocol="tcp" domain="test-domain-name" target="." port="1024" priority="10" weight="10"/> <host ip='192.168.122.2'> <hostname>myhost</hostname> <hostname>myhostalias</hostname> @@ -396,6 +397,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). + <span class="since">Since 0.9.9</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 937e180..fac0eda 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> @@ -217,6 +230,19 @@ </element> </define> + <define name='unsignedShort'> + <data type='integer'> + <param name="minInclusive">0</param> + <param name="maxInclusive">65535</param> + </data> + </define> + + <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 1058b07..5b293f9 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -138,6 +138,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); VIR_FREE(def); } } @@ -553,8 +562,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); + goto error; + } + + if (!(protocol = virXMLPropString(cur, "protocol"))) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("Missing required protocol attribute in dns srv record '%s'"), 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); + 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; + + /* Following attributes are optional but we had to make sure their NULL above */ + if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { + snprintf(xpath, sizeof(xpath), "string(//network/dns/srv[@service='%s']/@port)", service); + 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; + } + + def->nsrvrecords++; + + goto cleanup; + +error: + VIR_FREE(domain); + VIR_FREE(service); + VIR_FREE(protocol); + VIR_FREE(target); + + ret = 1; + +cleanup: + return ret; +} + +static int virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, - xmlNodePtr node) + xmlNodePtr node, + xmlXPathContextPtr ctxt) { xmlNodePtr cur; int ret = -1; @@ -599,6 +699,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) @@ -887,7 +992,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; } @@ -1146,6 +1251,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); + + virBufferAsprintf(buf, "/>\n"); + } + } + if (def->nhosts) { int ii, j; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 1be20f8..5ef4878 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> @@ -69,6 +71,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; @@ -82,6 +96,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 63338a2..0c0e7a7 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -527,6 +527,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; + } + } + + 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) { + virReportOOMError(); + goto cleanup; + } + + virCommandAddArgPair(cmd, "--srv-host", record); + VIR_FREE(record); + } + } } /* diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv index 7ea2e94..4936d1b 100644 --- a/tests/networkxml2argvdata/isolated-network.argv +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -1,6 +1,12 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ ---except-interface lo --dhcp-option=3 --no-resolv \ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--conf-file= \ +--except-interface lo \ +--dhcp-option=3 \ +--no-resolv \ --listen-address 192.168.152.1 \ --dhcp-range 192.168.152.2,192.168.152.254 \ ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases --dhcp-lease-max=253 \ ---dhcp-no-override\ +--dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases \ +--dhcp-lease-max=253 \ +--dhcp-no-override diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv b/tests/networkxml2argvdata/nat-network-dns-hosts.argv index 2158df8..bdf8376 100644 --- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv @@ -1,3 +1,9 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ ---conf-file= --except-interface lo --listen-address 192.168.122.1 \ ---expand-hosts --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--domain example.com \ +--conf-file= \ +--except-interface lo \ +--listen-address 192.168.122.1 \ +--expand-hosts \ +--addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts 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..7005078 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv @@ -0,0 +1,16 @@ +/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 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..ebb3942 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv @@ -0,0 +1,16 @@ +/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 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/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv index be6ba4b..5c35aae 100644 --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -1,9 +1,16 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ ---except-interface lo --txt-record=example,example value \ ---listen-address 192.168.122.1 --listen-address 192.168.123.1 \ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--conf-file= \ +--except-interface lo \ +--txt-record=example,example value \ +--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 \ +--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\ +--dhcp-lease-max=253 \ +--dhcp-no-override \ +--dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile diff --git a/tests/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv index d7faee2..e22de4c 100644 --- a/tests/networkxml2argvdata/nat-network.argv +++ b/tests/networkxml2argvdata/nat-network.argv @@ -1,8 +1,15 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ ---except-interface lo --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 \ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--conf-file= \ +--except-interface lo \ +--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\ +--dhcp-lease-max=253 \ +--dhcp-no-override \ +--dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile diff --git a/tests/networkxml2argvdata/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv index 78e873c..bf8ff5e 100644 --- a/tests/networkxml2argvdata/netboot-network.argv +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -1,6 +1,15 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ ---conf-file= --except-interface lo --listen-address 192.168.122.1 \ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--domain example.com \ +--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/netboot.leases \ ---dhcp-lease-max=253 --dhcp-no-override --expand-hosts --enable-tftp \ ---tftp-root /var/lib/tftproot --dhcp-boot pxeboot.img\ +--dhcp-lease-max=253 \ +--dhcp-no-override \ +--expand-hosts \ +--enable-tftp \ +--tftp-root /var/lib/tftproot \ +--dhcp-boot pxeboot.img diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv index 5fe1b8e..29935ea 100644 --- a/tests/networkxml2argvdata/netboot-proxy-network.argv +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -1,6 +1,13 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ ---conf-file= --except-interface lo --listen-address 192.168.122.1 \ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--domain example.com \ +--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/netboot.leases \ ---dhcp-lease-max=253 --dhcp-no-override --expand-hosts \ ---dhcp-boot pxeboot.img,,10.20.30.40\ +--dhcp-lease-max=253 \ +--dhcp-no-override \ +--expand-hosts \ +--dhcp-boot pxeboot.img,,10.20.30.40 diff --git a/tests/networkxml2argvdata/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv index d059630..79c319b 100644 --- a/tests/networkxml2argvdata/routed-network.argv +++ b/tests/networkxml2argvdata/routed-network.argv @@ -1,2 +1,6 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ ---except-interface lo --listen-address 192.168.122.1\ +/usr/sbin/dnsmasq \ +--strict-order \ +--bind-interfaces \ +--conf-file= \ +--except-interface lo \ +--listen-address 192.168.122.1 diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c index 4a11d6f..4223cb7 100644 --- a/tests/networkxml2argvtest.c +++ b/tests/networkxml2argvtest.c @@ -18,6 +18,7 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { char *inXmlData = NULL; char *outArgvData = NULL; + char *actual_tmp = NULL; char *actual = NULL; int ret = -1; virNetworkDefPtr dev = NULL; @@ -47,9 +48,17 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx) < 0) goto fail; - if (!(actual = virCommandToString(cmd))) + if (!(actual_tmp = virCommandToString(cmd))) goto fail; + if (VIR_ALLOC_N( actual, (strlen(actual_tmp) + 3) * sizeof(char) ) < 0) + goto fail; + + /* A little hack to make it working for both check and syntax-check */ + memcpy(actual, actual_tmp, strlen(actual_tmp)); + actual[ strlen(actual_tmp) ] = '\n'; + actual[ strlen(actual_tmp) + 1 ] = 0; + if (STRNEQ(outArgvData, actual)) { virtTestDifference(stderr, outArgvData, actual); goto fail; @@ -61,6 +70,7 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { free(inXmlData); free(outArgvData); free(actual); + free(actual_tmp); VIR_FREE(pidfile); virCommandFree(cmd); virNetworkObjFree(obj); @@ -120,6 +130,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> -- 1.7.7.3

On 12/05/2011 07:03 AM, Michal Novotny wrote:
Hi, this is the fourth version of my SRV record for DNSMasq patch rebased for the current codebase 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. This is basically un-reviewed version 3 of this patch rebased to the current codebase with changed version information to 0.9.9 as it's supposed to go in 0.9.9 or later because of current 0.9.8 features freeze.
Also, the second part of this patch is fixing the networkxml2argv test to pass both checks, i.e. both unit tests and also syntax check.
Please review, Michal
@@ -217,6 +230,19 @@ </element> </define>
+ <define name='unsignedShort'> + <data type='integer'> + <param name="minInclusive">0</param> + <param name="maxInclusive">65535</param> + </data> + </define>
I'm a bit surprised we didn't have a common definition for this before. It looks like docs/schemas/nwfilter.rng could be made shorter if we moved this to a common file between the two, but that can be a separate cleanup.
@@ -553,8 +562,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 };
I'm not a fan of a fixed-width array,...
+ /* Following attributes are optional but we had to make sure their NULL above */
s/their/they're/
+ if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { + snprintf(xpath, sizeof(xpath), "string(//network/dns/srv[@service='%s']/@port)", service);
...along with an unchecked(!) snprintf into that array. It might be better to modify ctxt->node (for example, see virDomainDiskDefParseXML), and construct your query directly relative to the node containing the srv in question.
+error: + VIR_FREE(domain); + VIR_FREE(service); + VIR_FREE(protocol); + VIR_FREE(target); + + ret = 1;
Should this be -1?
@@ -1146,6 +1251,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' ",
Drop the trailing space here...
+ def->srvrecords[i].service, + def->srvrecords[i].protocol); + + if (def->srvrecords[i].domain) + virBufferAsprintf(buf, "domain='%s' ", def->srvrecords[i].domain);
...and use leading space rather than trailing space for each of these...
+ 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); + + virBufferAsprintf(buf, "/>\n");
so that this line does not result in a space prior to the />.
--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\ +--dhcp-lease-max=253 \ +--dhcp-no-override \ +--dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
This changes the file to add a trailing newline, where it used to end without one (thanks to the backslash at the end). This has a negative consequence...
+++ b/tests/networkxml2argvtest.c @@ -18,6 +18,7 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { char *inXmlData = NULL; char *outArgvData = NULL; + char *actual_tmp = NULL; char *actual = NULL; int ret = -1; virNetworkDefPtr dev = NULL; @@ -47,9 +48,17 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx) < 0) goto fail;
- if (!(actual = virCommandToString(cmd))) + if (!(actual_tmp = virCommandToString(cmd))) goto fail;
+ if (VIR_ALLOC_N( actual, (strlen(actual_tmp) + 3) * sizeof(char) ) < 0)
Style. Libvirt code tends not to use spaces before or after nested layers of arguments. This would be: if (VIR_ALLOC_N(actual, (strlen(actual_tmp) + 3) * sizeof(char)) < 0) That said, sizeof(char) is ALWAYS 1, so multiplying by sizeof(char) is useless. And why the +3? Since you are only adding one byte (plus room for trailing NUL), this could be: if (VIR_ALLOC_N(actual, strlen(actual_tmp) + 2) < 0) but see below why I think we can get away without this malloc in the first place.
+ goto fail; + + /* A little hack to make it working for both check and syntax-check */ + memcpy(actual, actual_tmp, strlen(actual_tmp)); + actual[ strlen(actual_tmp) ] = '\n'; + actual[ strlen(actual_tmp) + 1 ] = 0;
...as mentioned above, your change means that there is now a trailing newline in the expected but not in the actual. Do we really need a trailing newline in the expected? And if so, it's more efficient to remove the trailing newline from expected than it is to alloc and memcpy the actual just to add a newline (that is, it's more efficient to hack on expected to remove newline than it is to hack actual to add newline). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Michal Novotny