[libvirt] [PATCH v1 2/3] virhostdev: introduce virHostdevReattachAllPCIDevices

[Daniel Henrique Barboza]

There are two places in virhostdev that executes a re-attach operation
in all pci devices of a virPCIDeviceListPtr array:
virHostdevPreparePCIDevices and virHostdevReAttachPCIDevices. The
difference is that the code inside virHostdevPreparePCIDevices uses
virPCIDeviceReattach(), while inside virHostdevReAttachPCIDevices
virHostdevReattachPCIDevice() is used. Both are called in the
same manner inside a loop.

To make the code easier to follow and to standardize it further,
a new virHostdevReattachAllPCIDevices() helper function is created,
which is now called in both places. virHostdevReattachPCIDevice()
was chosen as re-attach function because it is an improved version
of the raw virPCIDeviceReattach(), including a timer to wait for
device cleanup in case of pci_stub_kvm.

Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
 src/util/virhostdev.c | 114 +++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 62 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 7cb0beb545..53aacb59b4 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -613,6 +613,56 @@ 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)
+{
+    size_t i;
+
+    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
+        virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+        virPCIDevicePtr actual;
+
+        /* We need to look up the actual device because that's what
+         * virHostdevReattachPCIDevice() expects as its argument */
+        if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
+            continue;
+
+        if (virPCIDeviceGetManaged(actual))
+            virHostdevReattachPCIDevice(mgr, actual);
+        else
+            VIR_DEBUG("Not reattaching unmanaged PCI device %s",
+                      virPCIDeviceGetName(actual));
+    }
+}
+
 static int
 virHostdevResetAllPCIDevices(virPCIDeviceListPtr pcidevs,
                              virHostdevManagerPtr mgr)
@@ -899,26 +949,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
     }
 
  reattachdevs:
-    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
-        virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
-        virPCIDevicePtr actual;
-
-        /* We need to look up the actual device because that's what
-         * virPCIDeviceReattach() expects as its argument */
-        if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
-            continue;
-
-        if (virPCIDeviceGetManaged(actual)) {
-            VIR_DEBUG("Reattaching managed PCI device %s",
-                      virPCIDeviceGetName(pci));
-            ignore_value(virPCIDeviceReattach(actual,
-                                              mgr->activePCIHostdevs,
-                                              mgr->inactivePCIHostdevs));
-        } else {
-            VIR_DEBUG("Not reattaching unmanaged PCI device %s",
-                      virPCIDeviceGetName(pci));
-        }
-    }
+    virHostdevReattachAllPCIDevices(pcidevs, mgr);
 
  cleanup:
     virObjectUnlock(mgr->activePCIHostdevs);
@@ -927,33 +958,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
     return ret;
 }
 
-/*
- * 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();
-    }
-}
-
 /* @oldStateDir:
  * For upgrade purpose: see virHostdevRestoreNetConfig
  */
@@ -1067,21 +1071,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
 
     /* Step 5: Reattach managed devices to their host drivers; unmanaged
      *         devices don't need to be processed further */
-    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
-        virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
-        virPCIDevicePtr actual;
-
-        /* We need to look up the actual device because that's what
-         * virHostdevReattachPCIDevice() expects as its argument */
-        if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
-            continue;
-
-        if (virPCIDeviceGetManaged(actual))
-            virHostdevReattachPCIDevice(mgr, actual);
-        else
-            VIR_DEBUG("Not reattaching unmanaged PCI device %s",
-                      virPCIDeviceGetName(actual));
-    }
+    virHostdevReattachAllPCIDevices(pcidevs, mgr);
 
     virObjectUnlock(mgr->activePCIHostdevs);
     virObjectUnlock(mgr->inactivePCIHostdevs);
-- 
2.21.0