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(a)redhat.com&gt; --- .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 1b2fd46..89b5613 100644 --- a/cfg.mk +++ b/cfg.mk @@ -946,7 +946,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(a)redhat.com&gt; + */ + +#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(a)redhat.com&gt; + */ + +#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

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(a)redhat.com&gt; --- cfg.mk | 2 +- tests/Makefile.am | 10 +- tests/virpcimock.c | 603 ++++++++++++++++++++++++++++++++++++++++++++++++++++- tests/virpcitest.c | 42 ++++ 4 files changed, 652 insertions(+), 5 deletions(-) diff --git a/cfg.mk b/cfg.mk index 89b5613..f210057 100644 --- a/cfg.mk +++ b/cfg.mk @@ -968,7 +968,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..2adc337 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -32,11 +32,16 @@ # 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 int (*realstat)(const char *path, struct stat *sb); +static int (*real__xstat)(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 +69,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 +101,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 +173,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 +229,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 +323,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 +710,10 @@ init_syms(void) LOAD_SYM(access); LOAD_SYM_ALT(lstat, __lxstat); + LOAD_SYM_ALT(stat, __xstat); + LOAD_SYM(canonicalize_file_name); LOAD_SYM(open); + LOAD_SYM(close); } static void @@ -207,6 +725,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 +740,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); } @@ -282,6 +810,63 @@ lstat(const char *path, struct stat *sb) } int +__xstat(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__xstat(ver, newpath, sb); + VIR_FREE(newpath); + } else { + ret = real__xstat(ver, path, sb); + } + return ret; +} + +int +stat(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 = realstat(newpath, sb); + VIR_FREE(newpath); + } else { + ret = realstat(path, 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, ...) { int ret; @@ -303,10 +888,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..3eaa469 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -60,6 +60,47 @@ 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); + + ret = 0; +cleanup: + virPCIDeviceFree(dev); + virObjectUnref(activeDevs); + virObjectUnref(inactiveDevs); + return ret; +} + static int mymain(void) { @@ -84,6 +125,7 @@ mymain(void) } while (0) DO_TEST(testVirPCIDeviceNew); + DO_TEST(testVirPCIDeviceDetach); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.1.5

This test will reattach the PCI device detached in the previous test. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/virpcitest.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 3eaa469..8811add 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -102,6 +102,42 @@ cleanup: } static int +testVirPCIDeviceReattach(const void *oaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + virPCIDevicePtr dev = NULL; + 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) { + virPCIDeviceFree(dev); + 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); + + ret = 0; +cleanup: + virObjectUnref(activeDevs); + virObjectUnref(inactiveDevs); + return ret; +} +static int mymain(void) { int ret = 0; @@ -126,6 +162,7 @@ mymain(void) DO_TEST(testVirPCIDeviceNew); DO_TEST(testVirPCIDeviceDetach); + DO_TEST(testVirPCIDeviceReattach); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.1.5

If a PCI deivce is not binded to any driver (e.g. there's yet no PCI driver in the linux kernel) but still users want to passthru the device we fail the whole operation as we fail to resolve the 'driver' link under the PCI device sysfs tree. Obviously, this is not a fatal error and it shouldn't be error at all. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/virpci.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 65d7168..0727085 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1095,10 +1095,16 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, const char *newDriverName = NULL; if (virPCIDriverDir(&stubDriverPath, stubDriverName) < 0 || - virPCIFile(&driverLink, dev->name, "driver") < 0 || - virPCIDeviceGetDriverPathAndName(dev, &oldDriverPath, - &oldDriverName) < 0) { + virPCIFile(&driverLink, dev->name, "driver") < 0) goto cleanup; + + if (virPCIDeviceGetDriverPathAndName(dev, &oldDriverPath, + &oldDriverName) < 0) { + /* It's okay if device is not binded to any driver. If that's the case, + * there's no /sys/bus/pci/devices/.../driver symlink. */ + if (errno != ENOENT) + goto cleanup; + virResetLastError(); } if (virFileExists(driverLink)) { -- 1.8.1.5

On 10/31/2013 01:23 PM, Michal Privoznik wrote: The problem with fixing it like this is that the log will already have an error message, and we'll end up getting a bug report about it even though the operation is successful. I think this should instead be fixed by having virPCIDeviceDriverPathAndName() return success with NULL path and name in the case that the device isn't bound to anything, then check for that specific condition in all the callers of virPCIDeviceDriverPathAndName(), and change the behavior accordingly. In virPCIDeviceBindToStub and virPCIDeviceReset, no change would be needed. In virPCIDeviceUnbindFromStub, I'm not sure what would be the appropriate action - doing a "drivers_reprobe" might be correct, but depending on what led to the current state, that might end up re-binding the stub driver; it almost seems that what's needed in that case is to do the reprobe, then do the entire function a 2nd time in case the first try caused a re-bind to the stub (and then generate an error if it has failed after the 2nd time).