[libvirt] [PATCH 0/3] Test our PCI device handling functions

*** BLURB HERE *** Michal Privoznik (3): tests: Introduce virpcitest virpcitest: Test virPCIDeviceDetach virpcitest: Introduce testVirPCIDeviceReattach .gitignore | 1 + cfg.mk | 4 +- tests/Makefile.am | 21 +- tests/virpcimock.c | 872 +++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/virpcitest.c | 184 +++++++++++ 5 files changed, 1078 insertions(+), 4 deletions(-) create mode 100644 tests/virpcimock.c create mode 100644 tests/virpcitest.c -- 1.8.1.5

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@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$$) 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@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; + +static void init_env(void); + +static void pci_device_new_from_stub(const struct pciDevice *data); + + +/* + * 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(); + + 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"); + + 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); +} + + +/* + * + * Mocked functions + * + */ + +int +access(const char *path, int mode) +{ + int ret; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + char *newpath; + if (getrealpath(&newpath, path) < 0) + return -1; + ret = realaccess(newpath, mode); + VIR_FREE(newpath); + } else { + ret = realaccess(path, mode); + } + return ret; +} + +int +__lxstat(int ver, const char *path, struct stat *sb) +{ + int ret; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + char *newpath; + if (getrealpath(&newpath, path) < 0) + return -1; + ret = real__lxstat(ver, newpath, sb); + VIR_FREE(newpath); + } else { + ret = real__lxstat(ver, path, sb); + } + return ret; +} + +int +lstat(const char *path, struct stat *sb) +{ + int ret; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + char *newpath; + if (getrealpath(&newpath, path) < 0) + return -1; + ret = reallstat(newpath, sb); + VIR_FREE(newpath); + } else { + ret = reallstat(path, sb); + } + return ret; +} + +int +open(const char *path, int flags, ...) +{ + int ret; + char *newpath = NULL; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX) && + getrealpath(&newpath, path) < 0) + return -1; + + if (flags & O_CREAT) { + va_list ap; + mode_t mode; + va_start(ap, flags); + mode = va_arg(ap, mode_t); + va_end(ap); + ret = realopen(newpath ? newpath : path, flags, mode); + } else { + ret = realopen(newpath ? newpath : path, flags); + } + VIR_FREE(newpath); + return ret; +} + +#else +/* Nothing to override on non-__linux__ platforms */ +#endif 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@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; + + 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) { + fprintf(stderr, "Out of memory\n"); + abort(); + } + + if (!mkdtemp(fakesysfsdir)) { + fprintf(stderr, "Cannot create fakesysfsdir"); + abort(); + } + + setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); + +# define DO_TEST(fnc) \ + do { \ + if (virtTestRun(#fnc, fnc, NULL) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST(testVirPCIDeviceNew); + + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) + virFileDeleteTree(fakesysfsdir); + + VIR_FREE(fakesysfsdir); + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virpcimock.so") +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif -- 1.8.1.5

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@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@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@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

This commit introduces yet another test under virpcitest: virPCIDeviceDetach. However, in order to be able to do this, the virpcimock needs to be extended to model the kernel behavior on PCI device binding and unbinding (create 'driver' symlinks under the device tree, check for device ID in driver's ID table, etc.) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- cfg.mk | 2 +- tests/Makefile.am | 10 +- tests/virpcimock.c | 562 ++++++++++++++++++++++++++++++++++++++++++++++++++++- tests/virpcitest.c | 44 +++++ 4 files changed, 613 insertions(+), 5 deletions(-) diff --git a/cfg.mk b/cfg.mk index 15114a9..306b052 100644 --- a/cfg.mk +++ b/cfg.mk @@ -964,7 +964,7 @@ exclude_file_name_regexp--sc_prohibit_strdup = \ ^(docs/|examples/|python/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vircgroupmock\.c)$$) + (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vir(cgroup|pci)mock\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|nodeinfo)data/|\.(gif|ico|png|diff)$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6d3245b..2c2e5a9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -48,12 +48,15 @@ if WITH_DTRACE_PROBES PROBES_O += ../src/libvirt_probes.lo endif WITH_DTRACE_PROBES +GNULIB_LIBS = \ + ../gnulib/lib/libgnu.la + LDADDS = \ $(WARN_CFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(PROBES_O) \ - ../src/libvirt.la \ - ../gnulib/lib/libgnu.la + $(GNULIB_LIBS) \ + ../src/libvirt.la EXTRA_DIST = \ capabilityschemadata \ @@ -751,7 +754,8 @@ virpcitest_LDADD = $(LDADDS) virpcimock_la_SOURCES = \ virpcimock.c virpcimock_la_CFLAGS = $(AM_CFLAGS) -virpcimock_la_LIBADD = ../src/libvirt.la +virpcimock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la virpcimock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation diff --git a/tests/virpcimock.c b/tests/virpcimock.c index d545361..16a5ff3 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -32,11 +32,14 @@ # include "viralloc.h" # include "virstring.h" # include "virfile.h" +# include "dirname.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 char *(*realcanonicalize_file_name)(const char *path); static int (*realopen)(const char *path, int flags, ...); +static int (*realclose)(int fd); /* Don't make static, since it causes problems with clang * when passed as an arg to virAsprintf() @@ -64,7 +67,30 @@ char *fakesysfsdir; * * 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. + * created by us. There are some actions that we must take if some special + * files are written to. Here's the list of files we watch: + * + * /sys/bus/pci/drivers/<driver>/new_id: + * Learn the driver new vendor and device combination. + * Data in format "VVVV DDDD". + * + * /sys/bus/pci/drivers/<driver>/remove_id + * Make the driver forget about vendor and device. + * Data in format "VVVV DDDD". + * + * /sys/bus/pci/drivers/<driver>/bind + * Check if driver supports the device and bind driver to it (create symlink + * called 'driver' pointing to the /sys/but/pci/drivers/<driver>). + * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). + * + * /sys/bus/pci/drivers/<driver>/unbind + * Unbind driver from the device. + * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). + * + * As a little hack, we are not mocking write to these files, but close() + * instead. The advantage is we don't need any self growing array to hold the + * partial writes and construct them back. We can let all the writes finish, + * and then just read the file content back. */ /* @@ -73,18 +99,51 @@ char *fakesysfsdir; * */ +struct pciDriver { + char *name; + int *vendor; /* List of vendor:device IDs the driver can handle */ + int *device; + size_t len; /* @len is used for both @vendor and @device */ +}; + struct pciDevice { char *id; int vendor; int device; + struct pciDriver *driver; /* Driver attached. NULL if attached to no driver */ +}; + +struct fdCallback { + int fd; + char *path; }; struct pciDevice **pciDevices = NULL; size_t pciDevices_size = 0; +struct pciDriver **pciDrivers = NULL; +size_t pciDrivers_size = 0; + +struct fdCallback *callbacks = NULL; +size_t callbacks_size = 0; + static void init_env(void); +static int pci_device_autobind(struct pciDevice *dev); static void pci_device_new_from_stub(const struct pciDevice *data); +static struct pciDevice *pci_device_find_by_id(const char *id); +static struct pciDevice *pci_device_find_by_content(const char *path); + +static void pci_driver_new(const char *name, ...); +static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev); +static struct pciDriver *pci_driver_find_by_path(const char *path); +static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev); +static int pci_driver_unbind(struct pciDriver *driver, struct pciDevice *dev); +static int pci_driver_handle_change(int fd, const char *path); +static int pci_driver_handle_bind(const char *path); +static int pci_driver_handle_unbind(const char *path); +static int pci_driver_handle_new_id(const char *path); +static int pci_driver_handle_remove_id(const char *path); /* @@ -112,6 +171,41 @@ make_file(const char *path, } static int +pci_read_file(const char *path, + char *buf, + size_t buf_size) +{ + int ret = -1; + int fd = -1; + char *newpath; + + if (virAsprintfQuiet(&newpath, "%s/%s", + fakesysfsdir, + path + strlen(PCI_SYSFS_PREFIX)) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if ((fd = realopen(newpath, O_RDWR)) < 0) + goto cleanup; + + bzero(buf, buf_size); + if (saferead(fd, buf, buf_size - 1) < 0) { + STDERR("Unable to read from %s", newpath); + goto cleanup; + } + + if (ftruncate(fd, 0) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(newpath); + realclose(fd); + return ret; +} + +static int getrealpath(char **newpath, const char *path) { @@ -133,6 +227,70 @@ getrealpath(char **newpath, return 0; } +static bool +find_fd(int fd, size_t *indx) +{ + size_t i; + + for (i = 0; i < callbacks_size; i++) { + if (callbacks[i].fd == fd) { + if (indx) + *indx = i; + return true; + } + } + + return false; +} + +static int +add_fd(int fd, const char *path) +{ + int ret = -1; + size_t i; + + if (find_fd(fd, &i)) { + struct fdCallback cb = callbacks[i]; + ABORT("FD %d %s already present in the array as %d %s", + fd, path, cb.fd, cb.path); + } + + if (VIR_REALLOC_N_QUIET(callbacks, callbacks_size + 1) < 0 || + VIR_STRDUP_QUIET(callbacks[callbacks_size].path, path) < 0) { + errno = ENOMEM; + goto cleanup; + } + + callbacks[callbacks_size++].fd = fd; + ret = 0; +cleanup: + return ret; +} + +static int +remove_fd(int fd) +{ + int ret = -1; + size_t i; + + if (find_fd(fd, &i)) { + struct fdCallback cb = callbacks[i]; + + if (pci_driver_handle_change(cb.fd, cb.path) < 0) + goto cleanup; + + VIR_FREE(cb.path); + if (VIR_DELETE_ELEMENT(callbacks, i, callbacks_size) < 0) { + errno = EINVAL; + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} + /* * PCI Device functions @@ -163,12 +321,367 @@ pci_device_new_from_stub(const struct pciDevice *data) ABORT("@tmp overflow"); make_file(devpath, "device", tmp); + if (pci_device_autobind(dev) < 0) + ABORT("Unable to bind: %s", data->id); + if (VIR_APPEND_ELEMENT_QUIET(pciDevices, pciDevices_size, dev) < 0) ABORT_OOM(); VIR_FREE(devpath); } +static struct pciDevice * +pci_device_find_by_id(const char *id) +{ + size_t i; + for (i = 0; i < pciDevices_size; i++) { + struct pciDevice *dev = pciDevices[i]; + + if (STREQ(dev->id, id)) + return dev; + } + + return NULL; +} + +static struct pciDevice * +pci_device_find_by_content(const char *path) +{ + char tmp[32]; + + if (pci_read_file(path, tmp, sizeof(tmp)) < 0) + return NULL; + + return pci_device_find_by_id(tmp); +} + +static int +pci_device_autobind(struct pciDevice *dev) +{ + struct pciDriver *driver = pci_driver_find_by_dev(dev); + + if (!driver) { + /* No driver found. Nothing to do */ + return 0; + } + + return pci_driver_bind(driver, dev); +} + + +/* + * PCI Driver functions + */ +static void +pci_driver_new(const char *name, ...) +{ + struct pciDriver *driver; + va_list args; + int vendor, device; + char *driverpath; + + if (VIR_ALLOC_QUIET(driver) < 0 || + VIR_STRDUP_QUIET(driver->name, name) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfsdir, name) < 0) + ABORT_OOM(); + + if (virFileMakePath(driverpath) < 0) + ABORT("Unable to create: %s", driverpath); + + va_start(args, name); + + while ((vendor = va_arg(args, int)) != -1) { + if ((device = va_arg(args, int)) == -1) + ABORT("Invalid vendor device pair for driver %s", name); + + if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 || + VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0) + ABORT_OOM(); + + driver->vendor[driver->len] = vendor; + driver->device[driver->len] = device; + driver->len++; + } + + make_file(driverpath, "bind", NULL); + make_file(driverpath, "unbind", NULL); + make_file(driverpath, "new_id", NULL); + make_file(driverpath, "remove_id", NULL); + + if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, pciDrivers_size, driver) < 0) + ABORT_OOM(); +} + +static struct pciDriver * +pci_driver_find_by_dev(struct pciDevice *dev) +{ + size_t i; + + for (i = 0; i < pciDrivers_size; i++) { + struct pciDriver *driver = pciDrivers[i]; + size_t j; + + for (j = 0; j < driver->len; j++) { + if (driver->vendor[j] == dev->vendor && + driver->device[j] == dev->device) + return driver; + } + } + + return NULL; +} + +static struct pciDriver * +pci_driver_find_by_path(const char *path) +{ + size_t i; + + for (i = 0; i < pciDrivers_size; i++) { + struct pciDriver *driver = pciDrivers[i]; + + if (strstr(path, driver->name)) + return driver; + } + + return NULL; +} + +static int +pci_driver_bind(struct pciDriver *driver, + struct pciDevice *dev) +{ + int ret = -1; + char *devpath = NULL, *driverpath = NULL; + + if (dev->driver) { + /* Device already binded */ + errno = ENODEV; + return ret; + } + + /* Make symlink under device tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s", + fakesysfsdir, driver->name) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (symlink(driverpath, devpath) < 0) + goto cleanup; + + /* Make symlink under driver tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", + fakesysfsdir, driver->name, dev->id) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (symlink(devpath, driverpath) < 0) + goto cleanup; + + dev->driver = driver; + ret = 0; +cleanup: + VIR_FREE(devpath); + VIR_FREE(driverpath); + return ret; +} + +static int +pci_driver_unbind(struct pciDriver *driver, + struct pciDevice *dev) +{ + int ret = -1; + char *devpath = NULL, *driverpath = NULL; + + if (!dev->driver) { + /* Device not binded */ + errno = ENODEV; + return ret; + } + + /* Make symlink under device tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", + fakesysfsdir, driver->name, dev->id) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (unlink(devpath) < 0 || + unlink(driverpath) < 0) + goto cleanup; + + dev->driver = NULL; + ret = 0; +cleanup: + VIR_FREE(devpath); + VIR_FREE(driverpath); + return ret; +} + +static int +pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const char *path) +{ + int ret; + const char *file = last_component(path); + + if (STREQ(file, "bind")) { + /* handle write to bind */ + ret = pci_driver_handle_bind(path); + } else if (STREQ(file, "unbind")) { + /* handle write to unbind */ + ret = pci_driver_handle_unbind(path); + } else if (STREQ(file, "new_id")) { + /* handle write to new_id */ + ret = pci_driver_handle_new_id(path); + } else if (STREQ(file, "remove_id")) { + /* handle write to remove_id */ + ret = pci_driver_handle_remove_id(path); + } else { + /* yet not handled write */ + ABORT("Not handled write to: %s", path); + } + return ret; +} + +static int +pci_driver_handle_bind(const char *path) +{ + int ret = -1; + struct pciDevice *dev = pci_device_find_by_content(path); + struct pciDriver *driver = pci_driver_find_by_path(path); + + if (!driver || !dev) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + ret = pci_driver_bind(driver, dev); +cleanup: + return ret; +} + +static int +pci_driver_handle_unbind(const char *path) +{ + int ret = -1; + struct pciDevice *dev = pci_device_find_by_content(path); + + if (!dev || !dev->driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + ret = pci_driver_unbind(dev->driver, dev); +cleanup: + return ret; +} +static int +pci_driver_handle_new_id(const char *path) +{ + int ret = -1; + struct pciDriver *driver = pci_driver_find_by_path(path); + size_t i; + int vendor, device; + char buf[32]; + + if (!driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + if (pci_read_file(path, buf, sizeof(buf)) < 0) + goto cleanup; + + if (sscanf(buf, "%x %x", &vendor, &device) < 2) { + errno = EINVAL; + goto cleanup; + } + + for (i = 0; i < driver->len; i++) { + if (driver->vendor[i] == vendor && + driver->device[i] == device) + break; + } + + if (i == driver->len) { + if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 || + VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0) { + errno = ENOMEM; + goto cleanup; + } + + driver->vendor[driver->len] = vendor; + driver->device[driver->len] = device; + driver->len++; + } + + for (i = 0; i < pciDevices_size; i++) { + struct pciDevice *dev = pciDevices[i]; + + if (!dev->driver && + dev->vendor == vendor && + dev->device == device && + pci_driver_bind(driver, dev) < 0) + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} + +static int +pci_driver_handle_remove_id(const char *path) +{ + int ret = -1; + struct pciDriver *driver = pci_driver_find_by_path(path); + size_t i; + int vendor, device; + char buf[32]; + + if (!driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + if (pci_read_file(path, buf, sizeof(buf)) < 0) + goto cleanup; + + if (sscanf(buf, "%x %x", &vendor, &device) < 2) { + errno = EINVAL; + goto cleanup; + } + + for (i = 0; i < driver->len; i++) { + if (driver->vendor[i] == vendor && + driver->device[i] == device) + continue; + } + + if (i != driver->len) { + if (VIR_DELETE_ELEMENT(driver->vendor, i, driver->len) < 0) + goto cleanup; + driver->len++; + if (VIR_DELETE_ELEMENT(driver->device, i, driver->len) < 0) + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} + /* * Functions to load the symbols and init the environment @@ -195,7 +708,9 @@ init_syms(void) LOAD_SYM(access); LOAD_SYM_ALT(lstat, __lxstat); + LOAD_SYM(canonicalize_file_name); LOAD_SYM(open); + LOAD_SYM(close); } static void @@ -207,6 +722,13 @@ init_env(void) if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); +# define MAKE_PCI_DRIVER(name, ...) \ + pci_driver_new(name, __VA_ARGS__, -1, -1) + + MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044); + MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047); + MAKE_PCI_DRIVER("pci-stub", -1, -1); + # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \ do { \ struct pciDevice dev = {.id = (char *)Id, .vendor = Vendor, \ @@ -215,6 +737,9 @@ init_env(void) } while (0) MAKE_PCI_DEVICE("0000:00:00.0", 0x8086, 0x0044); + MAKE_PCI_DEVICE("0000:00:01.0", 0x8086, 0x0044); + MAKE_PCI_DEVICE("0000:00:02.0", 0x8086, 0x0046); + MAKE_PCI_DEVICE("0000:00:03.0", 0x8086, 0x0048); } @@ -281,6 +806,25 @@ lstat(const char *path, struct stat *sb) return ret; } +char * +canonicalize_file_name(const char *path) +{ + char *ret; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + char *newpath; + if (getrealpath(&newpath, path) < 0) + return NULL; + ret = realcanonicalize_file_name(newpath); + VIR_FREE(newpath); + } else { + ret = realcanonicalize_file_name(path); + } + return ret; +} + int open(const char *path, int flags, ...) { @@ -303,10 +847,26 @@ open(const char *path, int flags, ...) } else { ret = realopen(newpath ? newpath : path, flags); } + + /* Catch both: /sys/bus/pci/drivers/... and + * /sys/bus/pci/device/.../driver/... */ + if (ret >= 0 && STRPREFIX(path, PCI_SYSFS_PREFIX) && + strstr(path, "driver") && add_fd(ret, path) < 0) { + realclose(ret); + ret = -1; + } + VIR_FREE(newpath); return ret; } +int +close(int fd) +{ + if (remove_fd(fd) < 0) + return -1; + return realclose(fd); +} #else /* Nothing to override on non-__linux__ platforms */ #endif diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 96f11d6..6740a5d 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -60,6 +60,49 @@ cleanup: return ret; } +# define CHECK_LIST_COUNT(list, cnt) \ + if ((count = virPCIDeviceListCount(list)) != cnt) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "Unexpected count of items in " #list ": %d, " \ + "expecting " #cnt, count); \ + goto cleanup; \ + } + +static int +testVirPCIDeviceDetach(const void *oaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + virPCIDevicePtr dev; + virPCIDeviceListPtr activeDevs = NULL, inactiveDevs = NULL; + int count; + + if (!(dev = virPCIDeviceNew(0, 0, 1, 0)) || + !(activeDevs = virPCIDeviceListNew()) || + !(inactiveDevs = virPCIDeviceListNew())) + goto cleanup; + + CHECK_LIST_COUNT(activeDevs, 0); + CHECK_LIST_COUNT(inactiveDevs, 0); + + if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0) + goto cleanup; + + if (virPCIDeviceDetach(dev, activeDevs, inactiveDevs) < 0) + goto cleanup; + + CHECK_LIST_COUNT(activeDevs, 0); + CHECK_LIST_COUNT(inactiveDevs, 1); + + /* @dev is in @inactiveDevs, don't double free it */ + dev = NULL; + ret = 0; +cleanup: + virPCIDeviceFree(dev); + virObjectUnref(activeDevs); + virObjectUnref(inactiveDevs); + return ret; +} + static int mymain(void) { @@ -84,6 +127,7 @@ mymain(void) } while (0) DO_TEST(testVirPCIDeviceNew); + DO_TEST(testVirPCIDeviceDetach); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.1.5

On Fri, Oct 25, 2013 at 02:03:42PM +0100, Michal Privoznik wrote:
This commit introduces yet another test under virpcitest: virPCIDeviceDetach. However, in order to be able to do this, the virpcimock needs to be extended to model the kernel behavior on PCI device binding and unbinding (create 'driver' symlinks under the device tree, check for device ID in driver's ID table, etc.)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- cfg.mk | 2 +- tests/Makefile.am | 10 +- tests/virpcimock.c | 562 ++++++++++++++++++++++++++++++++++++++++++++++++++++- tests/virpcitest.c | 44 +++++ 4 files changed, 613 insertions(+), 5 deletions(-)
diff --git a/cfg.mk b/cfg.mk index 15114a9..306b052 100644 --- a/cfg.mk +++ b/cfg.mk @@ -964,7 +964,7 @@ exclude_file_name_regexp--sc_prohibit_strdup = \ ^(docs/|examples/|python/|src/util/virstring\.c|tests/virnetserverclientmock.c$$)
exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vircgroupmock\.c)$$) + (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vir(cgroup|pci)mock\.c)$$)
exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|nodeinfo)data/|\.(gif|ico|png|diff)$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6d3245b..2c2e5a9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -48,12 +48,15 @@ if WITH_DTRACE_PROBES PROBES_O += ../src/libvirt_probes.lo endif WITH_DTRACE_PROBES
+GNULIB_LIBS = \ + ../gnulib/lib/libgnu.la + LDADDS = \ $(WARN_CFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(PROBES_O) \ - ../src/libvirt.la \ - ../gnulib/lib/libgnu.la + $(GNULIB_LIBS) \ + ../src/libvirt.la
EXTRA_DIST = \ capabilityschemadata \ @@ -751,7 +754,8 @@ virpcitest_LDADD = $(LDADDS) virpcimock_la_SOURCES = \ virpcimock.c virpcimock_la_CFLAGS = $(AM_CFLAGS) -virpcimock_la_LIBADD = ../src/libvirt.la +virpcimock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la virpcimock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation
diff --git a/tests/virpcimock.c b/tests/virpcimock.c index d545361..16a5ff3 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -32,11 +32,14 @@ # include "viralloc.h" # include "virstring.h" # include "virfile.h" +# include "dirname.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 char *(*realcanonicalize_file_name)(const char *path); static int (*realopen)(const char *path, int flags, ...); +static int (*realclose)(int fd);
/* Don't make static, since it causes problems with clang * when passed as an arg to virAsprintf() @@ -64,7 +67,30 @@ char *fakesysfsdir; * * 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. + * created by us. There are some actions that we must take if some special + * files are written to. Here's the list of files we watch: + * + * /sys/bus/pci/drivers/<driver>/new_id: + * Learn the driver new vendor and device combination. + * Data in format "VVVV DDDD". + * + * /sys/bus/pci/drivers/<driver>/remove_id + * Make the driver forget about vendor and device. + * Data in format "VVVV DDDD". + * + * /sys/bus/pci/drivers/<driver>/bind + * Check if driver supports the device and bind driver to it (create symlink + * called 'driver' pointing to the /sys/but/pci/drivers/<driver>). + * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). + * + * /sys/bus/pci/drivers/<driver>/unbind + * Unbind driver from the device. + * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). + * + * As a little hack, we are not mocking write to these files, but close() + * instead. The advantage is we don't need any self growing array to hold the + * partial writes and construct them back. We can let all the writes finish, + * and then just read the file content back. */
/* @@ -73,18 +99,51 @@ char *fakesysfsdir; * */
+struct pciDriver { + char *name; + int *vendor; /* List of vendor:device IDs the driver can handle */ + int *device; + size_t len; /* @len is used for both @vendor and @device */ +}; +
Don't you want to use few typedefs to save a bunch of 'struct' keywords?
struct pciDevice { char *id; int vendor; int device; + struct pciDriver *driver; /* Driver attached. NULL if attached to no driver */ +}; + +struct fdCallback { + int fd; + char *path; };
struct pciDevice **pciDevices = NULL; size_t pciDevices_size = 0;
+struct pciDriver **pciDrivers = NULL; +size_t pciDrivers_size = 0; + +struct fdCallback *callbacks = NULL; +size_t callbacks_size = 0; + static void init_env(void);
+static int pci_device_autobind(struct pciDevice *dev); static void pci_device_new_from_stub(const struct pciDevice *data); +static struct pciDevice *pci_device_find_by_id(const char *id); +static struct pciDevice *pci_device_find_by_content(const char *path); + +static void pci_driver_new(const char *name, ...); +static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev); +static struct pciDriver *pci_driver_find_by_path(const char *path); +static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev); +static int pci_driver_unbind(struct pciDriver *driver, struct pciDevice *dev); +static int pci_driver_handle_change(int fd, const char *path); +static int pci_driver_handle_bind(const char *path); +static int pci_driver_handle_unbind(const char *path); +static int pci_driver_handle_new_id(const char *path); +static int pci_driver_handle_remove_id(const char *path);
/* @@ -112,6 +171,41 @@ make_file(const char *path, }
static int +pci_read_file(const char *path, + char *buf, + size_t buf_size) +{ + int ret = -1; + int fd = -1; + char *newpath; + + if (virAsprintfQuiet(&newpath, "%s/%s", + fakesysfsdir, + path + strlen(PCI_SYSFS_PREFIX)) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if ((fd = realopen(newpath, O_RDWR)) < 0) + goto cleanup; + + bzero(buf, buf_size); + if (saferead(fd, buf, buf_size - 1) < 0) { + STDERR("Unable to read from %s", newpath); + goto cleanup; + } + + if (ftruncate(fd, 0) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(newpath); + realclose(fd); + return ret; +} + +static int getrealpath(char **newpath, const char *path) { @@ -133,6 +227,70 @@ getrealpath(char **newpath, return 0; }
+static bool +find_fd(int fd, size_t *indx) +{ + size_t i; + + for (i = 0; i < callbacks_size; i++) { + if (callbacks[i].fd == fd) { + if (indx) + *indx = i; + return true; + } + } + + return false; +} + +static int +add_fd(int fd, const char *path) +{ + int ret = -1; + size_t i; + + if (find_fd(fd, &i)) { + struct fdCallback cb = callbacks[i]; + ABORT("FD %d %s already present in the array as %d %s", + fd, path, cb.fd, cb.path); + } + + if (VIR_REALLOC_N_QUIET(callbacks, callbacks_size + 1) < 0 || + VIR_STRDUP_QUIET(callbacks[callbacks_size].path, path) < 0) { + errno = ENOMEM; + goto cleanup; + } + + callbacks[callbacks_size++].fd = fd; + ret = 0; +cleanup: + return ret; +} + +static int +remove_fd(int fd) +{ + int ret = -1; + size_t i; + + if (find_fd(fd, &i)) { + struct fdCallback cb = callbacks[i]; + + if (pci_driver_handle_change(cb.fd, cb.path) < 0) + goto cleanup; + + VIR_FREE(cb.path); + if (VIR_DELETE_ELEMENT(callbacks, i, callbacks_size) < 0) { + errno = EINVAL; + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} +
/* * PCI Device functions @@ -163,12 +321,367 @@ pci_device_new_from_stub(const struct pciDevice *data) ABORT("@tmp overflow"); make_file(devpath, "device", tmp);
+ if (pci_device_autobind(dev) < 0) + ABORT("Unable to bind: %s", data->id); + if (VIR_APPEND_ELEMENT_QUIET(pciDevices, pciDevices_size, dev) < 0) ABORT_OOM();
VIR_FREE(devpath); }
+static struct pciDevice * +pci_device_find_by_id(const char *id) +{ + size_t i; + for (i = 0; i < pciDevices_size; i++) { + struct pciDevice *dev = pciDevices[i]; + + if (STREQ(dev->id, id)) + return dev; + } + + return NULL; +} + +static struct pciDevice * +pci_device_find_by_content(const char *path) +{ + char tmp[32]; + + if (pci_read_file(path, tmp, sizeof(tmp)) < 0) + return NULL; + + return pci_device_find_by_id(tmp); +} + +static int +pci_device_autobind(struct pciDevice *dev) +{ + struct pciDriver *driver = pci_driver_find_by_dev(dev); + + if (!driver) { + /* No driver found. Nothing to do */ + return 0; + } + + return pci_driver_bind(driver, dev); +} + + +/* + * PCI Driver functions + */ +static void +pci_driver_new(const char *name, ...) +{ + struct pciDriver *driver; + va_list args; + int vendor, device; + char *driverpath; + + if (VIR_ALLOC_QUIET(driver) < 0 || + VIR_STRDUP_QUIET(driver->name, name) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfsdir, name) < 0) + ABORT_OOM(); + + if (virFileMakePath(driverpath) < 0) + ABORT("Unable to create: %s", driverpath); + + va_start(args, name); + + while ((vendor = va_arg(args, int)) != -1) { + if ((device = va_arg(args, int)) == -1) + ABORT("Invalid vendor device pair for driver %s", name); + + if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 || + VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0) + ABORT_OOM(); + + driver->vendor[driver->len] = vendor; + driver->device[driver->len] = device; + driver->len++; + } + + make_file(driverpath, "bind", NULL); + make_file(driverpath, "unbind", NULL); + make_file(driverpath, "new_id", NULL); + make_file(driverpath, "remove_id", NULL); + + if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, pciDrivers_size, driver) < 0) + ABORT_OOM(); +} + +static struct pciDriver * +pci_driver_find_by_dev(struct pciDevice *dev) +{ + size_t i; + + for (i = 0; i < pciDrivers_size; i++) { + struct pciDriver *driver = pciDrivers[i]; + size_t j; + + for (j = 0; j < driver->len; j++) { + if (driver->vendor[j] == dev->vendor && + driver->device[j] == dev->device) + return driver; + } + } + + return NULL; +} + +static struct pciDriver * +pci_driver_find_by_path(const char *path) +{ + size_t i; + + for (i = 0; i < pciDrivers_size; i++) { + struct pciDriver *driver = pciDrivers[i]; + + if (strstr(path, driver->name)) + return driver; + } + + return NULL; +} + +static int +pci_driver_bind(struct pciDriver *driver, + struct pciDevice *dev) +{ + int ret = -1; + char *devpath = NULL, *driverpath = NULL; + + if (dev->driver) { + /* Device already binded */ + errno = ENODEV; + return ret; + } + + /* Make symlink under device tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s", + fakesysfsdir, driver->name) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (symlink(driverpath, devpath) < 0) + goto cleanup; + + /* Make symlink under driver tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", + fakesysfsdir, driver->name, dev->id) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (symlink(devpath, driverpath) < 0) + goto cleanup; + + dev->driver = driver; + ret = 0; +cleanup: + VIR_FREE(devpath); + VIR_FREE(driverpath); + return ret; +} + +static int +pci_driver_unbind(struct pciDriver *driver, + struct pciDevice *dev) +{ + int ret = -1; + char *devpath = NULL, *driverpath = NULL; + + if (!dev->driver) { + /* Device not binded */ + errno = ENODEV; + return ret; + } + + /* Make symlink under device tree */ + if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", + fakesysfsdir, dev->id) < 0 || + virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", + fakesysfsdir, driver->name, dev->id) < 0) { + errno = ENOMEM; + goto cleanup; + } + + if (unlink(devpath) < 0 || + unlink(driverpath) < 0) + goto cleanup; + + dev->driver = NULL; + ret = 0; +cleanup: + VIR_FREE(devpath); + VIR_FREE(driverpath); + return ret; +} + +static int +pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const char *path) +{ + int ret; + const char *file = last_component(path); + + if (STREQ(file, "bind")) { + /* handle write to bind */ + ret = pci_driver_handle_bind(path); + } else if (STREQ(file, "unbind")) { + /* handle write to unbind */ + ret = pci_driver_handle_unbind(path); + } else if (STREQ(file, "new_id")) { + /* handle write to new_id */ + ret = pci_driver_handle_new_id(path); + } else if (STREQ(file, "remove_id")) { + /* handle write to remove_id */ + ret = pci_driver_handle_remove_id(path); + } else { + /* yet not handled write */ + ABORT("Not handled write to: %s", path); + } + return ret; +} + +static int +pci_driver_handle_bind(const char *path) +{ + int ret = -1; + struct pciDevice *dev = pci_device_find_by_content(path); + struct pciDriver *driver = pci_driver_find_by_path(path); + + if (!driver || !dev) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + ret = pci_driver_bind(driver, dev); +cleanup: + return ret; +} + +static int +pci_driver_handle_unbind(const char *path) +{ + int ret = -1; + struct pciDevice *dev = pci_device_find_by_content(path); + + if (!dev || !dev->driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + ret = pci_driver_unbind(dev->driver, dev); +cleanup: + return ret; +} +static int +pci_driver_handle_new_id(const char *path) +{ + int ret = -1; + struct pciDriver *driver = pci_driver_find_by_path(path); + size_t i; + int vendor, device; + char buf[32]; + + if (!driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + if (pci_read_file(path, buf, sizeof(buf)) < 0) + goto cleanup; + + if (sscanf(buf, "%x %x", &vendor, &device) < 2) { + errno = EINVAL; + goto cleanup; + } + + for (i = 0; i < driver->len; i++) { + if (driver->vendor[i] == vendor && + driver->device[i] == device) + break; + } + + if (i == driver->len) { + if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 || + VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0) { + errno = ENOMEM; + goto cleanup; + } + + driver->vendor[driver->len] = vendor; + driver->device[driver->len] = device; + driver->len++; + } + + for (i = 0; i < pciDevices_size; i++) { + struct pciDevice *dev = pciDevices[i]; + + if (!dev->driver && + dev->vendor == vendor && + dev->device == device && + pci_driver_bind(driver, dev) < 0) + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} + +static int +pci_driver_handle_remove_id(const char *path) +{ + int ret = -1; + struct pciDriver *driver = pci_driver_find_by_path(path); + size_t i; + int vendor, device; + char buf[32]; + + if (!driver) { + /* This should never happen (TM) */ + errno = ENODEV; + goto cleanup; + } + + if (pci_read_file(path, buf, sizeof(buf)) < 0) + goto cleanup; + + if (sscanf(buf, "%x %x", &vendor, &device) < 2) { + errno = EINVAL; + goto cleanup; + } + + for (i = 0; i < driver->len; i++) { + if (driver->vendor[i] == vendor && + driver->device[i] == device) + continue; + } + + if (i != driver->len) { + if (VIR_DELETE_ELEMENT(driver->vendor, i, driver->len) < 0) + goto cleanup; + driver->len++; + if (VIR_DELETE_ELEMENT(driver->device, i, driver->len) < 0) + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} +
/* * Functions to load the symbols and init the environment @@ -195,7 +708,9 @@ init_syms(void)
LOAD_SYM(access); LOAD_SYM_ALT(lstat, __lxstat); + LOAD_SYM(canonicalize_file_name); LOAD_SYM(open); + LOAD_SYM(close); }
static void @@ -207,6 +722,13 @@ init_env(void) if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n");
+# define MAKE_PCI_DRIVER(name, ...) \ + pci_driver_new(name, __VA_ARGS__, -1, -1) + + MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044); + MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047); + MAKE_PCI_DRIVER("pci-stub", -1, -1); + # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \ do { \ struct pciDevice dev = {.id = (char *)Id, .vendor = Vendor, \ @@ -215,6 +737,9 @@ init_env(void) } while (0)
MAKE_PCI_DEVICE("0000:00:00.0", 0x8086, 0x0044); + MAKE_PCI_DEVICE("0000:00:01.0", 0x8086, 0x0044); + MAKE_PCI_DEVICE("0000:00:02.0", 0x8086, 0x0046); + MAKE_PCI_DEVICE("0000:00:03.0", 0x8086, 0x0048); }
@@ -281,6 +806,25 @@ lstat(const char *path, struct stat *sb) return ret; }
+char * +canonicalize_file_name(const char *path) +{ + char *ret; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + char *newpath; + if (getrealpath(&newpath, path) < 0) + return NULL; + ret = realcanonicalize_file_name(newpath); + VIR_FREE(newpath); + } else { + ret = realcanonicalize_file_name(path); + } + return ret; +} + int open(const char *path, int flags, ...) { @@ -303,10 +847,26 @@ open(const char *path, int flags, ...) } else { ret = realopen(newpath ? newpath : path, flags); } + + /* Catch both: /sys/bus/pci/drivers/... and + * /sys/bus/pci/device/.../driver/... */ + if (ret >= 0 && STRPREFIX(path, PCI_SYSFS_PREFIX) && + strstr(path, "driver") && add_fd(ret, path) < 0) { + realclose(ret); + ret = -1; + } +
Wouldn't it be safer to canonicalize the path (which should be done in other functions anyway, since there could be a call to 'open("/sys//bus/pci/...", ...)', which this code would miss? Not that we're doing this somewhere right now, but there's no performance impact when it is done in tests and it's a bit future-safer. [...] Other than that, this patch looks OK, so ACK (conditional; when [1/3] gets in). Martin

This test will reattach the PCI device detached in the previous test. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virpcitest.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 6740a5d..727cad2 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -104,6 +104,42 @@ cleanup: } static int +testVirPCIDeviceReattach(const void *oaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + virPCIDevicePtr dev; + virPCIDeviceListPtr activeDevs = NULL, inactiveDevs = NULL; + int count; + + if (!(dev = virPCIDeviceNew(0, 0, 1, 0)) || + !(activeDevs = virPCIDeviceListNew()) || + !(inactiveDevs = virPCIDeviceListNew())) + goto cleanup; + + if (virPCIDeviceListAdd(inactiveDevs, dev) < 0) + goto cleanup; + + CHECK_LIST_COUNT(activeDevs, 0); + CHECK_LIST_COUNT(inactiveDevs, 1); + + if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0) + goto cleanup; + + if (virPCIDeviceReattach(dev, activeDevs, inactiveDevs) < 0) + goto cleanup; + + CHECK_LIST_COUNT(activeDevs, 0); + CHECK_LIST_COUNT(inactiveDevs, 0); + + dev = NULL; + ret = 0; +cleanup: + virPCIDeviceFree(dev); + virObjectUnref(activeDevs); + virObjectUnref(inactiveDevs); + return ret; +} +static int mymain(void) { int ret = 0; @@ -128,6 +164,7 @@ mymain(void) DO_TEST(testVirPCIDeviceNew); DO_TEST(testVirPCIDeviceDetach); + DO_TEST(testVirPCIDeviceReattach); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.1.5

On Fri, Oct 25, 2013 at 02:03:43PM +0100, Michal Privoznik wrote:
This test will reattach the PCI device detached in the previous test.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virpcitest.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
Looks OK, ACK under same condition as [2/3] (when [1/3] gets in). Martin
participants (2)
-
Martin Kletzander
-
Michal Privoznik