[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