[PATCH 1/4] virpci: changed the work with PCI via libpciaccess
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/meson.build | 1 + src/util/virpci.c | 464 ++++++++++------------------------------ tests/qemuhotplugtest.c | 11 +- tests/qemumemlocktest.c | 9 + tests/qemuxmlconftest.c | 9 + tests/virpcimock.c | 21 +- 6 files changed, 160 insertions(+), 355 deletions(-) diff --git a/src/meson.build b/src/meson.build index 9413192a55..39788ac4d7 100644 --- a/src/meson.build +++ b/src/meson.build @@ -411,6 +411,7 @@ libvirt_lib = shared_library( dtrace_gen_objects, dependencies: [ src_dep, + pciaccess_dep ], link_args: libvirt_link_args, link_whole: [ diff --git a/src/util/virpci.c b/src/util/virpci.c index 90617e69c6..3f95fa2b3f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -29,6 +29,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> +#include <pciaccess.h> #ifdef __linux__ # include <sys/utsname.h> @@ -72,7 +73,7 @@ struct _virPCIDevice { char *name; /* domain:bus:slot.function */ char id[PCI_ID_LEN]; /* product vendor */ - char *path; + struct pci_device *device; /* The driver:domain which uses the device */ char *used_by_drvname; @@ -359,121 +360,6 @@ virPCIDeviceGetCurrentDriverNameAndType(virPCIDevice *dev, } -static int -virPCIDeviceConfigOpenInternal(virPCIDevice *dev, bool readonly, bool fatal) -{ - int fd; - - fd = open(dev->path, readonly ? O_RDONLY : O_RDWR); - - if (fd < 0) { - if (fatal) { - virReportSystemError(errno, - _("Failed to open config space file '%1$s'"), - dev->path); - } else { - VIR_WARN("Failed to open config space file '%s': %s", - dev->path, g_strerror(errno)); - } - return -1; - } - - VIR_DEBUG("%s %s: opened %s", dev->id, dev->name, dev->path); - return fd; -} - -static int -virPCIDeviceConfigOpen(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, true, true); -} - -static int -virPCIDeviceConfigOpenTry(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, true, false); -} - -static int -virPCIDeviceConfigOpenWrite(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, false, true); -} - -static void -virPCIDeviceConfigClose(virPCIDevice *dev, int cfgfd) -{ - if (VIR_CLOSE(cfgfd) < 0) { - VIR_WARN("Failed to close config space file '%s': %s", - dev->path, g_strerror(errno)); - } -} - - -static int -virPCIDeviceRead(virPCIDevice *dev, - int cfgfd, - unsigned int pos, - uint8_t *buf, - unsigned int buflen) -{ - memset(buf, 0, buflen); - errno = 0; - - if (lseek(cfgfd, pos, SEEK_SET) != pos || - saferead(cfgfd, buf, buflen) != buflen) { - VIR_DEBUG("Failed to read %u bytes at %u from '%s' : %s", - buflen, pos, dev->path, g_strerror(errno)); - return -1; - } - return 0; -} - - -/** - * virPCIDeviceReadN: - * @dev: virPCIDevice object (used only to log name of config file) - * @cfgfd: open file descriptor for device config file in sysfs - * @pos: byte offset in the file to read from - * - * read "N" (where "N" is "8", "16", or "32", and appears at the end - * of the function name) bytes from a PCI device's already-opened - * sysfs config file and return them as the return value from the - * function. - * - * Returns the value at @pos in the file, or 0 if there was an - * error. NB: since 0 could be a valid value, occurrence of an error - * must be determined by examining errno. errno is always reset to 0 - * before the seek/read is attempted (see virPCIDeviceRead()), so if - * errno != 0 on return from one of these functions, then either the - * seek or the read operation failed for some reason. If errno == 0 - * and the return value is 0, then the config file really does contain - * the value 0 at @pos. - */ -static uint8_t -virPCIDeviceRead8(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf; - virPCIDeviceRead(dev, cfgfd, pos, &buf, sizeof(buf)); - return buf; -} - -static uint16_t -virPCIDeviceRead16(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf[2]; - virPCIDeviceRead(dev, cfgfd, pos, &buf[0], sizeof(buf)); - return (buf[0] << 0) | (buf[1] << 8); -} - -static uint32_t -virPCIDeviceRead32(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf[4]; - virPCIDeviceRead(dev, cfgfd, pos, &buf[0], sizeof(buf)); - return (buf[0] << 0) | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24); -} - static int virPCIDeviceReadClass(virPCIDevice *dev, uint16_t *device_class) { @@ -499,36 +385,6 @@ virPCIDeviceReadClass(virPCIDevice *dev, uint16_t *device_class) return 0; } -static int -virPCIDeviceWrite(virPCIDevice *dev, - int cfgfd, - unsigned int pos, - uint8_t *buf, - unsigned int buflen) -{ - if (lseek(cfgfd, pos, SEEK_SET) != pos || - safewrite(cfgfd, buf, buflen) != buflen) { - VIR_WARN("Failed to write to '%s' : %s", dev->path, - g_strerror(errno)); - return -1; - } - return 0; -} - -static void -virPCIDeviceWrite16(virPCIDevice *dev, int cfgfd, unsigned int pos, uint16_t val) -{ - uint8_t buf[2] = { (val >> 0), (val >> 8) }; - virPCIDeviceWrite(dev, cfgfd, pos, &buf[0], sizeof(buf)); -} - -static void -virPCIDeviceWrite32(virPCIDevice *dev, int cfgfd, unsigned int pos, uint32_t val) -{ - uint8_t buf[4] = { (val >> 0), (val >> 8), (val >> 16), (val >> 24) }; - virPCIDeviceWrite(dev, cfgfd, pos, &buf[0], sizeof(buf)); -} - typedef int (*virPCIDeviceIterPredicate)(virPCIDevice *, virPCIDevice *, void *); @@ -610,7 +466,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, */ static int virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, - int cfgfd, unsigned int capability, unsigned int *offset) { @@ -619,11 +474,13 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, *offset = 0; /* assume failure (*nothing* can be at offset 0) */ - status = virPCIDeviceRead16(dev, cfgfd, PCI_STATUS); + pci_device_cfg_read_u16(dev->device, &status, PCI_STATUS); + if (errno != 0 || !(status & PCI_STATUS_CAP_LIST)) goto error; - pos = virPCIDeviceRead8(dev, cfgfd, PCI_CAPABILITY_LIST); + pci_device_cfg_read_u8(dev->device, &pos, PCI_CAPABILITY_LIST); + if (errno != 0) goto error; @@ -635,7 +492,9 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, * capabilities here. */ while (pos >= PCI_CONF_HEADER_LEN && pos != 0xff) { - uint8_t capid = virPCIDeviceRead8(dev, cfgfd, pos); + uint8_t capid; + pci_device_cfg_read_u8(dev->device, &capid, pos); + if (errno != 0) goto error; @@ -646,7 +505,8 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, return 0; } - pos = virPCIDeviceRead8(dev, cfgfd, pos + 1); + pci_device_cfg_read_u8(dev->device, &pos, pos + 1); + if (errno != 0) goto error; } @@ -665,7 +525,6 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, static unsigned int virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, - int cfgfd, unsigned int capability) { int ttl; @@ -677,7 +536,7 @@ virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, pos = PCI_EXT_CAP_BASE; while (ttl > 0 && pos >= PCI_EXT_CAP_BASE) { - header = virPCIDeviceRead32(dev, cfgfd, pos); + header = pci_device_cfg_read_u32(dev->device, &header, pos); if ((header & PCI_EXT_CAP_ID_MASK) == capability) return pos; @@ -693,7 +552,7 @@ virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, * not have FLR, 1 if it does, and -1 on error */ static bool -virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev) { uint32_t caps; unsigned int pos; @@ -707,7 +566,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * on SR-IOV NICs at the moment. */ if (dev->pcie_cap_pos) { - caps = virPCIDeviceRead32(dev, cfgfd, dev->pcie_cap_pos + PCI_EXP_DEVCAP); + pci_device_cfg_read_u32(dev->device, &caps, dev->pcie_cap_pos + PCI_EXP_DEVCAP); if (caps & PCI_EXP_DEVCAP_FLR) { VIR_DEBUG("%s %s: detected PCIe FLR capability", dev->id, dev->name); return true; @@ -718,11 +577,13 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * the same thing, except for conventional PCI * devices. This is not common yet. */ - if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF, &pos) < 0) + if (virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_AF, &pos) < 0) goto error; if (pos) { - caps = virPCIDeviceRead16(dev, cfgfd, pos + PCI_AF_CAP); + uint16_t caps16; + pci_device_cfg_read_u16(dev->device, &caps16, pos + PCI_AF_CAP); + caps = caps16; if (caps & PCI_AF_CAP_FLR) { VIR_DEBUG("%s %s: detected PCI FLR capability", dev->id, dev->name); return true; @@ -754,13 +615,13 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * internal reset, not just a soft reset. */ static bool -virPCIDeviceDetectPowerManagementReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceDetectPowerManagementReset(virPCIDevice *dev) { if (dev->pci_pm_cap_pos) { uint32_t ctl; /* require the NO_SOFT_RESET bit is clear */ - ctl = virPCIDeviceRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL); + pci_device_cfg_read_u32(dev->device, &ctl, dev->pci_pm_cap_pos + PCI_PM_CTRL); if (!(ctl & PCI_PM_CTRL_NO_SOFT_RESET)) { VIR_DEBUG("%s %s: detected PM reset capability", dev->id, dev->name); return true; @@ -811,26 +672,22 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) uint8_t header_type, secondary, subordinate; virPCIDevice **best = data; int ret = 0; - int fd; if (dev->address.domain != check->address.domain) return 0; - if ((fd = virPCIDeviceConfigOpenTry(check)) < 0) - return 0; - /* Is it a bridge? */ ret = virPCIDeviceReadClass(check, &device_class); if (ret < 0 || device_class != PCI_CLASS_BRIDGE_PCI) - goto cleanup; + return ret; /* Is it a plane? */ - header_type = virPCIDeviceRead8(check, fd, PCI_HEADER_TYPE); + pci_device_cfg_read_u8(check->device, &header_type, PCI_HEADER_TYPE); if ((header_type & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_BRIDGE) - goto cleanup; + return ret; - secondary = virPCIDeviceRead8(check, fd, PCI_SECONDARY_BUS); - subordinate = virPCIDeviceRead8(check, fd, PCI_SUBORDINATE_BUS); + pci_device_cfg_read_u8(check->device, &secondary, PCI_SECONDARY_BUS); + pci_device_cfg_read_u8(check->device, &subordinate, PCI_SUBORDINATE_BUS); VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name); @@ -838,8 +695,7 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) * the direct parent. No further work is necessary */ if (dev->address.bus == secondary) { - ret = 1; - goto cleanup; + return 1; } /* otherwise, SRIOV allows VFs to be on different buses than their PFs. @@ -850,35 +706,26 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) if (*best == NULL) { *best = virPCIDeviceNew(&check->address); if (*best == NULL) { - ret = -1; - goto cleanup; + return -1; } } else { /* OK, we had already recorded a previous "best" match for the * parent. See if the current device is more restrictive than the * best, and if so, make it the new best */ - int bestfd; uint8_t best_secondary; - if ((bestfd = virPCIDeviceConfigOpenTry(*best)) < 0) - goto cleanup; - best_secondary = virPCIDeviceRead8(*best, bestfd, PCI_SECONDARY_BUS); - virPCIDeviceConfigClose(*best, bestfd); + pci_device_cfg_read_u8((*best)->device, &best_secondary, PCI_SECONDARY_BUS); if (secondary > best_secondary) { virPCIDeviceFree(*best); *best = virPCIDeviceNew(&check->address); if (*best == NULL) { - ret = -1; - goto cleanup; + return -1; } } } } - - cleanup: - virPCIDeviceConfigClose(check, fd); return ret; } @@ -902,15 +749,14 @@ virPCIDeviceGetParent(virPCIDevice *dev, virPCIDevice **parent) */ static int virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, - int cfgfd, virPCIDeviceList *inactiveDevs) { g_autoptr(virPCIDevice) parent = NULL; g_autoptr(virPCIDevice) conflict = NULL; uint8_t config_space[PCI_CONF_LEN]; uint16_t ctl; - int ret = -1; - int parentfd; + pciaddr_t bytes_read; + pciaddr_t bytes_written; /* Refuse to do a secondary bus reset if there are other * devices/functions behind the bus are used by the host @@ -932,8 +778,6 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, dev->name); return -1; } - if ((parentfd = virPCIDeviceConfigOpenWrite(parent)) < 0) - goto out; VIR_DEBUG("%s %s: doing a secondary bus reset", dev->id, dev->name); @@ -941,38 +785,37 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, * for the supplied device since we refuse to do a reset if there * are multiple devices/functions */ - if (virPCIDeviceRead(dev, cfgfd, 0, config_space, PCI_CONF_LEN) < 0) { + pci_device_cfg_read(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_read); + if (bytes_read < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to read PCI config space for %1$s"), dev->name); - goto out; + return -1; } /* Read the control register, set the reset flag, wait 200ms, * unset the reset flag and wait 200ms. */ - ctl = virPCIDeviceRead16(dev, parentfd, PCI_BRIDGE_CONTROL); - virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, - ctl | PCI_BRIDGE_CTL_RESET); + pci_device_cfg_read_u16(parent->device, &ctl, PCI_BRIDGE_CONTROL); + + pci_device_cfg_write_u16(parent->device, ctl | PCI_BRIDGE_CTL_RESET, PCI_BRIDGE_CONTROL); g_usleep(200 * 1000); /* sleep 200ms */ - virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, ctl); + pci_device_cfg_write_u16(parent->device, ctl, PCI_BRIDGE_CONTROL); g_usleep(200 * 1000); /* sleep 200ms */ - if (virPCIDeviceWrite(dev, cfgfd, 0, config_space, PCI_CONF_LEN) < 0) { + pci_device_cfg_write(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_written); + if (bytes_written < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restore PCI config space for %1$s"), dev->name); - goto out; + return -1; } - ret = 0; - out: - virPCIDeviceConfigClose(parent, parentfd); - return ret; + return 0; } /* Power management reset attempts to reset a device using a @@ -980,16 +823,19 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, * above we require the device supports a full internal reset. */ static int -virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceTryPowerManagementReset(virPCIDevice *dev) { uint8_t config_space[PCI_CONF_LEN]; uint32_t ctl; + pciaddr_t bytes_read; + pciaddr_t bytes_written; if (!dev->pci_pm_cap_pos) return -1; /* Save and restore the device's config space. */ - if (virPCIDeviceRead(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) { + pci_device_cfg_read(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_read); + if (bytes_read < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to read PCI config space for %1$s"), dev->name); @@ -998,20 +844,19 @@ virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd) VIR_DEBUG("%s %s: doing a power management reset", dev->id, dev->name); - ctl = virPCIDeviceRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL); + pci_device_cfg_read_u32(dev->device, &ctl, dev->pci_pm_cap_pos + PCI_PM_CTRL); ctl &= ~PCI_PM_CTRL_STATE_MASK; - virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, - ctl | PCI_PM_CTRL_STATE_D3hot); + pci_device_cfg_write_u32(dev->device, ctl | PCI_PM_CTRL_STATE_D3hot, dev->pci_pm_cap_pos + PCI_PM_CTRL); g_usleep(10 * 1000); /* sleep 10ms */ - virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, - ctl | PCI_PM_CTRL_STATE_D0); + pci_device_cfg_write_u32(dev->device, ctl | PCI_PM_CTRL_STATE_D0, dev->pci_pm_cap_pos + PCI_PM_CTRL); g_usleep(10 * 1000); /* sleep 10ms */ - if (virPCIDeviceWrite(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) { + pci_device_cfg_write(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_written); + if (bytes_written < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restore PCI config space for %1$s"), dev->name); @@ -1046,10 +891,10 @@ virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd) * Always returns success (0) (for now) */ static int -virPCIDeviceInit(virPCIDevice *dev, int cfgfd) +virPCIDeviceInit(virPCIDevice *dev) { dev->is_pcie = false; - if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP, &dev->pcie_cap_pos) < 0) { + if (virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_EXP, &dev->pcie_cap_pos) < 0) { /* an unprivileged process is unable to read *all* of a * device's PCI config (it can only read the first 64 * bytes, which isn't enough for see the Express @@ -1065,18 +910,13 @@ virPCIDeviceInit(virPCIDevice *dev, int cfgfd) * -1), then we blindly assume the most likely outcome - * PCIe. */ - off_t configLen = virFileLength(virPCIDeviceGetConfigPath(dev), -1); - - if (configLen != 256) - dev->is_pcie = true; - } else { dev->is_pcie = (dev->pcie_cap_pos != 0); } - virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM, &dev->pci_pm_cap_pos); - dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd); - dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd); + virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_PM, &dev->pci_pm_cap_pos); + dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev); + dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev); return 0; } @@ -1089,7 +929,6 @@ virPCIDeviceReset(virPCIDevice *dev, g_autofree char *drvName = NULL; virPCIStubDriver drvType; int ret = -1; - int fd = -1; int hdrType = -1; if (virPCIGetHeaderType(dev, &hdrType) < 0) @@ -1114,29 +953,26 @@ virPCIDeviceReset(virPCIDevice *dev, * be redundant. */ if (virPCIDeviceGetCurrentDriverNameAndType(dev, &drvName, &drvType) < 0) - goto cleanup; + return -1; if (drvType == VIR_PCI_STUB_DRIVER_VFIO) { VIR_DEBUG("Device %s is bound to %s - skip reset", dev->name, drvName); ret = 0; - goto cleanup; + return 0; } VIR_DEBUG("Resetting device %s", dev->name); - if ((fd = virPCIDeviceConfigOpenWrite(dev)) < 0) - goto cleanup; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + if (virPCIDeviceInit(dev) < 0) + return -1; /* KVM will perform FLR when starting and stopping * a guest, so there is no need for us to do it here. */ if (dev->has_flr) { ret = 0; - goto cleanup; + return 0; } /* If the device supports PCI power management reset, @@ -1144,11 +980,11 @@ virPCIDeviceReset(virPCIDevice *dev, * the function, not the whole device. */ if (dev->has_pm_reset) - ret = virPCIDeviceTryPowerManagementReset(dev, fd); + ret = virPCIDeviceTryPowerManagementReset(dev); /* Bus reset is not an option with the root bus */ if (ret < 0 && dev->address.bus != 0) - ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs); + ret = virPCIDeviceTrySecondaryBusReset(dev, inactiveDevs); if (ret < 0) { virErrorPtr err = virGetLastError(); @@ -1159,8 +995,6 @@ virPCIDeviceReset(virPCIDevice *dev, _("no FLR, PM reset or bus reset available")); } - cleanup: - virPCIDeviceConfigClose(dev, fd); return ret; } @@ -1756,28 +1590,6 @@ virPCIDeviceReattach(virPCIDevice *dev, return 0; } -static char * -virPCIDeviceReadID(virPCIDevice *dev, const char *id_name) -{ - g_autofree char *path = NULL; - g_autofree char *id_str = NULL; - - path = virPCIFile(dev->name, id_name); - - /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ - if (virFileReadAll(path, 7, &id_str) < 0) - return NULL; - - /* Check for 0x suffix */ - if (id_str[0] != '0' || id_str[1] != 'x') - return NULL; - - /* Chop off the newline; we know the string is 7 bytes */ - id_str[6] = '\0'; - - return g_steal_pointer(&id_str); -} - bool virPCIDeviceAddressIsValid(virPCIDeviceAddress *addr, bool report) @@ -1865,9 +1677,9 @@ virPCIDeviceExists(const virPCIDeviceAddress *addr) virPCIDevice * virPCIDeviceNew(const virPCIDeviceAddress *address) { + struct pci_device * device; + g_autoptr(virPCIDevice) dev = NULL; - g_autofree char *vendor = NULL; - g_autofree char *product = NULL; dev = g_new0(virPCIDevice, 1); @@ -1875,31 +1687,21 @@ virPCIDeviceNew(const virPCIDeviceAddress *address) dev->name = virPCIDeviceAddressAsString(&dev->address); - dev->path = g_strdup_printf(PCI_SYSFS "devices/%s/config", dev->name); - - if (!virFileExists(dev->path)) { - virReportSystemError(errno, - _("Device %1$s not found: could not access %2$s"), - dev->name, dev->path); + pci_system_init(); + device = pci_device_find_by_slot(address->domain, address->bus, address->slot, address->function); + if (!device) return NULL; - } - - vendor = virPCIDeviceReadID(dev, "vendor"); - product = virPCIDeviceReadID(dev, "device"); - - if (!vendor || !product) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to read product/vendor ID for %1$s"), - dev->name); + dev->device = g_memdup(device, sizeof(*dev->device)); + if (!dev->device) { + virReportSystemError(errno, + _("Not found device domain: %1$d, bus: %2$d, slot: %3$d, function: %4$d"), + address->domain, address->bus, address->slot, address->function); return NULL; } - - /* strings contain '0x' prefix */ - if (g_snprintf(dev->id, sizeof(dev->id), "%s %s", &vendor[2], - &product[2]) >= sizeof(dev->id)) { + if (g_snprintf(dev->id, sizeof(dev->id), "%x %x", dev->device->vendor_id, dev->device->device_id) >= sizeof(dev->id)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("dev->id buffer overflow: %1$s %2$s"), - &vendor[2], &product[2]); + _("dev->id buffer overflow: %1$d %2$d"), + dev->device->vendor_id, dev->device->device_id); return NULL; } @@ -1918,10 +1720,10 @@ virPCIDeviceCopy(virPCIDevice *dev) /* shallow copy to take care of most attributes */ *copy = *dev; - copy->path = NULL; - copy->used_by_drvname = copy->used_by_domname = NULL; + copy->device = NULL; + copy->device = g_memdup(dev->device, sizeof(*dev->device)); + copy->name = copy->used_by_drvname = copy->used_by_domname = copy->stubDriverName = NULL; copy->name = g_strdup(dev->name); - copy->path = g_strdup(dev->path); copy->used_by_drvname = g_strdup(dev->used_by_drvname); copy->used_by_domname = g_strdup(dev->used_by_domname); copy->stubDriverName = g_strdup(dev->stubDriverName); @@ -1936,10 +1738,11 @@ virPCIDeviceFree(virPCIDevice *dev) return; VIR_DEBUG("%s %s: freeing", dev->id, dev->name); g_free(dev->name); - g_free(dev->path); g_free(dev->used_by_drvname); g_free(dev->used_by_domname); g_free(dev->stubDriverName); + if (dev->device) + g_free(dev->device); g_free(dev); } @@ -1971,9 +1774,9 @@ virPCIDeviceGetName(virPCIDevice *dev) * config file. */ const char * -virPCIDeviceGetConfigPath(virPCIDevice *dev) +virPCIDeviceGetConfigPath(virPCIDevice *dev G_GNUC_UNUSED) { - return dev->path; + return NULL; } void virPCIDeviceSetManaged(virPCIDevice *dev, bool managed) @@ -2484,47 +2287,37 @@ virPCIDeviceDownstreamLacksACS(virPCIDevice *dev) uint16_t flags; uint16_t ctrl; unsigned int pos; - int fd; - int ret = 0; uint16_t device_class; - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) + if (virPCIDeviceInit(dev) < 0) { return -1; - - if (virPCIDeviceInit(dev, fd) < 0) { - ret = -1; - goto cleanup; } if (virPCIDeviceReadClass(dev, &device_class) < 0) - goto cleanup; + return 0; pos = dev->pcie_cap_pos; if (!pos || device_class != PCI_CLASS_BRIDGE_PCI) - goto cleanup; + return 0; - flags = virPCIDeviceRead16(dev, fd, pos + PCI_EXP_FLAGS); + pci_device_cfg_read_u16(dev->device, &flags, pos + PCI_EXP_FLAGS); if (((flags & PCI_EXP_FLAGS_TYPE) >> 4) != PCI_EXP_TYPE_DOWNSTREAM) - goto cleanup; + return 0; - pos = virPCIDeviceFindExtendedCapabilityOffset(dev, fd, PCI_EXT_CAP_ID_ACS); + pos = virPCIDeviceFindExtendedCapabilityOffset(dev, PCI_EXT_CAP_ID_ACS); if (!pos) { VIR_DEBUG("%s %s: downstream port lacks ACS", dev->id, dev->name); - ret = 1; - goto cleanup; + return 1; } - ctrl = virPCIDeviceRead16(dev, fd, pos + PCI_EXT_ACS_CTRL); + pci_device_cfg_read_u16(dev->device, &ctrl, pos + PCI_EXT_ACS_CTRL); if ((ctrl & PCI_EXT_CAP_ACS_ENABLED) != PCI_EXT_CAP_ACS_ENABLED) { VIR_DEBUG("%s %s: downstream port has ACS disabled", dev->id, dev->name); - ret = 1; - goto cleanup; + return 1; } - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return 0; } static int @@ -3189,48 +2982,27 @@ virPCIDeviceGetVPD(virPCIDevice *dev G_GNUC_UNUSED) int virPCIDeviceIsPCIExpress(virPCIDevice *dev) { - int fd; - int ret = -1; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; - - ret = dev->is_pcie; + if (virPCIDeviceInit(dev) < 0) + return -1; - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return dev->is_pcie; } int virPCIDeviceHasPCIExpressLink(virPCIDevice *dev) { - int fd; - int ret = -1; uint16_t cap, type; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + if (virPCIDeviceInit(dev) < 0) + return -1; if (dev->pcie_cap_pos == 0) { - ret = 0; - goto cleanup; + return 0; } - cap = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_CAP_FLAGS); + pci_device_cfg_read_u16(dev->device, &cap, dev->pcie_cap_pos + PCI_CAP_FLAGS); type = (cap & PCI_EXP_FLAGS_TYPE) >> 4; - ret = type != PCI_EXP_TYPE_ROOT_INT_EP && type != PCI_EXP_TYPE_ROOT_EC; - - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return type != PCI_EXP_TYPE_ROOT_INT_EP && type != PCI_EXP_TYPE_ROOT_EC; } int @@ -3242,53 +3014,39 @@ virPCIDeviceGetLinkCapSta(virPCIDevice *dev, unsigned int *sta_width) { uint32_t t; - int fd; - int ret = -1; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + uint16_t t16; + if (virPCIDeviceInit(dev) < 0) + return -1; if (!dev->pcie_cap_pos) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pci device %1$s is not a PCI-Express device"), dev->name); - goto cleanup; + return -1; } - t = virPCIDeviceRead32(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKCAP); + pci_device_cfg_read_u32(dev->device, &t, dev->pcie_cap_pos + PCI_EXP_LNKCAP); *cap_port = t >> 24; *cap_speed = t & PCI_EXP_LNKCAP_SPEED; *cap_width = (t & PCI_EXP_LNKCAP_WIDTH) >> 4; - t = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKSTA); + pci_device_cfg_read_u16(dev->device, &t16, dev->pcie_cap_pos + PCI_EXP_LNKSTA); + t = t16; *sta_speed = t & PCI_EXP_LNKSTA_SPEED; *sta_width = (t & PCI_EXP_LNKSTA_WIDTH) >> 4; - ret = 0; - - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return 0; } int virPCIGetHeaderType(virPCIDevice *dev, int *hdrType) { - int fd; uint8_t type; *hdrType = -1; - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return -1; - - type = virPCIDeviceRead8(dev, fd, PCI_HEADER_TYPE); - - virPCIDeviceConfigClose(dev, fd); + pci_device_cfg_read_u8(dev->device, &type, PCI_HEADER_TYPE); type &= PCI_HEADER_TYPE_MASK; if (type >= VIR_PCI_HEADER_LAST) { diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d2a1f5acf1..fd8339bd30 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -18,12 +18,14 @@ */ #include <config.h> +#include "testutils.h" + +#ifdef __linux__ #include "qemu/qemu_alias.h" #include "qemu/qemu_conf.h" #include "qemu/qemu_hotplug.h" #include "qemumonitortestutils.h" -#include "testutils.h" #include "testutilsqemu.h" #include "testutilsqemuschema.h" #include "virhostdev.h" @@ -816,3 +818,10 @@ VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("domaincaps"), VIR_TEST_MOCK("virprocess"), VIR_TEST_MOCK("qemuhotplug")); +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index a2888732f7..5f62c80665 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -7,6 +7,8 @@ #include "testutils.h" +#ifdef __linux__ + #ifdef WITH_QEMU # include "internal.h" @@ -137,3 +139,10 @@ main(void) } #endif /* WITH_QEMU */ +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 1f0068864a..6f84812109 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -7,6 +7,8 @@ #include "testutils.h" +#ifdef __linux__ + #ifdef WITH_QEMU # include "internal.h" @@ -3051,3 +3053,10 @@ int main(void) } #endif /* WITH_QEMU */ +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 5b923c63ce..2fa12903d2 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -22,7 +22,7 @@ #include "virpcivpdpriv.h" -#if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__) +#if defined(__linux__) || defined(__APPLE__) # define VIR_MOCK_LOOKUP_MAIN # include "virmock.h" # include "virpci.h" @@ -42,6 +42,10 @@ static int (*real___open_2)(const char *path, int flags); static int (*real_close)(int fd); static DIR * (*real_opendir)(const char *name); static char *(*real_virFileCanonicalizePath)(const char *path); +static int (*real_scandir)(const char *restrict dirp, + struct dirent ***restrict namelist, + typeof(int(const struct dirent *)) *filter, + typeof(int(const struct dirent **, const struct dirent **)) *compar); static char *fakerootdir; @@ -955,6 +959,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(opendir); # endif VIR_MOCK_REAL_INIT(virFileCanonicalizePath); + VIR_MOCK_REAL_INIT(scandir); } static void @@ -1172,6 +1177,20 @@ virFileCanonicalizePath(const char *path) return real_virFileCanonicalizePath(newpath); } +int scandir(const char *restrict dirp, struct dirent ***restrict namelist, + typeof(int(const struct dirent *)) *filter, + typeof(int(const struct dirent **, const struct dirent **)) *compar) +{ + g_autofree char *newpath = NULL; + + init_syms(); + + if (getrealpath(&newpath, dirp) < 0) + return -1; + + return real_scandir(newpath, namelist, filter, compar); +} + # include "virmockstathelpers.c" #else -- 2.47.1
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/bhyve/bhyve_capabilities.c | 2 +- src/conf/node_device_conf.c | 2 +- src/node_device/node_device_driver.c | 2 +- src/node_device/node_device_udev.c | 2 ++ src/util/virmdev.c | 2 +- src/util/virpci.c | 2 +- tests/domaincapsdata/bhyve_basic.x86_64.xml | 3 ++- tests/domaincapsdata/bhyve_fbuf.x86_64.xml | 3 ++- tests/domaincapsdata/bhyve_uefi.x86_64.xml | 3 ++- 9 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index b065256cf0..fcef91c435 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -108,7 +108,7 @@ virBhyveDomainCapsFill(virDomainCaps *caps, VIR_DOMAIN_CAPS_ENUM_SET(caps->video.modelType, VIR_DOMAIN_VIDEO_TYPE_GOP); } - caps->hostdev.supported = VIR_TRISTATE_BOOL_NO; + caps->hostdev.supported = VIR_TRISTATE_BOOL_YES; caps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS] = VIR_TRISTATE_BOOL_NO; caps->features[VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO] = VIR_TRISTATE_BOOL_NO; caps->features[VIR_DOMAIN_CAPS_FEATURE_GENID] = VIR_TRISTATE_BOOL_NO; diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 88486a5f2d..0a1d649a40 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -3107,7 +3107,7 @@ virNodeDeviceSyncMdevActiveConfig(virNodeDeviceDef *def) } } -#ifdef __linux__ +#if defined(__linux__) || defined(__FreeBSD__) int virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHost *scsi_host) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 123b16a292..592f72fe52 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -111,7 +111,7 @@ int nodeConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) return 1; } -#if defined (__linux__) && defined(WITH_UDEV) +#if (defined(__linux__) || defined(__FreeBSD__)) && defined(WITH_UDEV) /* NB: It was previously believed that changes in driver name were * relayed to libvirt as "change" events by udev, and the udev event * notification is setup to recognize such events and effectively diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 344b39e97a..e5dc021a6a 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -432,6 +432,7 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state, char *p; bool privileged = false; +#ifndef __FreeBSD__ linkpath = g_strdup_printf("%s/config", udev_device_get_syspath(device)); if (virFileWaitForExists(linkpath, 10, 100) < 0) { virReportSystemError(errno, @@ -439,6 +440,7 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state, linkpath); goto cleanup; } +#endif VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) { privileged = driver_state->privileged; diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 6ecdbdf0ab..3a07ba75f2 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -565,7 +565,7 @@ virMediatedDeviceParentGetAddress(const char *sysfspath, return -1; } -#ifdef __linux__ +#if defined(__linux__) || defined(__FreeBSD__) ssize_t virMediatedDeviceGetMdevTypes(const char *sysfspath, diff --git a/src/util/virpci.c b/src/util/virpci.c index 3f95fa2b3f..7acdc56545 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2482,7 +2482,7 @@ virPCIGetVirtualFunctions(const char *sysfs_path, } -#ifdef __linux__ +#if defined(__linux__) || defined(__FreeBSD__) virPCIDeviceAddress * virPCIGetDeviceAddressFromSysfsLink(const char *device_link) diff --git a/tests/domaincapsdata/bhyve_basic.x86_64.xml b/tests/domaincapsdata/bhyve_basic.x86_64.xml index dd054577c0..fb9bf54a92 100644 --- a/tests/domaincapsdata/bhyve_basic.x86_64.xml +++ b/tests/domaincapsdata/bhyve_basic.x86_64.xml @@ -26,7 +26,8 @@ </disk> <graphics supported='no'/> <video supported='no'/> - <hostdev supported='no'/> + <hostdev supported='yes'> + </hostdev> </devices> <features> <gic supported='no'/> diff --git a/tests/domaincapsdata/bhyve_fbuf.x86_64.xml b/tests/domaincapsdata/bhyve_fbuf.x86_64.xml index 0b1d9c17d7..1ec5706aed 100644 --- a/tests/domaincapsdata/bhyve_fbuf.x86_64.xml +++ b/tests/domaincapsdata/bhyve_fbuf.x86_64.xml @@ -43,7 +43,8 @@ <value>gop</value> </enum> </video> - <hostdev supported='no'/> + <hostdev supported='yes'> + </hostdev> </devices> <features> <gic supported='no'/> diff --git a/tests/domaincapsdata/bhyve_uefi.x86_64.xml b/tests/domaincapsdata/bhyve_uefi.x86_64.xml index 69fff197a7..f76cf91acb 100644 --- a/tests/domaincapsdata/bhyve_uefi.x86_64.xml +++ b/tests/domaincapsdata/bhyve_uefi.x86_64.xml @@ -35,7 +35,8 @@ </disk> <graphics supported='no'/> <video supported='no'/> - <hostdev supported='no'/> + <hostdev supported='yes'> + </hostdev> </devices> <features> <gic supported='no'/> -- 2.47.1
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/bhyve/bhyve_command.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index bc287307c8..5da88ec9bd 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -152,6 +152,30 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd) return 0; } +static int +bhyveBuildHostdevArgStr(const virDomainDef *def, virCommand *cmd) +{ + size_t i; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDef *hostdev = def->hostdevs[i]; + virDomainHostdevSubsys *subsys = &hostdev->source.subsys; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported hostdev")); + return -1; + } + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d,passthru,%d/%d/%d", + hostdev->info->addr.pci.slot, subsys->u.pci.addr.bus, + subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + } + return 0; +} + static int bhyveBuildAHCIControllerArgStr(const virDomainDef *def, virDomainControllerDef *controller, @@ -819,6 +843,9 @@ virBhyveProcessBuildBhyveCmd(struct _bhyveConn *driver, virDomainDef *def, virCommandAddArg(cmd, bhyvecmd->args[i]); } + if (bhyveBuildHostdevArgStr(def, cmd) < 0) + return NULL; + virCommandAddArg(cmd, def->name); return g_steal_pointer(&cmd); -- 2.47.1
On Wed, Feb 12, 2025 at 09:46:09AM +0300, Alexander Shursha wrote:
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/bhyve/bhyve_command.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
This should come with additions to bhyvexml2argvtest.c data files.
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index bc287307c8..5da88ec9bd 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -152,6 +152,30 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd) return 0; }
+static int +bhyveBuildHostdevArgStr(const virDomainDef *def, virCommand *cmd) +{ + size_t i; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDef *hostdev = def->hostdevs[i]; + virDomainHostdevSubsys *subsys = &hostdev->source.subsys; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported hostdev")); + return -1; + }
We should probably be diagnosing this in bhyveDomainDeviceDefValidate so it gets reported to the user much earlier.
+ virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d,passthru,%d/%d/%d", + hostdev->info->addr.pci.slot, subsys->u.pci.addr.bus, + subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + } + return 0; +} + static int bhyveBuildAHCIControllerArgStr(const virDomainDef *def, virDomainControllerDef *controller, @@ -819,6 +843,9 @@ virBhyveProcessBuildBhyveCmd(struct _bhyveConn *driver, virDomainDef *def, virCommandAddArg(cmd, bhyvecmd->args[i]); }
+ if (bhyveBuildHostdevArgStr(def, cmd) < 0) + return NULL; + virCommandAddArg(cmd, def->name);
return g_steal_pointer(&cmd); -- 2.47.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> writes:
src/bhyve/bhyve_command.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
This should come with additions to bhyvexml2argvtest.c data files.
Done
+static int +bhyveBuildHostdevArgStr(const virDomainDef *def, virCommand *cmd) +{ + size_t i; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDef *hostdev = def->hostdevs[i]; + virDomainHostdevSubsys *subsys = &hostdev->source.subsys; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported hostdev")); + return -1; + }
We should probably be diagnosing this in bhyveDomainDeviceDefValidate so it gets reported to the user much earlier.
Done. -- Alexander Shursha
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/bhyve/bhyve_parse_command.c | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 29d3a678bf..14916c401d 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -639,6 +639,63 @@ bhyveParsePCIFbuf(virDomainDef *def, return -1; } +static int +bhyveParsePassthru(virDomainDef *def G_GNUC_UNUSED, + unsigned pcibus, + unsigned pcislot, + unsigned pcifunction, + char *addr) +{ + /* -s slot,bus/slot/function */ + /* -s slot,pcibus:slot:function */ + virDomainHostdevDef *hostdev = NULL; + g_auto(GStrv) params = NULL; + GStrv param; + char *p = NULL; + + if (!(hostdev = virDomainHostdevDefNew())) + return 0; + + hostdev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + hostdev->info->addr.pci.bus = pcibus; + hostdev->info->addr.pci.slot = pcislot; + hostdev->info->addr.pci.function = pcifunction; + + if (!addr) + goto error; + + if (!(params = g_strsplit(addr, ":", -1))) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + } + if (g_str_equal(addr, *params)) + if (!(params = g_strsplit(addr, "/", -1))) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + } + if (g_strv_length(params) != 3) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + } + param = params; + hostdev->source.subsys.u.pci.addr.bus = g_ascii_strtoull(*param++, &p, 10); + hostdev->source.subsys.u.pci.addr.slot = g_ascii_strtoull(*param++, &p, 10); + hostdev->source.subsys.u.pci.addr.function = g_ascii_strtoull(*param, &p, 10); + + hostdev->source.subsys.u.pci.addr.domain = 0; + hostdev->managed = false; + + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); + return 0; + + error: + virDomainHostdevDefFree(hostdev); + return -1; +} + static int bhyveParseBhyvePCIArg(virDomainDef *def, virDomainXMLOption *xmlopt, @@ -703,6 +760,8 @@ bhyveParseBhyvePCIArg(virDomainDef *def, VIR_DOMAIN_NET_MODEL_E1000, conf); else if (STREQ(emulation, "fbuf")) bhyveParsePCIFbuf(def, xmlopt, caps, bus, slot, function, conf); + else if (STREQ(emulation, "passthru")) + bhyveParsePassthru(def, bus, slot, function, conf); VIR_FREE(emulation); VIR_FREE(slotdef); -- 2.47.1
On Wed, Feb 12, 2025 at 09:46:10AM +0300, Alexander Shursha wrote:
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/bhyve/bhyve_parse_command.c | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
Should come with additions to bhyveargv2xmltest.c data files
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 29d3a678bf..14916c401d 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -639,6 +639,63 @@ bhyveParsePCIFbuf(virDomainDef *def, return -1; }
+static int +bhyveParsePassthru(virDomainDef *def G_GNUC_UNUSED, + unsigned pcibus, + unsigned pcislot, + unsigned pcifunction, + char *addr) +{ + /* -s slot,bus/slot/function */ + /* -s slot,pcibus:slot:function */ + virDomainHostdevDef *hostdev = NULL; + g_auto(GStrv) params = NULL; + GStrv param; + char *p = NULL; + + if (!(hostdev = virDomainHostdevDefNew())) + return 0;
This method can't fail so don't check for NULL.
+ + hostdev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + hostdev->info->addr.pci.bus = pcibus; + hostdev->info->addr.pci.slot = pcislot; + hostdev->info->addr.pci.function = pcifunction; + + if (!addr) + goto error; + + if (!(params = g_strsplit(addr, ":", -1))) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + } + if (g_str_equal(addr, *params)) + if (!(params = g_strsplit(addr, "/", -1))) {
This overwrites the pointer currently stored in 'params' without free'ing it. The 'g_auto' will only free data when it goes out of scope, not when the pointer is overwritten directly.
+ virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + }
Add {} on the outer if too for clarity.
+ if (g_strv_length(params) != 3) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + }
+ param = params; + hostdev->source.subsys.u.pci.addr.bus = g_ascii_strtoull(*param++, &p, 10); + hostdev->source.subsys.u.pci.addr.slot = g_ascii_strtoull(*param++, &p, 10); + hostdev->source.subsys.u.pci.addr.function = g_ascii_strtoull(*param, &p, 10); + + hostdev->source.subsys.u.pci.addr.domain = 0; + hostdev->managed = false; + + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); + return 0; + + error: + virDomainHostdevDefFree(hostdev); + return -1; +} + static int bhyveParseBhyvePCIArg(virDomainDef *def, virDomainXMLOption *xmlopt, @@ -703,6 +760,8 @@ bhyveParseBhyvePCIArg(virDomainDef *def, VIR_DOMAIN_NET_MODEL_E1000, conf); else if (STREQ(emulation, "fbuf")) bhyveParsePCIFbuf(def, xmlopt, caps, bus, slot, function, conf); + else if (STREQ(emulation, "passthru")) + bhyveParsePassthru(def, bus, slot, function, conf);
VIR_FREE(emulation); VIR_FREE(slotdef); -- 2.47.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> writes:
src/bhyve/bhyve_parse_command.c | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
Should come with additions to bhyveargv2xmltest.c data files
Done
+ if (!(hostdev = virDomainHostdevDefNew())) + return 0;
This method can't fail so don't check for NULL.
Done
+ if (!(params = g_strsplit(addr, ":", -1))) { + virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + } + if (g_str_equal(addr, *params)) + if (!(params = g_strsplit(addr, "/", -1))) {
This overwrites the pointer currently stored in 'params' without free'ing it. The 'g_auto' will only free data when it goes out of scope, not when the pointer is overwritten directly.
Done
+ virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to parse PCI address %1$s"), addr); + goto error; + }
Add {} on the outer if too for clarity.
Done. -- Alexander Shursha
On Wed, Feb 12, 2025 at 09:46:07 +0300, Alexander Shursha wrote:
Sponsored by: Future Crew, LLC Signed-off-by: Alexander Shursha <kekek2@ya.ru>
As mentioned in the PR upstream the commit message serves as a place to describe the patch and most importantly the justification/why it's done, rather than serving as advertisment space. (You can keep the sponsorship line, but adding a proper commit message will be mandatory)
On Wed, Feb 12, 2025 at 09:46:07AM +0300, Alexander Shursha wrote: Please provide a commit message that describes the problem you're solving with this refactoring. Likewise for other commits in this series which all have empty commit messages.
Sponsored by: Future Crew, LLC
While it is nice that they're sponsoring you to work on this, IMHO, we don't need to be advertizing commercial relationships in the commit log. It suffices to put such a note in the cover letter when sending a patch series.
Signed-off-by: Alexander Shursha <kekek2@ya.ru> --- src/meson.build | 1 + src/util/virpci.c | 464 ++++++++++------------------------------ tests/qemuhotplugtest.c | 11 +- tests/qemumemlocktest.c | 9 + tests/qemuxmlconftest.c | 9 + tests/virpcimock.c | 21 +- 6 files changed, 160 insertions(+), 355 deletions(-)
diff --git a/src/meson.build b/src/meson.build index 9413192a55..39788ac4d7 100644 --- a/src/meson.build +++ b/src/meson.build @@ -411,6 +411,7 @@ libvirt_lib = shared_library( dtrace_gen_objects, dependencies: [ src_dep, + pciaccess_dep ], link_args: libvirt_link_args, link_whole: [ diff --git a/src/util/virpci.c b/src/util/virpci.c index 90617e69c6..3f95fa2b3f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -29,6 +29,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> +#include <pciaccess.h>
#ifdef __linux__ # include <sys/utsname.h> @@ -72,7 +73,7 @@ struct _virPCIDevice {
char *name; /* domain:bus:slot.function */ char id[PCI_ID_LEN]; /* product vendor */ - char *path; + struct pci_device *device;
/* The driver:domain which uses the device */ char *used_by_drvname; @@ -359,121 +360,6 @@ virPCIDeviceGetCurrentDriverNameAndType(virPCIDevice *dev, }
-static int -virPCIDeviceConfigOpenInternal(virPCIDevice *dev, bool readonly, bool fatal) -{ - int fd; - - fd = open(dev->path, readonly ? O_RDONLY : O_RDWR); - - if (fd < 0) { - if (fatal) { - virReportSystemError(errno, - _("Failed to open config space file '%1$s'"), - dev->path); - } else { - VIR_WARN("Failed to open config space file '%s': %s", - dev->path, g_strerror(errno)); - } - return -1; - } - - VIR_DEBUG("%s %s: opened %s", dev->id, dev->name, dev->path); - return fd; -} - -static int -virPCIDeviceConfigOpen(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, true, true); -} - -static int -virPCIDeviceConfigOpenTry(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, true, false); -} - -static int -virPCIDeviceConfigOpenWrite(virPCIDevice *dev) -{ - return virPCIDeviceConfigOpenInternal(dev, false, true); -} - -static void -virPCIDeviceConfigClose(virPCIDevice *dev, int cfgfd) -{ - if (VIR_CLOSE(cfgfd) < 0) { - VIR_WARN("Failed to close config space file '%s': %s", - dev->path, g_strerror(errno)); - } -} - - -static int -virPCIDeviceRead(virPCIDevice *dev, - int cfgfd, - unsigned int pos, - uint8_t *buf, - unsigned int buflen) -{ - memset(buf, 0, buflen); - errno = 0; - - if (lseek(cfgfd, pos, SEEK_SET) != pos || - saferead(cfgfd, buf, buflen) != buflen) { - VIR_DEBUG("Failed to read %u bytes at %u from '%s' : %s", - buflen, pos, dev->path, g_strerror(errno)); - return -1; - } - return 0; -} - - -/** - * virPCIDeviceReadN: - * @dev: virPCIDevice object (used only to log name of config file) - * @cfgfd: open file descriptor for device config file in sysfs - * @pos: byte offset in the file to read from - * - * read "N" (where "N" is "8", "16", or "32", and appears at the end - * of the function name) bytes from a PCI device's already-opened - * sysfs config file and return them as the return value from the - * function. - * - * Returns the value at @pos in the file, or 0 if there was an - * error. NB: since 0 could be a valid value, occurrence of an error - * must be determined by examining errno. errno is always reset to 0 - * before the seek/read is attempted (see virPCIDeviceRead()), so if - * errno != 0 on return from one of these functions, then either the - * seek or the read operation failed for some reason. If errno == 0 - * and the return value is 0, then the config file really does contain - * the value 0 at @pos. - */ -static uint8_t -virPCIDeviceRead8(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf; - virPCIDeviceRead(dev, cfgfd, pos, &buf, sizeof(buf)); - return buf; -} - -static uint16_t -virPCIDeviceRead16(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf[2]; - virPCIDeviceRead(dev, cfgfd, pos, &buf[0], sizeof(buf)); - return (buf[0] << 0) | (buf[1] << 8); -} - -static uint32_t -virPCIDeviceRead32(virPCIDevice *dev, int cfgfd, unsigned int pos) -{ - uint8_t buf[4]; - virPCIDeviceRead(dev, cfgfd, pos, &buf[0], sizeof(buf)); - return (buf[0] << 0) | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24); -} - static int virPCIDeviceReadClass(virPCIDevice *dev, uint16_t *device_class) { @@ -499,36 +385,6 @@ virPCIDeviceReadClass(virPCIDevice *dev, uint16_t *device_class) return 0; }
-static int -virPCIDeviceWrite(virPCIDevice *dev, - int cfgfd, - unsigned int pos, - uint8_t *buf, - unsigned int buflen) -{ - if (lseek(cfgfd, pos, SEEK_SET) != pos || - safewrite(cfgfd, buf, buflen) != buflen) { - VIR_WARN("Failed to write to '%s' : %s", dev->path, - g_strerror(errno)); - return -1; - } - return 0; -} - -static void -virPCIDeviceWrite16(virPCIDevice *dev, int cfgfd, unsigned int pos, uint16_t val) -{ - uint8_t buf[2] = { (val >> 0), (val >> 8) }; - virPCIDeviceWrite(dev, cfgfd, pos, &buf[0], sizeof(buf)); -} - -static void -virPCIDeviceWrite32(virPCIDevice *dev, int cfgfd, unsigned int pos, uint32_t val) -{ - uint8_t buf[4] = { (val >> 0), (val >> 8), (val >> 16), (val >> 24) }; - virPCIDeviceWrite(dev, cfgfd, pos, &buf[0], sizeof(buf)); -} - typedef int (*virPCIDeviceIterPredicate)(virPCIDevice *, virPCIDevice *, void *);
@@ -610,7 +466,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, */ static int virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, - int cfgfd, unsigned int capability, unsigned int *offset) { @@ -619,11 +474,13 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev,
*offset = 0; /* assume failure (*nothing* can be at offset 0) */
- status = virPCIDeviceRead16(dev, cfgfd, PCI_STATUS); + pci_device_cfg_read_u16(dev->device, &status, PCI_STATUS); + if (errno != 0 || !(status & PCI_STATUS_CAP_LIST)) goto error;
- pos = virPCIDeviceRead8(dev, cfgfd, PCI_CAPABILITY_LIST); + pci_device_cfg_read_u8(dev->device, &pos, PCI_CAPABILITY_LIST); + if (errno != 0) goto error;
@@ -635,7 +492,9 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, * capabilities here. */ while (pos >= PCI_CONF_HEADER_LEN && pos != 0xff) { - uint8_t capid = virPCIDeviceRead8(dev, cfgfd, pos); + uint8_t capid; + pci_device_cfg_read_u8(dev->device, &capid, pos); + if (errno != 0) goto error;
@@ -646,7 +505,8 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev, return 0; }
- pos = virPCIDeviceRead8(dev, cfgfd, pos + 1); + pci_device_cfg_read_u8(dev->device, &pos, pos + 1); + if (errno != 0) goto error; } @@ -665,7 +525,6 @@ virPCIDeviceFindCapabilityOffset(virPCIDevice *dev,
static unsigned int virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, - int cfgfd, unsigned int capability) { int ttl; @@ -677,7 +536,7 @@ virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, pos = PCI_EXT_CAP_BASE;
while (ttl > 0 && pos >= PCI_EXT_CAP_BASE) { - header = virPCIDeviceRead32(dev, cfgfd, pos); + header = pci_device_cfg_read_u32(dev->device, &header, pos);
if ((header & PCI_EXT_CAP_ID_MASK) == capability) return pos; @@ -693,7 +552,7 @@ virPCIDeviceFindExtendedCapabilityOffset(virPCIDevice *dev, * not have FLR, 1 if it does, and -1 on error */ static bool -virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev) { uint32_t caps; unsigned int pos; @@ -707,7 +566,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * on SR-IOV NICs at the moment. */ if (dev->pcie_cap_pos) { - caps = virPCIDeviceRead32(dev, cfgfd, dev->pcie_cap_pos + PCI_EXP_DEVCAP); + pci_device_cfg_read_u32(dev->device, &caps, dev->pcie_cap_pos + PCI_EXP_DEVCAP); if (caps & PCI_EXP_DEVCAP_FLR) { VIR_DEBUG("%s %s: detected PCIe FLR capability", dev->id, dev->name); return true; @@ -718,11 +577,13 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * the same thing, except for conventional PCI * devices. This is not common yet. */ - if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF, &pos) < 0) + if (virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_AF, &pos) < 0) goto error;
if (pos) { - caps = virPCIDeviceRead16(dev, cfgfd, pos + PCI_AF_CAP); + uint16_t caps16; + pci_device_cfg_read_u16(dev->device, &caps16, pos + PCI_AF_CAP); + caps = caps16; if (caps & PCI_AF_CAP_FLR) { VIR_DEBUG("%s %s: detected PCI FLR capability", dev->id, dev->name); return true; @@ -754,13 +615,13 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevice *dev, int cfgfd) * internal reset, not just a soft reset. */ static bool -virPCIDeviceDetectPowerManagementReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceDetectPowerManagementReset(virPCIDevice *dev) { if (dev->pci_pm_cap_pos) { uint32_t ctl;
/* require the NO_SOFT_RESET bit is clear */ - ctl = virPCIDeviceRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL); + pci_device_cfg_read_u32(dev->device, &ctl, dev->pci_pm_cap_pos + PCI_PM_CTRL); if (!(ctl & PCI_PM_CTRL_NO_SOFT_RESET)) { VIR_DEBUG("%s %s: detected PM reset capability", dev->id, dev->name); return true; @@ -811,26 +672,22 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) uint8_t header_type, secondary, subordinate; virPCIDevice **best = data; int ret = 0; - int fd;
if (dev->address.domain != check->address.domain) return 0;
- if ((fd = virPCIDeviceConfigOpenTry(check)) < 0) - return 0; - /* Is it a bridge? */ ret = virPCIDeviceReadClass(check, &device_class); if (ret < 0 || device_class != PCI_CLASS_BRIDGE_PCI) - goto cleanup; + return ret;
/* Is it a plane? */ - header_type = virPCIDeviceRead8(check, fd, PCI_HEADER_TYPE); + pci_device_cfg_read_u8(check->device, &header_type, PCI_HEADER_TYPE); if ((header_type & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_BRIDGE) - goto cleanup; + return ret;
- secondary = virPCIDeviceRead8(check, fd, PCI_SECONDARY_BUS); - subordinate = virPCIDeviceRead8(check, fd, PCI_SUBORDINATE_BUS); + pci_device_cfg_read_u8(check->device, &secondary, PCI_SECONDARY_BUS); + pci_device_cfg_read_u8(check->device, &subordinate, PCI_SUBORDINATE_BUS);
VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name);
@@ -838,8 +695,7 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) * the direct parent. No further work is necessary */ if (dev->address.bus == secondary) { - ret = 1; - goto cleanup; + return 1; }
/* otherwise, SRIOV allows VFs to be on different buses than their PFs. @@ -850,35 +706,26 @@ virPCIDeviceIsParent(virPCIDevice *dev, virPCIDevice *check, void *data) if (*best == NULL) { *best = virPCIDeviceNew(&check->address); if (*best == NULL) { - ret = -1; - goto cleanup; + return -1; } } else { /* OK, we had already recorded a previous "best" match for the * parent. See if the current device is more restrictive than the * best, and if so, make it the new best */ - int bestfd; uint8_t best_secondary;
- if ((bestfd = virPCIDeviceConfigOpenTry(*best)) < 0) - goto cleanup; - best_secondary = virPCIDeviceRead8(*best, bestfd, PCI_SECONDARY_BUS); - virPCIDeviceConfigClose(*best, bestfd); + pci_device_cfg_read_u8((*best)->device, &best_secondary, PCI_SECONDARY_BUS);
if (secondary > best_secondary) { virPCIDeviceFree(*best); *best = virPCIDeviceNew(&check->address); if (*best == NULL) { - ret = -1; - goto cleanup; + return -1; } } } } - - cleanup: - virPCIDeviceConfigClose(check, fd); return ret; }
@@ -902,15 +749,14 @@ virPCIDeviceGetParent(virPCIDevice *dev, virPCIDevice **parent) */ static int virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, - int cfgfd, virPCIDeviceList *inactiveDevs) { g_autoptr(virPCIDevice) parent = NULL; g_autoptr(virPCIDevice) conflict = NULL; uint8_t config_space[PCI_CONF_LEN]; uint16_t ctl; - int ret = -1; - int parentfd; + pciaddr_t bytes_read; + pciaddr_t bytes_written;
/* Refuse to do a secondary bus reset if there are other * devices/functions behind the bus are used by the host @@ -932,8 +778,6 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, dev->name); return -1; } - if ((parentfd = virPCIDeviceConfigOpenWrite(parent)) < 0) - goto out;
VIR_DEBUG("%s %s: doing a secondary bus reset", dev->id, dev->name);
@@ -941,38 +785,37 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, * for the supplied device since we refuse to do a reset if there * are multiple devices/functions */ - if (virPCIDeviceRead(dev, cfgfd, 0, config_space, PCI_CONF_LEN) < 0) { + pci_device_cfg_read(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_read); + if (bytes_read < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to read PCI config space for %1$s"), dev->name); - goto out; + return -1; }
/* Read the control register, set the reset flag, wait 200ms, * unset the reset flag and wait 200ms. */ - ctl = virPCIDeviceRead16(dev, parentfd, PCI_BRIDGE_CONTROL);
- virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, - ctl | PCI_BRIDGE_CTL_RESET); + pci_device_cfg_read_u16(parent->device, &ctl, PCI_BRIDGE_CONTROL); + + pci_device_cfg_write_u16(parent->device, ctl | PCI_BRIDGE_CTL_RESET, PCI_BRIDGE_CONTROL);
g_usleep(200 * 1000); /* sleep 200ms */
- virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, ctl); + pci_device_cfg_write_u16(parent->device, ctl, PCI_BRIDGE_CONTROL);
g_usleep(200 * 1000); /* sleep 200ms */
- if (virPCIDeviceWrite(dev, cfgfd, 0, config_space, PCI_CONF_LEN) < 0) { + pci_device_cfg_write(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_written); + if (bytes_written < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restore PCI config space for %1$s"), dev->name); - goto out; + return -1; } - ret = 0;
- out: - virPCIDeviceConfigClose(parent, parentfd); - return ret; + return 0; }
/* Power management reset attempts to reset a device using a @@ -980,16 +823,19 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevice *dev, * above we require the device supports a full internal reset. */ static int -virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd) +virPCIDeviceTryPowerManagementReset(virPCIDevice *dev) { uint8_t config_space[PCI_CONF_LEN]; uint32_t ctl; + pciaddr_t bytes_read; + pciaddr_t bytes_written;
if (!dev->pci_pm_cap_pos) return -1;
/* Save and restore the device's config space. */ - if (virPCIDeviceRead(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) { + pci_device_cfg_read(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_read); + if (bytes_read < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to read PCI config space for %1$s"), dev->name); @@ -998,20 +844,19 @@ virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd)
VIR_DEBUG("%s %s: doing a power management reset", dev->id, dev->name);
- ctl = virPCIDeviceRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL); + pci_device_cfg_read_u32(dev->device, &ctl, dev->pci_pm_cap_pos + PCI_PM_CTRL); ctl &= ~PCI_PM_CTRL_STATE_MASK;
- virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, - ctl | PCI_PM_CTRL_STATE_D3hot); + pci_device_cfg_write_u32(dev->device, ctl | PCI_PM_CTRL_STATE_D3hot, dev->pci_pm_cap_pos + PCI_PM_CTRL);
g_usleep(10 * 1000); /* sleep 10ms */
- virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, - ctl | PCI_PM_CTRL_STATE_D0); + pci_device_cfg_write_u32(dev->device, ctl | PCI_PM_CTRL_STATE_D0, dev->pci_pm_cap_pos + PCI_PM_CTRL);
g_usleep(10 * 1000); /* sleep 10ms */
- if (virPCIDeviceWrite(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) { + pci_device_cfg_write(dev->device, config_space, 0, PCI_CONF_LEN, &bytes_written); + if (bytes_written < PCI_CONF_LEN) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restore PCI config space for %1$s"), dev->name); @@ -1046,10 +891,10 @@ virPCIDeviceTryPowerManagementReset(virPCIDevice *dev, int cfgfd) * Always returns success (0) (for now) */ static int -virPCIDeviceInit(virPCIDevice *dev, int cfgfd) +virPCIDeviceInit(virPCIDevice *dev) { dev->is_pcie = false; - if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP, &dev->pcie_cap_pos) < 0) { + if (virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_EXP, &dev->pcie_cap_pos) < 0) { /* an unprivileged process is unable to read *all* of a * device's PCI config (it can only read the first 64 * bytes, which isn't enough for see the Express @@ -1065,18 +910,13 @@ virPCIDeviceInit(virPCIDevice *dev, int cfgfd) * -1), then we blindly assume the most likely outcome - * PCIe. */ - off_t configLen = virFileLength(virPCIDeviceGetConfigPath(dev), -1); - - if (configLen != 256) - dev->is_pcie = true; - } else { dev->is_pcie = (dev->pcie_cap_pos != 0); }
- virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM, &dev->pci_pm_cap_pos); - dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd); - dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd); + virPCIDeviceFindCapabilityOffset(dev, PCI_CAP_ID_PM, &dev->pci_pm_cap_pos); + dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev); + dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev);
return 0; } @@ -1089,7 +929,6 @@ virPCIDeviceReset(virPCIDevice *dev, g_autofree char *drvName = NULL; virPCIStubDriver drvType; int ret = -1; - int fd = -1; int hdrType = -1;
if (virPCIGetHeaderType(dev, &hdrType) < 0) @@ -1114,29 +953,26 @@ virPCIDeviceReset(virPCIDevice *dev, * be redundant. */ if (virPCIDeviceGetCurrentDriverNameAndType(dev, &drvName, &drvType) < 0) - goto cleanup; + return -1;
if (drvType == VIR_PCI_STUB_DRIVER_VFIO) {
VIR_DEBUG("Device %s is bound to %s - skip reset", dev->name, drvName); ret = 0; - goto cleanup; + return 0; }
VIR_DEBUG("Resetting device %s", dev->name);
- if ((fd = virPCIDeviceConfigOpenWrite(dev)) < 0) - goto cleanup; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + if (virPCIDeviceInit(dev) < 0) + return -1;
/* KVM will perform FLR when starting and stopping * a guest, so there is no need for us to do it here. */ if (dev->has_flr) { ret = 0; - goto cleanup; + return 0; }
/* If the device supports PCI power management reset, @@ -1144,11 +980,11 @@ virPCIDeviceReset(virPCIDevice *dev, * the function, not the whole device. */ if (dev->has_pm_reset) - ret = virPCIDeviceTryPowerManagementReset(dev, fd); + ret = virPCIDeviceTryPowerManagementReset(dev);
/* Bus reset is not an option with the root bus */ if (ret < 0 && dev->address.bus != 0) - ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs); + ret = virPCIDeviceTrySecondaryBusReset(dev, inactiveDevs);
if (ret < 0) { virErrorPtr err = virGetLastError(); @@ -1159,8 +995,6 @@ virPCIDeviceReset(virPCIDevice *dev, _("no FLR, PM reset or bus reset available")); }
- cleanup: - virPCIDeviceConfigClose(dev, fd); return ret; }
@@ -1756,28 +1590,6 @@ virPCIDeviceReattach(virPCIDevice *dev, return 0; }
-static char * -virPCIDeviceReadID(virPCIDevice *dev, const char *id_name) -{ - g_autofree char *path = NULL; - g_autofree char *id_str = NULL; - - path = virPCIFile(dev->name, id_name); - - /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ - if (virFileReadAll(path, 7, &id_str) < 0) - return NULL; - - /* Check for 0x suffix */ - if (id_str[0] != '0' || id_str[1] != 'x') - return NULL; - - /* Chop off the newline; we know the string is 7 bytes */ - id_str[6] = '\0'; - - return g_steal_pointer(&id_str); -} - bool virPCIDeviceAddressIsValid(virPCIDeviceAddress *addr, bool report) @@ -1865,9 +1677,9 @@ virPCIDeviceExists(const virPCIDeviceAddress *addr) virPCIDevice * virPCIDeviceNew(const virPCIDeviceAddress *address) { + struct pci_device * device; + g_autoptr(virPCIDevice) dev = NULL; - g_autofree char *vendor = NULL; - g_autofree char *product = NULL;
dev = g_new0(virPCIDevice, 1);
@@ -1875,31 +1687,21 @@ virPCIDeviceNew(const virPCIDeviceAddress *address)
dev->name = virPCIDeviceAddressAsString(&dev->address);
- dev->path = g_strdup_printf(PCI_SYSFS "devices/%s/config", dev->name); - - if (!virFileExists(dev->path)) { - virReportSystemError(errno, - _("Device %1$s not found: could not access %2$s"), - dev->name, dev->path); + pci_system_init(); + device = pci_device_find_by_slot(address->domain, address->bus, address->slot, address->function); + if (!device) return NULL; - } - - vendor = virPCIDeviceReadID(dev, "vendor"); - product = virPCIDeviceReadID(dev, "device"); - - if (!vendor || !product) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to read product/vendor ID for %1$s"), - dev->name); + dev->device = g_memdup(device, sizeof(*dev->device)); + if (!dev->device) { + virReportSystemError(errno, + _("Not found device domain: %1$d, bus: %2$d, slot: %3$d, function: %4$d"), + address->domain, address->bus, address->slot, address->function); return NULL; } - - /* strings contain '0x' prefix */ - if (g_snprintf(dev->id, sizeof(dev->id), "%s %s", &vendor[2], - &product[2]) >= sizeof(dev->id)) { + if (g_snprintf(dev->id, sizeof(dev->id), "%x %x", dev->device->vendor_id, dev->device->device_id) >= sizeof(dev->id)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("dev->id buffer overflow: %1$s %2$s"), - &vendor[2], &product[2]); + _("dev->id buffer overflow: %1$d %2$d"), + dev->device->vendor_id, dev->device->device_id); return NULL; }
@@ -1918,10 +1720,10 @@ virPCIDeviceCopy(virPCIDevice *dev)
/* shallow copy to take care of most attributes */ *copy = *dev; - copy->path = NULL; - copy->used_by_drvname = copy->used_by_domname = NULL; + copy->device = NULL; + copy->device = g_memdup(dev->device, sizeof(*dev->device)); + copy->name = copy->used_by_drvname = copy->used_by_domname = copy->stubDriverName = NULL; copy->name = g_strdup(dev->name); - copy->path = g_strdup(dev->path); copy->used_by_drvname = g_strdup(dev->used_by_drvname); copy->used_by_domname = g_strdup(dev->used_by_domname); copy->stubDriverName = g_strdup(dev->stubDriverName); @@ -1936,10 +1738,11 @@ virPCIDeviceFree(virPCIDevice *dev) return; VIR_DEBUG("%s %s: freeing", dev->id, dev->name); g_free(dev->name); - g_free(dev->path); g_free(dev->used_by_drvname); g_free(dev->used_by_domname); g_free(dev->stubDriverName); + if (dev->device) + g_free(dev->device); g_free(dev); }
@@ -1971,9 +1774,9 @@ virPCIDeviceGetName(virPCIDevice *dev) * config file. */ const char * -virPCIDeviceGetConfigPath(virPCIDevice *dev) +virPCIDeviceGetConfigPath(virPCIDevice *dev G_GNUC_UNUSED) { - return dev->path; + return NULL; }
void virPCIDeviceSetManaged(virPCIDevice *dev, bool managed) @@ -2484,47 +2287,37 @@ virPCIDeviceDownstreamLacksACS(virPCIDevice *dev) uint16_t flags; uint16_t ctrl; unsigned int pos; - int fd; - int ret = 0; uint16_t device_class;
- if ((fd = virPCIDeviceConfigOpen(dev)) < 0) + if (virPCIDeviceInit(dev) < 0) { return -1; - - if (virPCIDeviceInit(dev, fd) < 0) { - ret = -1; - goto cleanup; }
if (virPCIDeviceReadClass(dev, &device_class) < 0) - goto cleanup; + return 0;
pos = dev->pcie_cap_pos; if (!pos || device_class != PCI_CLASS_BRIDGE_PCI) - goto cleanup; + return 0;
- flags = virPCIDeviceRead16(dev, fd, pos + PCI_EXP_FLAGS); + pci_device_cfg_read_u16(dev->device, &flags, pos + PCI_EXP_FLAGS); if (((flags & PCI_EXP_FLAGS_TYPE) >> 4) != PCI_EXP_TYPE_DOWNSTREAM) - goto cleanup; + return 0;
- pos = virPCIDeviceFindExtendedCapabilityOffset(dev, fd, PCI_EXT_CAP_ID_ACS); + pos = virPCIDeviceFindExtendedCapabilityOffset(dev, PCI_EXT_CAP_ID_ACS); if (!pos) { VIR_DEBUG("%s %s: downstream port lacks ACS", dev->id, dev->name); - ret = 1; - goto cleanup; + return 1; }
- ctrl = virPCIDeviceRead16(dev, fd, pos + PCI_EXT_ACS_CTRL); + pci_device_cfg_read_u16(dev->device, &ctrl, pos + PCI_EXT_ACS_CTRL); if ((ctrl & PCI_EXT_CAP_ACS_ENABLED) != PCI_EXT_CAP_ACS_ENABLED) { VIR_DEBUG("%s %s: downstream port has ACS disabled", dev->id, dev->name); - ret = 1; - goto cleanup; + return 1; }
- cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return 0; }
static int @@ -3189,48 +2982,27 @@ virPCIDeviceGetVPD(virPCIDevice *dev G_GNUC_UNUSED) int virPCIDeviceIsPCIExpress(virPCIDevice *dev) { - int fd; - int ret = -1; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; - - ret = dev->is_pcie; + if (virPCIDeviceInit(dev) < 0) + return -1;
- cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return dev->is_pcie; }
int virPCIDeviceHasPCIExpressLink(virPCIDevice *dev) { - int fd; - int ret = -1; uint16_t cap, type; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + if (virPCIDeviceInit(dev) < 0) + return -1;
if (dev->pcie_cap_pos == 0) { - ret = 0; - goto cleanup; + return 0; }
- cap = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_CAP_FLAGS); + pci_device_cfg_read_u16(dev->device, &cap, dev->pcie_cap_pos + PCI_CAP_FLAGS); type = (cap & PCI_EXP_FLAGS_TYPE) >> 4;
- ret = type != PCI_EXP_TYPE_ROOT_INT_EP && type != PCI_EXP_TYPE_ROOT_EC; - - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return type != PCI_EXP_TYPE_ROOT_INT_EP && type != PCI_EXP_TYPE_ROOT_EC; }
int @@ -3242,53 +3014,39 @@ virPCIDeviceGetLinkCapSta(virPCIDevice *dev, unsigned int *sta_width) { uint32_t t; - int fd; - int ret = -1; - - if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return ret; - - if (virPCIDeviceInit(dev, fd) < 0) - goto cleanup; + uint16_t t16; + if (virPCIDeviceInit(dev) < 0) + return -1;
if (!dev->pcie_cap_pos) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pci device %1$s is not a PCI-Express device"), dev->name); - goto cleanup; + return -1; }
- t = virPCIDeviceRead32(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKCAP); + pci_device_cfg_read_u32(dev->device, &t, dev->pcie_cap_pos + PCI_EXP_LNKCAP);
*cap_port = t >> 24; *cap_speed = t & PCI_EXP_LNKCAP_SPEED; *cap_width = (t & PCI_EXP_LNKCAP_WIDTH) >> 4;
- t = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKSTA); + pci_device_cfg_read_u16(dev->device, &t16, dev->pcie_cap_pos + PCI_EXP_LNKSTA); + t = t16;
*sta_speed = t & PCI_EXP_LNKSTA_SPEED; *sta_width = (t & PCI_EXP_LNKSTA_WIDTH) >> 4; - ret = 0; - - cleanup: - virPCIDeviceConfigClose(dev, fd); - return ret; + return 0; }
int virPCIGetHeaderType(virPCIDevice *dev, int *hdrType) { - int fd; uint8_t type;
*hdrType = -1;
- if ((fd = virPCIDeviceConfigOpen(dev)) < 0) - return -1; - - type = virPCIDeviceRead8(dev, fd, PCI_HEADER_TYPE); - - virPCIDeviceConfigClose(dev, fd); + pci_device_cfg_read_u8(dev->device, &type, PCI_HEADER_TYPE);
type &= PCI_HEADER_TYPE_MASK; if (type >= VIR_PCI_HEADER_LAST) { diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d2a1f5acf1..fd8339bd30 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -18,12 +18,14 @@ */
#include <config.h> +#include "testutils.h" + +#ifdef __linux__
#include "qemu/qemu_alias.h" #include "qemu/qemu_conf.h" #include "qemu/qemu_hotplug.h" #include "qemumonitortestutils.h" -#include "testutils.h" #include "testutilsqemu.h" #include "testutilsqemuschema.h" #include "virhostdev.h" @@ -816,3 +818,10 @@ VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("domaincaps"), VIR_TEST_MOCK("virprocess"), VIR_TEST_MOCK("qemuhotplug")); +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index a2888732f7..5f62c80665 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -7,6 +7,8 @@
#include "testutils.h"
+#ifdef __linux__ + #ifdef WITH_QEMU
# include "internal.h" @@ -137,3 +139,10 @@ main(void) }
#endif /* WITH_QEMU */ +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 1f0068864a..6f84812109 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -7,6 +7,8 @@
#include "testutils.h"
+#ifdef __linux__ + #ifdef WITH_QEMU
# include "internal.h" @@ -3051,3 +3053,10 @@ int main(void) }
#endif /* WITH_QEMU */ +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 5b923c63ce..2fa12903d2 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -22,7 +22,7 @@
#include "virpcivpdpriv.h"
-#if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__) +#if defined(__linux__) || defined(__APPLE__) # define VIR_MOCK_LOOKUP_MAIN # include "virmock.h" # include "virpci.h" @@ -42,6 +42,10 @@ static int (*real___open_2)(const char *path, int flags); static int (*real_close)(int fd); static DIR * (*real_opendir)(const char *name); static char *(*real_virFileCanonicalizePath)(const char *path); +static int (*real_scandir)(const char *restrict dirp, + struct dirent ***restrict namelist, + typeof(int(const struct dirent *)) *filter, + typeof(int(const struct dirent **, const struct dirent **)) *compar);
static char *fakerootdir;
@@ -955,6 +959,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(opendir); # endif VIR_MOCK_REAL_INIT(virFileCanonicalizePath); + VIR_MOCK_REAL_INIT(scandir); }
static void @@ -1172,6 +1177,20 @@ virFileCanonicalizePath(const char *path) return real_virFileCanonicalizePath(newpath); }
+int scandir(const char *restrict dirp, struct dirent ***restrict namelist, + typeof(int(const struct dirent *)) *filter, + typeof(int(const struct dirent **, const struct dirent **)) *compar) +{ + g_autofree char *newpath = NULL; + + init_syms(); + + if (getrealpath(&newpath, dirp) < 0) + return -1; + + return real_scandir(newpath, namelist, filter, compar); +} + # include "virmockstathelpers.c"
#else -- 2.47.1
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> writes: Should I remove Sponsored from the commit log and add Copyright to the file? * virpci.c: helper APIs for managing host PCI devices * * Copyright (C) 2009-2015 Red Hat, Inc. + * Copyright (C) 2024-2025 Future Crew, LLC * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public
Please provide a commit message that describes the problem you're solving with this refactoring. Likewise for other commits in this series which all have empty commit messages.
Sponsored by: Future Crew, LLC
While it is nice that they're sponsoring you to work on this, IMHO, we don't need to be advertizing commercial relationships in the commit log.
It suffices to put such a note in the cover letter when sending a patch series.
-- Alexander Shursha
On Wed, Feb 12, 2025 at 02:54:17PM +0300, Alexander Shursha wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:
Should I remove Sponsored from the commit log and add Copyright to the file?
* virpci.c: helper APIs for managing host PCI devices * * Copyright (C) 2009-2015 Red Hat, Inc. + * Copyright (C) 2024-2025 Future Crew, LLC
That depends on the terms of your work with Future Crew ie whether you have assigned your copyright over to them. As a project, we don't require contributors provide explicit Copyright lines on files. Some companies require their employees to do that, others don't. We just defer to contributors to comply with whatever rules they are working under in respect of 'Copyright' notices in files.
* * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public
Please provide a commit message that describes the problem you're solving with this refactoring. Likewise for other commits in this series which all have empty commit messages.
Sponsored by: Future Crew, LLC
While it is nice that they're sponsoring you to work on this, IMHO, we don't need to be advertizing commercial relationships in the commit log.
It suffices to put such a note in the cover letter when sending a patch series.
-- Alexander Shursha
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> writes: Done.
Please provide a commit message that describes the problem you're solving with this refactoring. Likewise for other commits in this series which all have empty commit messages.
Sponsored by: Future Crew, LLC
While it is nice that they're sponsoring you to work on this, IMHO, we don't need to be advertizing commercial relationships in the commit log.
It suffices to put such a note in the cover letter when sending a patch series.
-- Alexander Shursha
participants (3)
-
Alexander Shursha -
Daniel P. Berrangé -
Peter Krempa