[libvirt] [PATCH 3/4] qemu: Fix memory (and FD) leak on PCI device detach
Jiri Denemark
jdenemar at redhat.com
Tue Dec 4 17:48:12 UTC 2012
On Tue, Dec 04, 2012 at 22:29:23 +0800, Osier Yang wrote:
> On 2012年12月04日 18:38, Jiri Denemark wrote:
> > Unmanaged PCI devices were only leaked if pciDeviceListAdd failed but
> > managed devices were always leaked. And leaking PCI device is likely to
> > leave PCI config file descriptor open. This patch fixes
> > qemuReattachPciDevice to either free the PCI device or add it to the
> > inactivePciHostdevs list.
> > ---
> > src/qemu/qemu_hostdev.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> > index b79319e..a748b8b 100644
> > --- a/src/qemu/qemu_hostdev.c
> > +++ b/src/qemu/qemu_hostdev.c
> > @@ -546,10 +546,8 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
> > }
> >
> > /* Loop 9: Now steal all the devices from pcidevs */
> > - while (pciDeviceListCount(pcidevs)> 0) {
> > - pciDevice *dev = pciDeviceListGet(pcidevs, 0);
> > - pciDeviceListSteal(pcidevs, dev);
> > - }
> > + while (pciDeviceListCount(pcidevs)> 0)
> > + pciDeviceListStealIndex(pcidevs, 0);
> >
> > ret = 0;
> > goto cleanup;
> > @@ -818,7 +816,8 @@ void qemuReattachPciDevice(pciDevice *dev, virQEMUDriverPtr driver)
> > * successfully, it must have been inactive.
> > */
> > if (!pciDeviceGetManaged(dev)) {
> > - pciDeviceListAdd(driver->inactivePciHostdevs, dev);
> > + if (pciDeviceListAdd(driver->inactivePciHostdevs, dev)< 0)
> > + pciFreeDevice(dev);
> > return;
> > }
> >
> > @@ -835,6 +834,7 @@ void qemuReattachPciDevice(pciDevice *dev, virQEMUDriverPtr driver)
> > err ? err->message : _("unknown error"));
> > virResetError(err);
> > }
> > + pciFreeDevice(dev);
> > }
>
> This produces the similar problem 1/4 tries to fix. pciDeviceListFree
> right after will free the whole list. Except this, the using of
> pciDeviceListStealIndex is nice.
I don't think it does. There are only two places where qemuReattachPciDevice
is called. The first one is the hunk you removed from this patch:
@@ -905,8 +905,8 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
}
}
- for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
- pciDevice *dev = pciDeviceListGet(pcidevs, i);
+ while (pciDeviceListCount(pcidevs) > 0) {
+ pciDevice *dev = pciDeviceListStealIndex(pcidevs, 0);
qemuReattachPciDevice(dev, driver);
}
and its purpose is to fix the caller to call qemuReattachPciDevice with a
device that is not referenced from pcidevs list (hence, it changes
pciDeviceListGet into pciDeviceListStealIndex). And the second one is in
qemu_hotplug.c:
activePci = pciDeviceListSteal(driver->activePciHostdevs, pci);
if (activePci &&
pciResetDevice(activePci, driver->activePciHostdevs,
driver->inactivePciHostdevs) == 0) {
qemuReattachPciDevice(activePci, driver);
} else {
/* reset of the device failed, treat it as if it was returned */
pciFreeDevice(activePci);
ret = -1;
}
That is, it has already been relying on qemuReattachPciDevice to properly free
activePci if needed.
Jirka
More information about the libvir-list
mailing list