Long ago I incorrectly associated libxl fd and timer registrations
with per-domain libxl_ctx objects. When creating a libxlDomainObjPrivate,
a libxl_ctx is allocated, and libxl_osevent_register_hooks is called
passing a pointer to the libxlDomainObjPrivate. When an fd or timer
registration occurred, the registration callback received the
libxlDomainObjPrivate, containing the per-domain libxl_ctx. This
libxl_ctx was then used when informing libxl about fd events or
timer expirations.
The problem with this approach is that fd and timer registrations do not
share the same lifespan as libxlDomainObjPrivate, and hence the per-domain
libxl_ctx ojects. The result is races between per-domain libxl_ctx's being
destoryed and events firing on associated fds/timers, typically manifesting
as an assert in libxl
libxl_internal.h:2788: libxl__ctx_unlock: Assertion `!r' failed
There is no need to associate libxlDomainObjPrivate objects with libxl's
desire to use libvirt's event loop. Instead, the driver-wide libxl_ctx can
be used for the fd and timer registrations.
This patch moves the fd and timer handling code away from the
domain-specific code in libxl_domain.c into libxl_driver.c. While at it,
function names were changed a bit to better describe their purpose.
The unnecessary locking was also removed since the code simply provides a
wrapper over the event loop interface. Indeed the locks may have been
causing some deadlocks when repeatedly creating/destroying muliple domains.
There have also been rumors about such deadlocks during parallel OpenStack
Tempest runs.
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
src/libxl/libxl_domain.c | 234 +----------------------------------------------
src/libxl/libxl_driver.c | 201 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 201 insertions(+), 234 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 856cfb4..c44799b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1,7 +1,7 @@
/*
* libxl_domain.c: libxl domain object private state
*
- * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -46,16 +46,6 @@ VIR_ENUM_IMPL(libxlDomainJob, LIBXL_JOB_LAST,
"modify",
);
-/* Object used to store info related to libxl event registrations */
-typedef struct _libxlEventHookInfo libxlEventHookInfo;
-typedef libxlEventHookInfo *libxlEventHookInfoPtr;
-struct _libxlEventHookInfo {
- libxlEventHookInfoPtr next;
- libxlDomainObjPrivatePtr priv;
- void *xl_priv;
- int id;
-};
-
static virClassPtr libxlDomainObjPrivateClass;
static void
@@ -75,227 +65,6 @@ libxlDomainObjPrivateOnceInit(void)
VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate)
-static void
-libxlDomainObjFDEventHookInfoFree(void *obj)
-{
- VIR_FREE(obj);
-}
-
-static void
-libxlDomainObjTimerEventHookInfoFree(void *obj)
-{
- libxlEventHookInfoPtr info = obj;
-
- /* Drop reference on libxlDomainObjPrivate */
- virObjectUnref(info->priv);
- VIR_FREE(info);
-}
-
-static void
-libxlDomainObjFDEventCallback(int watch ATTRIBUTE_UNUSED,
- int fd,
- int vir_events,
- void *fd_info)
-{
- libxlEventHookInfoPtr info = fd_info;
- int events = 0;
-
- virObjectLock(info->priv);
- if (vir_events & VIR_EVENT_HANDLE_READABLE)
- events |= POLLIN;
- if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
- events |= POLLOUT;
- if (vir_events & VIR_EVENT_HANDLE_ERROR)
- events |= POLLERR;
- if (vir_events & VIR_EVENT_HANDLE_HANGUP)
- events |= POLLHUP;
-
- virObjectUnlock(info->priv);
- libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events);
-}
-
-static int
-libxlDomainObjFDRegisterEventHook(void *priv,
- int fd,
- void **hndp,
- short events,
- void *xl_priv)
-{
- int vir_events = VIR_EVENT_HANDLE_ERROR;
- libxlEventHookInfoPtr info;
-
- if (VIR_ALLOC(info) < 0)
- return -1;
-
- info->priv = priv;
- info->xl_priv = xl_priv;
-
- if (events & POLLIN)
- vir_events |= VIR_EVENT_HANDLE_READABLE;
- if (events & POLLOUT)
- vir_events |= VIR_EVENT_HANDLE_WRITABLE;
-
- info->id = virEventAddHandle(fd, vir_events, libxlDomainObjFDEventCallback,
- info, libxlDomainObjFDEventHookInfoFree);
- if (info->id < 0) {
- VIR_FREE(info);
- return -1;
- }
-
- *hndp = info;
-
- return 0;
-}
-
-static int
-libxlDomainObjFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void **hndp,
- short events)
-{
- libxlEventHookInfoPtr info = *hndp;
- int vir_events = VIR_EVENT_HANDLE_ERROR;
-
- virObjectLock(info->priv);
- if (events & POLLIN)
- vir_events |= VIR_EVENT_HANDLE_READABLE;
- if (events & POLLOUT)
- vir_events |= VIR_EVENT_HANDLE_WRITABLE;
-
- virEventUpdateHandle(info->id, vir_events);
- virObjectUnlock(info->priv);
-
- return 0;
-}
-
-static void
-libxlDomainObjFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void *hnd)
-{
- libxlEventHookInfoPtr info = hnd;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- virEventRemoveHandle(info->id);
- virObjectUnlock(p);
-}
-
-static void
-libxlDomainObjTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
-{
- libxlEventHookInfoPtr info = timer_info;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- /*
- * libxl expects the event to be deregistered when calling
- * libxl_osevent_occurred_timeout, but we dont want the event info
- * destroyed. Disable the timeout and only remove it after returning
- * from libxl.
- */
- virEventUpdateTimeout(info->id, -1);
- virObjectUnlock(p);
- libxl_osevent_occurred_timeout(p->ctx, info->xl_priv);
- virObjectLock(p);
- virEventRemoveTimeout(info->id);
- virObjectUnlock(p);
-}
-
-static int
-libxlDomainObjTimeoutRegisterEventHook(void *priv,
- void **hndp,
- struct timeval abs_t,
- void *xl_priv)
-{
- libxlEventHookInfoPtr info;
- struct timeval now;
- struct timeval res;
- static struct timeval zero;
- int timeout;
-
- if (VIR_ALLOC(info) < 0)
- return -1;
-
- info->priv = priv;
- /*
- * Also take a reference on the domain object. Reference is dropped in
- * libxlDomainObjEventHookInfoFree, ensuring the domain object outlives the
- * timeout event objects.
- */
- virObjectRef(info->priv);
- info->xl_priv = xl_priv;
-
- gettimeofday(&now, NULL);
- timersub(&abs_t, &now, &res);
- /* Ensure timeout is not overflowed */
- if (timercmp(&res, &zero, <)) {
- timeout = 0;
- } else if (res.tv_sec > INT_MAX / 1000) {
- timeout = INT_MAX;
- } else {
- timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
- }
- info->id = virEventAddTimeout(timeout, libxlDomainObjTimerCallback,
- info, libxlDomainObjTimerEventHookInfoFree);
- if (info->id < 0) {
- virObjectUnref(info->priv);
- VIR_FREE(info);
- return -1;
- }
-
- *hndp = info;
-
- return 0;
-}
-
-/*
- * Note: There are two changes wrt timeouts starting with xen-unstable
- * changeset 26469:
- *
- * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
- * i.e. make the timeout fire immediately. Prior to this commit, timeout
- * modify callbacks were never invoked.
- *
- * 2. Timeout deregister hooks will no longer be called.
- */
-static int
-libxlDomainObjTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- void **hndp,
- struct timeval abs_t ATTRIBUTE_UNUSED)
-{
- libxlEventHookInfoPtr info = *hndp;
-
- virObjectLock(info->priv);
- /* Make the timeout fire */
- virEventUpdateTimeout(info->id, 0);
- virObjectUnlock(info->priv);
-
- return 0;
-}
-
-static void
-libxlDomainObjTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
- void *hnd)
-{
- libxlEventHookInfoPtr info = hnd;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- virEventRemoveTimeout(info->id);
- virObjectUnlock(p);
-}
-
-
-static const libxl_osevent_hooks libxl_event_callbacks = {
- .fd_register = libxlDomainObjFDRegisterEventHook,
- .fd_modify = libxlDomainObjFDModifyEventHook,
- .fd_deregister = libxlDomainObjFDDeregisterEventHook,
- .timeout_register = libxlDomainObjTimeoutRegisterEventHook,
- .timeout_modify = libxlDomainObjTimeoutModifyEventHook,
- .timeout_deregister = libxlDomainObjTimeoutDeregisterEventHook,
-};
-
static int
libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv)
{
@@ -796,7 +565,6 @@ libxlDomainObjPrivateInitCtx(virDomainObjPtr vm)
goto cleanup;
}
- libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv);
libxl_childproc_setmode(priv->ctx, &libxl_child_hooks, priv);
ret = 0;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c95b387..00dab98 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2,7 +2,7 @@
* libxl_driver.c: core driver methods for managing libxenlight domains
*
* Copyright (C) 2006-2014 Red Hat, Inc.
- * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
* Copyright (C) 2011 Univention GmbH.
*
* This library is free software; you can redistribute it and/or
@@ -80,6 +80,15 @@ VIR_LOG_INIT("libxl.libxl_driver");
static libxlDriverPrivatePtr libxl_driver;
+/* Object used to store info related to libxl event registrations */
+typedef struct _libxlOSEventHookInfo libxlOSEventHookInfo;
+typedef libxlOSEventHookInfo *libxlOSEventHookInfoPtr;
+struct _libxlOSEventHookInfo {
+ libxl_ctx *ctx;
+ void *xl_priv;
+ int id;
+};
+
/* Function declarations */
static int
libxlDomainManagedSaveLoad(virDomainObjPtr vm,
@@ -87,6 +96,183 @@ libxlDomainManagedSaveLoad(virDomainObjPtr vm,
/* Function definitions */
+static void
+libxlOSEventHookInfoFree(void *obj)
+{
+ VIR_FREE(obj);
+}
+
+static void
+libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
+ int fd,
+ int vir_events,
+ void *fd_info)
+{
+ libxlOSEventHookInfoPtr info = fd_info;
+ int events = 0;
+
+ if (vir_events & VIR_EVENT_HANDLE_READABLE)
+ events |= POLLIN;
+ if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
+ events |= POLLOUT;
+ if (vir_events & VIR_EVENT_HANDLE_ERROR)
+ events |= POLLERR;
+ if (vir_events & VIR_EVENT_HANDLE_HANGUP)
+ events |= POLLHUP;
+
+ libxl_osevent_occurred_fd(info->ctx, info->xl_priv, fd, 0, events);
+}
+
+static int
+libxlFDRegisterEventHook(void *priv,
+ int fd,
+ void **hndp,
+ short events,
+ void *xl_priv)
+{
+ int vir_events = VIR_EVENT_HANDLE_ERROR;
+ libxlOSEventHookInfoPtr info;
+
+ if (VIR_ALLOC(info) < 0)
+ return -1;
+
+ info->ctx = priv;
+ info->xl_priv = xl_priv;
+
+ if (events & POLLIN)
+ vir_events |= VIR_EVENT_HANDLE_READABLE;
+ if (events & POLLOUT)
+ vir_events |= VIR_EVENT_HANDLE_WRITABLE;
+
+ info->id = virEventAddHandle(fd, vir_events, libxlFDEventCallback,
+ info, libxlOSEventHookInfoFree);
+ if (info->id < 0) {
+ VIR_FREE(info);
+ return -1;
+ }
+
+ *hndp = info;
+
+ return 0;
+}
+
+static int
+libxlFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void **hndp,
+ short events)
+{
+ libxlOSEventHookInfoPtr info = *hndp;
+ int vir_events = VIR_EVENT_HANDLE_ERROR;
+
+ if (events & POLLIN)
+ vir_events |= VIR_EVENT_HANDLE_READABLE;
+ if (events & POLLOUT)
+ vir_events |= VIR_EVENT_HANDLE_WRITABLE;
+
+ virEventUpdateHandle(info->id, vir_events);
+
+ return 0;
+}
+
+static void
+libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void *hnd)
+{
+ libxlOSEventHookInfoPtr info = hnd;
+
+ virEventRemoveHandle(info->id);
+}
+
+static void
+libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
+{
+ libxlOSEventHookInfoPtr info = timer_info;
+
+ /*
+ * libxl expects the event to be deregistered when calling
+ * libxl_osevent_occurred_timeout, but we dont want the event info
+ * destroyed. Disable the timeout and only remove it after returning
+ * from libxl.
+ */
+ virEventUpdateTimeout(info->id, -1);
+ libxl_osevent_occurred_timeout(info->ctx, info->xl_priv);
+ virEventRemoveTimeout(info->id);
+}
+
+static int
+libxlTimeoutRegisterEventHook(void *priv,
+ void **hndp,
+ struct timeval abs_t,
+ void *xl_priv)
+{
+ libxlOSEventHookInfoPtr info;
+ struct timeval now;
+ struct timeval res;
+ static struct timeval zero;
+ int timeout;
+
+ if (VIR_ALLOC(info) < 0)
+ return -1;
+
+ info->ctx = priv;
+ info->xl_priv = xl_priv;
+
+ gettimeofday(&now, NULL);
+ timersub(&abs_t, &now, &res);
+ /* Ensure timeout is not overflowed */
+ if (timercmp(&res, &zero, <)) {
+ timeout = 0;
+ } else if (res.tv_sec > INT_MAX / 1000) {
+ timeout = INT_MAX;
+ } else {
+ timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
+ }
+ info->id = virEventAddTimeout(timeout, libxlTimerCallback,
+ info, libxlOSEventHookInfoFree);
+ if (info->id < 0) {
+ VIR_FREE(info);
+ return -1;
+ }
+
+ *hndp = info;
+
+ return 0;
+}
+
+/*
+ * Note: There are two changes wrt timeouts starting with xen-unstable
+ * changeset 26469:
+ *
+ * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
+ * i.e. make the timeout fire immediately. Prior to this commit, timeout
+ * modify callbacks were never invoked.
+ *
+ * 2. Timeout deregister hooks will no longer be called.
+ */
+static int
+libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
+ void **hndp,
+ struct timeval abs_t ATTRIBUTE_UNUSED)
+{
+ libxlOSEventHookInfoPtr info = *hndp;
+
+ /* Make the timeout fire */
+ virEventUpdateTimeout(info->id, 0);
+
+ return 0;
+}
+
+static void
+libxlTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
+ void *hnd)
+{
+ libxlOSEventHookInfoPtr info = hnd;
+
+ virEventRemoveTimeout(info->id);
+}
+
static virDomainObjPtr
libxlDomObjFromDomain(virDomainPtr dom)
{
@@ -277,6 +463,16 @@ libxlDriverShouldLoad(bool privileged)
return ret;
}
+/* Callbacks wrapping libvirt's event loop interface */
+static const libxl_osevent_hooks libxl_osevent_callbacks = {
+ .fd_register = libxlFDRegisterEventHook,
+ .fd_modify = libxlFDModifyEventHook,
+ .fd_deregister = libxlFDDeregisterEventHook,
+ .timeout_register = libxlTimeoutRegisterEventHook,
+ .timeout_modify = libxlTimeoutModifyEventHook,
+ .timeout_deregister = libxlTimeoutDeregisterEventHook,
+};
+
static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
@@ -322,6 +518,9 @@ libxlStateInitialize(bool privileged,
if (!(cfg = libxlDriverConfigNew()))
goto error;
+ /* Register the callbacks providing access to libvirt's event loop */
+ libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx);
+
libxl_driver->config = cfg;
if (virFileMakePath(cfg->stateDir) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
1.8.4.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list