[libvirt] [PATCH 1/3] tests: Introduce virpcitest
Martin Kletzander
mkletzan at redhat.com
Fri Nov 1 09:24:23 UTC 2013
On Fri, Oct 25, 2013 at 02:03:41PM +0100, Michal Privoznik wrote:
> Among with this test introduce virpcimock as we need to mock some
> syscalls, e.g. redirect open() of a file under /sys/bus/pci to a
> stub sysfs tree.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> .gitignore | 1 +
> cfg.mk | 2 +-
> tests/Makefile.am | 13 +++
> tests/virpcimock.c | 312 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/virpcitest.c | 103 ++++++++++++++++++
> 5 files changed, 430 insertions(+), 1 deletion(-)
> create mode 100644 tests/virpcimock.c
> create mode 100644 tests/virpcitest.c
>
> diff --git a/.gitignore b/.gitignore
> index e372876..6b024e7 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -212,6 +212,7 @@
> /tests/virlockspacetest
> /tests/virlogtest
> /tests/virnet*test
> +/tests/virpcitest
> /tests/virportallocatortest
> /tests/virshtest
> /tests/virstoragetest
> diff --git a/cfg.mk b/cfg.mk
> index e9da282..15114a9 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -942,7 +942,7 @@ exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
> exclude_file_name_regexp--sc_copyright_usage = \
> ^COPYING(|\.LESSER)$$
>
> -exclude_file_name_regexp--sc_flags_usage = ^(docs/|src/util/virnetdevtap\.c$$|tests/vircgroupmock\.c$$)
> +exclude_file_name_regexp--sc_flags_usage = ^(docs/|src/util/virnetdevtap\.c$$|tests/(vircgroup|virpci)mock\.c$$)
>
vir(cgroup|pci)mock would look cleaner IMHO ;)
> exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
> ^(src/rpc/gendispatch\.pl$$|tests/)
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 866ecd4..6d3245b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -123,6 +123,7 @@ test_programs = virshtest sockettest \
> virauthconfigtest \
> virbitmaptest \
> vircgrouptest \
> + virpcitest \
> virendiantest \
> viridentitytest \
> virkeycodetest \
> @@ -306,6 +307,7 @@ test_libraries = libshunload.la \
> libvirportallocatormock.la \
> virnetserverclientmock.la \
> vircgroupmock.la \
> + virpcimock.la \
> $(NULL)
> if WITH_QEMU
> test_libraries += libqemumonitortestutils.la
> @@ -742,6 +744,17 @@ vircgroupmock_la_CFLAGS = $(AM_CFLAGS)
> vircgroupmock_la_LDFLAGS = -module -avoid-version \
> -rpath /evil/libtool/hack/to/force/shared/lib/creation
>
> +virpcitest_SOURCES = \
> + virpcitest.c testutils.h testutils.c
> +virpcitest_LDADD = $(LDADDS)
> +
> +virpcimock_la_SOURCES = \
> + virpcimock.c
> +virpcimock_la_CFLAGS = $(AM_CFLAGS)
> +virpcimock_la_LIBADD = ../src/libvirt.la
> +virpcimock_la_LDFLAGS = -module -avoid-version \
> + -rpath /evil/libtool/hack/to/force/shared/lib/creation
> +
> if WITH_DBUS
> virdbustest_SOURCES = \
> virdbustest.c testutils.h testutils.c
> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> new file mode 100644
> index 0000000..d545361
> --- /dev/null
> +++ b/tests/virpcimock.c
> @@ -0,0 +1,312 @@
> +/*
> + * Copyright (C) 2013 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Michal Privoznik <mprivozn at redhat.com>
> + */
> +
> +#include <config.h>
> +
> +#ifdef __linux__
> +# include "internal.h"
> +# include <stdio.h>
> +# include <dlfcn.h>
> +# include <stdlib.h>
> +# include <unistd.h>
> +# include <fcntl.h>
> +# include <sys/stat.h>
> +# include <stdarg.h>
> +# include "viralloc.h"
> +# include "virstring.h"
> +# include "virfile.h"
> +
> +static int (*realaccess)(const char *path, int mode);
> +static int (*reallstat)(const char *path, struct stat *sb);
> +static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
> +static int (*realopen)(const char *path, int flags, ...);
> +
> +/* Don't make static, since it causes problems with clang
> + * when passed as an arg to virAsprintf()
> + * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline]
> + */
> +char *fakesysfsdir;
> +
> +# define PCI_SYSFS_PREFIX "/sys/bus/pci/"
> +
> +# define STDERR(...) \
> + fprintf(stderr, "%s %zu: ", __FUNCTION__, (size_t) __LINE__); \
> + fprintf(stderr, __VA_ARGS__); \
> + fprintf(stderr, "\n"); \
> +
> +# define ABORT(...) \
> + do { \
> + STDERR(__VA_ARGS__); \
> + abort(); \
> + } while (0)
> +
> +# define ABORT_OOM() \
> + ABORT("Out of memory")
> +/*
> + * The plan:
> + *
> + * Mock some file handling functions. Redirect them into a stub tree passed via
> + * LIBVIRT_FAKE_SYSFS_DIR env variable. All files and links within stub tree is
> + * created by us.
> + */
> +
> +/*
> + *
> + * Functions to model kernel behavior
> + *
> + */
> +
> +struct pciDevice {
> + char *id;
> + int vendor;
> + int device;
> +};
> +
> +struct pciDevice **pciDevices = NULL;
> +size_t pciDevices_size = 0;
> +
Mixing camelCase_and_underscores looks a bit weird, how about our
usual nPciDevices (or similar)? ...
> +static void init_env(void);
> +
> +static void pci_device_new_from_stub(const struct pciDevice *data);
> +
... or pci_devices when you're using underscores in these functions.
> +
> +/*
> + * Helper functions
> + */
> +static void
> +make_file(const char *path,
> + const char *name,
> + const char *value)
> +{
> + int fd = -1;
> + char *filepath = NULL;
> +
> + if (virAsprintfQuiet(&filepath, "%s/%s", path, name) < 0)
> + ABORT_OOM();
> +
> + if ((fd = realopen(filepath, O_CREAT|O_WRONLY, 0666)) < 0)
> + ABORT("Unable to open: %s", filepath);
> +
> + if (value && safewrite(fd, value, strlen(value)) != strlen(value))
> + ABORT("Unable to write: %s", filepath);
> +
> + VIR_FORCE_CLOSE(fd);
> + VIR_FREE(filepath);
> +}
> +
> +static int
> +getrealpath(char **newpath,
> + const char *path)
> +{
> + if (!fakesysfsdir)
> + init_env();
(1)
> +
> + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) {
> + if (virAsprintfQuiet(newpath, "%s/%s",
> + fakesysfsdir,
> + path + strlen(PCI_SYSFS_PREFIX)) < 0) {
> + errno = ENOMEM;
> + return -1;
> + }
> + } else {
> + if (VIR_STRDUP_QUIET(*newpath, path) < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +/*
> + * PCI Device functions
> + */
> +static void
> +pci_device_new_from_stub(const struct pciDevice *data)
> +{
> + struct pciDevice *dev;
> + char *devpath;
> + char tmp[32];
> +
> + if (VIR_ALLOC_QUIET(dev) < 0 ||
> + virAsprintfQuiet(&devpath, "%s/devices/%s", fakesysfsdir, data->id) < 0)
> + ABORT_OOM();
> +
> + memcpy(dev, data, sizeof(*dev));
> +
> + if (virFileMakePath(devpath) < 0)
> + ABORT("Unable to create: %s", devpath);
> +
> + make_file(devpath, "config", "some dummy config");
> +
(3)
> + if (snprintf(tmp, sizeof(tmp), "0x%.4x", dev->vendor) < 0)
> + ABORT("@tmp overflow");
> + make_file(devpath, "vendor", tmp);
> +
> + if (snprintf(tmp, sizeof(tmp), "0x%.4x", dev->device) < 0)
> + ABORT("@tmp overflow");
> + make_file(devpath, "device", tmp);
> +
> + if (VIR_APPEND_ELEMENT_QUIET(pciDevices, pciDevices_size, dev) < 0)
> + ABORT_OOM();
> +
> + VIR_FREE(devpath);
> +}
> +
> +
> +/*
> + * Functions to load the symbols and init the environment
> + */
> +static void
> +init_syms(void)
> +{
> + if (realaccess)
> + return;
> +
> +# define LOAD_SYM(name) \
> + do { \
> + if (!(real ## name = dlsym(RTLD_NEXT, #name))) \
> + ABORT("Cannot find real '%s' symbol\n", #name); \
> + } while (0)
> +
> +# define LOAD_SYM_ALT(name1, name2) \
> + do { \
> + if (!(real ## name1 = dlsym(RTLD_NEXT, #name1)) && \
> + !(real ## name2 = dlsym(RTLD_NEXT, #name2))) \
> + ABORT("Cannot find real '%s' or '%s' symbol\n", \
> + #name1, #name2); \
> + } while (0)
> +
> + LOAD_SYM(access);
> + LOAD_SYM_ALT(lstat, __lxstat);
> + LOAD_SYM(open);
> +}
> +
> +static void
> +init_env(void)
> +{
> + if (fakesysfsdir)
> + return;
> +
> + if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR")))
> + ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n");
> +
> +# define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \
> + do { \
> + struct pciDevice dev = {.id = (char *)Id, .vendor = Vendor, \
> + .device = Device, __VA_ARGS__}; \
> + pci_device_new_from_stub(&dev); \
> + } while (0)
> +
> + MAKE_PCI_DEVICE("0000:00:00.0", 0x8086, 0x0044);
(2)
[...]
> diff --git a/tests/virpcitest.c b/tests/virpcitest.c
> new file mode 100644
> index 0000000..96f11d6
> --- /dev/null
> +++ b/tests/virpcitest.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright (C) 2013 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Michal Privoznik <mprivozn at redhat.com>
> + */
> +
> +#include <config.h>
> +
> +#include "testutils.h"
> +
> +#ifdef __linux__
> +
> +# include <stdlib.h>
> +# include <stdio.h>
> +# include <sys/types.h>
> +# include <sys/stat.h>
> +# include <fcntl.h>
> +# include <virpci.h>
> +
> +# define VIR_FROM_THIS VIR_FROM_NONE
> +
> +# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX"
> +char *fakesysfsdir;
> +
> +static int
> +testVirPCIDeviceNew(const void *opaque ATTRIBUTE_UNUSED)
> +{
> + int ret = -1;
> + virPCIDevicePtr dev;
> + const char *devName;
> +
> + if (!(dev = virPCIDeviceNew(0, 0, 0, 0)))
> + goto cleanup;
> +
(4)
> + devName = virPCIDeviceGetName(dev);
> + if (STRNEQ(devName, "0000:00:00.0")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "PCI device name mismatch: %s, expecting %s",
> + devName, "0000:00:00.0");
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + virPCIDeviceFree(dev);
> + return ret;
> +}
> +
> +static int
> +mymain(void)
> +{
> + int ret = 0;
> +
> + if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) {
Since you are initializing fakesysfsdir in here, init_env() is never
called, because conditions like (1) are false. That means
MAKE_PCI_DEVICE (2) is never called as well, mocked files are not
created (3) properly and the first call to virPCIDeviceNew() (4) fails
like this:
LIBVIRT_DEBUG=1 VIR_TEST_DEBUG=2 ./virpcitest
TEST: virpcitest
1) testVirPCIDeviceNew
... 2013-10-31 15:15:05.511+0000: 19020: info : libvirt version: 1.1.3
2013-10-31 15:15:05.511+0000: 19020: debug : virPCIDeviceFree:1572 : 0000:00:00.0: freeing
libvirt: error : Device 0000:00:00.0 not found: could not access /sys/bus/pci/devices/0000:00:00.0/config: No such file or directory
FAILED
Oh, I understand it now! The problem is that the variables couldn't
be made static. And the real issue here is that the variable names
are the same for virpcimock.c and virpcitest.c and you wanted to have
two different ones. Doing s/fakesysfsdir/fakesysfsdir_template/
on virpcitest.c makes it work as expected. I guess that's what you
had in mind, right?
[...]
It might sound silly, but I'd rather wait after release for this, even
though these are tests. OTOH it gives you some time for v2 in case my
change is not what you've intended ;-)
ACK (after release) if you just change the 'fakesysfsdir' variable
name to something different, but meaningful and clean up the
camelCase_underscore names.
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131101/a8ee60a8/attachment-0001.sig>
More information about the libvir-list
mailing list