On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
We might be just fine looking up the information in pcidevs, but it wouldn't save us any trouble and it's better to be consistent. --- src/util/virhostdev.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index a431f0a..03c3445 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -650,7 +650,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
/* Step 8: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { - virPCIDevicePtr pci; + virPCIDevicePtr actual; virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci;
@@ -659,24 +659,27 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue;
- pci = virPCIDeviceListFindByIDs(pcidevs, - pcisrc->addr.domain, - pcisrc->addr.bus, - pcisrc->addr.slot, - pcisrc->addr.function); + /* We need to look up the actual device because it's the one + * that contains the information we care about (unbind_from_stub, + * remove_slot, reprobe) */
When a device goes from the inactivePCIHostdevs list to the activePCIHostdevs list "at some point in time" in the future - does it do a similar save? That is this change only grabs devices from the active list for the save; whereas, prior to this change it would pull from all pcidevs
+ actual = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs, + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function);
/* Appropriate values for the unbind_from_stub, remove_slot * and reprobe properties of the device were set earlier * by virPCIDeviceDetach() */ - if (pci) { + if (actual) { VIR_DEBUG("Saving network configuration of PCI device %s", - virPCIDeviceGetName(pci)); + virPCIDeviceGetName(actual)); hostdev->origstates.states.pci.unbind_from_stub = - virPCIDeviceGetUnbindFromStub(pci); + virPCIDeviceGetUnbindFromStub(actual); hostdev->origstates.states.pci.remove_slot = - virPCIDeviceGetRemoveSlot(pci); + virPCIDeviceGetRemoveSlot(actual); hostdev->origstates.states.pci.reprobe = - virPCIDeviceGetReprobe(pci); + virPCIDeviceGetReprobe(actual); } }
@@ -844,17 +847,17 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
if (virHostdevIsPCINetDevice(hostdev)) { virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; - virPCIDevicePtr pci; + virPCIDevicePtr actual;
- pci = virPCIDeviceListFindByIDs(pcidevs, - pcisrc->addr.domain, - pcisrc->addr.bus, - pcisrc->addr.slot, - pcisrc->addr.function);
So the previous loop took care of the activePCIHostdevs, correct? And this loop takes care of the inactivePCIHostdevs for reattachement? I think this part is correct - although is it worthy of splitting into it's own separate patch? John Going to stop here for now (we have company).
+ actual = virPCIDeviceListFindByIDs(mgr->inactivePCIHostdevs, + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function);
- if (pci) { + if (actual) { VIR_DEBUG("Restoring network configuration of PCI device %s", - virPCIDeviceGetName(pci)); + virPCIDeviceGetName(actual)); virHostdevNetConfigRestore(hostdev, mgr->stateDir, oldStateDir); }