[libvirt] [PATCH 22/24] hostdev: Streamline device ownership tracking

After this patch, ownership of virPCIDevice instances is very easy to keep track of: for each host PCI device, the only instance that actually matters is the one inside one of the bookkeeping list. Whenever some operation needs to be performed on a PCI device, the actual device is looked up first; when this is not the case, a comment explains the reason. --- src/util/virhostdev.c | 118 +++++++++++++++++++++++++++----------------------- 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index d1529c5..b2f54cd 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -618,28 +618,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, last_processed_hostdev_vf = i; } - /* Step 5: Now mark all the devices as active */ + /* Step 5: Move devices from the inactive list to the active list */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; - VIR_DEBUG("Adding PCI device %s to active list", + VIR_DEBUG("Removing PCI device %s from inactive list", virPCIDeviceGetName(pci)); - if (virPCIDeviceListAdd(mgr->activePCIHostdevs, pci) < 0) - goto inactivedevs; - } - - /* Step 6: Now remove the devices from inactive list. */ - for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { - virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + actual = virPCIDeviceListSteal(mgr->inactivePCIHostdevs, pci); - VIR_DEBUG("Removing PCI device %s from inactive list", + VIR_DEBUG("Adding PCI device %s to active list", virPCIDeviceGetName(pci)); - virPCIDeviceListDel(mgr->inactivePCIHostdevs, pci); + if (!actual || virPCIDeviceListAdd(mgr->activePCIHostdevs, actual) < 0) + goto inactivedevs; } - /* Step 7: Now set the used_by_domain of the device in - * activePCIHostdevs as domain name. - */ + /* Step 6: Set driver and domain information */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci, actual; @@ -655,7 +649,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, virPCIDeviceSetUsedBy(actual, drv_name, dom_name); } - /* Step 8: Now set the original states for hostdev def */ + /* Step 7: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { virPCIDevicePtr actual; virDomainHostdevDefPtr hostdev = hostdevs[i]; @@ -690,23 +684,25 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, } } - /* Step 9: Now steal all the devices from pcidevs */ - while (virPCIDeviceListCount(pcidevs) > 0) - virPCIDeviceListStealIndex(pcidevs, 0); - ret = 0; goto cleanup; inactivedevs: - /* Only steal all the devices from activePCIHostdevs. We will - * free them in virObjectUnref(). - */ + /* Move devices back to the inactive list so that they can be + * processed properly below (reattachdevs label) */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + if (!(actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci))) + continue; + + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) + continue; } resetvfnetconfig: @@ -716,16 +712,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, reattachdevs: for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; - if (virPCIDeviceGetManaged(pci)) { + /* We need to look up the actual device because that's what + * virPCIDeviceReattach() exepects 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(pci, + virPCIDeviceGetName(actual)); + ignore_value(virPCIDeviceReattach(actual, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs)); } else { VIR_DEBUG("Not reattaching unmanaged PCI device %s", - virPCIDeviceGetName(pci)); + virPCIDeviceGetName(actual)); } } @@ -745,17 +747,6 @@ static void virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, virPCIDevicePtr actual) { - /* If the device is not managed and was attached to guest - * successfully, it must have been inactive. - */ - if (!virPCIDeviceGetManaged(actual)) { - VIR_DEBUG("Adding unmanaged PCI device %s to inactive list", - virPCIDeviceGetName(actual)); - if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) - virPCIDeviceFree(actual); - return; - } - /* Wait for device cleanup if it is qemu/kvm */ if (virPCIDeviceGetStubDriver(actual) == VIR_PCI_STUB_DRIVER_KVM) { int retries = 100; @@ -774,7 +765,6 @@ virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, err ? err->message : _("unknown error")); virResetError(err); } - virPCIDeviceFree(actual); } /* @oldStateDir: @@ -836,17 +826,29 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, continue; } + i++; + } + + /* Step 2: Move devices from the active list to the inactive list */ + for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { + virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; + VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListDel(mgr->activePCIHostdevs, pci); - i++; + actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + + if (actual) { + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual); + } } - /* At this point, any device that had been used by the guest is in - * pcidevs, but has been removed from activePCIHostdevs. - */ + /* At this point, any device that had been used by the guest has been + * moved to the inactive list */ - /* Step 2: restore original network config of hostdevs that used + /* Step 3: restore original network config of hostdevs that used * <interface type='hostdev'> */ for (i = 0; i < nhostdevs; i++) { @@ -871,7 +873,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, } } - /* Step 3: perform a PCI Reset on all devices */ + /* Step 4: perform a PCI Reset on all devices */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); @@ -888,16 +890,26 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, } } - /* Step 4: reattach devices to their host drivers (if managed) or place - * them on the inactive list (if not managed) - */ - while (virPCIDeviceListCount(pcidevs) > 0) { - virPCIDevicePtr pci = virPCIDeviceListStealIndex(pcidevs, 0); - virHostdevReattachPCIDevice(mgr, pci); + /* 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)); } - virObjectUnref(pcidevs); cleanup: + virObjectUnref(pcidevs); virObjectUnlock(mgr->activePCIHostdevs); virObjectUnlock(mgr->inactivePCIHostdevs); } -- 2.5.0