[libvirt] [PATCH 0/5] Another round of mem leak fixes
*** BLURB HERE *** Michal Privoznik (5): virDomainNetDefClear: Free @coalesce testCompareMemLock: Use correct free function for domain def securityselinuxtest: Don't leak @mgr vircgrouptest: Don't leak @cgroup virhostdevtest: Don't leak @mgr->activeSCSIHostdevs src/conf/domain_conf.c | 1 + tests/qemumemlocktest.c | 2 +- tests/securityselinuxtest.c | 1 + tests/vircgrouptest.c | 3 +++ tests/virhostdevtest.c | 3 ++- 5 files changed, 8 insertions(+), 2 deletions(-) -- 2.13.0
In virDomainNetDefParseXML() the def->coalesce is parsed and allocated by virDomainNetDefCoalesceParseXML() but in fact it's never freed . Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5ce2ecd9..b487ff2fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2040,6 +2040,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) VIR_FREE(def->ifname_guest); VIR_FREE(def->ifname_guest_actual); VIR_FREE(def->virtio); + VIR_FREE(def->coalesce); virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); -- 2.13.0
On Thu, Aug 10, 2017 at 09:29:06AM +0200, Michal Privoznik wrote:
In virDomainNetDefParseXML() the def->coalesce is parsed and allocated by virDomainNetDefCoalesceParseXML() but in fact it's never freed .
s/d \./d./
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5ce2ecd9..b487ff2fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2040,6 +2040,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) VIR_FREE(def->ifname_guest); VIR_FREE(def->ifname_guest_actual); VIR_FREE(def->virtio); + VIR_FREE(def->coalesce);
virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); -- 2.13.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
This is a nice example of volkswagened code. It passes tests but doesn't work as expected really. virDomainDef is not an instance of virObject thus virObjectUnref() is not the correct function to be called. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemumemlocktest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index 66ebabb65..62bd25450 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -50,7 +50,7 @@ testCompareMemLock(const void *data) ret = virTestCompareToULL(info->memlock, qemuDomainGetMemLockLimitBytes(def)); cleanup: - virObjectUnref(def); + virDomainDefFree(def); VIR_FREE(xml); return ret; -- 2.13.0
On Thu, Aug 10, 2017 at 09:29:07AM +0200, Michal Privoznik wrote:
This is a nice example of volkswagened code. It passes tests but
Please remove the brand reference before pushing Jan
doesn't work as expected really. virDomainDef is not an instance of virObject thus virObjectUnref() is not the correct function to be called.
The security manager is created so that test cases can use it. However, it is never released. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/securityselinuxtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 63161b9b3..767b6cc02 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -337,6 +337,7 @@ mymain(void) "svirt_t", "svirt_image_t", 0, 0, 0, 1023); + virObjectUnref(mgr); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.13.0
In these test cases we create internal representation of cgroup, however, never free it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/vircgrouptest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index b932b1ad5..1e551cfc7 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -353,6 +353,7 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED) } /* Should now work */ + virCgroupFree(&cgroup); if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment/production cgroup: %d\n", -rv); goto cleanup; @@ -401,12 +402,14 @@ static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED goto cleanup; } + virCgroupFree(&cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); goto cleanup; } /* Should now work */ + virCgroupFree(&cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user/production cgroup: %d\n", -rv); goto cleanup; -- 2.13.0
So the hostdev manager has some lists to keep track which devices are active (=assigned to a domain) or inactive. The manager and its lists are allocated in myInit and freed in myCleanup but one of them (activeSCSIHostdevs) was missing. Also, the order in which the cleanup was done doesn't make it easy to spot it, therefore reoder it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virhostdevtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index 655991c3c..0ad58ddf3 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -73,8 +73,9 @@ myCleanup(void) virFileDeleteTree(mgr->stateDir); virObjectUnref(mgr->activePCIHostdevs); - virObjectUnref(mgr->inactivePCIHostdevs); virObjectUnref(mgr->activeUSBHostdevs); + virObjectUnref(mgr->inactivePCIHostdevs); + virObjectUnref(mgr->activeSCSIHostdevs); VIR_FREE(mgr->stateDir); VIR_FREE(mgr); } -- 2.13.0
On Thu, Aug 10, 2017 at 09:29:05AM +0200, Michal Privoznik wrote:
*** BLURB HERE ***
Reblurbed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik (5): virDomainNetDefClear: Free @coalesce testCompareMemLock: Use correct free function for domain def securityselinuxtest: Don't leak @mgr vircgrouptest: Don't leak @cgroup virhostdevtest: Don't leak @mgr->activeSCSIHostdevs
src/conf/domain_conf.c | 1 + tests/qemumemlocktest.c | 2 +- tests/securityselinuxtest.c | 1 + tests/vircgrouptest.c | 3 +++ tests/virhostdevtest.c | 3 ++- 5 files changed, 8 insertions(+), 2 deletions(-)
-- 2.13.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Ján Tomko -
Martin Kletzander -
Michal Privoznik