Re: [PATCH 1/2] libxl: unref libxlDriverConfig object

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 11/18/2015 10:47 PM, Jim Fehlig wrote:
> Commits b6e19cf4 and 6472e54a missed unref'ing the
> libxlDriverConfig object. Add missing calls to virObjectUnref.
> 
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  src/libxl/libxl_driver.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index fd4bae1..4609c00 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -4648,18 +4648,21 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
>                              virTypedParameterPtr params,
>                              unsigned int nparams)
>  {
> -    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +    libxlDriverConfigPtr cfg;
>      libxl_dominfo d_info;
>      int ret = -1;
>  
>      if (nparams == 0)
>          return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
>  
> +    libxl_dominfo_init(&d_info);
> +    cfg = libxlDriverConfigGet(driver);
> +
>      if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("libxl_domain_info failed for domain '%d'"),
>                         vm->def->id);
> -        return ret;
> +        goto cleanup;
>      }
>  
>      if (virTypedParameterAssign(&params[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
> @@ -4670,6 +4673,7 @@ libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
>  
>   cleanup:
>      libxl_dominfo_dispose(&d_info);
> +    virObjectUnref(cfg);
>      return ret;
>  }
>  
> @@ -4684,7 +4688,7 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
>      libxl_vcpuinfo *vcpuinfo;
>      int maxcpu, hostcpus;
>      size_t i;
> -    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +    libxlDriverConfigPtr cfg;
>      int ret = -1;
>  
>      if (nparams == 0 && ncpus != 0)
> @@ -4692,12 +4696,13 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
>      else if (nparams == 0)
>          return vm->def->maxvcpus;
>  
> +    cfg = libxlDriverConfigGet(driver);
>      if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu,
>                                      &hostcpus)) == NULL) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Failed to list vcpus for domain '%d' with libxenlight"),
>                         vm->def->id);
> -        return ret;
> +        goto cleanup;
>      }
>  
>      for (i = start_cpu; i < maxcpu && i < ncpus; ++i) {
> @@ -4710,7 +4715,9 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
>      ret = nparams;
>  
>   cleanup:
> -    libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
> +    if (vcpuinfo)
> +        libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
> +    virObjectUnref(cfg);
>      return ret;
>  }
>  
> @@ -4766,7 +4773,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
>                         unsigned int flags)
>  {
>      libxlDriverPrivatePtr driver = dom->conn->privateData;
> -    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +    libxlDriverConfigPtr cfg;
>      virDomainObjPtr vm;
>      libxl_dominfo d_info;
>      unsigned mem, maxmem;
> @@ -4775,6 +4782,8 @@ libxlDomainMemoryStats(virDomainPtr dom,
>  
>      virCheckFlags(0, -1);
>  
> +    cfg = libxlDriverConfigGet(driver);
> +
>      if (!(vm = libxlDomObjFromDomain(dom)))
>          goto cleanup;
>  
> @@ -4815,6 +4824,7 @@ libxlDomainMemoryStats(virDomainPtr dom,
>   cleanup:
>      if (vm)
>          virObjectUnlock(vm);
> +    virObjectUnref(cfg);
>      return ret;
>  }
>  
> 
Looks great and also tested this series. I see you also moved cfg init further
down which avoids unnecessary initialization for some cases.

Regards,
Joao

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