On 04/01/2011 06:45 AM, Michal Novotny wrote:
Make: passed
Make check: passed
Make syntax-check: passed
Hi,
this is the patch that is adding regression tests for the network
bridge driver command-line arguments similar way it is done for
QEMU driver. This is checking the built dnsmasq parameters (using
networkBuildDhcpDaemonCommandLine() function) and comparing them
to excepted arguments in the *.argv files.
This has been tested and working fine.
Michal
Same comments about the commit message as in 1/5 - don't include stuff
about what tests passed, salutations, signatures; *do* include a short
sample of the XML.
Signed-off-by: Michal Novotny<minovotn@xxxxxxxxxx>
---
src/network/bridge_driver.c | 27 ++++-
src/network/bridge_driver.h | 3 +
tests/Makefile.am | 11 ++
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 | 119 ++++++++++++++++++++
16 files changed, 255 insertions(+), 3 deletions(-)
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/network/bridge_driver.c b/src/network/bridge_driver.c
index 2e299f5..b6ce39d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -613,11 +613,11 @@ cleanup:
return ret;
}
-static int
-networkStartDhcpDaemon(virNetworkObjPtr network)
+int
+networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout,
+ char *pidfile)
If you make a function global, you should add it to a .syms file. In
this case (as we just discussed on IRC with eblake) you should create a
new src/libvirt_network.syms file and add it to that (then add that
.syms file to the appropriate places in src/Makefile.am)
{
virCommandPtr cmd = NULL;
- char *pidfile = NULL;
As patched, this will not compile - you removed pidfile from the new
function, but left the assignment to it in. (actually, all of the
directory and file creation items should be moved down into
networkStartDhcpDaemon, so that networkBuildDhcpDaemonCommandLine
doesn't have any side effects.)
int ret = -1, err, ii;
virNetworkIpDefPtr ipdef;
@@ -666,6 +666,27 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
goto cleanup;
}
+ if (cmdout)
+ *cmdout = cmd;
+
+ ret = 0;
+cleanup:
+ if (ret != 0)
The standard practice in libvirt is to use "ret < 0" rather than "ret != 0".
+ virCommandFree(cmd);
+ return ret;
+}
+
+static int
+networkStartDhcpDaemon(virNetworkObjPtr network)
+{
+ virCommandPtr cmd = NULL;
+ char *pidfile = NULL;
+ int ret = -1;
+
+ ret = networkBuildDhcpDaemonCommandLine(network,&cmd, pidfile);
+ if (ret != 0)
Again, 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 5896442..a3f8d00 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,6 +50,7 @@ EXTRA_DIST = \
networkschematest \
networkxml2xmlin \
networkxml2xmlout \
+ networkxml2argvdata \
Yay for new tests!
nodedevschemadata \
nodedevschematest \
nodeinfodata \
@@ -104,6 +105,8 @@ endif
check_PROGRAMS += networkxml2xmltest
+check_PROGRAMS += networkxml2argvtest
+
check_PROGRAMS += nwfilterxml2xmltest
check_PROGRAMS += storagevolxml2xmltest storagepoolxml2xmltest
@@ -191,6 +194,8 @@ endif
TESTS += networkxml2xmltest
+TESTS += networkxml2argvtest
+
TESTS += storagevolxml2xmltest storagepoolxml2xmltest
TESTS += nodedevxml2xmltest
@@ -308,6 +313,12 @@ networkxml2xmltest_SOURCES = \
testutils.c testutils.h
networkxml2xmltest_LDADD = $(LDADDS)
+networkxml2argvtest_SOURCES = \
+ networkxml2argvtest.c \
+ ../src/network/bridge_driver.c network/bridge_driver.h \
Rather than adding the source files, you should be adding the .la file
libvirt_network.la. See other .la file additions for the proper pattern
to follow.
+ testutils.c testutils.h
+networkxml2argvtest_LDADD = $(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..1c173db
--- /dev/null
+++ b/tests/networkxml2argvdata/isolated-network.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/private.pid --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
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..55dcf02
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --except-interface lo --txt-record=example,example value --listen-address 192.168.122.1
Does the argument to ==txt-record need to be quoted? (probably not
needed, but it might help readability in the logs - will it *hurt*
anything to quote it?
--listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 --dhcp-range 192.168.122.2,192.168.122.254 --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases --dhcp-lease-max=253 --dhcp-no-override --dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.xml b/tests/networkxml2argvdata/nat-network-dns-txt-record.xml
new file mode 100644
index 0000000..d3e795d
--- /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' />
+<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>
+<dns>
+<txt-record name='example' value='example value' />
+</dns>
+</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..95ee6d9
--- /dev/null
+++ b/tests/networkxml2argvdata/nat-network.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --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
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..36c2360
--- /dev/null
+++ b/tests/networkxml2argvdata/netboot-network.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=/var/run/libvirt/network/netboot.pid --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
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..da97b72
--- /dev/null
+++ b/tests/networkxml2argvdata/netboot-proxy-network.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com --pid-file=/var/run/libvirt/network/netboot.pid --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
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..443087c
--- /dev/null
+++ b/tests/networkxml2argvdata/routed-network.argv
@@ -0,0 +1 @@
+/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/local.pid --conf-file= --except-interface lo --listen-address 192.168.122.1
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..e3a8bb4
--- /dev/null
+++ b/tests/networkxml2argvtest.c
@@ -0,0 +1,119 @@
+#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 char *progname;
+static char *abs_srcdir;
+
+#define MAX_FILE 4096
+
+
+static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) {
+ char inXmlData[MAX_FILE];
+ char *inXmlPtr =&(inXmlData[0]);
+ char outArgvData[MAX_FILE];
+ char *outArgvPtr =&(outArgvData[0]);
+ char *actual = NULL;
+ int ret = -1;
+ virNetworkDefPtr dev = NULL;
+ virNetworkObjPtr obj = NULL;
+ virCommandPtr cmd = NULL;
+ char *pidfile = NULL;
+
+ if (virtTestLoadFile(inxml,&inXmlPtr, MAX_FILE)< 0)
+ goto fail;
+
+ if (virtTestLoadFile(outargv,&outArgvPtr, MAX_FILE)< 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;
+
+ /* There is a new line character but syntax-check would complain
+ * about this in argv files so just trim it now
+ */
+ outArgvData[ strlen(outArgvData) - 1] = 0;
+
+ 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(int argc, char **argv)
+{
+ int ret = 0;
+ char cwd[PATH_MAX];
+
+ progname = argv[0];
+
+ if (argc> 1) {
+ fprintf(stderr, "Usage: %s\n", progname);
+ return (EXIT_FAILURE);
+ }
+
+ 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)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list