[libvirt] [PATCH v2 2/5] Network: Add regression tests for the command line arguments
Laine Stump
laine at laine.org
Wed Apr 27 16:33:01 UTC 2011
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 at redhat.com>
> ---
> 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)
More information about the libvir-list
mailing list