[PATCH v1 3/3] virhostdev: remove virHostdevReattachPCIDevice

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

We have 3 pieces of code that do slightly the same thing, but
it varies depending on where it is called:

- virPCIDeviceReattach(). This is where the actual re-attach
work happens;

- virHostdevReattachPCIDevice(). This is a static function from
virhostdev.c that calls virPCIDeviceReattach() after waiting
for device cleanup for the KVM PCI stub driver;

- virHostdevPCINodeDeviceReAttach(). This function also
calls virPCIDeviceReattach(), but setting some device properties
beforehand.

All these extra operations that are done before virPCIDeviceReattach()
can be moved inside the function itself,  allowing for the
same re-attach behavior everywhere.

This patch consolidates all these pre-conditions inside the
body of virPCIDeviceReattach(). With that, virHostdevReattachPCIDevice()
can be removed and the callers can use virPCIDeviceReattach()
directly instead.

Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
 src/util/virhostdev.c | 43 +++++++++----------------------------------
 src/util/virpci.c     | 29 ++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 53aacb59b4..23be037a39 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -613,33 +613,6 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
     }
 }
 
-/*
- * Pre-condition: inactivePCIHostdevs & activePCIHostdevs
- * are locked
- */
-static void
-virHostdevReattachPCIDevice(virHostdevManagerPtr mgr,
-                            virPCIDevicePtr actual)
-{
-    /* Wait for device cleanup if it is qemu/kvm */
-    if (virPCIDeviceGetStubDriver(actual) == VIR_PCI_STUB_DRIVER_KVM) {
-        int retries = 100;
-        while (virPCIDeviceWaitForCleanup(actual, "kvm_assigned_device")
-               && retries) {
-            usleep(100*1000);
-            retries--;
-        }
-    }
-
-    VIR_DEBUG("Reattaching PCI device %s", virPCIDeviceGetName(actual));
-    if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs,
-                             mgr->inactivePCIHostdevs) < 0) {
-        VIR_ERROR(_("Failed to re-attach PCI device: %s"),
-                  virGetLastErrorMessage());
-        virResetLastError();
-    }
-}
-
 static void
 virHostdevReattachAllPCIDevices(virPCIDeviceListPtr pcidevs,
                                 virHostdevManagerPtr mgr)
@@ -655,11 +628,17 @@ virHostdevReattachAllPCIDevices(virPCIDeviceListPtr pcidevs,
         if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
             continue;
 
-        if (virPCIDeviceGetManaged(actual))
-            virHostdevReattachPCIDevice(mgr, actual);
-        else
+        if (virPCIDeviceGetManaged(actual)) {
+            if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs,
+                                     mgr->inactivePCIHostdevs) < 0) {
+                VIR_ERROR(_("Failed to re-attach PCI device: %s"),
+                          virGetLastErrorMessage());
+                virResetLastError();
+            }
+        } else {
             VIR_DEBUG("Not reattaching unmanaged PCI device %s",
                       virPCIDeviceGetName(actual));
+        }
     }
 }
 
@@ -2050,10 +2029,6 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr,
     if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data))
         goto cleanup;
 
-    virPCIDeviceSetUnbindFromStub(pci, true);
-    virPCIDeviceSetRemoveSlot(pci, true);
-    virPCIDeviceSetReprobe(pci, true);
-
     if (virPCIDeviceReattach(pci, mgr->activePCIHostdevs,
                              mgr->inactivePCIHostdevs) < 0)
         goto cleanup;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 75e8daadd5..4594643d3c 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1509,19 +1509,39 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
     return 0;
 }
 
+/*
+ * Pre-condition: inactivePCIHostdevs & activePCIHostdevs
+ * are locked
+ */
 int
 virPCIDeviceReattach(virPCIDevicePtr dev,
                      virPCIDeviceListPtr activeDevs,
                      virPCIDeviceListPtr inactiveDevs)
 {
+    int ret = -1;
+
     if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Not reattaching active device %s"), dev->name);
-        return -1;
+        goto exit;
+    }
+
+    virPCIDeviceSetUnbindFromStub(dev, true);
+    virPCIDeviceSetRemoveSlot(dev, true);
+    virPCIDeviceSetReprobe(dev, true);
+
+    /* Wait for device cleanup if it is qemu/kvm */
+    if (virPCIDeviceGetStubDriver(dev) == VIR_PCI_STUB_DRIVER_KVM) {
+        int retries = 100;
+        while (virPCIDeviceWaitForCleanup(dev, "kvm_assigned_device")
+               && retries) {
+            usleep(100*1000);
+            retries--;
+        }
     }
 
     if (virPCIDeviceUnbindFromStub(dev) < 0)
-        return -1;
+        goto exit;
 
     /* Steal the dev from list inactiveDevs */
     if (inactiveDevs) {
@@ -1529,7 +1549,10 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
         virPCIDeviceListDel(inactiveDevs, dev);
     }
 
-    return 0;
+    ret = 0;
+
+ exit:
+    return ret;
 }
 
 /* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on
-- 
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list