[libvirt] [PATCH 00/10] libxl: switch driver to use a single libxl_ctx

This series is a follow up to https://www.redhat.com/archives/libvir-list/2015-February/msg00024.html It goes a step further and changes the libxl driver to use one, driver-wide libxl_ctx. Currently the libxl driver has one driver-wide ctx for operations that are not domain-specific and a ctx for each domain. This approach was necessary back in the old Xen4.1 libxl days, but with the newer libxl it is more of a hinderance than benefit. Ian Jackson suggested moving to a single ctx while discussing some deadlocks and assertions encountered in the libxl driver when under load from tests such as OpenStack Tempest. Making such a change involves quite a bit of code movement. I've tried to split that up into a reviewable series, the result of which are the 9 patches that follow. I've ran this through all of my automated tests as well as some hacky tests I created to reproduce failures revealed by Tempest. One downside of moving to a single ctx is losing the per-domain log files. Currently, a single log stream can be associated with ctx, hence all logging from libxl will go to a single file. Ian is going to investigate possibilities to accommodate per-domain log files in libxl, but in the meantime folks using Xen are accustomed to a single log file from the xend days. I've been testing this series on xen-unstable and Xen 4.4.1 + commits 2ffeb5d7, 4b9143e4, 5a968257, 60ce518a, 66bff9fd, 77a1bf37, f49f9b41, 6b5a5bba, 93699882d, f1335f0d, and 8bc64413. Results are much better than before applying the series, but I do notice a stuck hypercall after many (hundreds) concurrent domain create/destroy operations. The single libxl_ctx is locked in the callpath, essentially deadlocking the driver. Thread 1 (Thread 0x7f0649a198c0 (LWP 2235)): 0 0x00007f0645272397 in ioctl () from /lib64/libc.so.6 1 0x00007f0645d8e353 in linux_privcmd_hypercall (xch=<optimized out>, h=<optimized out>, hypercall=<optimized out>) at xc_linux_osdep.c:134 2 0x00007f0645d854b8 in do_xen_hypercall (xch=xch@entry=0x7f0630039390, hypercall=hypercall@entry=0x7fffd53f80e0) at xc_private.c:249 3 0x00007f0645d86aa4 in do_sysctl (sysctl=sysctl@entry=0x7fffd53f8080, xch=xch@entry=0x7f0630039390) at xc_private.h:281 4 xc_sysctl (xch=xch@entry=0x7f0630039390, sysctl=sysctl@entry=0x7fffd53f8170) at xc_private.c:656 5 0x00007f0645d7bfbf in xc_domain_getinfolist (xch=0x7f0630039390, first_domain=first_domain@entry=119, max_domains=max_domains@entry=1, info=info@entry=0x7fffd53f8260) at xc_domain.c:382 6 0x00007f0645fabca6 in domain_death_xswatch_callback (egc=0x7fffd53f83f0, w=<optimized out>, wpath=<optimized out>, epath=<optimized out>) at libxl.c:1041 7 0x00007f0645fd75a8 in watchfd_callback (egc=0x7fffd53f83f0, ev=<optimized out>, fd=<optimized out>, events=<optimized out>, revents=<optimized out>) at libxl_event.c:515 8 0x00007f0645fd8ac3 in libxl_osevent_occurred_fd (ctx=<optimized out>, for_libxl=<optimized out>, fd=<optimized out>, events_ign=<optimized out>, revents_ign=<optimized out>) at libxl_event.c:1259 9 0x00007f063a23402c in libxlFDEventCallback (watch=454, fd=33, vir_events=1, fd_info=0x7f0608007e70) at libxl/libxl_driver.c:123 There is no hint in any logs or dmesg suggesting a cause for the stuck hypercall. Any suggestions for further debugging tips appreciated. Jim Fehlig (10): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: use driver-wide ctx in fd and timer event handling libxl: Move setup of child processing code to driver initialization libxl: move event registration to driver initialization libxl: use global libxl_ctx in event handler libxl: remove unnecessary libxlDomainEventsRegister libxl: make libxlDomainFreeMem static libxl: remove per-domain libxl_ctx libxl: change libxl log stream to ERROR log level src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 438 ++++++--------------------------------- src/libxl/libxl_domain.h | 27 +-- src/libxl/libxl_driver.c | 484 +++++++++++++++++++++++++++++++------------- src/libxl/libxl_migration.c | 17 +- 5 files changed, 426 insertions(+), 542 deletions(-) -- 1.8.4.5

Domain death watch is already disabled in libxlDomainCleanup. No need to disable it a second and third time. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 21c41d7..e186c53 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -439,9 +439,6 @@ libxlDomainObjPrivateDispose(void *obj) { libxlDomainObjPrivatePtr priv = obj; - if (priv->deathW) - libxl_evdisable_domain_death(priv->ctx, priv->deathW); - libxlDomainObjFreeJob(priv); virChrdevFree(priv->devs); libxl_ctx_free(priv->ctx); @@ -456,11 +453,6 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data; - if (priv->deathW) { - libxl_evdisable_domain_death(priv->ctx, priv->deathW); - priv->deathW = NULL; - } - virObjectUnref(priv); } -- 1.8.4.5

Instead of using the libxl_ctx in the libxlDomainObjPrivatePtr, use the ctx passed to the callback. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index e186c53..9af5758 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1158,10 +1158,9 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) } static void -libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback) +libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) { virDomainObjPtr vm = for_callback; - libxlDomainObjPrivatePtr priv = vm->privateData; size_t i; virObjectLock(vm); @@ -1175,7 +1174,7 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback) console_type = (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ? LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV); - ret = libxl_console_get_tty(priv->ctx, ev->domid, + ret = libxl_console_get_tty(ctx, ev->domid, chr->target.port, console_type, &console); if (!ret) { -- 1.8.4.5

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@suse.com> --- 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 9af5758..4872abe 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) { @@ -804,7 +573,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 ce3a99b..4b459eb 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

Informing libxl how to handle its child proceses should be done once during driver initialization, not once for each domain-specific libxl_ctx object. The related libxl documentation in $xen-src/tools/libxl/libxl_event.h even mentions that "it is best to call this at initialisation". Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 10 ---------- src/libxl/libxl_driver.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 4872abe..0fc03f7 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -529,14 +529,6 @@ const struct libxl_event_hooks ev_hooks = { .disaster = NULL, }; -static const libxl_childproc_hooks libxl_child_hooks = { -#ifdef LIBXL_HAVE_SIGCHLD_OWNER_SELECTIVE_REAP - .chldowner = libxl_sigchld_owner_libxl_always_selective_reap, -#else - .chldowner = libxl_sigchld_owner_libxl, -#endif -}; - int libxlDomainObjPrivateInitCtx(virDomainObjPtr vm) { @@ -573,8 +565,6 @@ libxlDomainObjPrivateInitCtx(virDomainObjPtr vm) goto cleanup; } - libxl_childproc_setmode(priv->ctx, &libxl_child_hooks, priv); - ret = 0; cleanup: diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4b459eb..5503c07 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -473,6 +473,14 @@ static const libxl_osevent_hooks libxl_osevent_callbacks = { .timeout_deregister = libxlTimeoutDeregisterEventHook, }; +static const libxl_childproc_hooks libxl_child_hooks = { +#ifdef LIBXL_HAVE_SIGCHLD_OWNER_SELECTIVE_REAP + .chldowner = libxl_sigchld_owner_libxl_always_selective_reap, +#else + .chldowner = libxl_sigchld_owner_libxl, +#endif +}; + static int libxlStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, @@ -521,6 +529,9 @@ libxlStateInitialize(bool privileged, /* Register the callbacks providing access to libvirt's event loop */ libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx); + /* Setup child process handling. See $xen-src/tools/libxl/libxl_event.h */ + libxl_childproc_setmode(cfg->ctx, &libxl_child_hooks, cfg->ctx); + libxl_driver->config = cfg; if (virFileMakePath(cfg->stateDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.8.4.5

Register a domain event handler with the driver-wide libxl_ctx during driver initialization. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 21 ++------------------- src/libxl/libxl_domain.h | 15 +++++++++++++++ src/libxl/libxl_driver.c | 9 +++++++++ 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0fc03f7..73bd5d0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -463,19 +463,9 @@ libxlDomainShutdownThread(void *opaque) /* * Handle previously registered domain event notification from libxenlight. - * - * Note: Xen 4.3 removed the const from the event handler signature. - * Detect which signature to use based on - * LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG. */ -#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, VIR_LIBXL_EVENT_CONST libxl_event *event) +void +libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) { virDomainObjPtr vm = data; libxlDomainObjPrivatePtr priv = vm->privateData; @@ -523,12 +513,6 @@ libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) libxl_event_free(priv->ctx, (libxl_event *)event); } -const struct libxl_event_hooks ev_hooks = { - .event_occurs_mask = LIBXL_EVENTMASK_ALL, - .event_occurs = libxlEventHandler, - .disaster = NULL, -}; - int libxlDomainObjPrivateInitCtx(virDomainObjPtr vm) { @@ -762,7 +746,6 @@ libxlDomainEventsRegister(libxlDriverPrivatePtr driver, virDomainObjPtr vm) libxlDomainObjPrivatePtr priv = vm->privateData; priv->driver = driver; - libxl_event_register_callbacks(priv->ctx, &ev_hooks, vm); /* Always enable domain death events */ if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index f459fdf..96e238e 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -118,6 +118,21 @@ bool libxlDomainCleanupJob(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virDomainShutoffReason reason); +/* + * Note: Xen 4.3 removed the const from the event handler signature. + * Detect which signature to use based on + * LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG. + */ +# ifdef LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG +# define VIR_LIBXL_EVENT_CONST /* empty */ +# else +# define VIR_LIBXL_EVENT_CONST const +# endif + +void +libxlDomainEventHandler(void *data, + VIR_LIBXL_EVENT_CONST libxl_event *event); + int libxlDomainEventsRegister(libxlDriverPrivatePtr driver, virDomainObjPtr vm); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5503c07..8e88645 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -481,6 +481,12 @@ static const libxl_childproc_hooks libxl_child_hooks = { #endif }; +const struct libxl_event_hooks ev_hooks = { + .event_occurs_mask = LIBXL_EVENTMASK_ALL, + .event_occurs = libxlDomainEventHandler, + .disaster = NULL, +}; + static int libxlStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, @@ -532,6 +538,9 @@ libxlStateInitialize(bool privileged, /* Setup child process handling. See $xen-src/tools/libxl/libxl_event.h */ libxl_childproc_setmode(cfg->ctx, &libxl_child_hooks, cfg->ctx); + /* Register callback to handle domain events */ + libxl_event_register_callbacks(cfg->ctx, &ev_hooks, libxl_driver); + libxl_driver->config = cfg; if (virFileMakePath(cfg->stateDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.8.4.5

Change the domain event handler code to use the driver-wide libxl_ctx instead of the domain-specific one. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 73bd5d0..ea3276c 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -349,6 +349,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { struct libxlShutdownThreadInfo { + libxlDriverPrivatePtr driver; virDomainObjPtr vm; libxl_event *event; }; @@ -359,15 +360,14 @@ libxlDomainShutdownThread(void *opaque) { struct libxlShutdownThreadInfo *shutdown_info = opaque; virDomainObjPtr vm = shutdown_info->vm; - libxlDomainObjPrivatePtr priv = vm->privateData; libxl_event *ev = shutdown_info->event; - libxlDriverPrivatePtr driver = priv->driver; - libxl_ctx *ctx = priv->ctx; + libxlDriverPrivatePtr driver = shutdown_info->driver; virObjectEventPtr dom_event = NULL; libxl_shutdown_reason xl_reason = ev->u.domain_shutdown.shutdown_reason; virDomainShutoffReason reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; + libxlDriverConfigPtr cfg; - virObjectLock(vm); + cfg = libxlDriverConfigGet(driver); if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) { dom_event = virDomainEventLifecycleNewFromObj(vm, @@ -430,7 +430,7 @@ libxlDomainShutdownThread(void *opaque) libxlDomainEventQueue(driver, dom_event); dom_event = NULL; } - libxl_domain_destroy(ctx, vm->def->id, NULL); + libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); if (libxlDomainCleanupJob(driver, vm, reason)) { if (!vm->persistent) { virDomainObjListRemove(driver->domains, vm); @@ -444,7 +444,7 @@ libxlDomainShutdownThread(void *opaque) libxlDomainEventQueue(driver, dom_event); dom_event = NULL; } - libxl_domain_destroy(ctx, vm->def->id, NULL); + libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); if (libxlDomainStart(driver, vm, false, -1) < 0) { virErrorPtr err = virGetLastError(); @@ -457,8 +457,9 @@ libxlDomainShutdownThread(void *opaque) virObjectUnlock(vm); if (dom_event) libxlDomainEventQueue(driver, dom_event); - libxl_event_free(ctx, ev); + libxl_event_free(cfg->ctx, ev); VIR_FREE(shutdown_info); + virObjectUnref(cfg); } /* @@ -467,11 +468,12 @@ libxlDomainShutdownThread(void *opaque) void libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) { - virDomainObjPtr vm = data; - libxlDomainObjPrivatePtr priv = vm->privateData; + libxlDriverPrivatePtr driver = data; + virDomainObjPtr vm = NULL; libxl_shutdown_reason xl_reason = event->u.domain_shutdown.shutdown_reason; struct libxlShutdownThreadInfo *shutdown_info; virThread thread; + libxlDriverConfigPtr cfg; if (event->type != LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) { VIR_INFO("Unhandled event type %d", event->type); @@ -485,6 +487,12 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND) goto error; + vm = virDomainObjListFindByID(driver->domains, event->domid); + if (!vm) { + VIR_INFO("Received event for unknown domain ID %d", event->domid); + goto error; + } + /* * Start a thread to handle shutdown. We don't want to be tying up * libxl's event machinery by doing a potentially lengthy shutdown. @@ -492,7 +500,8 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) if (VIR_ALLOC(shutdown_info) < 0) goto error; - shutdown_info->vm = data; + shutdown_info->driver = driver; + shutdown_info->vm = vm; shutdown_info->event = (libxl_event *)event; if (virThreadCreate(&thread, false, libxlDomainShutdownThread, shutdown_info) < 0) { @@ -504,13 +513,17 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) } /* - * libxl_event freed in shutdown thread + * VM is unlocked and libxl_event freed in shutdown thread */ return; error: + cfg = libxlDriverConfigGet(driver); /* Cast away any const */ - libxl_event_free(priv->ctx, (libxl_event *)event); + libxl_event_free(cfg->ctx, (libxl_event *)event); + virObjectUnref(cfg); + if (vm) + virObjectUnlock(vm); } int -- 1.8.4.5

This function now only enables domain death events. Simply call libxl_evenable_domain_death() instead of an unnecessary wrapper. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 32 +++++++------------------------- src/libxl/libxl_domain.h | 5 ----- src/libxl/libxl_driver.c | 5 +++-- 3 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ea3276c..69d5c5b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -751,30 +751,6 @@ libxlDomainCleanupJob(libxlDriverPrivatePtr driver, } /* - * Register for domain events emitted by libxl. - */ -int -libxlDomainEventsRegister(libxlDriverPrivatePtr driver, virDomainObjPtr vm) -{ - libxlDomainObjPrivatePtr priv = vm->privateData; - - priv->driver = driver; - - /* Always enable domain death events */ - if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) - goto error; - - return 0; - - error: - if (priv->deathW) { - libxl_evdisable_domain_death(priv->ctx, priv->deathW); - priv->deathW = NULL; - } - return -1; -} - -/* * Core dump domain to default dump path. * * virDomainObjPtr must be locked on invocation @@ -1076,7 +1052,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, * be cleaned up if there are any subsequent failures. */ vm->def->id = domid; - if (libxlDomainEventsRegister(driver, vm) < 0) + + /* Always enable domain death events */ + if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) goto cleanup_dom; if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) @@ -1116,6 +1094,10 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto endjob; cleanup_dom: + if (priv->deathW) { + libxl_evdisable_domain_death(priv->ctx, priv->deathW); + priv->deathW = NULL; + } libxl_domain_destroy(priv->ctx, domid, NULL); vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 96e238e..297dffb 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -68,7 +68,6 @@ struct _libxlDomainObjPrivate { /* console */ virChrdevsPtr devs; libxl_evgen_domain_death *deathW; - libxlDriverPrivatePtr driver; unsigned short migrationPort; struct libxlDomainJobObj job; @@ -134,10 +133,6 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event); int -libxlDomainEventsRegister(libxlDriverPrivatePtr driver, - virDomainObjPtr vm); - -int libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, virDomainObjPtr vm); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8e88645..43ee629 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -367,8 +367,9 @@ libxlReconnectDomain(virDomainObjPtr vm, if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - /* Re-register domain death et. al. events */ - libxlDomainEventsRegister(driver, vm); + /* Enable domain death events */ + libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW); + virObjectUnlock(vm); return 0; -- 1.8.4.5

libxlDomainFreeMem() is only used in libxl_domain.c and thus should be declared static. While at it, change the signature to take a libxl_ctx instead of libxlDomainObjPrivatePtr, since only the libxl_ctx is needed. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 16 ++++++++-------- src/libxl/libxl_domain.h | 4 ---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 69d5c5b..6bd38c5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -848,8 +848,8 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) return ret; } -int -libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) +static int +libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config) { uint32_t needed_mem; uint32_t free_mem; @@ -858,10 +858,10 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) int tries = 3; int wait_secs = 10; - if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, + if ((ret = libxl_domain_need_memory(ctx, &d_config->b_info, &needed_mem)) >= 0) { for (i = 0; i < tries; ++i) { - if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0) + if ((ret = libxl_get_free_memory(ctx, &free_mem)) < 0) break; if (free_mem >= needed_mem) { @@ -869,17 +869,17 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) break; } - if ((ret = libxl_set_memory_target(priv->ctx, 0, + if ((ret = libxl_set_memory_target(ctx, 0, free_mem - needed_mem, /* relative */ 1, 0)) < 0) break; - ret = libxl_wait_for_free_memory(priv->ctx, 0, needed_mem, + ret = libxl_wait_for_free_memory(ctx, 0, needed_mem, wait_secs); if (ret == 0 || ret != ERROR_NOMEM) break; - if ((ret = libxl_wait_for_memory_target(priv->ctx, 0, 1)) < 0) + if ((ret = libxl_wait_for_memory_target(ctx, 0, 1)) < 0) break; } } @@ -1003,7 +1003,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, priv->ctx, &d_config) < 0) goto endjob; - if (cfg->autoballoon && libxlDomainFreeMem(priv, &d_config) < 0) { + if (cfg->autoballoon && libxlDomainFreeMem(priv->ctx, &d_config) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to get free memory for domain '%s'"), d_config.c_info.name); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 297dffb..2db4d77 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -141,10 +141,6 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm); int -libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, - libxl_domain_config *d_config); - -int libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, bool start_paused, -- 1.8.4.5

Although needed in the Xen 4.1 libxl days, there is no longer any benefit to having per-domain libxl_ctx. On the contrary, their use makes the code unecessarily complicated and prone to deadlocks under load. As suggested by the libxl maintainers, use a single libxl_ctx as a handle to libxl instead of per-domain ctx's. One downside to using a single libxl_ctx is there are no longer per-domain log files for log messages emitted by libxl. Messages for all domains will be sent to /var/log/libvirt/libxl/libxl-driver.log. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 81 +++----------- src/libxl/libxl_domain.h | 5 - src/libxl/libxl_driver.c | 260 +++++++++++++++++++++----------------------- src/libxl/libxl_migration.c | 17 ++- 4 files changed, 147 insertions(+), 216 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 6bd38c5..2fbfe73 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -210,11 +210,6 @@ libxlDomainObjPrivateDispose(void *obj) libxlDomainObjFreeJob(priv); virChrdevFree(priv->devs); - libxl_ctx_free(priv->ctx); - if (priv->logger_file) - VIR_FORCE_FCLOSE(priv->logger_file); - - xtl_logger_destroy(priv->logger); } static void @@ -526,49 +521,6 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) virObjectUnlock(vm); } -int -libxlDomainObjPrivateInitCtx(virDomainObjPtr vm) -{ - libxlDomainObjPrivatePtr priv = vm->privateData; - char *log_file; - int ret = -1; - - if (priv->ctx) - return 0; - - if (virAsprintf(&log_file, "%s/%s.log", LIBXL_LOG_DIR, vm->def->name) < 0) - return -1; - - if ((priv->logger_file = fopen(log_file, "a")) == NULL) { - virReportSystemError(errno, - _("failed to open logfile %s"), - log_file); - goto cleanup; - } - - priv->logger = - (xentoollog_logger *)xtl_createlogger_stdiostream(priv->logger_file, - XTL_DEBUG, 0); - if (!priv->logger) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot create libxenlight logger for domain %s"), - vm->def->name); - goto cleanup; - } - - if (libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, priv->logger)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed libxl context initialization")); - goto cleanup; - } - - ret = 0; - - cleanup: - VIR_FREE(log_file); - return ret; -} - void libxlDomainEventQueue(libxlDriverPrivatePtr driver, virObjectEventPtr event) { @@ -683,7 +635,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, vm->def->id = -1; if (priv->deathW) { - libxl_evdisable_domain_death(priv->ctx, priv->deathW); + libxl_evdisable_domain_death(cfg->ctx, priv->deathW); priv->deathW = NULL; } @@ -759,7 +711,6 @@ int libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { - libxlDomainObjPrivatePtr priv = vm->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); time_t curtime = time(NULL); char timestr[100]; @@ -781,7 +732,7 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, /* Unlock virDomainObj while dumping core */ virObjectUnlock(vm); - libxl_domain_core_dump(priv->ctx, vm->def->id, dumpfile, NULL); + libxl_domain_core_dump(cfg->ctx, vm->def->id, dumpfile, NULL); virObjectLock(vm); ignore_value(libxlDomainObjEndJob(driver, vm)); @@ -797,7 +748,7 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, int libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { - libxlDomainObjPrivatePtr priv = vm->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainDefPtr def = vm->def; libxl_bitmap map; virBitmapPtr cpumask = NULL; @@ -832,7 +783,7 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) map.size = cpumaplen; map.map = cpumap; - if (libxl_set_vcpuaffinity(priv->ctx, def->id, vcpu, &map) != 0) { + if (libxl_set_vcpuaffinity(cfg->ctx, def->id, vcpu, &map) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to pin vcpu '%d' with libxenlight"), vcpu); goto cleanup; @@ -845,6 +796,7 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) cleanup: VIR_FREE(cpumap); + virObjectUnref(cfg); return ret; } @@ -950,9 +902,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxl_domain_config_init(&d_config); - if (libxlDomainObjPrivateInitCtx(vm) < 0) - return ret; - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) return ret; @@ -1000,10 +949,10 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, - priv->ctx, &d_config) < 0) + cfg->ctx, &d_config) < 0) goto endjob; - if (cfg->autoballoon && libxlDomainFreeMem(priv->ctx, &d_config) < 0) { + if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to get free memory for domain '%s'"), d_config.c_info.name); @@ -1020,16 +969,16 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, aop_console_how.for_callback = vm; aop_console_how.callback = libxlConsoleCallback; if (restore_fd < 0) { - ret = libxl_domain_create_new(priv->ctx, &d_config, + ret = libxl_domain_create_new(cfg->ctx, &d_config, &domid, NULL, &aop_console_how); } else { #ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS params.checkpointed_stream = 0; - ret = libxl_domain_create_restore(priv->ctx, &d_config, &domid, + ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid, restore_fd, ¶ms, NULL, &aop_console_how); #else - ret = libxl_domain_create_restore(priv->ctx, &d_config, &domid, + ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid, restore_fd, NULL, &aop_console_how); #endif } @@ -1054,13 +1003,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, vm->def->id = domid; /* Always enable domain death events */ - if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) + if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW)) goto cleanup_dom; if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) goto cleanup_dom; - if (libxl_userdata_store(priv->ctx, domid, "libvirt-xml", + if (libxl_userdata_store(cfg->ctx, domid, "libvirt-xml", (uint8_t *)dom_xml, strlen(dom_xml) + 1)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxenlight failed to store userdata")); @@ -1071,7 +1020,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup_dom; if (!start_paused) { - libxl_domain_unpause(priv->ctx, domid); + libxl_domain_unpause(cfg->ctx, domid); virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); @@ -1095,10 +1044,10 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, cleanup_dom: if (priv->deathW) { - libxl_evdisable_domain_death(priv->ctx, priv->deathW); + libxl_evdisable_domain_death(cfg->ctx, priv->deathW); priv->deathW = NULL; } - libxl_domain_destroy(priv->ctx, domid, NULL); + libxl_domain_destroy(cfg->ctx, domid, NULL); vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 2db4d77..a032e46 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -60,11 +60,6 @@ typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr; struct _libxlDomainObjPrivate { virObjectLockable parent; - /* per domain log stream for libxl messages */ - FILE *logger_file; - xentoollog_logger *logger; - /* per domain libxl ctx */ - libxl_ctx *ctx; /* console */ virChrdevsPtr devs; libxl_evgen_domain_death *deathW; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 43ee629..bf64d15 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -328,6 +328,7 @@ libxlReconnectDomain(virDomainObjPtr vm, { libxlDriverPrivatePtr driver = opaque; libxlDomainObjPrivatePtr priv = vm->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); int rc; libxl_dominfo d_info; int len; @@ -336,9 +337,8 @@ libxlReconnectDomain(virDomainObjPtr vm, virObjectLock(vm); - libxlDomainObjPrivateInitCtx(vm); /* Does domain still exist? */ - rc = libxl_domain_info(priv->ctx, &d_info, vm->def->id); + rc = libxl_domain_info(cfg->ctx, &d_info, vm->def->id); if (rc == ERROR_INVAL) { goto out; } else if (rc != 0) { @@ -348,7 +348,7 @@ libxlReconnectDomain(virDomainObjPtr vm, } /* Is this a domain that was under libvirt control? */ - if (libxl_userdata_retrieve(priv->ctx, vm->def->id, + if (libxl_userdata_retrieve(cfg->ctx, vm->def->id, "libvirt-xml", &data, &len)) { VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", vm->def->id); goto out; @@ -368,9 +368,10 @@ libxlReconnectDomain(virDomainObjPtr vm, driver->inhibitCallback(true, driver->inhibitOpaque); /* Enable domain death events */ - libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW); + libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); virObjectUnlock(vm); + virObjectUnref(cfg); return 0; out: @@ -379,6 +380,7 @@ libxlReconnectDomain(virDomainObjPtr vm, virDomainObjListRemoveLocked(driver->domains, vm); else virObjectUnlock(vm); + virObjectUnref(cfg); return -1; } @@ -986,7 +988,6 @@ libxlDomainSuspend(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; - libxlDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; int ret = -1; @@ -1004,10 +1005,8 @@ libxlDomainSuspend(virDomainPtr dom) goto endjob; } - priv = vm->privateData; - if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - if (libxl_domain_pause(priv->ctx, vm->def->id) != 0) { + if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to suspend domain '%d' with libxenlight"), vm->def->id); @@ -1045,7 +1044,6 @@ libxlDomainResume(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; - libxlDomainObjPrivatePtr priv; virObjectEventPtr event = NULL; int ret = -1; @@ -1063,10 +1061,8 @@ libxlDomainResume(virDomainPtr dom) goto endjob; } - priv = vm->privateData; - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (libxl_domain_unpause(priv->ctx, vm->def->id) != 0) { + if (libxl_domain_unpause(cfg->ctx, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to resume domain '%d' with libxenlight"), vm->def->id); @@ -1101,9 +1097,10 @@ libxlDomainResume(virDomainPtr dom) static int libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; int ret = -1; - libxlDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | VIR_DOMAIN_SHUTDOWN_PARAVIRT, -1); @@ -1123,9 +1120,8 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - priv = vm->privateData; if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) { - ret = libxl_domain_shutdown(priv->ctx, vm->def->id); + ret = libxl_domain_shutdown(cfg->ctx, vm->def->id); if (ret == 0) goto cleanup; @@ -1140,7 +1136,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) } if (flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) { - ret = libxl_send_trigger(priv->ctx, vm->def->id, + ret = libxl_send_trigger(cfg->ctx, vm->def->id, LIBXL_TRIGGER_POWER, 0); if (ret == 0) goto cleanup; @@ -1154,6 +1150,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -1167,9 +1164,10 @@ libxlDomainShutdown(virDomainPtr dom) static int libxlDomainReboot(virDomainPtr dom, unsigned int flags) { + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; int ret = -1; - libxlDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_REBOOT_PARAVIRT, -1); if (flags == 0) @@ -1187,9 +1185,8 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) goto cleanup; } - priv = vm->privateData; if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) { - ret = libxl_domain_reboot(priv->ctx, vm->def->id); + ret = libxl_domain_reboot(cfg->ctx, vm->def->id); if (ret == 0) goto cleanup; @@ -1202,6 +1199,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -1210,10 +1208,10 @@ libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; int ret = -1; virObjectEventPtr event = NULL; - libxlDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -1232,8 +1230,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - priv = vm->privateData; - if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto cleanup; @@ -1253,6 +1250,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); + virObjectUnref(cfg); return ret; } @@ -1309,7 +1307,6 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; virDomainDefPtr persistentDef = NULL; bool isActive; @@ -1365,8 +1362,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, /* resize the maximum memory */ if (flags & VIR_DOMAIN_MEM_LIVE) { - priv = vm->privateData; - if (libxl_domain_setmaxmem(priv->ctx, vm->def->id, newmem) < 0) { + if (libxl_domain_setmaxmem(cfg->ctx, vm->def->id, newmem) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set maximum memory for domain '%d'" " with libxenlight"), vm->def->id); @@ -1396,10 +1392,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_MEM_LIVE) { int res; - priv = vm->privateData; /* Unlock virDomainObj while ballooning memory */ virObjectUnlock(vm); - res = libxl_set_memory_target(priv->ctx, vm->def->id, newmem, 0, + res = libxl_set_memory_target(cfg->ctx, vm->def->id, newmem, 0, /* force */ 1); virObjectLock(vm); if (res < 0) { @@ -1446,9 +1441,10 @@ libxlDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) static int libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; libxl_dominfo d_info; - libxlDomainObjPrivatePtr priv; int ret = -1; if (!(vm = libxlDomObjFromDomain(dom))) @@ -1457,13 +1453,12 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - priv = vm->privateData; if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; info->memory = vm->def->mem.cur_balloon; info->maxMem = vm->def->mem.max_balloon; } else { - if (libxl_domain_info(priv->ctx, &d_info, vm->def->id) != 0) { + if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxl_domain_info failed for domain '%d'"), vm->def->id); @@ -1481,6 +1476,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -1517,7 +1513,7 @@ static int libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, const char *to) { - libxlDomainObjPrivatePtr priv = vm->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); libxlSavefileHeader hdr; virObjectEventPtr event = NULL; char *xml = NULL; @@ -1562,7 +1558,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, /* Unlock virDomainObj while saving domain */ virObjectUnlock(vm); - ret = libxl_domain_suspend(priv->ctx, vm->def->id, fd, 0, NULL); + ret = libxl_domain_suspend(cfg->ctx, vm->def->id, fd, 0, NULL); virObjectLock(vm); if (ret != 0) { @@ -1576,7 +1572,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); - if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto cleanup; @@ -1592,6 +1588,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virReportSystemError(errno, "%s", _("cannot close file")); if (event) libxlDomainEventQueue(driver, event); + virObjectUnref(cfg); return ret; } @@ -1728,7 +1725,7 @@ static int libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; - libxlDomainObjPrivatePtr priv; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; virObjectEventPtr event = NULL; bool remove_dom = false; @@ -1751,11 +1748,9 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) goto endjob; } - priv = vm->privateData; - if (!(flags & VIR_DUMP_LIVE) && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { - if (libxl_domain_pause(priv->ctx, vm->def->id) != 0) { + if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Before dumping core, failed to suspend domain '%d'" " with libxenlight"), @@ -1768,7 +1763,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) /* Unlock virDomainObj while dumping core */ virObjectUnlock(vm); - ret = libxl_domain_core_dump(priv->ctx, vm->def->id, to, NULL); + ret = libxl_domain_core_dump(cfg->ctx, vm->def->id, to, NULL); virObjectLock(vm); if (ret != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1779,7 +1774,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) } if (flags & VIR_DUMP_CRASH) { - if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto unpause; @@ -1796,7 +1791,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) unpause: if (virDomainObjIsActive(vm) && paused) { - if (libxl_domain_unpause(priv->ctx, vm->def->id) != 0) { + if (libxl_domain_unpause(cfg->ctx, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("After dumping core, failed to resume domain '%d' with" " libxenlight"), vm->def->id); @@ -1819,6 +1814,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); + virObjectUnref(cfg); return ret; } @@ -1961,7 +1957,6 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - libxlDomainObjPrivatePtr priv; virDomainDefPtr def; virDomainObjPtr vm; libxl_bitmap map; @@ -2028,8 +2023,6 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - priv = vm->privateData; - if (!(def = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm))) goto endjob; @@ -2057,7 +2050,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE: - if (libxl_set_vcpuonline(priv->ctx, vm->def->id, &map) != 0) { + if (libxl_set_vcpuonline(cfg->ctx, vm->def->id, &map) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain '%d'" " with libxenlight"), vm->def->id); @@ -2066,7 +2059,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: - if (libxl_set_vcpuonline(priv->ctx, vm->def->id, &map) != 0) { + if (libxl_set_vcpuonline(cfg->ctx, vm->def->id, &map) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain '%d'" " with libxenlight"), vm->def->id); @@ -2201,10 +2194,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, if (flags & VIR_DOMAIN_AFFECT_LIVE) { libxl_bitmap map = { .size = maplen, .map = cpumap }; - libxlDomainObjPrivatePtr priv; - - priv = vm->privateData; - if (libxl_set_vcpuaffinity(priv->ctx, vm->def->id, vcpu, &map) != 0) { + if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, vcpu, &map) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to pin vcpu '%d' with libxenlight"), vcpu); @@ -2300,7 +2290,6 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, if (ncpumaps > targetDef->vcpus) ncpumaps = targetDef->vcpus; - /* we use cfg->ctx, as vm->privateData->ctx may be NULL if VM is down. */ if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0) goto cleanup; @@ -2343,7 +2332,8 @@ static int libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen) { - libxlDomainObjPrivatePtr priv; + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; int ret = -1; libxl_vcpuinfo *vcpuinfo; @@ -2362,8 +2352,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, goto cleanup; } - priv = vm->privateData; - if ((vcpuinfo = libxl_list_vcpu(priv->ctx, vm->def->id, &maxcpu, + if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu, &hostcpus)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to list vcpus for domain '%d' with libxenlight"), @@ -2399,6 +2388,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -2754,9 +2744,9 @@ libxlDomainUndefine(virDomainPtr dom) } static int -libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, - virDomainObjPtr vm, virDomainDiskDefPtr disk) +libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr origdisk = NULL; libxl_device_disk x_disk; size_t i; @@ -2787,7 +2777,7 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_cdrom_insert(priv->ctx, vm->def->id, &x_disk, NULL)) < 0) { + if ((ret = libxl_cdrom_insert(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to change media for disk '%s'"), disk->dst); @@ -2803,20 +2793,21 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, ret = 0; cleanup: + virObjectUnref(cfg); return ret; } static int -libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, - virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr l_disk = dev->data.disk; libxl_device_disk x_disk; int ret = -1; switch (l_disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: - ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk); + ret = libxlDomainChangeEjectableMedia(vm, l_disk); break; case VIR_DOMAIN_DISK_DEVICE_DISK: if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { @@ -2838,7 +2829,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(l_disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_device_disk_add(priv->ctx, vm->def->id, + if ((ret = libxl_device_disk_add(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to attach disk '%s'"), @@ -2862,43 +2853,45 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, } cleanup: + virObjectUnref(cfg); return ret; } static int libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { - libxl_device_pci pcidev; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + libxl_device_pci pcidev; virDomainHostdevDefPtr found; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; + int ret = -1; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - return -1; + goto cleanup; if (virDomainHostdevFind(vm->def, hostdev, &found) >= 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("target pci device %.4x:%.2x:%.2x.%.1x already exists"), pcisrc->addr.domain, pcisrc->addr.bus, pcisrc->addr.slot, pcisrc->addr.function); - return -1; + goto cleanup; } if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) - return -1; + goto cleanup; if (virHostdevPreparePCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def->name, vm->def->uuid, &hostdev, 1, 0) < 0) - return -1; + goto cleanup; if (libxlMakePCI(hostdev, &pcidev) < 0) goto error; - if (libxl_device_pci_add(priv->ctx, vm->def->id, &pcidev, 0) < 0) { + if (libxl_device_pci_add(cfg->ctx, vm->def->id, &pcidev, 0) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to attach pci device %.4x:%.2x:%.2x.%.1x"), pcisrc->addr.domain, pcisrc->addr.bus, @@ -2907,17 +2900,20 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, } vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; - return 0; + ret = 0; + goto cleanup; error: virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def->name, &hostdev, 1, NULL); - return -1; + + cleanup: + virObjectUnref(cfg); + return ret; } static int libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { @@ -2930,7 +2926,7 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, switch (hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (libxlDomainAttachHostPCIDevice(driver, priv, vm, hostdev) < 0) + if (libxlDomainAttachHostPCIDevice(driver, vm, hostdev) < 0) return -1; break; @@ -2945,9 +2941,9 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, } static int -libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, - virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr l_disk = NULL; libxl_device_disk x_disk; int idx; @@ -2970,7 +2966,7 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(l_disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_device_disk_remove(priv->ctx, vm->def->id, + if ((ret = libxl_device_disk_remove(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to detach disk '%s'"), @@ -2995,29 +2991,30 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, } cleanup: + virObjectUnref(cfg); return ret; } static int libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainNetDefPtr net) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); int actualType; libxl_device_nic nic; int ret = -1; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) - return -1; + goto out; /* If appropriate, grab a physical device from the configured * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ if (networkAllocateActualDevice(vm->def, net) < 0) - return -1; + goto out; actualType = virDomainNetGetActualType(net); @@ -3027,7 +3024,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, * netdev-specific code as appropriate), then also added to * the nets list (see out:) if successful. */ - ret = libxlDomainAttachHostDevice(driver, priv, vm, + ret = libxlDomainAttachHostDevice(driver, vm, virDomainNetGetActualHostdev(net)); goto out; } @@ -3036,7 +3033,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, if (libxlMakeNic(vm->def, net, &nic) < 0) goto cleanup; - if (libxl_device_nic_add(priv->ctx, vm->def->id, &nic, 0)) { + if (libxl_device_nic_add(cfg->ctx, vm->def->id, &nic, 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxenlight failed to attach network device")); goto cleanup; @@ -3049,12 +3046,12 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, out: if (!ret) vm->def->nets[vm->def->nnets++] = net; + virObjectUnref(cfg); return ret; } static int libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3062,20 +3059,20 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev); + ret = libxlDomainAttachDeviceDiskLive(vm, dev); if (!ret) dev->data.disk = NULL; break; case VIR_DOMAIN_DEVICE_NET: - ret = libxlDomainAttachNetDevice(driver, priv, vm, + ret = libxlDomainAttachNetDevice(driver, vm, dev->data.net); if (!ret) dev->data.net = NULL; break; case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = libxlDomainAttachHostDevice(driver, priv, vm, + ret = libxlDomainAttachHostDevice(driver, vm, dev->data.hostdev); if (!ret) dev->data.hostdev = NULL; @@ -3180,19 +3177,20 @@ libxlIsMultiFunctionDevice(virDomainDefPtr def, static int libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci; libxl_device_pci pcidev; virDomainHostdevDefPtr detach; int idx; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + int ret = -1; if (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - return -1; + goto cleanup; idx = virDomainHostdevFind(vm->def, hostdev, &detach); if (idx < 0) { @@ -3200,7 +3198,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, _("host pci device %.4x:%.2x:%.2x.%.1x not found"), pcisrc->addr.domain, pcisrc->addr.bus, pcisrc->addr.slot, pcisrc->addr.function); - return -1; + goto cleanup; } if (libxlIsMultiFunctionDevice(vm->def, detach->info)) { @@ -3217,7 +3215,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, if (libxlMakePCI(detach, &pcidev) < 0) goto error; - if (libxl_device_pci_remove(priv->ctx, vm->def->id, &pcidev, 0) < 0) { + if (libxl_device_pci_remove(cfg->ctx, vm->def->id, &pcidev, 0) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to detach pci device\ %.4x:%.2x:%.2x.%.1x"), @@ -3233,16 +3231,18 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def->name, &hostdev, 1, NULL); - return 0; + ret = 0; error: virDomainHostdevDefFree(detach); - return -1; + + cleanup: + virObjectUnref(cfg); + return ret; } static int libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { @@ -3257,7 +3257,7 @@ libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver, switch (subsys->type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - return libxlDomainDetachHostPCIDevice(driver, priv, vm, hostdev); + return libxlDomainDetachHostPCIDevice(driver, vm, hostdev); default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3270,10 +3270,10 @@ libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver, static int libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainNetDefPtr net) { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); int detachidx; virDomainNetDefPtr detach = NULL; libxl_device_nic nic; @@ -3281,7 +3281,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, int ret = -1; if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) - return -1; + goto out; detach = vm->def->nets[detachidx]; @@ -3289,17 +3289,17 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, /* This is really a "smart hostdev", so it should be attached as a * hostdev, then also removed from nets list (see out:) if successful. */ - ret = libxlDomainDetachHostDevice(driver, priv, vm, + ret = libxlDomainDetachHostDevice(driver, vm, virDomainNetGetActualHostdev(detach)); goto out; } libxl_device_nic_init(&nic); - if (libxl_mac_to_device_nic(priv->ctx, vm->def->id, + if (libxl_mac_to_device_nic(cfg->ctx, vm->def->id, virMacAddrFormat(&detach->mac, mac), &nic)) goto cleanup; - if (libxl_device_nic_remove(priv->ctx, vm->def->id, &nic, 0)) { + if (libxl_device_nic_remove(cfg->ctx, vm->def->id, &nic, 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxenlight failed to detach network device")); goto cleanup; @@ -3312,12 +3312,12 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, out: if (!ret) virDomainNetRemove(vm->def, detachidx); + virObjectUnref(cfg); return ret; } static int libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, - libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3325,16 +3325,16 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev); + ret = libxlDomainDetachDeviceDiskLive(vm, dev); break; case VIR_DOMAIN_DEVICE_NET: - ret = libxlDomainDetachNetDevice(driver, priv, vm, + ret = libxlDomainDetachNetDevice(driver, vm, dev->data.net); break; case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = libxlDomainDetachHostDevice(driver, priv, vm, + ret = libxlDomainDetachHostDevice(driver, vm, dev->data.hostdev); break; @@ -3399,8 +3399,7 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) } static int -libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv, - virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; int ret = -1; @@ -3410,7 +3409,7 @@ libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv, disk = dev->data.disk; switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: - ret = libxlDomainChangeEjectableMedia(priv, vm, disk); + ret = libxlDomainChangeEjectableMedia(vm, disk); if (ret == 0) dev->data.disk = NULL; break; @@ -3483,7 +3482,6 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL; - libxlDomainObjPrivatePtr priv; int ret = -1; virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | @@ -3518,8 +3516,6 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } - priv = vm->privateData; - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, @@ -3543,7 +3539,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto endjob; - if (libxlDomainAttachDeviceLive(driver, priv, vm, dev) < 0) + if (libxlDomainAttachDeviceLive(driver, vm, dev) < 0) goto endjob; /* @@ -3594,7 +3590,6 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL; - libxlDomainObjPrivatePtr priv; int ret = -1; virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | @@ -3629,8 +3624,6 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } - priv = vm->privateData; - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, @@ -3654,7 +3647,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto endjob; - if (libxlDomainDetachDeviceLive(driver, priv, vm, dev) < 0) + if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0) goto endjob; /* @@ -3705,7 +3698,6 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL; - libxlDomainObjPrivatePtr priv; int ret = -1; virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | @@ -3737,8 +3729,6 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } - priv = vm->privateData; - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, @@ -3764,7 +3754,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup; - if ((ret = libxlDomainUpdateDeviceLive(priv, vm, dev)) < 0) + if ((ret = libxlDomainUpdateDeviceLive(vm, dev)) < 0) goto cleanup; /* @@ -3998,7 +3988,8 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart) static char * libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) { - libxlDomainObjPrivatePtr priv; + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; char * ret = NULL; const char *name = NULL; @@ -4015,8 +4006,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) goto cleanup; } - priv = vm->privateData; - sched_id = libxl_get_scheduler(priv->ctx); + sched_id = libxl_get_scheduler(cfg->ctx); if (nparams) *nparams = 0; @@ -4047,6 +4037,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -4056,7 +4047,8 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, int *nparams, unsigned int flags) { - libxlDomainObjPrivatePtr priv; + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; libxl_domain_sched_params sc_info; libxl_scheduler sched_id; @@ -4079,9 +4071,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - priv = vm->privateData; - - sched_id = libxl_get_scheduler(priv->ctx); + sched_id = libxl_get_scheduler(cfg->ctx); if (sched_id != LIBXL_SCHEDULER_CREDIT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4089,7 +4079,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - if (libxl_domain_sched_params_get(priv->ctx, vm->def->id, &sc_info) != 0) { + if (libxl_domain_sched_params_get(cfg->ctx, vm->def->id, &sc_info) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get scheduler parameters for domain '%d'" " with libxenlight"), vm->def->id); @@ -4113,6 +4103,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -4130,7 +4121,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; - libxlDomainObjPrivatePtr priv; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; libxl_domain_sched_params sc_info; int sched_id; @@ -4160,9 +4151,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - priv = vm->privateData; - - sched_id = libxl_get_scheduler(priv->ctx); + sched_id = libxl_get_scheduler(cfg->ctx); if (sched_id != LIBXL_SCHEDULER_CREDIT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4170,7 +4159,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (libxl_domain_sched_params_get(priv->ctx, vm->def->id, &sc_info) != 0) { + if (libxl_domain_sched_params_get(cfg->ctx, vm->def->id, &sc_info) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get scheduler parameters for domain '%d'" " with libxenlight"), vm->def->id); @@ -4186,7 +4175,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, sc_info.cap = params[i].value.ui; } - if (libxl_domain_sched_params_set(priv->ctx, vm->def->id, &sc_info) != 0) { + if (libxl_domain_sched_params_set(cfg->ctx, vm->def->id, &sc_info) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set scheduler parameters for domain '%d'" " with libxenlight"), vm->def->id); @@ -4202,6 +4191,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } @@ -4295,7 +4285,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom, int *nparams, unsigned int flags) { - libxlDomainObjPrivatePtr priv; + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; libxl_bitmap nodemap; virBitmapPtr nodes = NULL; @@ -4326,8 +4317,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom, goto cleanup; } - priv = vm->privateData; - if ((*nparams) == 0) { *nparams = LIBXL_NUMA_NPARAM; ret = 0; @@ -4355,18 +4344,18 @@ libxlDomainGetNumaParameters(virDomainPtr dom, /* Node affinity */ /* Let's allocate both libxl and libvirt bitmaps */ - numnodes = libxl_get_max_nodes(priv->ctx); + numnodes = libxl_get_max_nodes(cfg->ctx); if (numnodes <= 0) goto cleanup; - if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0)) { + if (libxl_node_bitmap_alloc(cfg->ctx, &nodemap, 0)) { virReportOOMError(); goto cleanup; } if (!(nodes = virBitmapNew(numnodes))) goto cleanup; - rc = libxl_domain_get_nodeaffinity(priv->ctx, + rc = libxl_domain_get_nodeaffinity(cfg->ctx, vm->def->id, &nodemap); if (rc != 0) { @@ -4409,6 +4398,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom, libxl_bitmap_dispose(&nodemap); if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } #endif diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 271390a..3d0c96e 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1,7 +1,7 @@ /* * libxl_migration.c: methods for handling migration with libxenlight * - * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. + * Copyright (C) 2014-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 @@ -176,7 +176,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver, unsigned long flags, int sockfd) { - libxlDomainObjPrivatePtr priv; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virObjectEventPtr event = NULL; int xl_flags = 0; @@ -185,12 +184,11 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver, if (flags & VIR_MIGRATE_LIVE) xl_flags = LIBXL_SUSPEND_LIVE; - priv = vm->privateData; - ret = libxl_domain_suspend(priv->ctx, vm->def->id, sockfd, + ret = libxl_domain_suspend(cfg->ctx, vm->def->id, sockfd, xl_flags, NULL); if (ret != 0) { /* attempt to resume the domain on failure */ - if (libxl_domain_resume(priv->ctx, vm->def->id, 1, 0) != 0) { + if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) != 0) { VIR_DEBUG("Failed to resume domain following failed migration"); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_MIGRATION); @@ -547,7 +545,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, /* Unpause if requested */ if (!(flags & VIR_MIGRATE_PAUSED)) { - if (libxl_domain_unpause(priv->ctx, vm->def->id) != 0) { + if (libxl_domain_unpause(cfg->ctx, vm->def->id) != 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Failed to unpause domain")); goto cleanup; @@ -577,7 +575,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, cleanup: if (dom == NULL) { - libxl_domain_destroy(priv->ctx, vm->def->id, NULL); + libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); @@ -598,12 +596,11 @@ libxlDomainMigrationConfirm(libxlDriverPrivatePtr driver, int cancelled) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - libxlDomainObjPrivatePtr priv = vm->privateData; virObjectEventPtr event = NULL; int ret = -1; if (cancelled) { - if (libxl_domain_resume(priv->ctx, vm->def->id, 1, 0) == 0) { + if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) == 0) { ret = 0; } else { VIR_DEBUG("Unable to resume domain '%s' after failed migration", @@ -617,7 +614,7 @@ libxlDomainMigrationConfirm(libxlDriverPrivatePtr driver, goto cleanup; } - libxl_domain_destroy(priv->ctx, vm->def->id, NULL); + libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); -- 1.8.4.5

The DEBUG log level is very verbose and can quickly fill a filesystem hosting /var/log/libvirt/libxl/ Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- Ideally, the log level should match what is configured in libvirtd.conf. Are the logging knob settings available in the drivers? If not, this would be a good option for a future /etc/libvirt/libxl.conf. src/libxl/libxl_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index e092b11..34dbfe4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1350,7 +1350,7 @@ libxlDriverConfigNew(void) cfg->logger = (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file, - XTL_DEBUG, 0); + XTL_ERROR, 0); if (!cfg->logger) { VIR_ERROR(_("cannot create logger for libxenlight, disabling driver")); goto error; -- 1.8.4.5

On 18.02.2015 04:56, Jim Fehlig wrote:
This series is a follow up to
https://www.redhat.com/archives/libvir-list/2015-February/msg00024.html
It goes a step further and changes the libxl driver to use one, driver-wide libxl_ctx. Currently the libxl driver has one driver-wide ctx for operations that are not domain-specific and a ctx for each domain. This approach was necessary back in the old Xen4.1 libxl days, but with the newer libxl it is more of a hinderance than benefit. Ian Jackson suggested moving to a single ctx while discussing some deadlocks and assertions encountered in the libxl driver when under load from tests such as OpenStack Tempest.
Making such a change involves quite a bit of code movement. I've tried to split that up into a reviewable series, the result of which are the 9 patches that follow. I've ran this through all of my automated tests as well as some hacky tests I created to reproduce failures revealed by Tempest.
One downside of moving to a single ctx is losing the per-domain log files. Currently, a single log stream can be associated with ctx, hence all logging from libxl will go to a single file. Ian is going to investigate possibilities to accommodate per-domain log files in libxl, but in the meantime folks using Xen are accustomed to a single log file from the xend days.
I've been testing this series on xen-unstable and Xen 4.4.1 + commits 2ffeb5d7, 4b9143e4, 5a968257, 60ce518a, 66bff9fd, 77a1bf37, f49f9b41, 6b5a5bba, 93699882d, f1335f0d, and 8bc64413. Results are much better than before applying the series, but I do notice a stuck hypercall after many (hundreds) concurrent domain create/destroy operations. The single libxl_ctx is locked in the callpath, essentially deadlocking the driver.
Thread 1 (Thread 0x7f0649a198c0 (LWP 2235)): 0 0x00007f0645272397 in ioctl () from /lib64/libc.so.6 1 0x00007f0645d8e353 in linux_privcmd_hypercall (xch=<optimized out>, h=<optimized out>, hypercall=<optimized out>) at xc_linux_osdep.c:134 2 0x00007f0645d854b8 in do_xen_hypercall (xch=xch@entry=0x7f0630039390, hypercall=hypercall@entry=0x7fffd53f80e0) at xc_private.c:249 3 0x00007f0645d86aa4 in do_sysctl (sysctl=sysctl@entry=0x7fffd53f8080, xch=xch@entry=0x7f0630039390) at xc_private.h:281 4 xc_sysctl (xch=xch@entry=0x7f0630039390, sysctl=sysctl@entry=0x7fffd53f8170) at xc_private.c:656 5 0x00007f0645d7bfbf in xc_domain_getinfolist (xch=0x7f0630039390, first_domain=first_domain@entry=119, max_domains=max_domains@entry=1, info=info@entry=0x7fffd53f8260) at xc_domain.c:382 6 0x00007f0645fabca6 in domain_death_xswatch_callback (egc=0x7fffd53f83f0, w=<optimized out>, wpath=<optimized out>, epath=<optimized out>) at libxl.c:1041 7 0x00007f0645fd75a8 in watchfd_callback (egc=0x7fffd53f83f0, ev=<optimized out>, fd=<optimized out>, events=<optimized out>, revents=<optimized out>) at libxl_event.c:515 8 0x00007f0645fd8ac3 in libxl_osevent_occurred_fd (ctx=<optimized out>, for_libxl=<optimized out>, fd=<optimized out>, events_ign=<optimized out>, revents_ign=<optimized out>) at libxl_event.c:1259 9 0x00007f063a23402c in libxlFDEventCallback (watch=454, fd=33, vir_events=1, fd_info=0x7f0608007e70) at libxl/libxl_driver.c:123
There is no hint in any logs or dmesg suggesting a cause for the stuck hypercall. Any suggestions for further debugging tips appreciated.
Jim Fehlig (10): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: use driver-wide ctx in fd and timer event handling libxl: Move setup of child processing code to driver initialization libxl: move event registration to driver initialization libxl: use global libxl_ctx in event handler libxl: remove unnecessary libxlDomainEventsRegister libxl: make libxlDomainFreeMem static libxl: remove per-domain libxl_ctx libxl: change libxl log stream to ERROR log level
src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 438 ++++++--------------------------------- src/libxl/libxl_domain.h | 27 +-- src/libxl/libxl_driver.c | 484 +++++++++++++++++++++++++++++++------------- src/libxl/libxl_migration.c | 17 +- 5 files changed, 426 insertions(+), 542 deletions(-)
ACK series Michal

Michal Privoznik wrote:
On 18.02.2015 04:56, Jim Fehlig wrote:
This series is a follow up to
https://www.redhat.com/archives/libvir-list/2015-February/msg00024.html
It goes a step further and changes the libxl driver to use one, driver-wide libxl_ctx. Currently the libxl driver has one driver-wide ctx for operations that are not domain-specific and a ctx for each domain. This approach was necessary back in the old Xen4.1 libxl days, but with the newer libxl it is more of a hinderance than benefit. Ian Jackson suggested moving to a single ctx while discussing some deadlocks and assertions encountered in the libxl driver when under load from tests such as OpenStack Tempest.
Making such a change involves quite a bit of code movement. I've tried to split that up into a reviewable series, the result of which are the 9 patches that follow. I've ran this through all of my automated tests as well as some hacky tests I created to reproduce failures revealed by Tempest.
One downside of moving to a single ctx is losing the per-domain log files. Currently, a single log stream can be associated with ctx, hence all logging from libxl will go to a single file. Ian is going to investigate possibilities to accommodate per-domain log files in libxl, but in the meantime folks using Xen are accustomed to a single log file from the xend days.
I've been testing this series on xen-unstable and Xen 4.4.1 + commits 2ffeb5d7, 4b9143e4, 5a968257, 60ce518a, 66bff9fd, 77a1bf37, f49f9b41, 6b5a5bba, 93699882d, f1335f0d, and 8bc64413. Results are much better than before applying the series, but I do notice a stuck hypercall after many (hundreds) concurrent domain create/destroy operations. The single libxl_ctx is locked in the callpath, essentially deadlocking the driver.
Thread 1 (Thread 0x7f0649a198c0 (LWP 2235)): 0 0x00007f0645272397 in ioctl () from /lib64/libc.so.6 1 0x00007f0645d8e353 in linux_privcmd_hypercall (xch=<optimized out>, h=<optimized out>, hypercall=<optimized out>) at xc_linux_osdep.c:134 2 0x00007f0645d854b8 in do_xen_hypercall (xch=xch@entry=0x7f0630039390, hypercall=hypercall@entry=0x7fffd53f80e0) at xc_private.c:249 3 0x00007f0645d86aa4 in do_sysctl (sysctl=sysctl@entry=0x7fffd53f8080, xch=xch@entry=0x7f0630039390) at xc_private.h:281 4 xc_sysctl (xch=xch@entry=0x7f0630039390, sysctl=sysctl@entry=0x7fffd53f8170) at xc_private.c:656 5 0x00007f0645d7bfbf in xc_domain_getinfolist (xch=0x7f0630039390, first_domain=first_domain@entry=119, max_domains=max_domains@entry=1, info=info@entry=0x7fffd53f8260) at xc_domain.c:382 6 0x00007f0645fabca6 in domain_death_xswatch_callback (egc=0x7fffd53f83f0, w=<optimized out>, wpath=<optimized out>, epath=<optimized out>) at libxl.c:1041 7 0x00007f0645fd75a8 in watchfd_callback (egc=0x7fffd53f83f0, ev=<optimized out>, fd=<optimized out>, events=<optimized out>, revents=<optimized out>) at libxl_event.c:515 8 0x00007f0645fd8ac3 in libxl_osevent_occurred_fd (ctx=<optimized out>, for_libxl=<optimized out>, fd=<optimized out>, events_ign=<optimized out>, revents_ign=<optimized out>) at libxl_event.c:1259 9 0x00007f063a23402c in libxlFDEventCallback (watch=454, fd=33, vir_events=1, fd_info=0x7f0608007e70) at libxl/libxl_driver.c:123
There is no hint in any logs or dmesg suggesting a cause for the stuck hypercall. Any suggestions for further debugging tips appreciated.
FYI, this was not a hung hypercall, but looping clear back in frame 6 that I overlooked. It was fixed in libxl by the following commit http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=4783c99aab866f470bd59368...
Jim Fehlig (10): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: use driver-wide ctx in fd and timer event handling libxl: Move setup of child processing code to driver initialization libxl: move event registration to driver initialization libxl: use global libxl_ctx in event handler libxl: remove unnecessary libxlDomainEventsRegister libxl: make libxlDomainFreeMem static libxl: remove per-domain libxl_ctx libxl: change libxl log stream to ERROR log level
src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 438 ++++++--------------------------------- src/libxl/libxl_domain.h | 27 +-- src/libxl/libxl_driver.c | 484 +++++++++++++++++++++++++++++++------------- src/libxl/libxl_migration.c | 17 +- 5 files changed, 426 insertions(+), 542 deletions(-)
ACK series
Thanks! 1 and 2 were pushed earlier as part of this trivial series https://www.redhat.com/archives/libvir-list/2015-March/msg00102.html I've now pushed 3-9, but held off on pushing 10 since it removes the possibility to get debug level messages from libxl. I think a better approach would be to introduce /etc/libvirt/libxl.conf with a 'log_level' setting, giving users the ability to change this a bit more dynamically. Actually, an even better approach would be to set libxl debug level based on the level set in /etc/libvirt/libvirtd.conf. But AFAIK, the settings of various knobs in libvirtd.conf are generally not available to the individual drivers. Regards, Jim
participants (2)
-
Jim Fehlig
-
Michal Privoznik