Re: [libvirt] [PATCH 15/24] hostdev: Remove virHostdevGetActivePCIHostDeviceList()
On Fri, 2016-03-11 at 14:01 -0500, John Ferlan wrote:
Please read my logic in patch 20 before doing anything - I'm in
the
middle of it... scroll down for my short thoughts [1]...
Your comments to patch 20 are extremely detailed, so it will
take me a while to go through them and reply properly.
In the meantime, I'll give a short reply to your short
thoughts :)
> > > virPCIDeviceListDel(pcidevs, dev);
> > > continue;
[1] this...
> > > }
> > > + } else {
> > > + virPCIDeviceListDel(pcidevs, dev);
> > > + continue;
[1] ... and this mean...
> > > }
> > >
> > > VIR_DEBUG("Removing PCI device %s from active list",
[1] ... this doesn't happen!
The code goes like this:
activeDev = virPCIDeviceListFind(mgr->activePCIHostdevs, dev);
if (activeDev) {
const char *usedby_drvname;
const char *usedby_domname;
virPCIDeviceGetUsedBy(activeDev, &usedby_drvname, &usedby_domname);
if (STRNEQ_NULLABLE(drv_name, usedby_drvname) ||
STRNEQ_NULLABLE(dom_name, usedby_domname)) {
virPCIDeviceListDel(pcidevs, dev);
continue;
}
} else {
virPCIDeviceListDel(pcidevs, dev);
continue;
}
VIR_DEBUG("Removing PCI device %s from active list",
virPCIDeviceGetName(dev));
virPCIDeviceListDel(mgr->activePCIHostdevs, dev);
i++;
So 'dev' is used too look up 'activeDev' (should be 'actual', but
the variable renaming patch has not been applyed at this point in
time) in the active list.
If no match is found, it means that the PCI device 'dev' is
referring to is not actually active, and we should remove it from
'pcidevs' (so that it doesn't get processed further) and move on
to the next one.
If a match is found, but the actual device is used either by a
different driver or by a different domain, we do the same thing:
remove 'dev' from 'pcidevs' and move on.
If neither of the above is true, namely if the device *is* active
and *is* used by the driver and domain we're processing, *then*
we proceed to remove it from the active list and, later, reattach
it to the host.
It should be noted that "making sure the devices we're about to
reattach to the host are the ones this domain is using" and
"remove devices from the active list" are split in patch 22,
with the latter being joined with "add devices to the inactive
list".
All in all, I believe this is still correct and can be pushed
along with patches 18 and 19 (after taking care of your
comments there) before moving on with the more troublesome
patch 20. Let me know whether you agree :)
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team