Instead of replicating the information (domain, bus, slot, function)
inside the virPCIDevice structure, use the already-existing
virPCIDeviceAddress structure.
For users of the module, this means that the object returned by
virPCIDeviceGetAddress() can no longer be NULL and must no longer
be freed by the caller.
---
Changes in v3:
* don't check the return value of virPCIDeviceGetAddress(), it can no
longer be NULL
* don't use a variable to store the device address if it's only going
to be used a single time
Changes in v2:
* use virPCIDeviceAddress instead of virPCIDeviceAddressPtr to avoid
extra allocations
src/util/virhostdev.c | 20 ++---------
src/util/virpci.c | 97 +++++++++++++++++++++++----------------------------
2 files changed, 47 insertions(+), 70 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index de029a0..4065535 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -585,15 +585,12 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
goto cleanup;
}
- VIR_FREE(devAddr);
- if (!(devAddr = virPCIDeviceGetAddress(dev)))
- goto cleanup;
-
/* The device is in use by other active domain if
* the dev is in list activePCIHostdevs. VFIO devices
* belonging to same iommu group can't be shared
* across guests.
*/
+ devAddr = virPCIDeviceGetAddress(dev);
if (usesVfio) {
if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
virHostdevIsPCINodeDeviceUsed,
@@ -728,7 +725,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
virObjectUnlock(hostdev_mgr->activePCIHostdevs);
virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
virObjectUnref(pcidevs);
- VIR_FREE(devAddr);
return ret;
}
@@ -1558,7 +1554,6 @@ int
virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
virPCIDevicePtr pci)
{
- virPCIDeviceAddressPtr devAddr = NULL;
struct virHostdevIsPCINodeDeviceUsedData data = { hostdev_mgr, NULL,
false };
int ret = -1;
@@ -1566,10 +1561,7 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
virObjectLock(hostdev_mgr->activePCIHostdevs);
virObjectLock(hostdev_mgr->inactivePCIHostdevs);
- if (!(devAddr = virPCIDeviceGetAddress(pci)))
- goto out;
-
- if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
+ if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data))
goto out;
if (virPCIDeviceDetach(pci, hostdev_mgr->activePCIHostdevs,
@@ -1581,7 +1573,6 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
out:
virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
virObjectUnlock(hostdev_mgr->activePCIHostdevs);
- VIR_FREE(devAddr);
return ret;
}
@@ -1589,7 +1580,6 @@ int
virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
virPCIDevicePtr pci)
{
- virPCIDeviceAddressPtr devAddr = NULL;
struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, NULL,
false};
int ret = -1;
@@ -1597,10 +1587,7 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
virObjectLock(hostdev_mgr->activePCIHostdevs);
virObjectLock(hostdev_mgr->inactivePCIHostdevs);
- if (!(devAddr = virPCIDeviceGetAddress(pci)))
- goto out;
-
- if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
+ if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data))
goto out;
virPCIDeviceReattachInit(pci);
@@ -1613,7 +1600,6 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
out:
virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
virObjectUnlock(hostdev_mgr->activePCIHostdevs);
- VIR_FREE(devAddr);
return ret;
}
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 7ca3fef..6f0cb8c 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -56,10 +56,7 @@ VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
"", "2.5", "5", "8")
struct _virPCIDevice {
- unsigned int domain;
- unsigned int bus;
- unsigned int slot;
- unsigned int function;
+ virPCIDeviceAddress address;
char name[PCI_ADDR_LEN]; /* domain:bus:slot.function */
char id[PCI_ID_LEN]; /* product vendor */
@@ -655,10 +652,10 @@ virPCIDeviceSharesBusWithActive(virPCIDevicePtr dev, virPCIDevicePtr check, void
virPCIDeviceList *inactiveDevs = data;
/* Different domain, different bus, or simply identical device */
- if (dev->domain != check->domain ||
- dev->bus != check->bus ||
- (dev->slot == check->slot &&
- dev->function == check->function))
+ if (dev->address.domain != check->address.domain ||
+ dev->address.bus != check->address.bus ||
+ (dev->address.slot == check->address.slot &&
+ dev->address.function == check->address.function))
return 0;
/* same bus, but inactive, i.e. about to be assigned to guest */
@@ -689,7 +686,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
int ret = 0;
int fd;
- if (dev->domain != check->domain)
+ if (dev->address.domain != check->address.domain)
return 0;
if ((fd = virPCIDeviceConfigOpen(check, false)) < 0)
@@ -713,7 +710,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
/* if the secondary bus exactly equals the device's bus, then we found
* the direct parent. No further work is necessary
*/
- if (dev->bus == secondary) {
+ if (dev->address.bus == secondary) {
ret = 1;
goto cleanup;
}
@@ -722,10 +719,12 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
* In this case, what we need to do is look for the "best" match; i.e.
* the most restrictive match that still satisfies all of the conditions.
*/
- if (dev->bus > secondary && dev->bus <= subordinate) {
+ if (dev->address.bus > secondary && dev->address.bus <= subordinate) {
if (*best == NULL) {
- *best = virPCIDeviceNew(check->domain, check->bus, check->slot,
- check->function);
+ *best = virPCIDeviceNew(check->address.domain,
+ check->address.bus,
+ check->address.slot,
+ check->address.function);
if (*best == NULL) {
ret = -1;
goto cleanup;
@@ -745,8 +744,10 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
if (secondary > best_secondary) {
virPCIDeviceFree(*best);
- *best = virPCIDeviceNew(check->domain, check->bus, check->slot,
- check->function);
+ *best = virPCIDeviceNew(check->address.domain,
+ check->address.bus,
+ check->address.slot,
+ check->address.function);
if (*best == NULL) {
ret = -1;
goto cleanup;
@@ -970,7 +971,7 @@ virPCIDeviceReset(virPCIDevicePtr dev,
ret = virPCIDeviceTryPowerManagementReset(dev, fd);
/* Bus reset is not an option with the root bus */
- if (ret < 0 && dev->bus != 0)
+ if (ret < 0 && dev->address.bus != 0)
ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs);
if (ret < 0) {
@@ -1483,8 +1484,8 @@ virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher)
virStrToLong_ui(tmp + 1, &tmp, 16, &function) < 0 || *tmp != '\n')
continue;
- if (domain != dev->domain || bus != dev->bus || slot != dev->slot ||
- function != dev->function)
+ if (domain != dev->address.domain || bus != dev->address.bus ||
+ slot != dev->address.slot || function != dev->address.function)
continue;
in_matching_device = true;
match_depth = strspn(line, " ");
@@ -1560,17 +1561,16 @@ virPCIDeviceNew(unsigned int domain,
if (VIR_ALLOC(dev) < 0)
return NULL;
- dev->domain = domain;
- dev->bus = bus;
- dev->slot = slot;
- dev->function = function;
+ dev->address.domain = domain;
+ dev->address.bus = bus;
+ dev->address.slot = slot;
+ dev->address.function = function;
if (snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x",
- dev->domain, dev->bus, dev->slot,
- dev->function) >= sizeof(dev->name)) {
+ domain, bus, slot, function) >= sizeof(dev->name)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("dev->name buffer overflow: %.4x:%.2x:%.2x.%.1x"),
- dev->domain, dev->bus, dev->slot, dev->function);
+ domain, bus, slot, function);
goto error;
}
if (virAsprintf(&dev->path, PCI_SYSFS "devices/%s/config",
@@ -1661,25 +1661,14 @@ virPCIDeviceFree(virPCIDevicePtr dev)
* @dev: device to get address from
*
* Take a PCI device on input and return its PCI address. The
- * caller must free the returned value when no longer needed.
+ * returned object is owned by the device and must not be freed.
*
- * Returns NULL on failure, the device address on success.
+ * Returns: a pointer to the address, which can never be NULL.
*/
virPCIDeviceAddressPtr
virPCIDeviceGetAddress(virPCIDevicePtr dev)
{
-
- virPCIDeviceAddressPtr pciAddrPtr;
-
- if (!dev || (VIR_ALLOC(pciAddrPtr) < 0))
- return NULL;
-
- pciAddrPtr->domain = dev->domain;
- pciAddrPtr->bus = dev->bus;
- pciAddrPtr->slot = dev->slot;
- pciAddrPtr->function = dev->function;
-
- return pciAddrPtr;
+ return &(dev->address);
}
const char *
@@ -1890,12 +1879,14 @@ virPCIDeviceListFindIndex(virPCIDeviceListPtr list, virPCIDevicePtr dev)
{
size_t i;
- for (i = 0; i < list->count; i++)
- if (list->devs[i]->domain == dev->domain &&
- list->devs[i]->bus == dev->bus &&
- list->devs[i]->slot == dev->slot &&
- list->devs[i]->function == dev->function)
+ for (i = 0; i < list->count; i++) {
+ virPCIDevicePtr other = list->devs[i];
+ if (other->address.domain == dev->address.domain &&
+ other->address.bus == dev->address.bus &&
+ other->address.slot == dev->address.slot &&
+ other->address.function == dev->address.function)
return i;
+ }
return -1;
}
@@ -1910,10 +1901,11 @@ virPCIDeviceListFindByIDs(virPCIDeviceListPtr list,
size_t i;
for (i = 0; i < list->count; i++) {
- if (list->devs[i]->domain == domain &&
- list->devs[i]->bus == bus &&
- list->devs[i]->slot == slot &&
- list->devs[i]->function == function)
+ virPCIDevicePtr other = list->devs[i];
+ if (other->address.domain == domain &&
+ other->address.bus == bus &&
+ other->address.slot == slot &&
+ other->address.function == function)
return list->devs[i];
}
return NULL;
@@ -1944,7 +1936,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev,
int direrr;
if (virAsprintf(&pcidir, "/sys/bus/pci/devices/%04x:%02x:%02x.%x",
- dev->domain, dev->bus, dev->slot, dev->function) < 0)
+ dev->address.domain, dev->address.bus,
+ dev->address.slot, dev->address.function) < 0)
goto cleanup;
if (!(dir = opendir(pcidir))) {
@@ -2074,13 +2067,11 @@ virPCIDeviceListPtr
virPCIDeviceGetIOMMUGroupList(virPCIDevicePtr dev)
{
virPCIDeviceListPtr groupList = virPCIDeviceListNew();
- virPCIDeviceAddress devAddr = { dev->domain, dev->bus,
- dev->slot, dev->function };
if (!groupList)
goto error;
- if (virPCIDeviceAddressIOMMUGroupIterate(&devAddr,
+ if (virPCIDeviceAddressIOMMUGroupIterate(&(dev->address),
virPCIDeviceGetIOMMUGroupAddOne,
groupList) < 0)
goto error;
@@ -2294,7 +2285,7 @@ virPCIDeviceIsBehindSwitchLackingACS(virPCIDevicePtr dev)
* into play since devices on the root bus can't P2P without going
* through the root IOMMU.
*/
- if (dev->bus == 0) {
+ if (dev->address.bus == 0) {
return 0;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.5.0
