On 7/19/19 8:52 AM, Michal Privoznik wrote:
On 7/18/19 10:10 PM, Daniel Henrique Barboza wrote:
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@xxxxxxxxx>
---
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();
+ }
+}
What good is there in introducing this new function if you're removing
in the very next patch?
Yep, I'll probably merge this up with patch 3 in a re-spin.
+
+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);
Just move its internals here. I mean, even from the code you're moving
we would be calling virPCIDeviceReattach().
+ else
+ VIR_DEBUG("Not reattaching unmanaged PCI device %s",
+ virPCIDeviceGetName(actual));
+ }
+}
+
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
