pci: Use virPCIDeviceAddress in virPCIDevice

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.
This commit is contained in:
Andrea Bolognani 2015-12-15 09:44:35 +01:00
parent 716be2570a
commit 7743454165
2 changed files with 47 additions and 70 deletions

View File

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

View File

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