Re: [libvirt] [PATCH 2/3] virpcitest: Test virPCIDeviceDetach

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 | 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; + } +