[libvirt] [PATCH v6 0/5] Add TXT record and hosts support for DNS service and dnsmasq command-line regression testing

Hi, this is the 6th (and hopefully the last) version of my patch to introduce the TXT record and DNS hosts support for the DNS service on the virtual network. This can be defined using the txt subelement in the dns element of the network XML description. The definition of TXT record names containing spaces is rejected with the error message that TXT record names in DNS doesn't support spaces. Also, regression testing for the dnsmasq command line has been added to test whether the dnsmasq command-line is correct or not. The new XML definition syntax is: <dns> <txt name="example" value="example value" /> <host ip='192.168.122.1'> <hostname>gateway</hostname> <hostname>host</hostname> </host> </dns> Where multiple host elements can be defined to put the aliases. The <dns> element have to be present on the <ip> level, i.e. one level upper than it was in version 2 of the patch since the definition affects all the IP addresses on the network. The patch series has been tested for the configuration and it was working fine and also RelaxNG schema with the tests have been both altered to add test cases to test those patches. This is the new version for latest libvirt codebase so please review the patchset. Some fixes and code optimizations were done by Laine Stump, so I'm adding him into the SOB clause as well ;-) Please review. Thanks, Michal Signed-off-by: Michal Novotny <minovotn@redhat.com> Signed-off-by: Laine Stump <lstump@redhat.com> Michal Novotny (5): Add TXT record support for virtual DNS service Network: Add regression tests for the command-line arguments Network: Move dnsmasqContext creation to networkSaveDnsmasqHostsfile() Network: Add additional hosts internal infrastructure Network: Add support for DNS hosts definition to the network XML docs/formatnetwork.html.in | 29 ++- docs/schemas/network.rng | 28 ++ src/Makefile.am | 7 +- src/conf/network_conf.c | 206 ++++++++++++++ src/conf/network_conf.h | 26 ++ src/libvirt_network.syms | 6 + src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 123 ++++++--- src/network/bridge_driver.h | 3 + src/util/dnsmasq.c | 285 ++++++++++++++++++-- src/util/dnsmasq.h | 22 ++- tests/Makefile.am | 10 + tests/networkxml2argvdata/isolated-network.argv | 1 + tests/networkxml2argvdata/isolated-network.xml | 11 + .../networkxml2argvdata/nat-network-dns-hosts.argv | 1 + .../networkxml2argvdata/nat-network-dns-hosts.xml | 14 + .../nat-network-dns-txt-record.argv | 1 + .../nat-network-dns-txt-record.xml | 24 ++ tests/networkxml2argvdata/nat-network.argv | 1 + tests/networkxml2argvdata/nat-network.xml | 21 ++ tests/networkxml2argvdata/netboot-network.argv | 1 + tests/networkxml2argvdata/netboot-network.xml | 14 + .../networkxml2argvdata/netboot-proxy-network.argv | 1 + .../networkxml2argvdata/netboot-proxy-network.xml | 13 + tests/networkxml2argvdata/routed-network.argv | 1 + tests/networkxml2argvdata/routed-network.xml | 9 + tests/networkxml2argvtest.c | 109 ++++++++ tests/networkxml2xmlin/nat-network-dns-hosts.xml | 14 + .../nat-network-dns-txt-record.xml | 24 ++ tests/networkxml2xmlout/nat-network-dns-hosts.xml | 14 + .../nat-network-dns-txt-record.xml | 24 ++ tests/networkxml2xmltest.c | 2 + 32 files changed, 990 insertions(+), 56 deletions(-) create mode 100644 src/libvirt_network.syms create mode 100644 tests/networkxml2argvdata/isolated-network.argv create mode 100644 tests/networkxml2argvdata/isolated-network.xml create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.argv create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.xml create mode 100644 tests/networkxml2argvdata/nat-network-dns-txt-record.argv create mode 100644 tests/networkxml2argvdata/nat-network-dns-txt-record.xml create mode 100644 tests/networkxml2argvdata/nat-network.argv create mode 100644 tests/networkxml2argvdata/nat-network.xml create mode 100644 tests/networkxml2argvdata/netboot-network.argv create mode 100644 tests/networkxml2argvdata/netboot-network.xml create mode 100644 tests/networkxml2argvdata/netboot-proxy-network.argv create mode 100644 tests/networkxml2argvdata/netboot-proxy-network.xml create mode 100644 tests/networkxml2argvdata/routed-network.argv create mode 100644 tests/networkxml2argvdata/routed-network.xml create mode 100644 tests/networkxml2argvtest.c create mode 100644 tests/networkxml2xmlin/nat-network-dns-hosts.xml create mode 100644 tests/networkxml2xmlin/nat-network-dns-txt-record.xml create mode 100644 tests/networkxml2xmlout/nat-network-dns-hosts.xml create mode 100644 tests/networkxml2xmlout/nat-network-dns-txt-record.xml -- 1.7.3.2

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 ;-) Signed-off-by: Michal Novotny <minovotn@redhat.com> Signed-off-by: Laine Stump <lstump@redhat.com> --- docs/formatnetwork.html.in | 20 ++++- docs/schemas/network.rng | 20 ++++ src/conf/network_conf.c | 112 ++++++++++++++++++++ src/conf/network_conf.h | 16 +++ src/network/bridge_driver.c | 18 +++ .../nat-network-dns-txt-record.xml | 24 ++++ .../nat-network-dns-txt-record.xml | 24 ++++ tests/networkxml2xmltest.c | 1 + 8 files changed, 234 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..62e1e08 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,7 +169,22 @@ 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>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an + + </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> + <dt><code>dhcp</code></dt> + <dd>Also within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further contain one or more <code>range</code> elements. The 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..d8f1e25 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,69 @@ 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) < 0) { + 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; + def->ntxtrecords++; + name = NULL; + value = NULL; + } + + 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 +663,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) virNetworkDefPtr def; char *tmp; xmlNodePtr *ipNodes = NULL; + xmlNodePtr dnsNode = NULL; int nIps; if (VIR_ALLOC(def) < 0) { @@ -641,6 +721,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 +837,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 +990,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 4b94959..dc143db 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -510,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); } -- 1.7.3.2

On 06/24/2011 06:04 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 ;-)
ACK. I pushed it with one small change to the doc (described below)
Signed-off-by: Michal Novotny<minovotn@redhat.com> Signed-off-by: Laine Stump<lstump@redhat.com> --- docs/formatnetwork.html.in | 20 ++++- docs/schemas/network.rng | 20 ++++ src/conf/network_conf.c | 112 ++++++++++++++++++++ src/conf/network_conf.h | 16 +++ src/network/bridge_driver.c | 18 +++ .../nat-network-dns-txt-record.xml | 24 ++++ .../nat-network-dns-txt-record.xml | 24 ++++ tests/networkxml2xmltest.c | 1 + 8 files changed, 234 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..62e1e08 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,7 +169,22 @@ 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>dhcp</code></dt><dd>Also within the<code>ip</code> element there is an + +</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>
I added a <dl> </dl> to the above paragraph, so it would be obvious that txt is a subelement of <dns>

The regression testing done by comparison of command-line generated from the network XML file and the expected command-line arguments (read from file). Differences between v5 and v6 (this one: - For the tests the pidfile is set to NULL string so there's no --pidfile argument passed to the dnsmasq. Signed-off-by: Michal Novotny <minovotn@redhat.com> --- src/Makefile.am | 7 +- src/libvirt_network.syms | 6 + src/network/bridge_driver.c | 43 ++++++-- src/network/bridge_driver.h | 3 + tests/Makefile.am | 10 ++ tests/networkxml2argvdata/isolated-network.argv | 1 + tests/networkxml2argvdata/isolated-network.xml | 11 ++ .../nat-network-dns-txt-record.argv | 1 + .../nat-network-dns-txt-record.xml | 24 +++++ tests/networkxml2argvdata/nat-network.argv | 1 + tests/networkxml2argvdata/nat-network.xml | 21 ++++ tests/networkxml2argvdata/netboot-network.argv | 1 + tests/networkxml2argvdata/netboot-network.xml | 14 +++ .../networkxml2argvdata/netboot-proxy-network.argv | 1 + .../networkxml2argvdata/netboot-proxy-network.xml | 13 +++ tests/networkxml2argvdata/routed-network.argv | 1 + tests/networkxml2argvdata/routed-network.xml | 9 ++ tests/networkxml2argvtest.c | 108 ++++++++++++++++++++ 18 files changed, 264 insertions(+), 11 deletions(-) create mode 100644 src/libvirt_network.syms create mode 100644 tests/networkxml2argvdata/isolated-network.argv create mode 100644 tests/networkxml2argvdata/isolated-network.xml create mode 100644 tests/networkxml2argvdata/nat-network-dns-txt-record.argv create mode 100644 tests/networkxml2argvdata/nat-network-dns-txt-record.xml create mode 100644 tests/networkxml2argvdata/nat-network.argv create mode 100644 tests/networkxml2argvdata/nat-network.xml create mode 100644 tests/networkxml2argvdata/netboot-network.argv create mode 100644 tests/networkxml2argvdata/netboot-network.xml create mode 100644 tests/networkxml2argvdata/netboot-proxy-network.argv create mode 100644 tests/networkxml2argvdata/netboot-proxy-network.xml create mode 100644 tests/networkxml2argvdata/routed-network.argv create mode 100644 tests/networkxml2argvdata/routed-network.xml create mode 100644 tests/networkxml2argvtest.c diff --git a/src/Makefile.am b/src/Makefile.am index b292f9f..46a9e47 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1090,6 +1090,10 @@ if WITH_XENXS USED_SYM_FILES += libvirt_xenxs.syms endif +if WITH_NETWORK +USED_SYM_FILES += libvirt_network.syms +endif + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ @@ -1100,7 +1104,8 @@ EXTRA_DIST += \ libvirt_daemon.syms \ libvirt_nwfilter.syms \ libvirt_vmx.syms \ - libvirt_xenxs.syms + libvirt_xenxs.syms \ + libvirt_network.syms BUILT_SOURCES += libvirt.syms libvirt.def libvirt_qemu.def diff --git a/src/libvirt_network.syms b/src/libvirt_network.syms new file mode 100644 index 0000000..6be5e45 --- /dev/null +++ b/src/libvirt_network.syms @@ -0,0 +1,6 @@ +# +# Network-specific symbols +# + +# bridge_driver.h +networkBuildDhcpDaemonCommandLine; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index dc143db..9c0423a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -494,7 +494,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (network->def->domain) virCommandAddArgList(cmd, "--domain", network->def->domain, NULL); - virCommandAddArgPair(cmd, "--pid-file", pidfile); + if (pidfile) + virCommandAddArgPair(cmd, "--pid-file", pidfile); /* *no* conf file */ virCommandAddArg(cmd, "--conf-file="); @@ -599,8 +600,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, } if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) == 0) { - virCommandAddArgPair(cmd, "--dhcp-hostsfile", - dctx->hostsfile->path); + if (dctx->hostsfile->nhosts) + virCommandAddArgPair(cmd, "--dhcp-hostsfile", + dctx->hostsfile->path); } dnsmasqContextFree(dctx); } @@ -631,12 +633,12 @@ cleanup: return ret; } -static int -networkStartDhcpDaemon(virNetworkObjPtr network) +int +networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout, + char *pidfile) { virCommandPtr cmd = NULL; - char *pidfile = NULL; - int ret = -1, err, ii; + int ret = -1, ii; virNetworkIpDefPtr ipdef; network->dnsmasqPid = -1; @@ -661,6 +663,28 @@ networkStartDhcpDaemon(virNetworkObjPtr network) if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0)) return 0; + cmd = virCommandNew(DNSMASQ); + if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd) < 0) { + goto cleanup; + } + + if (cmdout) + *cmdout = cmd; + ret = 0; +cleanup: + if (ret < 0) + virCommandFree(cmd); + return ret; +} + +static int +networkStartDhcpDaemon(virNetworkObjPtr network) +{ + virCommandPtr cmd = NULL; + char *pidfile = NULL; + int ret = -1; + int err; + if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), @@ -686,10 +710,9 @@ networkStartDhcpDaemon(virNetworkObjPtr network) goto cleanup; } - cmd = virCommandNew(DNSMASQ); - if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd) < 0) { + ret = networkBuildDhcpDaemonCommandLine(network,&cmd, pidfile); + if (ret< 0) goto cleanup; - } if (virCommandRun(cmd, NULL) < 0) goto cleanup; diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 32d2ae7..8d82b67 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -28,7 +28,10 @@ # include <config.h> # include "internal.h" +# include "network_conf.h" +# include "command.h" int networkRegister(void); +int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout, char *pidfile); #endif /* __VIR_NETWORK__DRIVER_H */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 7ae50a2..40eb71e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -50,6 +50,7 @@ EXTRA_DIST = \ networkschematest \ networkxml2xmlin \ networkxml2xmlout \ + networkxml2argvdata \ nodedevschemadata \ nodedevschematest \ nodeinfodata \ @@ -109,6 +110,8 @@ endif check_PROGRAMS += networkxml2xmltest +check_PROGRAMS += networkxml2argvtest + check_PROGRAMS += nwfilterxml2xmltest check_PROGRAMS += storagevolxml2xmltest storagepoolxml2xmltest @@ -220,6 +223,8 @@ endif TESTS += networkxml2xmltest +TESTS += networkxml2argvtest + TESTS += storagevolxml2xmltest storagepoolxml2xmltest TESTS += nodedevxml2xmltest @@ -347,6 +352,11 @@ networkxml2xmltest_SOURCES = \ testutils.c testutils.h networkxml2xmltest_LDADD = $(LDADDS) +networkxml2argvtest_SOURCES = \ + networkxml2argvtest.c \ + testutils.c testutils.h +networkxml2argvtest_LDADD = ../src/libvirt_driver_network.la $(LDADDS) + nwfilterxml2xmltest_SOURCES = \ nwfilterxml2xmltest.c \ testutils.c testutils.h diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv new file mode 100644 index 0000000..e0993fb --- /dev/null +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= --except-interface lo --dhcp-option=3 --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 \ No newline at end of file diff --git a/tests/networkxml2argvdata/isolated-network.xml b/tests/networkxml2argvdata/isolated-network.xml new file mode 100644 index 0000000..cc320a9 --- /dev/null +++ b/tests/networkxml2argvdata/isolated-network.xml @@ -0,0 +1,11 @@ +<network> + <name>private</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name='virbr2' stp='on' delay='0' /> + <mac address='52:54:00:17:3F:37'/> + <ip address='192.168.152.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.152.2' end='192.168.152.254' /> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv new file mode 100644 index 0000000..c000808 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -0,0 +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 --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 \ No newline at end of file diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.xml b/tests/networkxml2argvdata/nat-network-dns-txt-record.xml new file mode 100644 index 0000000..bd16976 --- /dev/null +++ b/tests/networkxml2argvdata/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/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv new file mode 100644 index 0000000..d58672f --- /dev/null +++ b/tests/networkxml2argvdata/nat-network.argv @@ -0,0 +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 \ No newline at end of file diff --git a/tests/networkxml2argvdata/nat-network.xml b/tests/networkxml2argvdata/nat-network.xml new file mode 100644 index 0000000..eb71d9e --- /dev/null +++ b/tests/networkxml2argvdata/nat-network.xml @@ -0,0 +1,21 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <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/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv new file mode 100644 index 0000000..c5db9e0 --- /dev/null +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -0,0 +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 --enable-tftp --tftp-root /var/lib/tftproot --dhcp-boot pxeboot.img \ No newline at end of file diff --git a/tests/networkxml2argvdata/netboot-network.xml b/tests/networkxml2argvdata/netboot-network.xml new file mode 100644 index 0000000..b8a4d99 --- /dev/null +++ b/tests/networkxml2argvdata/netboot-network.xml @@ -0,0 +1,14 @@ +<network> + <name>netboot</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode='nat'/> + <bridge name='virbr1' stp='off' delay='1' /> + <domain name='example.com'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <tftp root='/var/lib/tftproot' /> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254' /> + <bootp file='pxeboot.img' /> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv new file mode 100644 index 0000000..06e5b08 --- /dev/null +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -0,0 +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 --dhcp-boot pxeboot.img,,10.20.30.40 \ No newline at end of file diff --git a/tests/networkxml2argvdata/netboot-proxy-network.xml b/tests/networkxml2argvdata/netboot-proxy-network.xml new file mode 100644 index 0000000..e11c50b --- /dev/null +++ b/tests/networkxml2argvdata/netboot-proxy-network.xml @@ -0,0 +1,13 @@ +<network> + <name>netboot</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode='nat'/> + <bridge name='virbr1' stp='off' delay='1' /> + <domain name='example.com'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254' /> + <bootp file='pxeboot.img' server='10.20.30.40' /> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2argvdata/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv new file mode 100644 index 0000000..bf5f579 --- /dev/null +++ b/tests/networkxml2argvdata/routed-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= --except-interface lo --listen-address 192.168.122.1 \ No newline at end of file diff --git a/tests/networkxml2argvdata/routed-network.xml b/tests/networkxml2argvdata/routed-network.xml new file mode 100644 index 0000000..3aa8109 --- /dev/null +++ b/tests/networkxml2argvdata/routed-network.xml @@ -0,0 +1,9 @@ +<network> + <name>local</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='route'/> + <bridge name='virbr1' stp='on' delay='0' /> + <mac address='12:34:56:78:9A:BC'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c new file mode 100644 index 0000000..98c84ad --- /dev/null +++ b/tests/networkxml2argvtest.c @@ -0,0 +1,108 @@ +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <string.h> + +#include <sys/types.h> +#include <fcntl.h> + +#include "internal.h" +#include "testutils.h" +#include "network_conf.h" +#include "command.h" +#include "memory.h" +#include "network/bridge_driver.h" + +static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { + char *inXmlData = NULL; + char *outArgvData = NULL; + char *actual = NULL; + int ret = -1; + virNetworkDefPtr dev = NULL; + virNetworkObjPtr obj = NULL; + virCommandPtr cmd = NULL; + char *pidfile = NULL; + + if (virtTestLoadFile(inxml, &inXmlData) < 0) + goto fail; + + if (virtTestLoadFile(outargv, &outArgvData) < 0) + goto fail; + + if (!(dev = virNetworkDefParseString(inXmlData))) + goto fail; + + if (VIR_ALLOC(obj) < 0) + goto fail; + + obj->def = dev; + + if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile) < 0) + goto fail; + + if (!(actual = virCommandToString(cmd))) + goto fail; + + if (STRNEQ(outArgvData, actual)) { + virtTestDifference(stderr, outArgvData, actual); + goto fail; + } + + ret = 0; + + fail: + free(inXmlData); + free(outArgvData); + free(actual); + VIR_FREE(pidfile); + virCommandFree(cmd); + virNetworkObjFree(obj); + return ret; +} + +static int +testCompareXMLToArgvHelper(const void *data) +{ + int result = -1; + char *inxml = NULL; + char *outxml = NULL; + + if (virAsprintf(&inxml, "%s/networkxml2argvdata/%s.xml", + abs_srcdir, (const char*)data) < 0 || + virAsprintf(&outxml, "%s/networkxml2argvdata/%s.argv", + abs_srcdir, (const char*)data) < 0) { + goto cleanup; + } + + result = testCompareXMLToArgvFiles(inxml, outxml); + +cleanup: + free(inxml); + free(outxml); + + return result; +} + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(name) \ + if (virtTestRun("Network XML-2-Argv " name, \ + 1, testCompareXMLToArgvHelper, (name)) < 0) \ + ret = -1 + + DO_TEST("isolated-network"); + DO_TEST("routed-network"); + 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); +} + +VIRT_TEST_MAIN(mymain) -- 1.7.3.2

On 06/24/2011 06:04 AM, Michal Novotny wrote:
The regression testing done by comparison of command-line generated from the network XML file and the expected command-line arguments (read from file).
Differences between v5 and v6 (this one: - For the tests the pidfile is set to NULL string so there's no --pidfile argument passed to the dnsmasq.
ACK (with modification detailed below) and pushed. I combined this patch with Patch 3/5. then split that into a new patch 2/5 that contains just the bridge_driver.[ch] changes (and libvirt_network.syms), and a new 3/5 that contains just the new tests. I verified that the end result after applying both patches was identical to yours, and pushed. Note that the test does still generate an error message if you've turned VIR_TEST_DEBUG on - it is trying to write the hosts file to /var/lib/libvirt/dnsmasq and failing. However, the test still succeeds when this happens, so although it's something we should fix, it shouldn't keep it from going in.

This patch moves the dnsmasqContext creation to the networkSaveDnsmasqHostsfile() function. Signed-off-by: Michal Novotny <minovotn@redhat.com> --- src/network/bridge_driver.c | 37 ++++++++++++++++++++----------------- 1 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9c0423a..e3a1225 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -433,13 +433,20 @@ networkShutdown(void) { } -static int +static dnsmasqContext* networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef, - dnsmasqContext *dctx, + char *name, bool force) { unsigned int i; + dnsmasqContext *dctx = dnsmasqContextNew(name, + DNSMASQ_STATE_DIR); + if (dctx == NULL) { + virReportOOMError(); + goto cleanup; + } + if (! force && virFileExists(dctx->hostsfile->path)) return 0; @@ -450,9 +457,14 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef, } if (dnsmasqSave(dctx) < 0) - return -1; + goto cleanup; - return 0; + return dctx; + +cleanup: + dnsmasqContextFree(dctx); + + return NULL; } @@ -465,6 +477,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int nbleases = 0; int ii; virNetworkIpDefPtr tmpipdef; + dnsmasqContext *dctx = NULL; /* * NB, be careful about syntax for dnsmasq options in long format. @@ -591,19 +604,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (ipdef->nranges || ipdef->nhosts) virCommandAddArg(cmd, "--dhcp-no-override"); - if (ipdef->nhosts > 0) { - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, - DNSMASQ_STATE_DIR); - if (dctx == NULL) { - virReportOOMError(); - goto cleanup; - } - - if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) == 0) { + if ((dctx = networkSaveDnsmasqHostsfile(ipdef, network->def->name, false))) { if (dctx->hostsfile->nhosts) virCommandAddArgPair(cmd, "--dhcp-hostsfile", dctx->hostsfile->path); - } + dnsmasqContextFree(dctx); } @@ -2231,11 +2236,9 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { } } if (ipv4def) { - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); + dnsmasqContext* dctx = networkSaveDnsmasqHostsfile(ipv4def, network->def->name, true); if (dctx == NULL) goto cleanup; - - networkSaveDnsmasqHostsfile(ipv4def, dctx, true); dnsmasqContextFree(dctx); } -- 1.7.3.2

On 06/24/2011 06:04 AM, Michal Novotny wrote:
This patch moves the dnsmasqContext creation to the networkSaveDnsmasqHostsfile() function.
ACK. As noted in the comment to 2/5, I recombined these two patches, and verified the result was identical before pushing.

Signed-off-by: Michal Novotny <minovotn@redhat.com> --- docs/formatnetwork.html.in | 14 ++ src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 3 + src/util/dnsmasq.c | 285 ++++++++++++++++++++++++++++++++++++++++--- src/util/dnsmasq.h | 22 +++- 5 files changed, 305 insertions(+), 20 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 62e1e08..a036545 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -218,6 +218,20 @@ element is used. The BOOTP options currently have to be the same for all address ranges and statically assigned addresses.<span class="since">Since 0.7.1 (<code>server</code> since 0.7.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> + <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> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9d3913..6a62766 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -194,6 +194,7 @@ virUnrefStream; # dnsmasq.h dnsmasqAddDhcpHost; +dnsmasqAddHost; dnsmasqContextFree; dnsmasqContextNew; dnsmasqDelete; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e3a1225..448afbd 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -608,6 +608,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (dctx->hostsfile->nhosts) virCommandAddArgPair(cmd, "--dhcp-hostsfile", dctx->hostsfile->path); + if (dctx->addnhostsfile->nhosts) + virCommandAddArgPair(cmd, "--addn-hosts", + dctx->addnhostsfile->path); dnsmasqContextFree(dctx); } diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c index 2ba9355..04a912a 100644 --- a/src/util/dnsmasq.c +++ b/src/util/dnsmasq.c @@ -48,6 +48,7 @@ #define VIR_FROM_THIS VIR_FROM_NETWORK #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" +#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts" static void dhcphostFree(dnsmasqDhcpHost *host) @@ -56,6 +57,235 @@ dhcphostFree(dnsmasqDhcpHost *host) } static void +addnhostFree(dnsmasqAddnHost *host) +{ + int i; + + for (i = 0; i < host->nhostnames; i++) + VIR_FREE(host->hostnames[i]); + VIR_FREE(host->hostnames); + VIR_FREE(host->ip); +} + +static void +addnhostsFree(dnsmasqAddnHostsfile *addnhostsfile) +{ + unsigned int i; + + if (addnhostsfile->hosts) { + for (i = 0; i < addnhostsfile->nhosts; i++) + addnhostFree(&addnhostsfile->hosts[i]); + + VIR_FREE(addnhostsfile->hosts); + + addnhostsfile->nhosts = 0; + } + + VIR_FREE(addnhostsfile->path); + + VIR_FREE(addnhostsfile); +} + +static int +addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile, + virSocketAddr *ip, + const char *name) +{ + char *ipstr = NULL; + int idx = -1; + int i; + + if (!(ipstr = virSocketFormatAddr(ip))) + return -1; + + for (i = 0; i < addnhostsfile->nhosts; i++) { + if (STREQ((const char *)addnhostsfile->hosts[i].ip, (const char *)ipstr)) { + idx = i; + break; + } + } + + if (idx < 0) { + if (VIR_REALLOC_N(addnhostsfile->hosts, addnhostsfile->nhosts + 1) < 0) + goto alloc_error; + + idx = addnhostsfile->nhosts; + if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames) < 0) + goto alloc_error; + + if (virAsprintf(&addnhostsfile->hosts[idx].ip, "%s", ipstr) < 0) + goto alloc_error; + + addnhostsfile->hosts[idx].nhostnames = 0; + addnhostsfile->nhosts++; + } + + if (VIR_REALLOC_N(addnhostsfile->hosts[idx].hostnames, addnhostsfile->hosts[idx].nhostnames + 1) < 0) + goto alloc_error; + + if (virAsprintf(&addnhostsfile->hosts[idx].hostnames[addnhostsfile->hosts[idx].nhostnames], "%s", name) < 0) + goto alloc_error; + + VIR_FREE(ipstr); + + addnhostsfile->hosts[idx].nhostnames++; + + return 0; + + alloc_error: + virReportOOMError(); + VIR_FREE(ipstr); + return -1; +} + +static dnsmasqAddnHostsfile * +addnhostsNew(const char *name, + const char *config_dir) +{ + int err; + dnsmasqAddnHostsfile *addnhostsfile; + + if (VIR_ALLOC(addnhostsfile) < 0) { + virReportOOMError(); + return NULL; + } + + addnhostsfile->hosts = NULL; + addnhostsfile->nhosts = 0; + + if (virAsprintf(&addnhostsfile->path, "%s/%s.%s", config_dir, name, + DNSMASQ_ADDNHOSTSFILE_SUFFIX) < 0) { + virReportOOMError(); + goto error; + } + + if ((err = virFileMakePath(config_dir))) { + virReportSystemError(err, _("cannot create config directory '%s'"), + config_dir); + goto error; + } + + return addnhostsfile; + + error: + addnhostsFree(addnhostsfile); + return NULL; +} + +static int +addnhostsWrite(const char *path, + dnsmasqAddnHost *hosts, + unsigned int nhosts) +{ + char *tmp; + FILE *f; + bool istmp = true; + unsigned int i, ii; + int rc = 0; + + if (nhosts == 0) + return rc; + + if (virAsprintf(&tmp, "%s.new", path) < 0) + return -ENOMEM; + + if (!(f = fopen(tmp, "w"))) { + istmp = false; + if (!(f = fopen(path, "w"))) { + rc = errno; + goto cleanup; + } + } + + for (i = 0; i < nhosts; i++) { + if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) { + rc = errno; + VIR_FORCE_FCLOSE(f); + + if (istmp) + unlink(tmp); + + goto cleanup; + } + + for (ii = 0; ii < hosts[i].nhostnames; ii++) { + if (fputs(hosts[i].hostnames[ii], f) == EOF || fputc('\t', f) == EOF) { + rc = errno; + VIR_FORCE_FCLOSE(f); + + if (istmp) + unlink(tmp); + + goto cleanup; + } + } + + if (fputc('\n', f) == EOF) { + rc = errno; + VIR_FORCE_FCLOSE(f); + + if (istmp) + unlink(tmp); + + goto cleanup; + } + } + + if (VIR_FCLOSE(f) == EOF) { + rc = errno; + goto cleanup; + } + + if (istmp) { + if (rename(tmp, path) < 0) { + rc = errno; + unlink(tmp); + goto cleanup; + } + + if (unlink(tmp) < 0) { + rc = errno; + goto cleanup; + } + } + + cleanup: + VIR_FREE(tmp); + + return rc; +} + +static int +addnhostsSave(dnsmasqAddnHostsfile *addnhostsfile) +{ + int err = addnhostsWrite(addnhostsfile->path, addnhostsfile->hosts, + addnhostsfile->nhosts); + + if (err < 0) { + virReportSystemError(err, _("cannot write config file '%s'"), + addnhostsfile->path); + return -1; + } + + return 0; +} + +static int +genericFileDelete(char *path) +{ + if (!virFileExists(path)) + return 0; + + if (unlink(path) < 0) { + virReportSystemError(errno, _("cannot remove config file '%s'"), + path); + return -1; + } + + return 0; +} + +static void hostsfileFree(dnsmasqHostsfile *hostsfile) { unsigned int i; @@ -220,21 +450,6 @@ hostsfileSave(dnsmasqHostsfile *hostsfile) return 0; } -static int -hostsfileDelete(dnsmasqHostsfile *hostsfile) -{ - if (!virFileExists(hostsfile->path)) - return 0; - - if (unlink(hostsfile->path) < 0) { - virReportSystemError(errno, _("cannot remove config file '%s'"), - hostsfile->path); - return -1; - } - - return 0; -} - /** * dnsmasqContextNew: * @@ -255,6 +470,8 @@ dnsmasqContextNew(const char *network_name, if (!(ctx->hostsfile = hostsfileNew(network_name, config_dir))) goto error; + if (!(ctx->addnhostsfile = addnhostsNew(network_name, config_dir))) + goto error; return ctx; @@ -277,6 +494,8 @@ dnsmasqContextFree(dnsmasqContext *ctx) if (ctx->hostsfile) hostsfileFree(ctx->hostsfile); + if (ctx->addnhostsfile) + addnhostsFree(ctx->addnhostsfile); VIR_FREE(ctx); } @@ -300,6 +519,24 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx, hostsfileAdd(ctx->hostsfile, mac, ip, name); } +/* + * dnsmasqAddHost: + * @ctx: pointer to the dnsmasq context for each network + * @ip: pointer to the socket address contains ip of the host + * @name: pointer to the string contains hostname of the host + * + * Add additional host entry. + */ + +void +dnsmasqAddHost(dnsmasqContext *ctx, + virSocketAddr *ip, + const char *name) +{ + if (ctx->addnhostsfile) + addnhostsAdd(ctx->addnhostsfile, ip, name); +} + /** * dnsmasqSave: * @ctx: pointer to the dnsmasq context for each network @@ -309,10 +546,16 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx, int dnsmasqSave(const dnsmasqContext *ctx) { + int ret = 0; + if (ctx->hostsfile) - return hostsfileSave(ctx->hostsfile); + ret = hostsfileSave(ctx->hostsfile); + if (ret == 0) { + if (ctx->addnhostsfile) + ret = addnhostsSave(ctx->addnhostsfile); + } - return 0; + return ret; } @@ -325,10 +568,14 @@ dnsmasqSave(const dnsmasqContext *ctx) int dnsmasqDelete(const dnsmasqContext *ctx) { + int ret = 0; + if (ctx->hostsfile) - return hostsfileDelete(ctx->hostsfile); + ret = genericFileDelete(ctx->hostsfile->path); + if (ctx->addnhostsfile) + ret = genericFileDelete(ctx->addnhostsfile->path); - return 0; + return ret; } /** diff --git a/src/util/dnsmasq.h b/src/util/dnsmasq.h index 02a961f..3f6320a 100644 --- a/src/util/dnsmasq.h +++ b/src/util/dnsmasq.h @@ -44,7 +44,24 @@ typedef struct typedef struct { - dnsmasqHostsfile *hostsfile; + unsigned int nhostnames; + char *ip; + char **hostnames; + +} dnsmasqAddnHost; + +typedef struct +{ + unsigned int nhosts; + dnsmasqAddnHost *hosts; + + char *path; /* Absolute path of dnsmasq's hostsfile. */ +} dnsmasqAddnHostsfile; + +typedef struct +{ + dnsmasqHostsfile *hostsfile; + dnsmasqAddnHostsfile *addnhostsfile; } dnsmasqContext; dnsmasqContext * dnsmasqContextNew(const char *network_name, @@ -54,6 +71,9 @@ void dnsmasqAddDhcpHost(dnsmasqContext *ctx, const char *mac, virSocketAddr *ip, const char *name); +void dnsmasqAddHost(dnsmasqContext *ctx, + virSocketAddr *ip, + const char *name); int dnsmasqSave(const dnsmasqContext *ctx); int dnsmasqDelete(const dnsmasqContext *ctx); int dnsmasqReload(pid_t pid); -- 1.7.3.2

On 06/24/2011 06:04 AM, Michal Novotny wrote:
Signed-off-by: Michal Novotny<minovotn@redhat.com> --- docs/formatnetwork.html.in | 14 ++ src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 3 + src/util/dnsmasq.c | 285 ++++++++++++++++++++++++++++++++++++++++--- src/util/dnsmasq.h | 22 +++- 5 files changed, 305 insertions(+), 20 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 62e1e08..a036545 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -218,6 +218,20 @@ element is used. The BOOTP options currently have to be the same for all address ranges and statically assigned addresses.<span class="since">Since 0.7.1 (<code>server</code> since 0.7.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> +<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>
I moved this documentation on the XML into patch 5/5, where the rest of the XML-related stuff is. ACK with that change. I made the change and pushed it.

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> Some of the improvements and fixes were done by Laine Stump so I'm putting him into the SOB clause again ;-) Signed-off-by: Michal Novotny <minovotn@redhat.com> Signed-off-by: Laine Stump <lstump@redhat.com> --- docs/formatnetwork.html.in | 9 +-- docs/schemas/network.rng | 8 ++ src/conf/network_conf.c | 94 ++++++++++++++++++++ src/conf/network_conf.h | 10 ++ src/network/bridge_driver.c | 28 ++++-- .../networkxml2argvdata/nat-network-dns-hosts.argv | 1 + .../networkxml2argvdata/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2argvtest.c | 1 + tests/networkxml2xmlin/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2xmlout/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2xmltest.c | 1 + 11 files changed, 177 insertions(+), 17 deletions(-) create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.argv create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.xml 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 d8f1e25..d0860d8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -114,6 +114,14 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def) } VIR_FREE(def->txtrecords); } + if (def->nhosts) { + while (def->nhosts--) { + while (def->hosts[def->nhosts].nnames--) + VIR_FREE(def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames]); + VIR_FREE(def->hosts[def->nhosts].names); + } + VIR_FREE(def->hosts); + } VIR_FREE(def); } } @@ -451,6 +459,71 @@ virNetworkDHCPRangeDefParseXML(const char *networkName, } static int +virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def, + xmlNodePtr node) +{ + xmlNodePtr cur; + char *ip; + virSocketAddr inaddr; + int ret = -1; + + if (def->hosts == NULL) { + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + goto error; + } + def->nhosts = 0; + } + + if (!(ip = virXMLPropString(node, "ip")) || + (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); + + if (VIR_REALLOC_N(def->hosts, def->nhosts + 1) < 0) { + virReportOOMError(); + goto error; + } + + def->hosts[def->nhosts].ip = inaddr; + def->hosts[def->nhosts].nnames = 0; + + if (VIR_ALLOC(def->hosts[def->nhosts].names) < 0) { + virReportOOMError(); + goto error; + } + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "hostname")) { + if (cur->children != NULL) { + if (VIR_REALLOC_N(def->hosts[def->nhosts].names, def->hosts[def->nhosts].nnames + 1) < 0) { + virReportOOMError(); + goto error; + } + + def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames] = strdup((char *)cur->children->content); + def->hosts[def->nhosts].nnames++; + } + } + + cur = cur->next; + } + + def->nhosts++; + + ret = 0; + +error: + return ret; +} + +static int virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, xmlNodePtr node) { @@ -496,6 +569,11 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, def->ntxtrecords++; name = NULL; value = NULL; + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "host")) { + ret = virNetworkDNSHostsDefParseXML(def, cur); + if (ret < 0) + goto error; } cur = cur->next; @@ -854,6 +932,22 @@ virNetworkDNSDefFormat(virBufferPtr buf, def->txtrecords[i].value); } + if (def->nhosts) { + int ii, j; + + for (ii = 0 ; ii < def->nhosts; ii++) { + char *ip = virSocketFormatAddr(&def->hosts[ii].ip); + + virBufferAsprintf(buf, " <host ip='%s'>\n", ip); + + for (j = 0; j < def->hosts[ii].nnames; j++) + virBufferAsprintf(buf, " <hostname>%s</hostname>\n", + def->hosts[ii].names[j]); + + virBufferAsprintf(buf, " </host>\n"); + } + } + virBufferAddLit(buf, " </dns>\n"); out: return result; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index d0dac02..d7d2951 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -64,9 +64,19 @@ struct _virNetworkDNSTxtRecordsDef { char *value; }; +struct virNetworkDNSHostsDef { + virSocketAddr ip; + int nnames; + char **names; +} virNetworkDNSHostsDef; + +typedef struct virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr; + struct virNetworkDNSDef { unsigned int ntxtrecords; virNetworkDNSTxtRecordsDefPtr txtrecords; + unsigned int nhosts; + virNetworkDNSHostsDefPtr hosts; } virNetworkDNSDef; typedef struct virNetworkDNSDef *virNetworkDNSDefPtr; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 448afbd..1b11132 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -435,10 +435,11 @@ networkShutdown(void) { static dnsmasqContext* networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef, + virNetworkDNSDefPtr dnsdef, char *name, bool force) { - unsigned int i; + unsigned int i, j; dnsmasqContext *dctx = dnsmasqContextNew(name, DNSMASQ_STATE_DIR); @@ -447,13 +448,22 @@ 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)) { + for (j = 0; j < host->nnames; j++) + dnsmasqAddHost(dctx, &host->ip, host->names[j]); + } + } } if (dnsmasqSave(dctx) < 0) @@ -604,7 +614,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 +2249,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/networkxml2argvdata/nat-network-dns-hosts.argv b/tests/networkxml2argvdata/nat-network-dns-hosts.argv new file mode 100644 index 0000000..d240bf4 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= --except-interface lo --listen-address 192.168.122.1 --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts \ No newline at end of file diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.xml b/tests/networkxml2argvdata/nat-network-dns-hosts.xml new file mode 100644 index 0000000..9a83fed --- /dev/null +++ b/tests/networkxml2argvdata/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/networkxml2argvtest.c b/tests/networkxml2argvtest.c index 98c84ad..16d57a9 100644 --- a/tests/networkxml2argvtest.c +++ b/tests/networkxml2argvtest.c @@ -101,6 +101,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); } 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); } -- 1.7.3.2

On Fri, Jun 24, 2011 at 12:04:40PM +0200, 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>
Some of the improvements and fixes were done by Laine Stump so I'm putting him into the SOB clause again ;-)
Signed-off-by: Michal Novotny <minovotn@redhat.com> Signed-off-by: Laine Stump <lstump@redhat.com> [...] + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "hostname")) { + if (cur->children != NULL) { + if (VIR_REALLOC_N(def->hosts[def->nhosts].names, def->hosts[def->nhosts].nnames + 1) < 0) { + virReportOOMError(); + goto error; + } + + def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames] = strdup((char *)cur->children->content); + def->hosts[def->nhosts].nnames++; + } + } + + cur = cur->next; + }
I would suggest to clean this up, as a separate patch to use a XPath nodeset query for hostname, and then use xmlNodeGetContent() to get the content string instead of trying to poke at cur->children->content (which may broke in some weird cases) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/24/2011 06:04 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>
Some of the improvements and fixes were done by Laine Stump so I'm putting him into the SOB clause again ;-)
Signed-off-by: Michal Novotny<minovotn@redhat.com> Signed-off-by: Laine Stump<lstump@redhat.com> --- docs/formatnetwork.html.in | 9 +-- docs/schemas/network.rng | 8 ++ src/conf/network_conf.c | 94 ++++++++++++++++++++ src/conf/network_conf.h | 10 ++ src/network/bridge_driver.c | 28 ++++-- .../networkxml2argvdata/nat-network-dns-hosts.argv | 1 + .../networkxml2argvdata/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2argvtest.c | 1 + tests/networkxml2xmlin/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2xmlout/nat-network-dns-hosts.xml | 14 +++ tests/networkxml2xmltest.c | 1 + 11 files changed, 177 insertions(+), 17 deletions(-) create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.argv create mode 100644 tests/networkxml2argvdata/nat-network-dns-hosts.xml 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>
Somehow you had added this text in twice in Patch 4/5, then removed one copy here. I removed it entirely from 4/5, and added it in once here (inside the indented section I added for <txt>).
+ def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames] = strdup((char *)cur->children->content);
As DV pointed out, we should make this a bit less knowledgeable of the internals of the libxml data structures. But that can be done later in a cleanup patch. ACK (with the change to documentation) and pushed.

On Fri, Jun 24, 2011 at 12:04:35PM +0200, Michal Novotny wrote:
Hi, this is the 6th (and hopefully the last) version of my patch to introduce the TXT record and DNS hosts support for the DNS service on the virtual network. This can be defined using the txt subelement in the dns element of the network XML description. The definition of TXT record names containing spaces is rejected with the error message that TXT record names in DNS doesn't support spaces.
ACK to the patch set, looks clean to me with the small exception of the code used to grab the hostnames, but that should be cleaned up as a separate patch, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard
-
Laine Stump
-
Michal Novotny