[libvirt] [PATCH] qemu: Get the dev from activePciHostdevs list before reattachment
Eric Blake
eblake at redhat.com
Wed Oct 12 17:31:09 UTC 2011
On 10/11/2011 01:59 AM, Osier Yang wrote:
> BZ# https://bugzilla.redhat.com/show_bug.cgi?id=736214
>
> The problem is caused by the original info of domain's PCI dev is
> maintained by qemu_driver->activePciHostdevs list, (E.g. dev->reprobe,
> which stands for whether need to reprobe driver for the dev when do
> reattachment). The fields (dev->reprobe, dev->unbind_from_stub, and
> dev->remove_slot) are initialized properly when preparing the PCI
> device for managed attachment. However, when do reattachment, it
> construct a complete new "pciDevice" without honoring the original
> dev info, and thus the dev won't get the original driver or can get
> other problem.
>
> This patch is to fix the problem by get the dev from list
> driver->activePciHostdevs if it's in the list, though it's unlikely
> the dev to be reattached is not in the list, as any PCI dev is added
> to the list when do preparation (qemuPrepareHostdevPCIDevices),
> the patch doesn't completely desert the old design so the device
> can be reattached even if it's not in list driver->activePciHostdevs.
>
> @@ -272,12 +273,23 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
> return;
> }
>
> + if (!(pci_list = pciDeviceListNew()))
> + return;
Memory leak. You must goto cleanup, so that pcidevs is not lost.
> +
> /* Again 3 loops; mark all devices as inactive before reset
> * them and reset all the devices before re-attach */
Comment says 3 loops, but you added a fourth one (although conceptually,
the last two loops are really a combination of one visit to the complete
set of devices, divided into two subsets according to whether we are
reusing active information).
> -
> for (i = 0; i< pciDeviceListCount(pcidevs); i++) {
> pciDevice *dev = pciDeviceListGet(pcidevs, i);
> - pciDeviceListDel(driver->activePciHostdevs, dev);
> + pciDevice *pci = NULL;
> + pci = pciDeviceListSteal(driver->activePciHostdevs, dev);
> +
> + if (pci) {
> + pciDeviceListDel(pcidevs, dev);
> + if (pciDeviceListAdd(pci_list, pci)< 0) {
> + pciFreeDevice(pci);
> + goto cleanup;
> + }
> + }
> }
I'm wondering if all this transfer work between two lists is really
necessary, or if there is a more efficient way to just copy the
information from driver->activePciHostdevs back into pcidevs, without
having to create a second pci_list.
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list