[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Get the dev from activePciHostdevs list before reattachment

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,

+    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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]