Markus Groß wrote:

--- src/libxl/libxl_conf.h | 8 ++- src/libxl/libxl_driver.c | 199 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 202 insertions(+), 5 deletions(-)

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index bb49d35..a86df9a 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -26,6 +26,7 @@

# include "internal.h" # include "domain_conf.h" +# include "domain_event.h" # include "capabilities.h" # include "configmake.h" # include "bitmap.h" @@ -57,6 +58,12 @@ struct _libxlDriverPrivate { virBitmapPtr reservedVNCPorts; virDomainObjList domains;

+ /* A list of callbacks */ + virDomainEventCallbackListPtr domainEventCallbacks; + virDomainEventQueuePtr domainEventQueue; + int domainEventTimer; + int domainEventDispatching; + char *configDir; char *autostartDir; char *logDir; @@ -87,5 +94,4 @@ int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config);

- #endif /* LIBXL_CONF_H */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 615cb47..ad95e2b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1,6 +1,7 @@ /*---------------------------------------------------------------------------*/ /* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. + * Copyright (C) 2006-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -15,6 +16,11 @@ * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Jim Fehlig <jfehlig@novell.com> + * Markus Groß <gross@univention.de> + * Daniel P. Berrange <berrange@redhat.com>

I have no objections to adding authors list, but we should probably add it to the other libxl files to be consistent. But that can be done with a subsequent patch.

*/ /*---------------------------------------------------------------------------*/

@@ -99,6 +105,58 @@ libxlDomainObjPrivateFree(void *data) VIR_FREE(priv); }

+static void +libxlDomainEventDispatchFunc(virConnectPtr conn, virDomainEventPtr event, + virConnectDomainEventGenericCallback cb, + void *cbopaque, void *opaque) +{ + libxlDriverPrivatePtr driver = opaque; + + /* Drop the lock whle dispatching, for sake of re-entrancy */ + libxlDriverUnlock(driver); + virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); + libxlDriverLock(driver); +} + +static void +libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) +{ + libxlDriverPrivatePtr driver = opaque; + virDomainEventQueue tempQueue; + + libxlDriverLock(driver); + + driver->domainEventDispatching = 1; + + /* Copy the queue, so we're reentrant safe */ + tempQueue.count = driver->domainEventQueue->count; + tempQueue.events = driver->domainEventQueue->events; + driver->domainEventQueue->count = 0; + driver->domainEventQueue->events = NULL; + + virEventUpdateTimeout(driver->domainEventTimer, -1); + virDomainEventQueueDispatch(&tempQueue, + driver->domainEventCallbacks, + libxlDomainEventDispatchFunc, + driver); + + /* Purge any deleted callbacks */ + virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); + + driver->domainEventDispatching = 0; + libxlDriverUnlock(driver); +} + +/* driver must be locked before calling */ +static void +libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) +{ + if (virDomainEventQueuePush(driver->domainEventQueue, event) < 0) + virDomainEventFree(event); + if (driver->domainEventQueue->count == 1) + virEventUpdateTimeout(driver->domainEventTimer, 0); +} + /* * Remove reference to domain object. */ @@ -187,6 +245,7 @@ static void libxlEventHandler(int watch, libxlDriverPrivatePtr driver = libxl_driver; virDomainObjPtr vm = data; libxlDomainObjPrivatePtr priv; + virDomainEventPtr dom_event = NULL; libxl_event event; libxl_dominfo info;

@@ -225,6 +284,9 @@ static void libxlEventHandler(int watch, switch (info.shutdown_reason) { case SHUTDOWN_poweroff: case SHUTDOWN_crash: + dom_event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED);

I don't think we want the CRASHED detail in case of a normal poweroff :-).

libxlVmReap(driver, vm, 0); if (!vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); @@ -244,6 +306,11 @@ static void libxlEventHandler(int watch, cleanup: if (vm) virDomainObjUnlock(vm); + if (dom_event) { + libxlDriverLock(driver); + libxlDomainEventQueue(driver, dom_event); + libxlDriverUnlock(driver); + } libxl_free_event(&event); }

@@ -303,6 +370,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, { libxl_domain_config d_config; virDomainDefPtr def = vm->def; + virDomainEventPtr event = NULL; int ret; uint32_t domid = 0; char *dom_xml = NULL; @@ -347,9 +415,14 @@ libxlVmStart(libxlDriverPrivatePtr driver, vm->state = VIR_DOMAIN_PAUSED; }

+ if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto error;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_BOOTED); + libxlDomainEventQueue(driver, event); + libxl_domain_config_destroy(&d_config); VIR_FREE(dom_xml); return 0; @@ -447,6 +520,13 @@ libxlShutdown(void) VIR_FREE(libxl_driver->libDir); VIR_FREE(libxl_driver->saveDir);

+ /* Free domain callback list */ + virDomainEventCallbackListFree(libxl_driver->domainEventCallbacks); + virDomainEventQueueFree(libxl_driver->domainEventQueue); + + if (libxl_driver->domainEventTimer != -1) + virEventRemoveTimeout(libxl_driver->domainEventTimer); + libxlDriverUnlock(libxl_driver); virMutexDestroy(&libxl_driver->lock); VIR_FREE(libxl_driver); @@ -556,6 +636,16 @@ libxlStartup(int privileged) { } VIR_FREE(log_file);

+ /* Init callback list */ + if (VIR_ALLOC(libxl_driver->domainEventCallbacks) < 0) + goto out_of_memory; + if (!(libxl_driver->domainEventQueue = virDomainEventQueueNew())) + goto out_of_memory; + if ((libxl_driver->domainEventTimer = + virEventAddTimeout(-1, libxlDomainEventFlush, libxl_driver, NULL)) < 0) + goto error; + + libxl_driver->logger = (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); if (!libxl_driver->logger) { @@ -698,6 +788,11 @@ libxlOpen(virConnectPtr conn, static int libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED) { + libxlDriverPrivatePtr driver = conn->privateData; + + libxlDriverLock(driver); + virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + libxlDriverUnlock(driver); conn->privateData = NULL; return 0; } @@ -1026,6 +1121,7 @@ libxlDomainDestroy(virDomainPtr dom) virDomainObjPtr vm; int ret = -1; libxlDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL;

libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1043,6 +1139,9 @@ libxlDomainDestroy(virDomainPtr dom) goto cleanup; }

+ event = virDomainEventNewFromObj(vm,VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_DESTROYED); + priv = vm->privateData; if (libxlVmReap(driver, vm, 1) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, @@ -1060,6 +1159,8 @@ libxlDomainDestroy(virDomainPtr dom) cleanup: if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return ret; } @@ -1193,6 +1294,7 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml) virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + virDomainEventPtr event = NULL; int dupVM;

libxlDriverLock(driver); @@ -1220,10 +1322,17 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml) if (dom) dom->id = vm->def->id;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, + !dupVM ? + VIR_DOMAIN_EVENT_DEFINED_ADDED : + VIR_DOMAIN_EVENT_DEFINED_UPDATED); + cleanup: virDomainDefFree(def); if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return dom; } @@ -1233,6 +1342,7 @@ libxlDomainUndefine(virDomainPtr dom) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; + virDomainEventPtr event = NULL; int ret = -1;

libxlDriverLock(driver); @@ -1264,6 +1374,9 @@ libxlDomainUndefine(virDomainPtr dom) vm) < 0) goto cleanup;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, + VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; ret = 0; @@ -1271,11 +1384,51 @@ libxlDomainUndefine(virDomainPtr dom) cleanup: if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return ret; }

static int +libxlDomainEventRegister(virConnectPtr conn, + virConnectDomainEventCallback callback, void *opaque, + virFreeCallback freecb) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + callback, opaque, freecb); + libxlDriverUnlock(driver); + + return ret; +} + + +static int +libxlDomainEventDeregister(virConnectPtr conn, + virConnectDomainEventCallback callback) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + if (driver->domainEventDispatching) + ret = virDomainEventCallbackListMarkDelete(conn, + driver->domainEventCallbacks, + callback); + else + ret = virDomainEventCallbackListRemove(conn, + driver->domainEventCallbacks, + callback); + libxlDriverUnlock(driver); + + return ret; +} + +static int libxlDomainIsActive(virDomainPtr dom) { libxlDriverPrivatePtr driver = dom->conn->privateData; @@ -1319,6 +1472,44 @@ libxlDomainIsPersistent(virDomainPtr dom) return ret; }

+static int +libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID, + virConnectDomainEventGenericCallback callback, + void *opaque, virFreeCallback freecb) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + dom, eventID, callback, opaque, + freecb); + libxlDriverUnlock(driver); + + return ret; +} + + +static int +libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + if (driver->domainEventDispatching) + ret = virDomainEventCallbackListMarkDeleteID(conn, + driver->domainEventCallbacks, + callbackID); + else + ret = virDomainEventCallbackListRemoveID(conn, + driver->domainEventCallbacks, + callbackID); + libxlDriverUnlock(driver); + + return ret; +} +

This all looks good and quite similar to event callback code in qemu driver, but I'd prefer a quick review by someone (danpb?) familiar with its subtleties. Regards, Jim

static virDriver libxlDriver = { VIR_DRV_LIBXL, @@ -1396,8 +1587,8 @@ static virDriver libxlDriver = { NULL, /* domainGetBlockInfo */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ - NULL, /* domainEventRegister */ - NULL, /* domainEventDeregister */ + libxlDomainEventRegister, /* domainEventRegister */ + libxlDomainEventDeregister, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ NULL, /* domainMigrateFinish2 */ NULL, /* nodeDeviceDettach */ @@ -1414,8 +1605,8 @@ static virDriver libxlDriver = { NULL, /* domainGetJobInfo */ NULL, /* domainAbortJob */ NULL, /* domainMigrateSetMaxDowntime */ - NULL, /* domainEventRegisterAny */ - NULL, /* domainEventDeregisterAny */ + libxlDomainEventRegisterAny,/* domainEventRegisterAny */ + libxlDomainEventDeregisterAny,/* domainEventDeregisterAny */ NULL, /* domainManagedSave */ NULL, /* domainHasManagedSaveImage */ NULL, /* domainManagedSaveRemove */

On Thu, Mar 24, 2011 at 10:24:26AM -0600, Jim Fehlig wrote:

Markus Groß wrote:

...

--- src/libxl/libxl_conf.h | 8 ++- src/libxl/libxl_driver.c | 199 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 202 insertions(+), 5 deletions(-)

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index bb49d35..a86df9a 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -26,6 +26,7 @@

# include "internal.h" # include "domain_conf.h" +# include "domain_event.h" # include "capabilities.h" # include "configmake.h" # include "bitmap.h" @@ -57,6 +58,12 @@ struct _libxlDriverPrivate { virBitmapPtr reservedVNCPorts; virDomainObjList domains;

+ /* A list of callbacks */ + virDomainEventCallbackListPtr domainEventCallbacks; + virDomainEventQueuePtr domainEventQueue; + int domainEventTimer; + int domainEventDispatching; + char *configDir; char *autostartDir; char *logDir; @@ -87,5 +94,4 @@ int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config);

- #endif /* LIBXL_CONF_H */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 615cb47..ad95e2b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1,6 +1,7 @@ /*---------------------------------------------------------------------------*/ /* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. + * Copyright (C) 2006-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -15,6 +16,11 @@ * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Jim Fehlig <jfehlig@novell.com> + * Markus Groß <gross@univention.de> + * Daniel P. Berrange <berrange@redhat.com>

I have no objections to adding authors list, but we should probably add it to the other libxl files to be consistent. But that can be done with a subsequent patch.

...

*/ /*---------------------------------------------------------------------------*/

@@ -99,6 +105,58 @@ libxlDomainObjPrivateFree(void *data) VIR_FREE(priv); }

+static void +libxlDomainEventDispatchFunc(virConnectPtr conn, virDomainEventPtr event, + virConnectDomainEventGenericCallback cb, + void *cbopaque, void *opaque) +{ + libxlDriverPrivatePtr driver = opaque; + + /* Drop the lock whle dispatching, for sake of re-entrancy */ + libxlDriverUnlock(driver); + virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL); + libxlDriverLock(driver); +} + +static void +libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) +{ + libxlDriverPrivatePtr driver = opaque; + virDomainEventQueue tempQueue; + + libxlDriverLock(driver); + + driver->domainEventDispatching = 1; + + /* Copy the queue, so we're reentrant safe */ + tempQueue.count = driver->domainEventQueue->count; + tempQueue.events = driver->domainEventQueue->events; + driver->domainEventQueue->count = 0; + driver->domainEventQueue->events = NULL; + + virEventUpdateTimeout(driver->domainEventTimer, -1); + virDomainEventQueueDispatch(&tempQueue, + driver->domainEventCallbacks, + libxlDomainEventDispatchFunc, + driver); + + /* Purge any deleted callbacks */ + virDomainEventCallbackListPurgeMarked(driver->domainEventCallbacks); + + driver->domainEventDispatching = 0; + libxlDriverUnlock(driver); +} + +/* driver must be locked before calling */ +static void +libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) +{ + if (virDomainEventQueuePush(driver->domainEventQueue, event) < 0) + virDomainEventFree(event); + if (driver->domainEventQueue->count == 1) + virEventUpdateTimeout(driver->domainEventTimer, 0); +} + /* * Remove reference to domain object. */ @@ -187,6 +245,7 @@ static void libxlEventHandler(int watch, libxlDriverPrivatePtr driver = libxl_driver; virDomainObjPtr vm = data; libxlDomainObjPrivatePtr priv; + virDomainEventPtr dom_event = NULL; libxl_event event; libxl_dominfo info;

@@ -225,6 +284,9 @@ static void libxlEventHandler(int watch, switch (info.shutdown_reason) { case SHUTDOWN_poweroff: case SHUTDOWN_crash: + dom_event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED);

I don't think we want the CRASHED detail in case of a normal poweroff :-).

...

libxlVmReap(driver, vm, 0); if (!vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); @@ -244,6 +306,11 @@ static void libxlEventHandler(int watch, cleanup: if (vm) virDomainObjUnlock(vm); + if (dom_event) { + libxlDriverLock(driver); + libxlDomainEventQueue(driver, dom_event); + libxlDriverUnlock(driver); + } libxl_free_event(&event); }

@@ -303,6 +370,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, { libxl_domain_config d_config; virDomainDefPtr def = vm->def; + virDomainEventPtr event = NULL; int ret; uint32_t domid = 0; char *dom_xml = NULL; @@ -347,9 +415,14 @@ libxlVmStart(libxlDriverPrivatePtr driver, vm->state = VIR_DOMAIN_PAUSED; }

+ if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto error;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_BOOTED); + libxlDomainEventQueue(driver, event); + libxl_domain_config_destroy(&d_config); VIR_FREE(dom_xml); return 0; @@ -447,6 +520,13 @@ libxlShutdown(void) VIR_FREE(libxl_driver->libDir); VIR_FREE(libxl_driver->saveDir);

+ /* Free domain callback list */ + virDomainEventCallbackListFree(libxl_driver->domainEventCallbacks); + virDomainEventQueueFree(libxl_driver->domainEventQueue); + + if (libxl_driver->domainEventTimer != -1) + virEventRemoveTimeout(libxl_driver->domainEventTimer); + libxlDriverUnlock(libxl_driver); virMutexDestroy(&libxl_driver->lock); VIR_FREE(libxl_driver); @@ -556,6 +636,16 @@ libxlStartup(int privileged) { } VIR_FREE(log_file);

+ /* Init callback list */ + if (VIR_ALLOC(libxl_driver->domainEventCallbacks) < 0) + goto out_of_memory; + if (!(libxl_driver->domainEventQueue = virDomainEventQueueNew())) + goto out_of_memory; + if ((libxl_driver->domainEventTimer = + virEventAddTimeout(-1, libxlDomainEventFlush, libxl_driver, NULL)) < 0) + goto error; + + libxl_driver->logger = (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); if (!libxl_driver->logger) { @@ -698,6 +788,11 @@ libxlOpen(virConnectPtr conn, static int libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED) { + libxlDriverPrivatePtr driver = conn->privateData; + + libxlDriverLock(driver); + virDomainEventCallbackListRemoveConn(conn, driver->domainEventCallbacks); + libxlDriverUnlock(driver); conn->privateData = NULL; return 0; } @@ -1026,6 +1121,7 @@ libxlDomainDestroy(virDomainPtr dom) virDomainObjPtr vm; int ret = -1; libxlDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL;

libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1043,6 +1139,9 @@ libxlDomainDestroy(virDomainPtr dom) goto cleanup; }

+ event = virDomainEventNewFromObj(vm,VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_DESTROYED); + priv = vm->privateData; if (libxlVmReap(driver, vm, 1) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, @@ -1060,6 +1159,8 @@ libxlDomainDestroy(virDomainPtr dom) cleanup: if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return ret; } @@ -1193,6 +1294,7 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml) virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + virDomainEventPtr event = NULL; int dupVM;

libxlDriverLock(driver); @@ -1220,10 +1322,17 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml) if (dom) dom->id = vm->def->id;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, + !dupVM ? + VIR_DOMAIN_EVENT_DEFINED_ADDED : + VIR_DOMAIN_EVENT_DEFINED_UPDATED); + cleanup: virDomainDefFree(def); if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return dom; } @@ -1233,6 +1342,7 @@ libxlDomainUndefine(virDomainPtr dom) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; + virDomainEventPtr event = NULL; int ret = -1;

libxlDriverLock(driver); @@ -1264,6 +1374,9 @@ libxlDomainUndefine(virDomainPtr dom) vm) < 0) goto cleanup;

+ event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, + VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; ret = 0; @@ -1271,11 +1384,51 @@ libxlDomainUndefine(virDomainPtr dom) cleanup: if (vm) virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); libxlDriverUnlock(driver); return ret; }

static int +libxlDomainEventRegister(virConnectPtr conn, + virConnectDomainEventCallback callback, void *opaque, + virFreeCallback freecb) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + ret = virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, + callback, opaque, freecb); + libxlDriverUnlock(driver); + + return ret; +} + + +static int +libxlDomainEventDeregister(virConnectPtr conn, + virConnectDomainEventCallback callback) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + if (driver->domainEventDispatching) + ret = virDomainEventCallbackListMarkDelete(conn, + driver->domainEventCallbacks, + callback); + else + ret = virDomainEventCallbackListRemove(conn, + driver->domainEventCallbacks, + callback); + libxlDriverUnlock(driver); + + return ret; +} + +static int libxlDomainIsActive(virDomainPtr dom) { libxlDriverPrivatePtr driver = dom->conn->privateData; @@ -1319,6 +1472,44 @@ libxlDomainIsPersistent(virDomainPtr dom) return ret; }

+static int +libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID, + virConnectDomainEventGenericCallback callback, + void *opaque, virFreeCallback freecb) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + ret = virDomainEventCallbackListAddID(conn, driver->domainEventCallbacks, + dom, eventID, callback, opaque, + freecb); + libxlDriverUnlock(driver); + + return ret; +} + + +static int +libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID) +{ + libxlDriverPrivatePtr driver = conn->privateData; + int ret; + + libxlDriverLock(driver); + if (driver->domainEventDispatching) + ret = virDomainEventCallbackListMarkDeleteID(conn, + driver->domainEventCallbacks, + callbackID); + else + ret = virDomainEventCallbackListRemoveID(conn, + driver->domainEventCallbacks, + callbackID); + libxlDriverUnlock(driver); + + return ret; +} +

This all looks good and quite similar to event callback code in qemu driver, but I'd prefer a quick review by someone (danpb?) familiar with its subtleties.

ACK, gets my vote with the changes you already mentioned. You might also want to add the PAUSED/RESUMED events to the Suspend and Resume driver methods. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Jim Fehlig wrote:

Markus Groß wrote:

...

--- src/libxl/libxl_driver.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ad95e2b..7ee3930 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1170,6 +1170,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; + libxl_dominfo d_info; int ret = -1;

libxlDriverLock(driver); @@ -1182,10 +1183,21 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) goto cleanup; }

+ if (!virDomainObjIsActive(vm)) { + info->cpuTime = 0; + info->memory = vm->def->mem.cur_balloon; + } else { + if (libxl_domain_info(&driver->ctx, &d_info, dom->id) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxl_domain_info failed for domain '%d'"), dom->id); + goto cleanup; + } + info->cpuTime = d_info.cpu_time; + info->memory = d_info.current_memkb; + } + info->state = vm->state; - info->cpuTime = 0; info->maxMem = vm->def->mem.max_balloon; - info->memory = vm->def->mem.cur_balloon; info->nrVirtCpu = vm->def->vcpus; ret = 0;

ACK

I've pushed this patch now. Thanks Markus. Regards, Jim

Markus Groß wrote:

--- src/libxl/libxl_driver.c | 295 +++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 290 insertions(+), 5 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7ee3930..49fc90f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -27,6 +27,7 @@ #include <config.h>

#include <sys/utsname.h> +#include <math.h> #include <libxl.h>

#include "internal.h" @@ -1207,6 +1208,290 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) return ret; }

+static int +libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, + unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDomainObjPrivatePtr priv; + virDomainDefPtr def; + virDomainObjPtr vm; + libxl_cpumap map; + uint8_t *bitmask = NULL; + unsigned int maplen; + unsigned int i, pos; + int max; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_VCPU_LIVE | + VIR_DOMAIN_VCPU_CONFIG | + VIR_DOMAIN_VCPU_MAXIMUM, -1); + + /* At least one of LIVE or CONFIG must be set. MAXIMUM cannot be + * mixed with LIVE. */ + if ((flags & (VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG)) == 0 || + (flags & (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_LIVE)) == + (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_LIVE)) { + libxlError(VIR_ERR_INVALID_ARG, + _("invalid flag combination: (0x%x)"), flags); + return -1; + }

I think we should ensure nvcpus != 0

+ + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + libxlDriverUnlock(driver); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_VCPU_LIVE)) { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot set vcpus on an inactive domain")); + goto cleanup; + } + + if (!vm->persistent && (flags & VIR_DOMAIN_VCPU_CONFIG)) { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + + if ((max = libxlGetMaxVcpus(dom->conn, NULL)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not determine max vcpus for the domain")); + goto cleanup; + } + + if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && vm->def->maxvcpus < max) { + max = vm->def->maxvcpus; + } + + if (nvcpus > max) { + libxlError(VIR_ERR_INVALID_ARG, + _("requested vcpus is greater than max allowable" + " vcpus for the domain: %d > %d"), nvcpus, max); + goto cleanup; + } + + priv = vm->privateData; + + if (!(def = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; +

[...]

+ maplen = (unsigned int) ceil((double) nvcpus / 8); + if (VIR_ALLOC_N(bitmask, maplen) < 0) { + virReportOOMError(); + goto cleanup; + } + + memset(bitmask, 0, maplen); + for (i = 0; i < nvcpus; ++i) { + pos = (unsigned int) floor((double) i / 8); + bitmask[pos] |= 1 << (i % 8); + } + + map.size = maplen; + map.map = bitmask;

Hmm, could this be simplified to if (libxl_cpumap_alloc(&priv->ctx, &map)) { virReportOOMError() goto cleanup; } for (i = 0; i < nvcpus; i++) libxl_cpumap_set(&map, i);

+ + switch (flags) { + case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_CONFIG: + def->maxvcpus = nvcpus; + if (nvcpus < def->vcpus) + def->vcpus = nvcpus; + ret = 0; + break; + + case VIR_DOMAIN_VCPU_CONFIG: + def->vcpus = nvcpus; + break; + + case VIR_DOMAIN_VCPU_LIVE: + if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to set vcpus for domain '%d'" + " with libxenlight"), dom->id); + goto cleanup; + } + break; + + case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: + if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to set vcpus for domain '%d'" + " with libxenlight"), dom->id); + goto cleanup; + } + def->vcpus = nvcpus; + break; + } + + ret = 0; + + if (flags & VIR_DOMAIN_VCPU_CONFIG) + ret = virDomainSaveConfig(driver->configDir, def); + +cleanup: + VIR_FREE(bitmask); + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int +libxlDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) +{ + return libxlDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_VCPU_LIVE); +} + +static int +libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr def; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_VCPU_LIVE | + VIR_DOMAIN_VCPU_CONFIG | + VIR_DOMAIN_VCPU_MAXIMUM, -1);

Documentation for this function says that it is an error to set both VIR_DOMAIN_VCPU_LIVE and VIR_DOMAIN_VCPU_CONFIG, so we should check for it.

+ + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + libxlDriverUnlock(driver); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (flags & VIR_DOMAIN_VCPU_LIVE) { + if (!virDomainObjIsActive(vm)) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + def = vm->def; + } else { + def = vm->newDef ? vm->newDef : vm->def; + } + + ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int +libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, + int maplen) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDomainObjPrivatePtr priv; + virDomainObjPtr vm; + int ret = -1; + libxl_cpumap map; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + libxlDriverUnlock(driver); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot pin vcpus on an inactive domain")); + goto cleanup; + } + + priv = vm->privateData; + + map.size = maplen; + map.map = cpumap; + if (libxl_set_vcpuaffinity(&priv->ctx, dom->id, vcpu, &map) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to pin vcpu '%d' with libxenlight"), vcpu); + goto cleanup; + } + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + + +static int +libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, + unsigned char *cpumaps, int maplen) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDomainObjPrivatePtr priv; + virDomainObjPtr vm; + int ret = -1; +

Extra newline.

+ libxl_vcpuinfo *vcpuinfo; + int maxcpu, hostcpus; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + libxlDriverUnlock(driver); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + if ((vcpuinfo = libxl_list_vcpu(&priv->ctx, dom->id, &maxcpu, + &hostcpus)) == NULL) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to list vcpus for domain '%d' with libxenlight"), + dom->id); + goto cleanup; + } + + memset(cpumaps, 0, maplen * maxinfo);

Documentation for this function says cpumaps can be NULL.

+ unsigned int i = 0;

I thought I recalled variable declaration only at start of a block but don't see anything about it in HACKING. Perhaps someone else can clarify. Regards, Jim

+ for (i = 0; i < maxcpu && i < maxinfo; ++i) { + info[i].number = vcpuinfo[i].vcpuid; + info[i].cpu = vcpuinfo[i].cpu; + info[i].cpuTime = vcpuinfo[i].vcpu_time; + if (vcpuinfo[i].running) + info[i].state = VIR_VCPU_RUNNING; + else if (vcpuinfo[i].blocked) + info[i].state = VIR_VCPU_BLOCKED; + else + info[i].state = VIR_VCPU_OFFLINE; + + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, i); + memcpy(cpumap, vcpuinfo[i].cpumap.map, + MIN(maplen, vcpuinfo[i].cpumap.size)); + + libxl_vcpuinfo_destroy(&vcpuinfo[i]); + } + VIR_FREE(vcpuinfo); + + ret = maxinfo; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static char * libxlDomainDumpXML(virDomainPtr dom, int flags) { @@ -1561,11 +1846,11 @@ static virDriver libxlDriver = { NULL, /* domainSave */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ - NULL, /* domainSetVcpus */ - NULL, /* domainSetVcpusFlags */ - NULL, /* domainGetVcpusFlags */ - NULL, /* domainPinVcpu */ - NULL, /* domainGetVcpus */ + libxlDomainSetVcpus, /* domainSetVcpus */ + libxlDomainSetVcpusFlags, /* domainSetVcpusFlags */ + libxlDomainGetVcpusFlags, /* domainGetVcpusFlags */ + libxlDomainPinVcpu, /* domainPinVcpu */ + libxlDomainGetVcpus, /* domainGetVcpus */ NULL, /* domainGetMaxVcpus */ NULL, /* domainGetSecurityLabel */ NULL, /* nodeGetSecurityModel */

Jim Fehlig wrote:

Markus Groß wrote:

...

--- src/libxl/libxl_driver.c | 131 ++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 49fc90f..32bf12e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1166,6 +1166,109 @@ cleanup: return ret; }

ACK.

Finally got around to pushing this patch. Thanks Markus. Regards, Jim

Markus Groß wrote:

--- configure.ac | 2 + daemon/Makefile.am | 3 + src/Makefile.am | 8 ++- src/libxl/libxl_driver.c | 94 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 102 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 12bf0f6..3519011 100644 --- a/configure.ac +++ b/configure.ac @@ -607,6 +607,8 @@ AM_CONDITIONAL([WITH_XEN], [test "$with_xen" = "yes"]) AC_SUBST([XEN_CFLAGS]) AC_SUBST([XEN_LIBS])

+AM_CONDITIONAL([WITH_XENXS], [test "$with_libxl" = "yes" || test "$with_xen" = "yes"]) + dnl dnl check for kernel headers required by xen_inotify dnl diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 9e3a557..4344127 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -108,6 +108,9 @@ endif

if WITH_LIBXL libvirtd_LDADD += ../src/libvirt_driver_libxl.la + libvirtd_LDADD += ../src/libvirt_xenxs.la + libvirtd_LDADD += ../src/libvirt_util.la + libvirtd_LDADD += ../src/libvirt_conf.la endif

I'd like to get another opinion on this hunk. Although I cant find any issues with it, I'd just like to ensure there is not a more appropriate way to add these libs.

if WITH_UML diff --git a/src/Makefile.am b/src/Makefile.am index c3729a6..ae0c6ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -474,7 +474,7 @@ libvirt_vmx_la_CFLAGS = \ libvirt_vmx_la_SOURCES = $(VMX_SOURCES) endif

-if WITH_XEN +if WITH_XENXS noinst_LTLIBRARIES += libvirt_xenxs.la libvirt_la_BUILT_LIBADD += libvirt_xenxs.la libvirt_xenxs_la_CFLAGS = \ @@ -704,8 +704,10 @@ noinst_LTLIBRARIES += libvirt_driver_libxl.la # Stateful, so linked to daemon instead #libvirt_la_BUILT_LIBADD += libvirt_driver_libxl.la endif -libvirt_driver_libxl_la_CFLAGS = $(LIBXL_CFLAGS) \ - -I@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_driver_libxl_la_CFLAGS = $(LIBXL_CFLAGS) \ + -I@top_srcdir@/src/conf \ + -I@top_srcdir@/src/xenxs \ + $(AM_CFLAGS) libvirt_driver_libxl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_libxl_la_LIBADD = $(LIBXL_LIBS) if WITH_DRIVER_MODULES diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 32bf12e..b05cd77 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -33,6 +33,7 @@ #include "internal.h" #include "logging.h" #include "virterror_internal.h" +#include "conf.h" #include "datatypes.h" #include "files.h" #include "memory.h" @@ -41,6 +42,7 @@ #include "command.h" #include "libxl_driver.h" #include "libxl_conf.h" +#include "xen_xm.h"

#define VIR_FROM_THIS VIR_FROM_LIBXL @@ -51,6 +53,8 @@ #define LIBXL_DOM_REQ_CRASH 3 #define LIBXL_DOM_REQ_HALT 4

+#define LIBXL_CONFIG_FORMAT_XM "xen-xm" + static libxlDriverPrivatePtr libxl_driver = NULL;

@@ -1620,6 +1624,92 @@ libxlDomainDumpXML(virDomainPtr dom, int flags) return ret; }

+static char * +libxlDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat, + const char * nativeConfig, + unsigned int flags ATTRIBUTE_UNUSED) +{ + libxlDriverPrivatePtr driver = conn->privateData; + const libxl_version_info *ver_info; + virDomainDefPtr def = NULL; + virConfPtr conf = NULL; + char *xml = NULL; + + if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + libxlError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); + goto cleanup; + } + + if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + VIR_ERROR0(_("cannot get version information from libxenlight")); + goto cleanup; + } + + if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) + goto cleanup; + + if (!(def = xenParseXM(conf, ver_info->xen_version_major, driver->caps))) { + libxlError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing xm config failed")); + goto cleanup; + } + + xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); + +cleanup: + virDomainDefFree(def); + if (conf) + virConfFree(conf);

At first glance this seems like a useless if-before-free, but I see that virConfFree emits an error if conf is NULL. ACK, assuming someone else agrees with the daemon/Makefile.am hunk. Regards, Jim

+ return xml; +} + +#define MAX_CONFIG_SIZE (1024 * 65) +static char * +libxlDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, + const char * domainXml, + unsigned int flags ATTRIBUTE_UNUSED) +{ + libxlDriverPrivatePtr driver = conn->privateData; + const libxl_version_info *ver_info; + virDomainDefPtr def = NULL; + virConfPtr conf = NULL; + int len = MAX_CONFIG_SIZE; + char *ret = NULL; + + if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + libxlError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); + goto cleanup; + } + + if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + VIR_ERROR0(_("cannot get version information from libxenlight")); + goto cleanup; + } + + if (!(def = virDomainDefParseString(driver->caps, domainXml, 0))) + goto cleanup; + + if (!(conf = xenFormatXM(conn, def, ver_info->xen_version_major))) + goto cleanup; + + if (VIR_ALLOC_N(ret, len) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virConfWriteMem(ret, &len, conf) < 0) { + VIR_FREE(ret); + goto cleanup; + } + +cleanup: + virDomainDefFree(def); + if (conf) + virConfFree(conf); + return ret; +} + static int libxlListDefinedDomains(virConnectPtr conn, char **const names, int nnames) @@ -1978,8 +2068,8 @@ static virDriver libxlDriver = { NULL, /* domainGetSecurityLabel */ NULL, /* nodeGetSecurityModel */ libxlDomainDumpXML, /* domainDumpXML */ - NULL, /* domainXmlFromNative */ - NULL, /* domainXmlToNative */ + libxlDomainXMLFromNative, /* domainXmlFromNative */ + libxlDomainXMLToNative, /* domainXmlToNative */ libxlListDefinedDomains, /* listDefinedDomains */ libxlNumDefinedDomains, /* numOfDefinedDomains */ libxlDomainCreate, /* domainCreate */

Markus Groß wrote:

--- src/libxl/libxl_driver.c | 64 +++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 63 insertions(+), 1 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b05cd77..c72a570 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -55,6 +55,11 @@

#define LIBXL_CONFIG_FORMAT_XM "xen-xm"

+/* Number of Xen scheduler parameters */ +#define XEN_SCHED_SEDF_NPARAM 5

I assume the similar definition in xend_internal.c is wrong since it is counting the domid as well. Is that correct? If so, we should fix it in xend_internal.c.

+#define XEN_SCHED_CREDIT_NPARAM 2 +#define XEN_SCHED_CREDIT2_NPARAM 1 + static libxlDriverPrivatePtr libxl_driver = NULL;

@@ -1918,6 +1923,63 @@ libxlDomainEventDeregister(virConnectPtr conn, return ret; }

+static char * +libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + libxlDomainObjPrivatePtr priv; + virDomainObjPtr vm; + char * ret = NULL; + int sched_id; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + libxlDriverUnlock(driver); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + priv = vm->privateData; + if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to get scheduler id for domain '%d'" + " with libxenlight"), dom->id); + goto cleanup; + } + + switch(sched_id) { + case XEN_SCHEDULER_SEDF: + ret = strdup("sedf"); + *nparams = XEN_SCHED_SEDF_NPARAM; + break; + case XEN_SCHEDULER_CREDIT: + ret = strdup("credit"); + *nparams = XEN_SCHED_CREDIT_NPARAM; + break; + case XEN_SCHEDULER_CREDIT2: + ret = strdup("credit2"); + *nparams = XEN_SCHED_CREDIT2_NPARAM; + break; + case XEN_SCHEDULER_ARINC653: + ret = strdup("arinc653"); + *nparams = 0; + break;

Hmm, seems these last two should be added to the xen-unified driver as well - but that obviously doesn't affect this patch. ACK, depending on the answer to my question above. Regards, Jim

+ default: + *nparams = 0; + goto cleanup; + } + + if (!ret) + virReportOOMError(); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static unsigned long long libxlNodeGetFreeMemory(virConnectPtr conn) { @@ -2083,7 +2145,7 @@ static virDriver libxlDriver = { NULL, /* domainUpdateDeviceFlags */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ - NULL, /* domainGetSchedulerType */ + libxlDomainGetSchedulerType,/* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ NULL, /* domainMigratePrepare */

Am Donnerstag 24 März 2011 16:55:23 schrieb Jim Fehlig:

Markus Groß wrote:

...

--- src/libxl/libxl_driver.c | 114 +++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4b31197..669c170 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1041,6 +1041,116 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) }

static int +libxlDomainSuspend(virDomainPtr dom) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + libxlDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL; + int ret = -1; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid);

Is there any reason the driver can't be unlocked here?

Thanks for pointing that out. I think it will be sufficient to limit the locking to the libxlDomainEventQueue call.

...

+ } + + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + goto cleanup; + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); + libxlDriverUnlock(driver); + return ret; +} + + +static int +libxlDomainResume(virDomainPtr dom) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + libxlDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL; + int ret = -1; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid);

Same question here about unlocking the driver.

See my answer above. Thanks, Markus