Re: [libvirt] [PATCH] libxl: dispose libxl_dominfo after libxl_domain_info()

Joao Martins wrote: > As suggested in a previous thread [0] this patch adds some missing calls > to libxl_dominfo_dispose when doing some of the libxl_domain_info > operations which would otherwise lead to memory leaks. > > [0] > https://www.redhat.com/archives/libvir-list/2015-September/msg00519.html > > Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com&gt; > --- > src/libxl/libxl_driver.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 73ed448..a449730 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -358,6 +358,8 @@ libxlReconnectDomain(virDomainObjPtr vm, > > virObjectLock(vm); > > + libxl_dominfo_init(&d_info); > + > /* Does domain still exist? */ > rc = libxl_domain_info(cfg->ctx, &d_info, vm->def->id); > if (rc == ERROR_INVAL) { > @@ -389,11 +391,13 @@ libxlReconnectDomain(virDomainObjPtr vm, > /* Enable domain death events */ > libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); > > + libxl_dominfo_dispose(&d_info); > virObjectUnlock(vm); > virObjectUnref(cfg); > return 0; > > out: > + libxl_dominfo_dispose(&d_info); > libxlDomainCleanup(driver, vm); > if (!vm->persistent) > virDomainObjListRemoveLocked(driver->domains, vm); > @@ -1598,6 +1602,8 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) > info->cpuTime = d_info.cpu_time; > info->memory = d_info.current_memkb; > info->maxMem = d_info.max_memkb; > + > + libxl_dominfo_dispose(&d_info); There should be a corresponding libxl_dominfo_init(). While looking at this, I audited all uses of libxl_domain_info() and found another missing _init(). With the below diff squashed in, I think this code is finally correct :-). Do you agree?

Regards, Jim diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 21c7610..4a9134e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1593,6 +1593,8 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) info->memory = vm->def->mem.cur_balloon; info->maxMem = virDomainDefGetMemoryActual(vm->def); } else { + libxl_dominfo_init(&d_info); + if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxl_domain_info failed for domain '%d'"), @@ -4797,6 +4799,7 @@ libxlDomainMemoryStats(virDomainPtr dom, virCheckFlags(0, -1); + libxl_dominfo_init(&d_info); cfg = libxlDriverConfigGet(driver); if (!(vm = libxlDomObjFromDomain(dom))) @@ -4828,13 +4831,12 @@ libxlDomainMemoryStats(virDomainPtr dom, ret = i; - libxl_dominfo_dispose(&d_info); - endjob: if (!libxlDomainObjEndJob(driver, vm)) vm = NULL; cleanup: + libxl_dominfo_dispose(&d_info); if (vm) virObjectUnlock(vm); virObjectUnref(cfg);