Re: [PATCH] Implement dispose method for libxlDomainObjPrivate

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

 



Frediano Ziglio wrote:
> When creating a timer/event handler reference counting is used. So it could
> be possible (in theory) that libxlDomainObjPrivateFree is called with
> reference counting >1. The problem is that libxlDomainObjPrivateFree leave
> the object in an invalid state with ctx freed (but still having dandling
> pointer). This can lead timer/event handler to core.
>   

Thanks for the analysis and the patch!

> This patch destroy the object before disposing it so at timer/event object
> is still valid.
>   

I'm not sure that is a good description of the fix :). IMO, it would be
clearer to say

This patch implements a dispose method for libxlDomainObjPrivate, and
moves freeing the libxl ctx to the dispose method, ensuring the ctx is
valid while the object's reference count is > 0.

> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> ---
>  src/libxl/libxl_driver.c |   20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 935919b..1c8cfd7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -110,6 +110,8 @@ static int
>  libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>               bool start_paused, int restore_fd);
>  
> +static void libxlDomainObjPrivateDispose(void *obj);
> +
>   

libvirt uses the pattern

static void
libxlDomainObjPrivateDispose(void *obj);

I've fixed these small nits and pushed the patch.

Regards,
Jim

>  /* Function definitions */
>  static int
>  libxlDomainObjPrivateOnceInit(void)
> @@ -117,7 +119,7 @@ libxlDomainObjPrivateOnceInit(void)
>      if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
>                                                     "libxlDomainObjPrivate",
>                                                     sizeof(libxlDomainObjPrivate),
> -                                                   NULL)))
> +                                                   libxlDomainObjPrivateDispose)))
>          return -1;
>  
>      return 0;
> @@ -418,14 +420,26 @@ libxlDomainObjPrivateAlloc(void)
>  }
>  
>  static void
> -libxlDomainObjPrivateFree(void *data)
> +libxlDomainObjPrivateDispose(void *obj)
>  {
> -    libxlDomainObjPrivatePtr priv = data;
> +    libxlDomainObjPrivatePtr priv = obj;
>  
>      if (priv->deathW)
>          libxl_evdisable_domain_death(priv->ctx, priv->deathW);
>  
>      libxl_ctx_free(priv->ctx);
> +}
> +
> +static void
> +libxlDomainObjPrivateFree(void *data)
> +{
> +    libxlDomainObjPrivatePtr priv = data;
> +
> +    if (priv->deathW) {
> +        libxl_evdisable_domain_death(priv->ctx, priv->deathW);
> +        priv->deathW = NULL;
> +    }
> +
>      virObjectUnref(priv);
>  }
>  
>   

--
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]