[libvirt] [PATCH v5 1/5] Add TXT record support for virtual DNS service
Laine Stump
laine at laine.org
Thu Jun 23 05:39:02 UTC 2011
On 06/14/2011 09:02 AM, Michal Novotny wrote:
> This commit introduces the<dns> element and<txt> record for the virtual DNS
> network. The DNS TXT record can be defined using following syntax in the
> network XML file:
>
> <dns>
> <txt name="example" value="example value" />
> </dns>
>
> Also, the Relax-NG scheme has been altered to allow the texts without spaces
> only for the name element and some nitpicks about memory free'ing have been
> fixed by Laine so therefore I'm adding Laine to the SOB clause ;-)
You won't be the first to have called me an SOB :-P
> Signed-off-by: Michal Novotny<minovotn at redhat.com>
> Signed-off-by: Laine Stump<lstump at redhat.com>
> ---
> docs/formatnetwork.html.in | 18 +++
> docs/schemas/network.rng | 20 ++++
> src/conf/network_conf.c | 110 ++++++++++++++++++++
> src/conf/network_conf.h | 16 +++
> src/network/bridge_driver.c | 19 +++-
> .../nat-network-dns-txt-record.xml | 24 +++++
> .../nat-network-dns-txt-record.xml | 24 +++++
> tests/networkxml2xmltest.c | 1 +
> 8 files changed, 231 insertions(+), 1 deletions(-)
> create mode 100644 tests/networkxml2xmlin/nat-network-dns-txt-record.xml
> create mode 100644 tests/networkxml2xmlout/nat-network-dns-txt-record.xml
>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 589aaff..1cf7636 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -114,6 +114,9 @@
> <pre>
> ...
> <mac address='00:16:3E:5D:C7:9E'/>
> +<dns>
> +<txt name="example" value="example value" />
> +</dns>
> <ip address="192.168.122.1" netmask="255.255.255.0">
> <dhcp>
> <range start="192.168.122.100" end="192.168.122.254" />
> @@ -166,6 +169,21 @@
> supported for IPv6 addresses, can only be specified on a single IPv4 address
> per network.
> <span class="since">Since 0.7.1</span>
> +
> +</dd><dt><code>dns</code></dt><dd>
> + The dns element of a network contains configuration information for the
> + virtual network's DNS server.<span class="since">Since 0.9.3</span>
> + Currently supported elements are:
> +</dd>
> +<dt><code>txt</code></dt>
> +<dd>A<code>dns</code> element can have 0 or more<code>txt</code> elements.
> + Each txt element defines a DNS TXT record and has two attributes, both
> + required: a name that can be queried via dns, and a value that will be
> + returned when that name is queried. names cannot contain embedded spaces
> + or commas. value is a single string that can contain multiple values
> + separated by commas.<span class="since">Since 0.9.3</span>
> +</dd>
> +
> </dd><dt><code>dhcp</code></dt><dd>Also within the<code>ip</code> element there is an
You got an extra </dd> in here, which you noticed and fixed in Patch
2/5. Better to just get it right here to begin with...
> optional<code>dhcp</code> element. The presence of this element
> enables DHCP services on the virtual network. It will further
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 6d01b06..c42382e 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -87,6 +87,19 @@
> </element>
> </optional>
>
> +<!-- Define the DNS related elements like TXT records
> + and other features in the<dns> element -->
> +<optional>
> +<element name="dns">
> +<zeroOrMore>
> +<element name="txt">
> +<attribute name="name"><ref name="dns-name"/></attribute>
> +<attribute name="value"><text/></attribute>
> +</element>
> +</zeroOrMore>
> +</element>
> +</optional>
> +
> <!--<ip> element -->
> <zeroOrMore>
> <!-- The IP element sets up NAT'ing and an optional DHCP server
> @@ -183,4 +196,11 @@
> </data>
> </define>
>
> +<!-- a valid DNS name -->
> +<define name='dns-name'>
> +<data type='string'>
> +<param name="pattern">([a-zA-Z\-]+)</param>
> +</data>
> +</define>
> +
> </grammar>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index e4765ea..93e931f 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -104,6 +104,20 @@ static void virNetworkIpDefClear(virNetworkIpDefPtr def)
> VIR_FREE(def->bootfile);
> }
>
> +static void virNetworkDNSDefFree(virNetworkDNSDefPtr def)
> +{
> + if (def) {
> + if (def->txtrecords) {
> + while (def->ntxtrecords--) {
> + VIR_FREE(def->txtrecords[def->ntxtrecords].name);
> + VIR_FREE(def->txtrecords[def->ntxtrecords].value);
> + }
> + VIR_FREE(def->txtrecords);
> + }
> + VIR_FREE(def);
> + }
> +}
> +
> void virNetworkDefFree(virNetworkDefPtr def)
> {
> int ii;
> @@ -121,6 +135,8 @@ void virNetworkDefFree(virNetworkDefPtr def)
> }
> VIR_FREE(def->ips);
>
> + virNetworkDNSDefFree(def->dns);
> +
> VIR_FREE(def);
> }
>
> @@ -435,6 +451,67 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
> }
>
> static int
> +virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef,
> + xmlNodePtr node)
> +{
> + xmlNodePtr cur;
> + int ret = -1;
> + char *name = NULL;
> + char *value = NULL;
> + virNetworkDNSDefPtr def = NULL;
> +
> + if (VIR_ALLOC(def)) {
Oops. I missed this the last time. The accepted practice for error
checking is "< 0", not "!=0" (which is what you're implying here).
> + virReportOOMError();
> + goto error;
> + }
> +
> + cur = node->children;
> + while (cur != NULL) {
> + if (cur->type == XML_ELEMENT_NODE&&
> + xmlStrEqual(cur->name, BAD_CAST "txt")) {
> + if (!(name = virXMLPropString(cur, "name"))) {
> + virNetworkReportError(VIR_ERR_XML_DETAIL,
> + "%s", _("Missing required name attribute in dns txt record"));
> + goto error;
> + }
> + if (!(value = virXMLPropString(cur, "value"))) {
> + virNetworkReportError(VIR_ERR_XML_DETAIL,
> + _("Missing required value attribute in dns txt record '%s'"), name);
> + goto error;
> + }
> +
> + if (strchr(name, ' ') != NULL) {
> + virNetworkReportError(VIR_ERR_XML_DETAIL,
> + _("spaces are not allowed in DNS TXT record names (name is '%s')"), name);
> + goto error;
> + }
> +
> + if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1)< 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + def->txtrecords[def->ntxtrecords].name = name;
> + def->txtrecords[def->ntxtrecords].value = value;
Eww. I just realized that if you were to make one successful trip
through the loop, then hit an error on "name" the next time through,
value would still point to a string that was also pointed to by the
txtrecords array, leading to a double free during the error recovery.
To avoid this, you need to add:
name = NULL;
value = NULL;
right here. (maybe the name=NULL; value=NULL; that was in the wrong
place in the previous version of the patch used to be here, and
accidentally got moved...)
> + def->ntxtrecords++;
> + }
> +
> + cur = cur->next;
> + }
> +
> + ret = 0;
> +error:
> + if (ret< 0) {
> + VIR_FREE(name);
> + VIR_FREE(value);
> + virNetworkDNSDefFree(def);
> + } else {
> + *dnsdef = def;
> + }
> + return ret;
> +}
> +
> +static int
> virNetworkIPParseXML(const char *networkName,
> virNetworkIpDefPtr def,
> xmlNodePtr node,
> @@ -584,6 +661,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> virNetworkDefPtr def;
> char *tmp;
> xmlNodePtr *ipNodes = NULL;
> + xmlNodePtr dnsNode = NULL;
> int nIps;
>
> if (VIR_ALLOC(def)< 0) {
> @@ -641,6 +719,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> def->mac_specified = true;
> }
>
> + dnsNode = virXPathNode("./dns", ctxt);
> + if (dnsNode != NULL) {
> + if (virNetworkDNSDefParseXML(&def->dns, dnsNode)< 0)
> + goto error;
> + }
> +
> nIps = virXPathNodeSet("./ip", ctxt,&ipNodes);
> if (nIps< 0)
> goto error;
> @@ -751,6 +835,29 @@ cleanup:
> }
>
> static int
> +virNetworkDNSDefFormat(virBufferPtr buf,
> + virNetworkDNSDefPtr def)
> +{
> + int result = 0;
> + int i;
> +
> + if (def == NULL)
> + goto out;
> +
> + virBufferAddLit(buf, "<dns>\n");
> +
> + for (i = 0 ; i< def->ntxtrecords ; i++) {
> + virBufferAsprintf(buf, "<txt name='%s' value='%s' />\n",
> + def->txtrecords[i].name,
> + def->txtrecords[i].value);
> + }
> +
> + virBufferAddLit(buf, "</dns>\n");
> +out:
> + return result;
> +}
> +
> +static int
> virNetworkIpDefFormat(virBufferPtr buf,
> const virNetworkIpDefPtr def)
> {
> @@ -881,6 +988,9 @@ char *virNetworkDefFormat(const virNetworkDefPtr def)
> if (def->domain)
> virBufferAsprintf(&buf, "<domain name='%s'/>\n", def->domain);
>
> + if (virNetworkDNSDefFormat(&buf, def->dns)< 0)
> + goto error;
> +
> for (ii = 0; ii< def->nips; ii++) {
> if (virNetworkIpDefFormat(&buf,&def->ips[ii])< 0)
> goto error;
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 281124b..d0dac02 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -57,6 +57,20 @@ struct _virNetworkDHCPHostDef {
> virSocketAddr ip;
> };
>
> +typedef struct _virNetworkDNSTxtRecordsDef virNetworkDNSTxtRecordsDef;
> +typedef virNetworkDNSTxtRecordsDef *virNetworkDNSTxtRecordsDefPtr;
> +struct _virNetworkDNSTxtRecordsDef {
> + char *name;
> + char *value;
> +};
> +
> +struct virNetworkDNSDef {
> + unsigned int ntxtrecords;
> + virNetworkDNSTxtRecordsDefPtr txtrecords;
> +} virNetworkDNSDef;
> +
> +typedef struct virNetworkDNSDef *virNetworkDNSDefPtr;
> +
> typedef struct _virNetworkIpDef virNetworkIpDef;
> typedef virNetworkIpDef *virNetworkIpDefPtr;
> struct _virNetworkIpDef {
> @@ -101,6 +115,8 @@ struct _virNetworkDef {
>
> size_t nips;
> virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */
> +
> + virNetworkDNSDefPtr dns; /* ptr to dns related configuration */
> };
>
> typedef struct _virNetworkObj virNetworkObj;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 5e4b4d9..a2cba05 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -456,7 +456,6 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
> return 0;
> }
>
> -
superfluous change in whitespace.
> static int
> networkBuildDnsmasqArgv(virNetworkObjPtr network,
> virNetworkIpDefPtr ipdef,
> @@ -511,6 +510,24 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
> if (network->def->forwardType == VIR_NETWORK_FORWARD_NONE)
> virCommandAddArg(cmd, "--dhcp-option=3");
>
> + if (network->def->dns != NULL) {
> + virNetworkDNSDefPtr dns = network->def->dns;
> + int i;
> +
> + for (i = 0; i< dns->ntxtrecords; i++) {
> + char *record = NULL;
> + if (virAsprintf(&record, "%s,%s",
> + dns->txtrecords[i].name,
> + dns->txtrecords[i].value)< 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + virCommandAddArgPair(cmd, "--txt-record", record);
> + VIR_FREE(record);
> + }
> + }
> +
> /*
> * --interface does not actually work with dnsmasq< 2.47,
> * due to DAD for ipv6 addresses on the interface.
> diff --git a/tests/networkxml2xmlin/nat-network-dns-txt-record.xml b/tests/networkxml2xmlin/nat-network-dns-txt-record.xml
> new file mode 100644
> index 0000000..bd16976
> --- /dev/null
> +++ b/tests/networkxml2xmlin/nat-network-dns-txt-record.xml
> @@ -0,0 +1,24 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +<forward dev='eth1' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<dns>
> +<txt name='example' value='example value' />
> +</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-txt-record.xml b/tests/networkxml2xmlout/nat-network-dns-txt-record.xml
> new file mode 100644
> index 0000000..bd16976
> --- /dev/null
> +++ b/tests/networkxml2xmlout/nat-network-dns-txt-record.xml
> @@ -0,0 +1,24 @@
> +<network>
> +<name>default</name>
> +<uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> +<forward dev='eth1' mode='nat'/>
> +<bridge name='virbr0' stp='on' delay='0' />
> +<dns>
> +<txt name='example' value='example value' />
> +</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 468785b..2cc8e56 100644
> --- a/tests/networkxml2xmltest.c
> +++ b/tests/networkxml2xmltest.c
> @@ -86,6 +86,7 @@ mymain(void)
> DO_TEST("nat-network");
> DO_TEST("netboot-network");
> DO_TEST("netboot-proxy-network");
> + DO_TEST("nat-network-dns-txt-record");
>
> return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
> }
I'm attaching a patch that fixes the problems I indicated above
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-squash-into-txt-record-1-5.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110623/bfa2b23d/attachment-0001.ksh>
More information about the libvir-list
mailing list