Re: [libvirt] [PATCH v2 1/4] hostdev: Streamline device ownership tracking
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);
+ }
}