On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
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 | 130 ++++++++++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 58 deletions(-)
ACK - couple of minor textual edits if you're interested below John
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 572aec0..e8efc53 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c
[...]
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) + VIR_WARN("Failed to add PCI device %s to the inactive list",
or "Failed to return PCI device ..."
+ virPCIDeviceGetName(pci)); }
[...]
+ /* 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); + + VIR_DEBUG("Adding PCI device %s to inactive list",
...to the inactive list
+ virPCIDeviceGetName(pci)); + if (!actual || + virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) { + + virErrorPtr err = virGetLastError(); + VIR_ERROR(_("Failed to add PCI device %s to inactive list"), + err ? err->message : _("unknown error")); + virResetError(err); + } }