Re: [libvirt] [PATCH 15/24] hostdev: Remove virHostdevGetActivePCIHostDeviceList()

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", > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list