[libvirt] [PATCH 05/10] hostdev: Simplify virHostdevIsPCIDeviceUsed()

[Andrea Bolognani]

This function, previously called virHostdevIsPCINodeDeviceUsed(), was
written to work both when used on its own and when used with
virPCIDeviceAddressIOMMUGroupIterate(); as a consequence, it was
slightly more convoluted than it needed to be.

Rework it to perform a single task (checking that a PCI device is not
used by a domain) and remove the redundant "Node" from its name. Add
some documentation as well.

The safety checks that are no longer performed by this function will be
reintroduced later on in a different form.
---
 src/util/virhostdev.c | 110 ++++++++++++++++----------------------------------
 1 file changed, 35 insertions(+), 75 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 33a45cb..77757c5 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -54,51 +54,43 @@ static virClassPtr virHostdevManagerClass;
 static void virHostdevManagerDispose(void *obj);
 static virHostdevManagerPtr virHostdevManagerNew(void);
 
-struct virHostdevIsPCINodeDeviceUsedData {
-    virHostdevManagerPtr hostdev_mgr;
-    const char *domainName;
-    const bool usesVfio;
-};
-
-static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *opaque)
+/**
+ * virHostdevIsPCIDeviceUsed:
+ * @dev: PCI device
+ * @hostdev_mgr: hostdev manager
+ *
+ * Checks whether @dev is already assigned to a domain, reporting a detailed
+ * error if that's the case.
+ *
+ * Returns: true if @dev is in use by some domain, false otherwise
+ */
+static bool
+virHostdevIsPCIDeviceUsed(virPCIDevicePtr dev,
+                          virHostdevManagerPtr hostdev_mgr)
 {
     virPCIDevicePtr other;
-    int ret = -1;
-    virPCIDevicePtr pci = NULL;
-    struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
+    const char *other_drvname = NULL;
+    const char *other_domname = NULL;
+    bool ret = false;
 
-    if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus,
-                                devAddr->slot, devAddr->function)))
-        goto cleanup;
+    if (!(other = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, dev)))
+        goto out;
 
-    other = virPCIDeviceListFind(helperData->hostdev_mgr->activePCIHostdevs,
-                                 pci);
-    if (other) {
-        const char *other_drvname = NULL;
-        const char *other_domname = NULL;
-        virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
+    virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
 
-        if (helperData->usesVfio &&
-            (other_domname && helperData->domainName) &&
-            (STREQ(other_domname, helperData->domainName)))
-            goto iommu_owner;
+    if (other_drvname && other_domname)
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("PCI device %s is in use by "
+                         "driver %s, domain %s"),
+                       virPCIDeviceGetName(dev),
+                       other_drvname, other_domname);
+    else
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("PCI device %s is in use"),
+                       virPCIDeviceGetName(dev));
+    ret = true;
 
-        if (other_drvname && other_domname)
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("PCI device %s is in use by "
-                             "driver %s, domain %s"),
-                           virPCIDeviceGetName(pci),
-                           other_drvname, other_domname);
-        else
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("PCI device %s is in use"),
-                           virPCIDeviceGetName(pci));
-        goto cleanup;
-    }
- iommu_owner:
-    ret = 0;
- cleanup:
-    virPCIDeviceFree(pci);
+ out:
     return ret;
 }
 
@@ -538,7 +530,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
     int last_processed_hostdev_vf = -1;
     size_t i;
     int ret = -1;
-    virPCIDeviceAddressPtr devAddr = NULL;
 
     if (!nhostdevs)
         return 0;
@@ -565,8 +556,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
         bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
         bool usesVfio = (virPCIDeviceGetStubDriver(dev) == VIR_PCI_STUB_DRIVER_VFIO);
-        struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, dom_name,
-                                                         usesVfio};
 
         if (!usesVfio && !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
             virReportError(VIR_ERR_OPERATION_INVALID,
@@ -575,23 +564,9 @@ 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.
-         */
-        if (usesVfio) {
-            if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
-                                                     virHostdevIsPCINodeDeviceUsed,
-                                                     &data) < 0)
-                goto cleanup;
-        } else if (virHostdevIsPCINodeDeviceUsed(devAddr, &data)) {
+        /* Host devices can't be shared between domains */
+        if (virHostdevIsPCIDeviceUsed(dev, hostdev_mgr))
             goto cleanup;
-        }
     }
 
     /* Loop 2: detach managed devices (i.e. bind to appropriate stub driver) */
@@ -718,7 +693,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
     virObjectUnlock(hostdev_mgr->activePCIHostdevs);
     virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
     virObjectUnref(pcidevs);
-    VIR_FREE(devAddr);
     return ret;
 }
 
@@ -1543,18 +1517,12 @@ int
 virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
                               virPCIDevicePtr pci)
 {
-    virPCIDeviceAddressPtr devAddr = NULL;
-    struct virHostdevIsPCINodeDeviceUsedData data = { hostdev_mgr, NULL,
-                                                     false };
     int ret = -1;
 
     virObjectLock(hostdev_mgr->activePCIHostdevs);
     virObjectLock(hostdev_mgr->inactivePCIHostdevs);
 
-    if (!(devAddr = virPCIDeviceGetAddress(pci)))
-        goto out;
-
-    if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
+    if (virHostdevIsPCIDeviceUsed(pci, hostdev_mgr))
         goto out;
 
     if (virPCIDeviceDetach(pci, hostdev_mgr->activePCIHostdevs,
@@ -1566,7 +1534,6 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
  out:
     virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
     virObjectUnlock(hostdev_mgr->activePCIHostdevs);
-    VIR_FREE(devAddr);
     return ret;
 }
 
@@ -1574,18 +1541,12 @@ int
 virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
                                 virPCIDevicePtr pci)
 {
-    virPCIDeviceAddressPtr devAddr = NULL;
-    struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, NULL,
-                                                     false};
     int ret = -1;
 
     virObjectLock(hostdev_mgr->activePCIHostdevs);
     virObjectLock(hostdev_mgr->inactivePCIHostdevs);
 
-    if (!(devAddr = virPCIDeviceGetAddress(pci)))
-        goto out;
-
-    if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
+    if (virHostdevIsPCIDeviceUsed(pci, hostdev_mgr))
         goto out;
 
     virPCIDeviceReattachInit(pci);
@@ -1598,7 +1559,6 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
  out:
     virObjectUnlock(hostdev_mgr->inactivePCIHostdevs);
     virObjectUnlock(hostdev_mgr->activePCIHostdevs);
-    VIR_FREE(devAddr);
     return ret;
 }
 
-- 
2.5.0