12:54 p.m.
On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
virHostdevGetPCIHostDeviceList() is similar but does not filter out
devices that are not in the active list; that said, we are looking
up the device in the active list just a few lines after anyway, so
we might as well just keep a single function around.
This also helps stress the fact the objects contained in pcidevs are
only for looking up the actual devices, which is something later
commits will make even more explicit.
---
src/util/virhostdev.c | 50 ++++----------------------------------------------
1 file changed, 4 insertions(+), 46 deletions(-)
Existing code uses virPCIDeviceListAddCopy (as does code that populates
inactiveDevs list). The AddCopy code will
1. virPCIDeviceCopy(virPCIDevicePtr dev)
VIR_ALLOC(copy);
*copy = *dev;
Update copy->path, copy->used_by_drvname, & copy->used_by_domname
2. Add to a virPCIDeviceListPtr
The new code.
1. virPCIDeviceNew for a virPCIDevicePtr pci;
VIR_ALLOC(dev);
copy in dev->address
generate dev->name, dev->id (similar to copy)
generate dev->path from dev->name
2. Add to a virPCIDeviceListPtr
3. Caller sets the managed and stubDriver backend.
NOTE: there's no copy of 'used_by_drvname' and 'used_by_domname',
like
the copy function nor are a few other fields set. This seems to be
important later [1].
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index fef7898..67e6e7b 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -245,49 +245,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int
nhostdevs)
}
-/*
- * virHostdevGetActivePCIHostDeviceList - make a new list with a *copy* of
- * every virPCIDevice object that is found on the activePCIHostdevs
- * list *and* is in the hostdev list for this domain.
- *
- * Return the new list, or NULL if there was a failure.
- *
- * Pre-condition: activePCIHostdevs is locked
- */
-static virPCIDeviceListPtr
-virHostdevGetActivePCIHostDeviceList(virHostdevManagerPtr mgr,
- virDomainHostdevDefPtr *hostdevs,
- int nhostdevs)
-{
- virPCIDeviceListPtr list;
- size_t i;
-
- if (!(list = virPCIDeviceListNew()))
- return NULL;
-
- for (i = 0; i < nhostdevs; i++) {
- virDomainHostdevDefPtr hostdev = hostdevs[i];
- virDevicePCIAddressPtr addr;
- virPCIDevicePtr activeDev;
-
- if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
- continue;
- if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
- continue;
-
- addr = &hostdev->source.subsys.u.pci.addr;
- activeDev = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
- addr->domain, addr->bus,
- addr->slot, addr->function);
- if (activeDev && virPCIDeviceListAddCopy(list, activeDev) < 0) {
- virObjectUnref(list);
- return NULL;
- }
- }
-
- return list;
-}
-
static int
virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev,
char **sysfs_path)
@@ -800,9 +757,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
virObjectLock(hostdev_mgr->activePCIHostdevs);
virObjectLock(hostdev_mgr->inactivePCIHostdevs);
- if (!(pcidevs = virHostdevGetActivePCIHostDeviceList(hostdev_mgr,
- hostdevs,
- nhostdevs))) {
+ if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs))) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to allocate PCI device list: %s"),
err ? err->message : _("unknown error"));
@@ -832,6 +787,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
[1] This code checks for usedby_drvname and usedby_domname
These are set by virHostdevPreparePCIDevices and
virHostdevUpdateActivePCIDevices. The virHostdevPreparePCIDevices is
the only current caller of virHostdevGetPCIHostDeviceList. The latter
will call the virPCIDeviceNew and then set the Managed, UsedBy, and
stubDriver fields.
The PreparePCIDevices code seems to do many of the same functions for
the activePCIHostdevs.
I think this one needs to be rethought..
John
virPCIDeviceListDel(pcidevs, dev);
continue;
}
+ } else {
+ virPCIDeviceListDel(pcidevs, dev);
+ continue;
}
VIR_DEBUG("Removing PCI device %s from active list",