[libvirt] [PATCH 0/2] libxl: a few fixes related to event handling
Xen upstream commit 5fb11a07 fixed the signature of the event handler, causing build failures of the libxl driver on xen-unstable / Xen 4.3. While at it, plug leaking of libxl_event objects in the event handler. Jim Fehlig (2): libxl: fix build with Xen4.3 libxl: fix leaking libxl events src/libxl/libxl_driver.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-) -- 1.7.7
Xen 4.3 fixes a mistake in the libxl event handler signature where the event owned by the application was defined as const. Detect this and define the libvirt libxl event handler signature appropriately. --- src/libxl/libxl_driver.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b39e005..74e8331 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -700,7 +700,12 @@ libxlVmReap(libxlDriverPrivatePtr driver, * Handle previously registered event notification from libxenlight */ static void -libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event) +libxlEventHandler(void *data ATTRIBUTE_UNUSED, +#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG + libxl_event *event) +#else + const libxl_event *event) +#endif { libxlDriverPrivatePtr driver = libxl_driver; virDomainObjPtr vm = NULL; -- 1.7.7
On 05/16/2013 10:01 AM, Jim Fehlig wrote:
Xen 4.3 fixes a mistake in the libxl event handler signature where the event owned by the application was defined as const. Detect this and define the libvirt libxl event handler signature appropriately. --- src/libxl/libxl_driver.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b39e005..74e8331 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -700,7 +700,12 @@ libxlVmReap(libxlDriverPrivatePtr driver, * Handle previously registered event notification from libxenlight */ static void -libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event) +libxlEventHandler(void *data ATTRIBUTE_UNUSED, +#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG + libxl_event *event) +#else + const libxl_event *event) +#endif
This works, but I'm not a fan of mid-() #ifdefs (it doesn't work if the () itself is part of a macro call). I'd much rather see: #ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG # define VIR_LIBXL_EVENT_CONST /* empty */ #else # define VIR_LIBXL_EVENT_CONST const #endif static void libxlEventHandler(void *data ATTRIBUTE_UNUSED, VIR_LIBXL_EVENT_CONST libxl_event *event) { -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
libxl expects the event handler to free the event passed to it. From libxl_event.h: event becomes owned by the application and must be freed, either by event_occurs or later --- src/libxl/libxl_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 74e8331..7b21b3d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -700,7 +700,7 @@ libxlVmReap(libxlDriverPrivatePtr driver, * Handle previously registered event notification from libxenlight */ static void -libxlEventHandler(void *data ATTRIBUTE_UNUSED, +libxlEventHandler(void *data, #ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG libxl_event *event) #else @@ -708,6 +708,7 @@ libxlEventHandler(void *data ATTRIBUTE_UNUSED, #endif { libxlDriverPrivatePtr driver = libxl_driver; + libxlDomainObjPrivatePtr priv = ((virDomainObjPtr)data)->privateData; virDomainObjPtr vm = NULL; virDomainEventPtr dom_event = NULL; libxl_shutdown_reason xl_reason = event->u.domain_shutdown.shutdown_reason; @@ -764,6 +765,11 @@ cleanup: libxlDomainEventQueue(driver, dom_event); libxlDriverUnlock(driver); } +#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG + libxl_event_free(priv->ctx, event); +#else + libxl_event_free(priv->ctx, (libxl_event *)event); +#endif } static const struct libxl_event_hooks ev_hooks = { -- 1.7.7
On 05/16/2013 10:01 AM, Jim Fehlig wrote:
libxl expects the event handler to free the event passed to it. From libxl_event.h:
event becomes owned by the application and must be freed, either by event_occurs or later --- src/libxl/libxl_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
@@ -764,6 +765,11 @@ cleanup: libxlDomainEventQueue(driver, dom_event); libxlDriverUnlock(driver); } +#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG + libxl_event_free(priv->ctx, event); +#else + libxl_event_free(priv->ctx, (libxl_event *)event); +#endif
At least here your #ifdef isn't inside (), but it is inside a function body. Have I mentioned that I like my #ifdefs hoisted to the top level, when possible? :) Here, you could avoid the #ifdef, and just write: /* cast away any const */ libxl_event_free(priv->ctx, (libxl_event *)event); At any rate, ACK to your series, whether or not you clean up the #ifdefs. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/16/2013 01:56 PM, Eric Blake wrote:
libxl expects the event handler to free the event passed to it. From libxl_event.h:
event becomes owned by the application and must be freed, either by event_occurs or later --- src/libxl/libxl_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
@@ -764,6 +765,11 @@ cleanup: libxlDomainEventQueue(driver, dom_event); libxlDriverUnlock(driver); } +#ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG + libxl_event_free(priv->ctx, event); +#else + libxl_event_free(priv->ctx, (libxl_event *)event); +#endif At least here your #ifdef isn't inside (), but it is inside a function
On 05/16/2013 10:01 AM, Jim Fehlig wrote: body. Have I mentioned that I like my #ifdefs hoisted to the top level, when possible? :)
Here, you could avoid the #ifdef, and just write:
/* cast away any const */ libxl_event_free(priv->ctx, (libxl_event *)event);
At any rate, ACK to your series, whether or not you clean up the #ifdefs.
Thanks Eric. I made your suggested changes and pushed the patches. Regards, Jim
participants (2)
-
Eric Blake -
Jim Fehlig