[libvirt] [PATCH] qemuDomainObjPrivateDataClear: Don't leak @migParams

Michal Privoznik posted 1 patch 5 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/29f05733ca7651abde28a8292ccec10db015683c.1524041533.git.mprivozn@redhat.com
Test syntax-check passed
There is a newer version of this series
src/qemu/qemu_domain.c | 2 ++
1 file changed, 2 insertions(+)
[libvirt] [PATCH] qemuDomainObjPrivateDataClear: Don't leak @migParams
Posted by Michal Privoznik 5 years, 12 months ago
Allocated in qemuMigrationParamsNew() we need to free
priv->job.migParams when no longer needed.

==8061== 234 (192 direct, 42 indirect) bytes in 1 blocks are definitely lost in loss record 112 of 123
==8061==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
==8061==    by 0x5325D05: virAlloc (viralloc.c:144)
==8061==    by 0x1984F9: qemuMigrationParamsNew (qemu_migration_params.c:218)
==8061==    by 0x19A352: qemuMigrationParamsParse (qemu_migration_params.c:1185)
==8061==    by 0x1604D8: qemuDomainObjPrivateXMLParseJob (qemu_domain.c:2390)
==8061==    by 0x160AE9: qemuDomainObjPrivateXMLParse (qemu_domain.c:2517)
==8061==    by 0x5419EAE: virDomainObjParseXML (domain_conf.c:20442)
==8061==    by 0x541A25E: virDomainObjParseNode (domain_conf.c:20555)
==8061==    by 0x541A2FC: virDomainObjParseFile (domain_conf.c:20574)
==8061==    by 0x13607D: testCompareStatusXMLToXMLFiles (qemuxml2xmltest.c:75)
==8061==    by 0x14F3E8: virTestRun (testutils.c:180)
==8061==    by 0x14DCD0: mymain (qemuxml2xmltest.c:1200)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 672f08b442..47595e244a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1895,6 +1895,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
 
     virBitmapFree(priv->migrationCaps);
     priv->migrationCaps = NULL;
+
+    qemuMigrationParamsFree(priv->job.migParams);
 }
 
 
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainObjPrivateDataClear: Don't leak @migParams
Posted by Jiri Denemark 5 years, 12 months ago
On Wed, Apr 18, 2018 at 10:52:13 +0200, Michal Privoznik wrote:
> Allocated in qemuMigrationParamsNew() we need to free
> priv->job.migParams when no longer needed.
> 
> ==8061== 234 (192 direct, 42 indirect) bytes in 1 blocks are definitely lost in loss record 112 of 123
> ==8061==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
> ==8061==    by 0x5325D05: virAlloc (viralloc.c:144)
> ==8061==    by 0x1984F9: qemuMigrationParamsNew (qemu_migration_params.c:218)
> ==8061==    by 0x19A352: qemuMigrationParamsParse (qemu_migration_params.c:1185)
> ==8061==    by 0x1604D8: qemuDomainObjPrivateXMLParseJob (qemu_domain.c:2390)
> ==8061==    by 0x160AE9: qemuDomainObjPrivateXMLParse (qemu_domain.c:2517)
> ==8061==    by 0x5419EAE: virDomainObjParseXML (domain_conf.c:20442)
> ==8061==    by 0x541A25E: virDomainObjParseNode (domain_conf.c:20555)
> ==8061==    by 0x541A2FC: virDomainObjParseFile (domain_conf.c:20574)
> ==8061==    by 0x13607D: testCompareStatusXMLToXMLFiles (qemuxml2xmltest.c:75)
> ==8061==    by 0x14F3E8: virTestRun (testutils.c:180)
> ==8061==    by 0x14DCD0: mymain (qemuxml2xmltest.c:1200)

Oh, it's triggered by our tests... I was wondering how this could even
happen in real life since the job is supposed to be clean at the point
qemuDomainObjPrivateDataClear is called. Anyway, clearing the job there
explicitly shouldn't harm.

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 672f08b442..47595e244a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1895,6 +1895,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
>  
>      virBitmapFree(priv->migrationCaps);
>      priv->migrationCaps = NULL;
> +
> +    qemuMigrationParamsFree(priv->job.migParams);

However, if we're going to do it, this change is not enough. We should
call
    qemuDomainObjResetJob(priv);
    qemuDomainObjResetAsyncJob(priv);

to make sure all parts of the job structure are properly cleared.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list