Re: [PATCH 2/3] libxl: fix resource leaks in libxlDomainStart error paths

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Reviewed-by: Chunyan Liu <cyliu@xxxxxxxx>

>>> On 3/29/2016 at 08:54 AM, in message
<1459212889-5490-3-git-send-email-jfehlig@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> wrote: 
> From: Chunyan Liu <cyliu@xxxxxxxx> 
>  
> libxlDomainStart allocates and reserves resources that were not 
> being released in error paths. libxlDomainCleanup already handles 
> the job of releasing resources, and libxlDomainStart should call 
> it when encountering a failure. 
>  
> Change the error handling logic to call libxlDomainCleanup on 
> failure. This includes acquiring the lease sooner and allowing 
> it to be released in libxlDomainCleanup on failure, similar to 
> the way other resources are reclaimed. With the lease now 
> released in libxlDomainCleanup, the release_dom label can be 
> renamed to cleanup_dom to better reflect its changed semantics. 
>  
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx> 
> --- 
>  src/libxl/libxl_domain.c | 28 ++++++++++++++-------------- 
>  1 file changed, 14 insertions(+), 14 deletions(-) 
>  
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c 
> index ead3d09..068bfb6 100644 
> --- a/src/libxl/libxl_domain.c 
> +++ b/src/libxl/libxl_domain.c 
> @@ -1036,17 +1036,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
>                                      vm, true) < 0) 
>          goto cleanup; 
>   
> -    if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, 
> -                               cfg->ctx, &d_config) < 0) 
> -        goto cleanup; 
> - 
> -    if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) 
> -        goto cleanup; 
> - 
> -    if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, 
> -                                       vm->def, VIR_HOSTDEV_SP_PCI) < 0) 
> -        goto cleanup; 
> - 
>      if (virDomainLockProcessStart(driver->lockManager, 
>                                    "xen:///system", 
>                                    vm, 
> @@ -1061,6 +1050,17 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
>          goto cleanup; 
>      VIR_FREE(priv->lockState); 
>   
> +    if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, 
> +                               cfg->ctx, &d_config) < 0) 
> +        goto cleanup_dom; 
> + 
> +    if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) 
> +        goto cleanup_dom; 
> + 
> +    if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, 
> +                                       vm->def, VIR_HOSTDEV_SP_PCI) < 0) 
> +        goto cleanup_dom; 
> + 
>      /* Unlock virDomainObj while creating the domain */ 
>      virObjectUnlock(vm); 
>   
> @@ -1091,7 +1091,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
>              virReportError(VIR_ERR_INTERNAL_ERROR, 
>                             _("libxenlight failed to restore domain '%s'"), 
>                             d_config.c_info.name); 
> -        goto release_dom; 
> +        goto cleanup_dom; 
>      } 
>   
>      /* 
> @@ -1152,8 +1152,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
>      vm->def->id = -1; 
>      virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF,  
> VIR_DOMAIN_SHUTOFF_FAILED); 
>   
> - release_dom: 
> -    virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState); 
> + cleanup_dom: 
> +    libxlDomainCleanup(driver, vm); 
>   
>   cleanup: 
>      libxl_domain_config_dispose(&d_config); 
 


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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]