[libvirt] [PATCH 0/2] bhyve: add xml2args unittest
Add testing for xml -> args generation. It's largely based on the similar test for qemu. As it's my first experience with unit testing in libvirt and in unit testing in C in general, I'm not sure I haven't miss some important pieces, but it works as expected for me. I'm also not sure about the license headers: some tests have them, some don't, so I decided not to add them for now. Roman Bogorodskiy (2): Move virBhyveTapGetRealDeviceName to virnetdevtap bhyve: add xml2args unittest src/bhyve/bhyve_command.c | 70 +--------- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 78 +++++++++++ src/util/virnetdevtap.h | 3 + tests/Makefile.am | 25 ++++ .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml | 24 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-base.xml | 20 +++ .../bhyvexml2argv-disk-virtio.args | 2 + .../bhyvexml2argv-disk-virtio.xml | 20 +++ tests/bhyvexml2argvmock.c | 38 +++++ tests/bhyvexml2argvtest.c | 154 +++++++++++++++++++++ 13 files changed, 370 insertions(+), 69 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml create mode 100644 tests/bhyvexml2argvmock.c create mode 100644 tests/bhyvexml2argvtest.c -- 1.8.4.2
To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName() out of bhyve_command.c to virnetdevtap and rename it to virNetDevTapGetRealDeviceName(). --- src/bhyve/bhyve_command.c | 70 +---------------------------------------- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevtap.h | 3 ++ 4 files changed, 84 insertions(+), 69 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..28a899b 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -21,10 +21,7 @@ #include <config.h> -#include <fcntl.h> #include <sys/types.h> -#include <dirent.h> -#include <sys/ioctl.h> #include <net/if.h> #include <net/if_tap.h> @@ -41,71 +38,6 @@ VIR_LOG_INIT("bhyve.bhyve_command"); -static char* -virBhyveTapGetRealDeviceName(char *name) -{ - /* This is an ugly hack, because if we rename - * tap device to vnet%d, its device name will be - * still /dev/tap%d, and bhyve tries to open /dev/tap%d, - * so we have to find the real name - */ - char *ret = NULL; - struct dirent *dp; - char *devpath = NULL; - int fd; - - DIR *dirp = opendir("/dev"); - if (dirp == NULL) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - "/dev"); - return NULL; - } - - while ((dp = readdir(dirp)) != NULL) { - if (STRPREFIX(dp->d_name, "tap")) { - struct ifreq ifr; - if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) { - goto cleanup; - } - if ((fd = open(devpath, O_RDWR)) < 0) { - virReportSystemError(errno, _("Unable to open '%s'"), devpath); - goto cleanup; - } - - if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) { - virReportSystemError(errno, "%s", - _("Unable to query tap interface name")); - goto cleanup; - } - - if (STREQ(name, ifr.ifr_name)) { - /* we can ignore the return value - * because we still have nothing - * to do but return; - */ - ignore_value(VIR_STRDUP(ret, dp->d_name)); - goto cleanup; - } - - VIR_FREE(devpath); - VIR_FORCE_CLOSE(fd); - } - - errno = 0; - } - - if (errno != 0) - virReportSystemError(errno, "%s", - _("Unable to iterate over TAP devices")); - -cleanup: - VIR_FREE(devpath); - VIR_FORCE_CLOSE(fd); - closedir(dirp); - return ret; -} - static int bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) { @@ -156,7 +88,7 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) } } - realifname = virBhyveTapGetRealDeviceName(net->ifname); + realifname = virNetDevTapGetRealDeviceName(net->ifname); if (realifname == NULL) { VIR_FREE(net->ifname); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3baf766..3adf201 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1556,6 +1556,7 @@ virNetDevTapCreate; virNetDevTapCreateInBridgePort; virNetDevTapDelete; virNetDevTapGetName; +virNetDevTapGetRealDeviceName; # util/virnetdevveth.h diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index a36af76..bd00c28 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -22,6 +22,10 @@ #include <config.h> +#include <fcntl.h> +#include <sys/types.h> +#include <dirent.h> + #include "virmacaddr.h" #include "virnetdevtap.h" #include "virnetdev.h" @@ -38,8 +42,11 @@ #include <fcntl.h> #ifdef __linux__ # include <linux/if_tun.h> /* IFF_TUN, IFF_NO_PI */ +#elif defined(__FreeBSD__) +# include <net/if_tap.h> #endif + #define VIR_FROM_THIS VIR_FROM_NONE VIR_LOG_INIT("util.netdevtap"); @@ -72,6 +79,78 @@ virNetDevTapGetName(int tapfd ATTRIBUTE_UNUSED, char **ifname ATTRIBUTE_UNUSED) #endif } +/** + * virNetDevTapGetRealDeviceName: + * @ifname: the interface name + * + * Lookup real interface name (i.e. name of the device entry in /dev), + * because e.g. on FreeBSD if we rename tap device to vnetN its device + * entry still remains unchanged (/dev/tapX), but bhyve needs a name + * that matches /dev entry. + * + * Returns the proper interface name or NULL if no corresponding interface + * found. + */ +char* +virNetDevTapGetRealDeviceName(char *ifname) +{ + char *ret = NULL; + struct dirent *dp; + char *devpath = NULL; + int fd; + + DIR *dirp = opendir("/dev"); + if (dirp == NULL) { + virReportSystemError(errno, + _("Failed to opendir path '%s'"), + "/dev"); + return NULL; + } + + while ((dp = readdir(dirp)) != NULL) { + if (STRPREFIX(dp->d_name, "tap")) { + struct ifreq ifr; + if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) { + goto cleanup; + } + if ((fd = open(devpath, O_RDWR)) < 0) { + virReportSystemError(errno, _("Unable to open '%s'"), devpath); + goto cleanup; + } + + if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query tap interface name")); + goto cleanup; + } + + if (STREQ(ifname, ifr.ifr_name)) { + /* we can ignore the return value + * because we still have nothing + * to do but return; + */ + ignore_value(VIR_STRDUP(ret, dp->d_name)); + goto cleanup; + } + + VIR_FREE(devpath); + VIR_FORCE_CLOSE(fd); + } + + errno = 0; + } + + if (errno != 0) + virReportSystemError(errno, "%s", + _("Unable to iterate over TAP devices")); + +cleanup: + VIR_FREE(devpath); + VIR_FORCE_CLOSE(fd); + closedir(dirp); + return ret; +} + /** * virNetDevProbeVnetHdr: diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 1e5bd19..03fb5f8 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -45,6 +45,9 @@ int virNetDevTapDelete(const char *ifname) int virNetDevTapGetName(int tapfd, char **ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +char* virNetDevTapGetRealDeviceName(char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + typedef enum { VIR_NETDEV_TAP_CREATE_NONE = 0, /* Bring the interface up */ -- 1.8.4.2
At this point unittest covers 3 basic cases: - minimal working XML for bhyve - same as above, but with virtio disk - ACPI and APIC args test --- src/util/virnetdevtap.c | 1 - tests/Makefile.am | 25 ++++ .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml | 24 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-base.xml | 20 +++ .../bhyvexml2argv-disk-virtio.args | 2 + .../bhyvexml2argv-disk-virtio.xml | 20 +++ tests/bhyvexml2argvmock.c | 38 +++++ tests/bhyvexml2argvtest.c | 154 +++++++++++++++++++++ 10 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml create mode 100644 tests/bhyvexml2argvmock.c create mode 100644 tests/bhyvexml2argvtest.c diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index bd00c28..6ecc1d6 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -22,7 +22,6 @@ #include <config.h> -#include <fcntl.h> #include <sys/types.h> #include <dirent.h> diff --git a/tests/Makefile.am b/tests/Makefile.am index 90f70ff..4fac8b3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -227,6 +227,10 @@ if WITH_VMWARE test_programs += vmwarevertest endif WITH_VMWARE +if WITH_BHYVE +test_programs += bhyvexml2argvtest +endif WITH_BHYVE + if WITH_CIL test_programs += objectlocking endif WITH_CIL @@ -348,6 +352,10 @@ test_libraries += libqemumonitortestutils.la \ $(NULL) endif WITH_QEMU +if WITH_BHYVE +test_libraries += bhyvexml2argvmock.la +endif WITH_BHYVE + if WITH_DBUS test_libraries += virsystemdmock.la endif WITH_DBUS @@ -597,6 +605,23 @@ else ! WITH_VMWARE EXTRA_DIST += vmwarevertest.c endif ! WITH_VMWARE +if WITH_BHYVE +bhyvexml2argvmock_la_SOURCES = \ + bhyvexml2argvmock.c +bhyvexml2argvmock_la_CFLAGS = $(AM_CFLAGS) +bhyvexml2argvmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation + +bhyve_LDADDS = ../src/libvirt_driver_bhyve_impl.la +bhyve_LDADDS += $(LDADDS) +bhyvexml2argvtest_SOURCES = \ + bhyvexml2argvtest.c \ + testutils.c testutils.h +bhyvexml2argvtest_LDADD = $(bhyve_LDADDS) +else ! WITH_BHYVE +EXTRA_DIST += bhyvexml2argvtest.c +endif ! WITH_BHYVE + networkxml2xmltest_SOURCES = \ networkxml2xmltest.c \ testutils.c testutils.h diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args new file mode 100644 index 0000000..c638f61 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args @@ -0,0 +1,2 @@ +/usr/sbin/bhyve -c 1 -m 214 -A -I -H -P -s 0:0,hostbridge \ +-s 1:0,virtio-net,faketapdev -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml new file mode 100644 index 0000000..b429fef --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <features> + <apic/> + <acpi/> + </features> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args new file mode 100644 index 0000000..afa94f1 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args @@ -0,0 +1,2 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 1:0,virtio-net,faketapdev -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml new file mode 100644 index 0000000..8c96f77 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml @@ -0,0 +1,20 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args new file mode 100644 index 0000000..e627d18 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args @@ -0,0 +1,2 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 1:0,virtio-net,faketapdev -s 2:0,virtio-blk,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml new file mode 100644 index 0000000..8cfb518 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml @@ -0,0 +1,20 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='vda' bus='virtio'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c new file mode 100644 index 0000000..5ad5a60 --- /dev/null +++ b/tests/bhyvexml2argvmock.c @@ -0,0 +1,38 @@ +#include <config.h> + +#include "util/virstring.h" +#include "util/virnetdev.h" +#include "util/virnetdevtap.h" +#include "internal.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +int virNetDevTapCreateInBridgePort(const char *brname ATTRIBUTE_UNUSED, + char **ifname, + const virMacAddr *macaddr ATTRIBUTE_UNUSED, + const unsigned char *vmuuid ATTRIBUTE_UNUSED, + int *tapfd ATTRIBUTE_UNUSED, + int tapfdSize ATTRIBUTE_UNUSED, + virNetDevVPortProfilePtr virtPortProfile ATTRIBUTE_UNUSED, + virNetDevVlanPtr virtVlan ATTRIBUTE_UNUSED, + unsigned int fakeflags ATTRIBUTE_UNUSED) +{ + if (VIR_STRDUP(*ifname, "vnet0") < 0) + return -1; + return 0; +} + +char *virNetDevTapGetRealDeviceName(char *name ATTRIBUTE_UNUSED) +{ + char *fakename; + + if (VIR_STRDUP(fakename, "faketapdev") < 0) + return NULL; + return fakename; +} + +int virNetDevSetOnline(const char *ifname ATTRIBUTE_UNUSED, + bool online ATTRIBUTE_UNUSED) +{ + return 0; +} diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c new file mode 100644 index 0000000..ae115f3 --- /dev/null +++ b/tests/bhyvexml2argvtest.c @@ -0,0 +1,154 @@ +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> + +#include "datatypes.h" +#include "bhyve/bhyve_utils.h" +#include "bhyve/bhyve_command.h" + +#include "testutils.h" + +#ifdef WITH_BHYVE + +# define VIR_FROM_THIS VIR_FROM_BHYVE + +static bhyveConn driver; + +static virCapsPtr +testBhyveBuildCapabilities(void) +{ + virCapsPtr caps; + virCapsGuestPtr guest; + + if ((caps = virCapabilitiesNew(virArchFromHost(), + 0, 0)) == NULL) + return NULL; + + if ((guest = virCapabilitiesAddGuest(caps, "hvm", + VIR_ARCH_X86_64, + "bhyve", + NULL, 0, NULL)) == NULL) + goto error; + + if (virCapabilitiesAddGuestDomain(guest, + "bhyve", NULL, NULL, 0, NULL) == NULL) + goto error; + + return caps; + +error: + virObjectUnref(caps); + return NULL; +} + +static int testCompareXMLToArgvFiles(const char *xml, + const char *cmdline) +{ + char *expectargv = NULL; + int len; + char *actualargv = NULL; + virConnectPtr conn; + virDomainDefPtr vmdef = NULL; + virDomainObj vm; + virCommandPtr cmd = NULL; + int ret = -1; + + + if (!(conn = virGetConnect())) + goto out; + + if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, + 1 << VIR_DOMAIN_VIRT_BHYVE, + VIR_DOMAIN_XML_INACTIVE))) + goto out; + + vm.def = vmdef; + + if (!(cmd = virBhyveProcessBuildBhyveCmd(&driver, &vm))) + goto out; + + if (!(actualargv = virCommandToString(cmd))) + goto out; + + len = virtTestLoadFile(cmdline, &expectargv); + if (len < 0) + goto out; + + if (len && expectargv[len - 1] == '\n') + expectargv[len - 1] = '\0'; + + if (STRNEQ(expectargv, actualargv)) { + virtTestDifference(stderr, expectargv, actualargv); + goto out; + } + + ret = 0; + +out: + VIR_FREE(expectargv); + VIR_FREE(actualargv); + virCommandFree(cmd); + virDomainDefFree(vmdef); + virObjectUnref(conn); + return ret; +} + +static int +testCompareXMLToArgvHelper(const void *data) +{ + int ret = -1; + const char *name = data; + char *xml = NULL; + char *args = NULL; + + if (virAsprintf(&xml, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.xml", + abs_srcdir, name) < 0 || + virAsprintf(&args, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.args", + abs_srcdir, name) < 0) + goto cleanup; + + ret = testCompareXMLToArgvFiles(xml, args); + +cleanup: + VIR_FREE(xml); + VIR_FREE(args); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if ((driver.caps = testBhyveBuildCapabilities()) == NULL) + return EXIT_FAILURE; + + if ((driver.xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL)) == NULL) + return EXIT_FAILURE; + +# define DO_TEST(name) \ + do { \ + if (virtTestRun("BHYVE XML-2-ARGV " name, \ + testCompareXMLToArgvHelper, name) < 0) \ + ret = -1; \ + } while (0) + + + DO_TEST("base"); + DO_TEST("acpiapic"); + DO_TEST("disk-virtio"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/bhyvexml2argvmock.so") + +#else + +int main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* WITH_BHYVE */ -- 1.8.4.2
Roman Bogorodskiy wrote:
Add testing for xml -> args generation. It's largely based on the similar test for qemu. As it's my first experience with unit testing in libvirt and in unit testing in C in general, I'm not sure I haven't miss some important pieces, but it works as expected for me.
I'm also not sure about the license headers: some tests have them, some don't, so I decided not to add them for now.
One more note on that. Original reason for the test was hostbridge device handling which happened in the network handling piece by mistake. This test doesn't test that because currently bhyve driver does NOT allow to create domains without networks. The reason for that is that currently networking is the only way to communicate with the guest and it makes no sense to start domain without network. However, I think it's a good base for future test and I plan to change this behaviour once I manage to bring in the nmdm/console support for bhyve. Roman Bogorodskiy
participants (1)
-
Roman Bogorodskiy