11:01 a.m.
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
11:01 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: fix build with Xen4.3
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
2:52 p.m.
New subject: [libvirt] [PATCH 1/2] libxl: fix build with Xen4.3
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
11:01 a.m.
New subject: [libvirt] [PATCH 2/2] libxl: fix leaking libxl events
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
2:56 p.m.
New subject: [libvirt] [PATCH 2/2] libxl: fix leaking libxl events
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
3:18 p.m.
New subject: [libvirt] [PATCH 2/2] libxl: fix leaking libxl events
On 05/16/2013 01:56 PM, Eric Blake wrote:
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.
Thanks Eric. I made your suggested changes and pushed the patches.
Regards,
Jim
4207
days inactive
4208
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
Jim Fehlig