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.

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index de029a01eb598cfa05c42b148d102312936d637f..4065535069a2c873dea6e82dbed7060968fdb489 100644 (file)
--- 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 7ca3fef9743a01e0f653a85aee000322b27bc9b9..6f0cb8ce5b6d4864766fa18d7ecc7a83aaf0a3c6 100644 (file)
--- 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,