[libvirt] [PATCH 0/4] qemu: process: Fix memleaks on VM restart
Few leaks with automatic pinning, USB addresses and TLS migration, since the private data structure needs to be cleared manually in qemuProcessStop. Peter Krempa (4): qemu: process: Clean automatic NUMA/cpu pinning information on shutdown qemu: process: Don't leak priv->usbaddrs after VM restart qemu: process: Clean up priv->migTLSAlias qemu: Move freeing of PCI address list to qemuProcessStop src/qemu/qemu_domain_address.c | 1 - src/qemu/qemu_process.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) -- 2.12.2
Clean the stale data after shutting down the VM. Otherwise the data would be leaked on next VM start. This happens due to the fact that the private data object is not freed on destroy of the VM. --- src/qemu/qemu_process.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 53170d732..e44359575 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6435,6 +6435,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, priv->qemuCaps = NULL; VIR_FREE(priv->pidfile); + /* remove automatic pinning data */ + virBitmapFree(priv->autoNodeset); + priv->autoNodeset = NULL; + virBitmapFree(priv->autoCpuset); + priv->autoCpuset = NULL; + /* The "release" hook cleans up additional resources */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); -- 2.12.2
Since the private data structure is not freed upon stopping a VM, the usbaddrs pointer would be leaked: ==15388== 136 (16 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 893 of 1,019 ==15388== at 0x4C2CF55: calloc (vg_replace_malloc.c:711) ==15388== by 0x54BF64A: virAlloc (viralloc.c:144) ==15388== by 0x5547588: virDomainUSBAddressSetCreate (domain_addr.c:1608) ==15388== by 0x144D38A2: qemuDomainAssignUSBAddresses (qemu_domain_address.c:2458) ==15388== by 0x144D38A2: qemuDomainAssignAddresses (qemu_domain_address.c:2515) ==15388== by 0x144ED1E3: qemuProcessPrepareDomain (qemu_process.c:5398) ==15388== by 0x144F51FF: qemuProcessStart (qemu_process.c:5979) [...] --- src/qemu/qemu_process.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e44359575..6787c47cf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6441,6 +6441,10 @@ void qemuProcessStop(virQEMUDriverPtr driver, virBitmapFree(priv->autoCpuset); priv->autoCpuset = NULL; + /* remove address data */ + virDomainUSBAddressSetFree(priv->usbaddrs); + priv->usbaddrs = NULL; + /* The "release" hook cleans up additional resources */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); -- 2.12.2
The alias would be leaked, since it's not freed on the vm stop path. --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6787c47cf..d3266e778 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6445,6 +6445,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainUSBAddressSetFree(priv->usbaddrs); priv->usbaddrs = NULL; + /* clean up migration data */ + VIR_FREE(priv->migTLSAlias); + /* The "release" hook cleans up additional resources */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); -- 2.12.2
Rather than freeing the list before starting a new VM clear it after stopping the old instance when the data becomes invalid. --- src/qemu/qemu_domain_address.c | 1 - src/qemu/qemu_process.c | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 064d05079..203a367ac 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2221,7 +2221,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (obj && obj->privateData) { priv = obj->privateData; /* if this is the live domain object, we persist the PCI addresses */ - virDomainPCIAddressSetFree(priv->pciaddrs); priv->pciaddrs = addrs; addrs = NULL; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d3266e778..3a18c96ca 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6442,6 +6442,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, priv->autoCpuset = NULL; /* remove address data */ + virDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; virDomainUSBAddressSetFree(priv->usbaddrs); priv->usbaddrs = NULL; -- 2.12.2
On 04/26/2017 09:03 AM, Peter Krempa wrote:
Few leaks with automatic pinning, USB addresses and TLS migration, since the private data structure needs to be cleared manually in qemuProcessStop.
Peter Krempa (4): qemu: process: Clean automatic NUMA/cpu pinning information on shutdown qemu: process: Don't leak priv->usbaddrs after VM restart qemu: process: Clean up priv->migTLSAlias qemu: Move freeing of PCI address list to qemuProcessStop
src/qemu/qemu_domain_address.c | 1 - src/qemu/qemu_process.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-)
ACK series, John
participants (2)
-
John Ferlan -
Peter Krempa