On 06/14/2011 09:28 AM, Daniel Veillard wrote:

On Mon, Jun 13, 2011 at 06:55:02PM +0200, 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>

Signed-off-by: Michal Novotny <minovotn@redhat.com> --- docs/formatnetwork.html.in | 19 ++++ docs/schemas/network.rng | 13 +++ src/conf/network_conf.c | 97 ++++++++++++++++++++ src/conf/network_conf.h | 16 +++ src/network/bridge_driver.c | 21 ++++- .../nat-network-dns-txt-record.xml | 24 +++++ .../nat-network-dns-txt-record.xml | 24 +++++ tests/networkxml2xmltest.c | 1 + 8 files changed, 214 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/schemas/network.rng b/docs/schemas/network.rng index 6d01b06..3780af5 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"><text/></attribute> Hum, we should probably restrict the name attribute value space to something like [a-zA-Z0-9-] , spaces are definitely forbidden here.

Yes, that's right. We should forbid spaces there. Michal -- Michal Novotny <minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat

I've attached a diff at the end that will address all the problems I've brought up in my review. I can either squash that in and push the result (once the rest of the series is reviewed and ready), or Michal can squash it into his local tree and resubmit. On 06/13/2011 12:55 PM, 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>

Signed-off-by: Michal Novotny<minovotn@redhat.com> --- docs/formatnetwork.html.in | 19 ++++ docs/schemas/network.rng | 13 +++ src/conf/network_conf.c | 97 ++++++++++++++++++++ src/conf/network_conf.h | 16 +++ src/network/bridge_driver.c | 21 ++++- .../nat-network-dns-txt-record.xml | 24 +++++ .../nat-network-dns-txt-record.xml | 24 +++++ tests/networkxml2xmltest.c | 1 + 8 files changed, 214 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..008897d 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,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>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.1</span>

s/0.9.1/0.9.3/

+ 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 which are + comma-separated which is allowed for the TXT records and it is represented a + single value.<span class="since">Since 0.9.1</span>

"a single string which can contain multiple values separated by commas." seems more concise. and again, s/0.9.1/0.9.3/

+</dd> + </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 diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 6d01b06..3780af5 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"><text/></attribute>

I see that Daniel recommends restricting this to A-Z, a-z, and -. I couldn't figure out from RFC 1035 if that's really what's intended (and am not sure if there's another later RFC that amends 1035). But since this is currently only used for tests, I guess it's okay to be too restrictive; we can always loosen it up later if we need to.

+<attribute name="value"><text/></attribute> +</element> +</zeroOrMore> +</element> +</optional> + <!--<ip> element --> <zeroOrMore> <!-- The IP element sets up NAT'ing and an optional DHCP server diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index e4765ea..133ac84 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c

There's no code here to critique, because you totally left it out - you aren't free'ing the virNetworkDNSDef when you free the virNetworkDef.

@@ -435,6 +435,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)) { + 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"))) { + cur = cur->next; + continue; + } + if (!(value = virXMLPropString(cur, "value"))) { + VIR_FREE(name); + cur = cur->next; + continue; + }

These should be errors rather than silently ignoring the record.

+ + if (strchr(name, ' ') != NULL) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("TXT record names in DNS don't support spaces in 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++; + } + + cur = cur->next; + } + + ret = 0; +error: + if (ret< 0) { + name = NULL; + value = NULL;

Uh, I don't think you want those assignments there :-)

+ VIR_FREE(name); + VIR_FREE(value);

Also you need to free def here, since you're not returning it.

+ } + else + if (dnsdef != NULL) + *dnsdef = def;

This isn't proper style for libvirt. The "}", "else", and "if" should be on the same line, and the else clause needs to have brackets around it (since the if clause has brackets). Also, dnsdef had *better* be non-NULL, or 1) you'll be leaking the virNetworkDNSDef, and 2) it would be pointless to call this function, since the whole purpose is to return the new DNSDef. Since you know it will always be non-NULL, we can just leave the if condition out of the else.

+ + return ret; +} + +static int virNetworkIPParseXML(const char *networkName, virNetworkIpDefPtr def, xmlNodePtr node, @@ -584,6 +647,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) virNetworkDefPtr def; char *tmp; xmlNodePtr *ipNodes = NULL; + xmlNodePtr dnsNode = NULL; int nIps;

if (VIR_ALLOC(def)< 0) { @@ -641,6 +705,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; @@ -695,6 +765,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) error: virNetworkDefFree(def); VIR_FREE(ipNodes); + VIR_FREE(dnsNode);

virXPathNode doesn't allocate anything, it just returns an existing pointer, so dnsNode shouldn't be freed.

return NULL; }

@@ -751,6 +822,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 +975,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..169bc08 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -456,7 +456,6 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef, return 0; }

- static int networkBuildDnsmasqArgv(virNetworkObjPtr network, virNetworkIpDefPtr ipdef, @@ -511,6 +510,26 @@ 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++) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + virBufferAsprintf(&buf, "%s,%s", + dns->txtrecords[i].name, + dns->txtrecords[i].value);

I can't see a reason for using virBufferAsprintf() here - since you have a small, fixed number of args, virAsprintf() would do just as well.

+ + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + + virCommandAddArgPair(cmd, "--txt-record", virBufferContentAndReset(&buf)); + virBufferFreeAndReset(&buf);

This is redundant.

+ } + } + /* * --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); }

[snip]

...

--- /dev/null +++ b/tests/networkxml2argvtest.c @@ -0,0 +1,99 @@ +#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(actual); + VIR_FREE(pidfile); + virCommandFree(cmd); + virNetworkObjFree(obj); + return ret; +} + +static int testCompareXMLToArgvHelper(const void *data) { + char inxml[PATH_MAX]; + char outargv[PATH_MAX]; + snprintf(inxml, PATH_MAX, "%s/networkxml2argvdata/%s.xml", + abs_srcdir, (const char*)data); + snprintf(outargv, PATH_MAX, "%s/networkxml2argvdata/%s.argv", + abs_srcdir, (const char*)data); + return testCompareXMLToArgvFiles(inxml, outargv); +} + + +static int +mymain(void) +{ + int ret = 0; + char cwd[PATH_MAX]; + + abs_srcdir = getenv("abs_srcdir"); + if (!abs_srcdir) + abs_srcdir = getcwd(cwd, sizeof(cwd)); + +#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) I like the idea of adding more tests but in my case this is crashing with a memory error. Running under valgrind I get:

paphio:~/libvirt/tests -> valgrind ./networkxml2argvtest TEST: networkxml2argvtest ..==1582== Source and destination overlap in memcpy(0x7feffc660, 0x7feffc660, 35) ==1582== at 0x4A073BA: memcpy (mc_replace_strmem.c:602) ==1582== by 0x4FC4F0: safe_copy (strerror_r.c:103) ==1582== by 0x4FC5FC: rpl_strerror_r (strerror_r.c:155) ==1582== by 0x4256A6: virStrerror (virterror.c:1239) ==1582== by 0x42577A: virReportSystemErrorFull (virterror.c:1268) ==1582== by 0x417C1E: hostsfileSave (dnsmasq.c:215) ==1582== by 0x417EB8: dnsmasqSave (dnsmasq.c:313) ==1582== by 0x409A2E: networkSaveDnsmasqHostsfile (bridge_driver.c:453) ==1582== by 0x409FE1: networkBuildDnsmasqArgv (bridge_driver.c:603) ==1582== by 0x40A23C: networkBuildDhcpDaemonCommandLine (bridge_driver.c:667) ==1582== by 0x40765E: testCompareXMLToArgvFiles (networkxml2argvtest.c:42) ==1582== by 0x4077AC: testCompareXMLToArgvHelper (networkxml2argvtest.c:70) ==1582== by 0x407D0F: virtTestRun (testutils.c:134) ==1582== by 0x407885: mymain (networkxml2argvtest.c:91) ==1582== by 0x408ACA: virtTestMain (testutils.c:619) ==1582== by 0x40795F: main (networkxml2argvtest.c:99) ==1582== !..!==1582== Invalid free() / delete / delete[] ==1582== at 0x4A05187: free (vg_replace_malloc.c:325) ==1582== by 0x41C45E: virFree (memory.c:310) ==1582== by 0x408ADD: virtTestMain (testutils.c:623) ==1582== by 0x40795F: main (networkxml2argvtest.c:99) ==1582== Address 0x7feffee50 is not stack'd, malloc'd or (recently) free'd ==1582== 6 FAIL paphio:~/libvirt/tests ->

I didn't run this under the valgrind however no tests were failing for me. I guess I need to run under the valgrind and it may be connected to the fact all of the tests were rewritten since the time of the previous version of my patch so this may be connected however this was working fine on my F-14 i386 box so I don't really know what's wrong on your setup. I need to investigate it further.

So two of the tests are failing here, and each of them seems to lead to a different issue for valgrind. The basic problem is that we seems to end up in hostsfileSave() within dnsmasq.c with a system path:

Breakpoint 1, hostsfileSave (hostsfile=0x865260) at util/dnsmasq.c:210 210 { (gdb) p *hostsfile $2 = {nhosts = 2, hosts = 0x862690, path = 0x862b40 "/var/lib/libvirt/dnsmasq/default.hostsfile"} (gdb) n 211 int err = hostsfileWrite(hostsfile->path, hostsfile->hosts, (gdb) n 214 if (err < 0) { (gdb) p err $3 = -13 (gdb)

and then in the end abs_srcdir_cleanup is set but when doing VIR_FREE(abs_srcdir); in testutils.c:622 it explodes due to an invalid address...

I'm a bit puzzled about what's ahppening in the first error, so we call virReportSystemErrorFull, normal, then first thing it does is

char strerror_buf[1024];

const char *errnoDetail = virStrerror(theerrno, strerror_buf, sizeof(strerror_buf));

which then jump into strerror_r and other code from gnulib I guess and that fails with valgrind. There is something fishy here, but what exactly ?!?

Daniel

Well, don't know exactly however I did to rewrite this for the latest codebase since test infrastructure changed since last version of my patch. However the strange thing is that it's working fine on my F-14 i386 box. Need to investigate this further. Michal -- Michal Novotny <minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat

On 06/13/2011 12:55 PM, 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).

For the tests the pidfile should be set to NULL string, i.e. (null) since no real invocation of the command is being done, just the command-line is being generated and compared to the expected one.

Signed-off-by: Michal Novotny<minovotn@redhat.com> --- src/Makefile.am | 11 ++- src/libvirt_network.syms | 6 + src/network/bridge_driver.c | 36 ++++++-- 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 | 99 ++++++++++++++++++++ 18 files changed, 255 insertions(+), 8 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 3612a24..a349f09 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -530,6 +530,10 @@ libvirt_driver_la_LIBADD = $(NUMACTL_LIBS) $(GNUTLS_LIBS)

USED_SYM_FILES = libvirt_private.syms

+if WITH_NETWORK +USED_SYM_FILES += libvirt_network.syms +endif +

This is an extra, isn't it (you've also done it below)?

if WITH_TEST if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_test.la @@ -1089,6 +1093,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 \ @@ -1099,7 +1107,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 169bc08..2726a5b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -633,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; @@ -663,6 +663,29 @@ 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"), @@ -688,10 +711,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..dce6034 --- /dev/null +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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..96fde34 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --txt-record=example,example value --listen-address 192.168.122.1 --listen-address 192.168.123.1 --listen-

Every time I see your example (with "example value"), I pause for a minute and try to think of how this won't eventually lead to problems. Should we be putting quotes around this? Or escaping the space, or something?

address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \ No newline at end of file diff --git a/tests/networkxml2argvdata/nat-network-dns-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..ccffa67 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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 \ 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..565c41b --- /dev/null +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=(null) --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..019367d --- /dev/null +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=(null) --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..2b51d90 --- /dev/null +++ b/tests/networkxml2argvdata/routed-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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..5e7f455 --- /dev/null +++ b/tests/networkxml2argvtest.c @@ -0,0 +1,99 @@ +#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)

Standard practice in libvirt code is to check "< 0" rather than just "!= 0".

+ goto fail; + + if (!(actual = virCommandToString(cmd))) + goto fail; + + if (STRNEQ(outArgvData, actual)) { + virtTestDifference(stderr, outArgvData, actual); + goto fail; + } + + ret = 0; + + fail: + free(actual); + VIR_FREE(pidfile); + virCommandFree(cmd); + virNetworkObjFree(obj); + return ret; +} + +static int testCompareXMLToArgvHelper(const void *data) { + char inxml[PATH_MAX]; + char outargv[PATH_MAX];

cc1: warnings being treated as errors networkxml2argvtest.c: In function 'testCompareXMLToArgvHelper': networkxml2argvtest.c:71: error: the frame size of 8224 bytes is larger than 4096 You need to use virAsprintf instead of snprintf. You probably started with qemuxml2argvtest.c, which has a similar function, and was changed awhile back to eliminate large buffers on the stack.

+ snprintf(inxml, PATH_MAX, "%s/networkxml2argvdata/%s.xml", + abs_srcdir, (const char*)data); + snprintf(outargv, PATH_MAX, "%s/networkxml2argvdata/%s.argv", + abs_srcdir, (const char*)data); + return testCompareXMLToArgvFiles(inxml, outargv); +} + + +static int +mymain(void) +{ + int ret = 0; + char cwd[PATH_MAX]; + + abs_srcdir = getenv("abs_srcdir"); + if (!abs_srcdir) + abs_srcdir = getcwd(cwd, sizeof(cwd));

This code to set abs_srcdir is no longer needed, as it's done in commandhelper.c:main() (a good thing, since the method you're using requires a large stack buffer that triggers the same error as above).

+#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)

After fixing the above problems, when I run this test as non-root, I get the following error: 3) Network XML-2-Argv nat-network ... 08:46:00.612: 20502: info : libvirt version: 0.9.2 08:46:00.612: 20502: error : hostsfileSave:216 : cannot write config file '/var/lib/libvirt/dnsmasq/default.hostsfile': Unknown error 18446744073709551603 The problem is that the test is calling networkBuildDhcpDaemonCommandLine, which calls networkBuildDnsmasqArgv, which calls networkSaveDnsmasqHostsfile. Here's a diff addressing all the above issues *except* the last one (that's beyond the scope of the time that I have :-):

On Mon, Jun 13, 2011 at 06:55:03PM +0200, Michal Novotny wrote:

...

diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv new file mode 100644 index 0000000..dce6034 --- /dev/null +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv new file mode 100644 index 0000000..96fde34 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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 --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \ No newline at end of file diff --git a/tests/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv new file mode 100644 index 0000000..ccffa67 --- /dev/null +++ b/tests/networkxml2argvdata/nat-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --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 \ No newline at end of file diff --git a/tests/networkxml2argvdata/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv new file mode 100644 index 0000000..565c41b --- /dev/null +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=(null) --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-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv new file mode 100644 index 0000000..019367d --- /dev/null +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=(null) --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/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv new file mode 100644 index 0000000..2b51d90 --- /dev/null +++ b/tests/networkxml2argvdata/routed-network.argv @@ -0,0 +1 @@ +/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --listen-address 192.168.122.1 \ No newline at end of file

The '--pid-file=(null)' surely cannot be right for any of these.

Also, as we did with the .args files in qemuxml2argvdata/ we really want to split these long lines

Regards, Daniel Well, the --pid-file is always set in real world scenario and since I'm not generating the pid file for the tests I'm passing NULL there so

On 06/14/2011 03:26 PM, Daniel P. Berrange wrote: that's why there's --pid-file=(null)... What do you mean by splitting those lines? To split to: /usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --listen-address 192.168.122.1 instead of: /usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=(null) --conf-file= --except-interface lo --listen-address 192.168.122.1 ? Thanks, Michal -- Michal Novotny <minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat