Re: [libvirt] [PATCH] Implement dispose method for libxlDomainObjPrivate
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(a)citrix.com>
---
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);
}