Once a domain has reached the shutdown state, set its ID to -1. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8e4242a..0cd0ec8 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -266,15 +266,15 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, char *file; size_t i; + vm->def->id = -1; + if (priv->deathW) { libxl_evdisable_domain_death(priv->ctx, priv->deathW); priv->deathW = NULL; } - if (vm->persistent) { - vm->def->id = -1; + if (vm->persistent) virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); - } if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); -- 1.8.1.4

This function, which only has five call sites, simply calls libxl_domain_destroy and libxlVmCleanup. Call those functions directly at the call sites, allowing more control over how a domain is destroyed and cleaned up. This patch maintains the existing semantic, leaving changes to a subsequent patch. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0cd0ec8..342ad3b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -317,28 +317,6 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, } /* - * Reap a domain from libxenlight. - * - * virDomainObjPtr should be locked on invocation - */ -static int -libxlVmReap(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, - virDomainShutoffReason reason) -{ - libxlDomainObjPrivatePtr priv = vm->privateData; - - if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to cleanup domain %d"), vm->def->id); - return -1; - } - - libxlVmCleanup(driver, vm, reason); - return 0; -} - -/* * Handle previously registered event notification from libxenlight. * * Note: Xen 4.3 removed the const from the event handler signature. @@ -385,14 +363,16 @@ libxlDomainShutdownThread(void *opaque) } else { reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; } - libxlVmReap(driver, vm, reason); + libxl_domain_destroy(ctx, vm->def->id, NULL); + libxlVmCleanup(driver, vm, reason); if (!vm->persistent) { virDomainObjListRemove(driver->domains, vm); vm = NULL; } break; case LIBXL_SHUTDOWN_REASON_REBOOT: - libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); + libxl_domain_destroy(ctx, vm->def->id, NULL); + libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); libxlVmStart(driver, vm, 0, -1); break; default: @@ -1533,6 +1513,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; virObjectEventPtr event = NULL; + libxlDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -1551,12 +1532,14 @@ libxlDomainDestroyFlags(virDomainPtr dom, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED) != 0) { + priv = vm->privateData; + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), dom->id); goto cleanup; } + libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); if (!vm->persistent) { virDomainObjListRemove(driver->domains, vm); vm = NULL; @@ -1872,12 +1855,13 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); - if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto cleanup; } + libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED); vm->hasManagedSave = true; ret = 0; @@ -2045,12 +2029,13 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) } if (flags & VIR_DUMP_CRASH) { - if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) { + if (libxl_domain_destroy(priv->ctx, dom->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), dom->id); goto cleanup_unpause; } + libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); if (!vm->persistent) { -- 1.8.1.4

Follows the pattern used in the QEMU driver for managing multiple, simultaneous jobs within the driver. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Add ATTRIBUTE_RETURN_CHECK to libxlDomainObjEndJob src/libxl/libxl_domain.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_domain.h | 38 ++++++++++++++ 2 files changed, 166 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index eb2e50e..fdc4661 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -30,10 +30,18 @@ #include "virerror.h" #include "virlog.h" #include "virstring.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_LIBXL +VIR_ENUM_IMPL(libxlDomainJob, LIBXL_JOB_LAST, + "none", + "query", + "destroy", + "modify", +); + /* Object used to store info related to libxl event registrations */ typedef struct _libxlEventHookInfo libxlEventHookInfo; typedef libxlEventHookInfo *libxlEventHookInfoPtr; @@ -284,6 +292,119 @@ static const libxl_osevent_hooks libxl_event_callbacks = { .timeout_deregister = libxlDomainObjTimeoutDeregisterEventHook, }; +static int +libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv) +{ + memset(&priv->job, 0, sizeof(priv->job)); + + if (virCondInit(&priv->job.cond) < 0) + return -1; + + return 0; +} + +static void +libxlDomainObjResetJob(libxlDomainObjPrivatePtr priv) +{ + struct libxlDomainJobObj *job = &priv->job; + + job->active = LIBXL_JOB_NONE; + job->owner = 0; +} + +static void +libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv) +{ + ignore_value(virCondDestroy(&priv->job.cond)); +} + +/* Give up waiting for mutex after 30 seconds */ +#define LIBXL_JOB_WAIT_TIME (1000ull * 30) + +/* + * obj must be locked before calling, libxlDriverPrivatePtr must NOT be locked + * + * This must be called by anything that will change the VM state + * in any way + * + * Upon successful return, the object will have its ref count increased, + * successful calls must be followed by EndJob eventually + */ +int +libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr obj, + enum libxlDomainJob job) +{ + libxlDomainObjPrivatePtr priv = obj->privateData; + unsigned long long now; + unsigned long long then; + + if (virTimeMillisNow(&now) < 0) + return -1; + then = now + LIBXL_JOB_WAIT_TIME; + + virObjectRef(obj); + + while (priv->job.active) { + VIR_DEBUG("Wait normal job condition for starting job: %s", + libxlDomainJobTypeToString(job)); + if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) < 0) + goto error; + } + + libxlDomainObjResetJob(priv); + + VIR_DEBUG("Starting job: %s", libxlDomainJobTypeToString(job)); + priv->job.active = job; + priv->job.owner = virThreadSelfID(); + + return 0; + +error: + VIR_WARN("Cannot start job (%s) for domain %s;" + " current job is (%s) owned by (%d)", + libxlDomainJobTypeToString(job), + obj->def->name, + libxlDomainJobTypeToString(priv->job.active), + priv->job.owner); + + if (errno == ETIMEDOUT) + virReportError(VIR_ERR_OPERATION_TIMEOUT, + "%s", _("cannot acquire state change lock")); + else + virReportSystemError(errno, + "%s", _("cannot acquire job mutex")); + + virObjectUnref(obj); + return -1; +} + +/* + * obj must be locked before calling + * + * To be called after completing the work associated with the + * earlier libxlDomainBeginJob() call + * + * Returns true if the remaining reference count on obj is + * non-zero, false if the reference count has dropped to zero + * and obj is disposed. + */ +bool +libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr obj) +{ + libxlDomainObjPrivatePtr priv = obj->privateData; + enum libxlDomainJob job = priv->job.active; + + VIR_DEBUG("Stopping job: %s", + libxlDomainJobTypeToString(job)); + + libxlDomainObjResetJob(priv); + virCondSignal(&priv->job.cond); + + return virObjectUnref(obj); +} + static void * libxlDomainObjPrivateAlloc(void) { @@ -300,6 +421,12 @@ libxlDomainObjPrivateAlloc(void) return NULL; } + if (libxlDomainObjInitJob(priv) < 0) { + virChrdevFree(priv->devs); + virObjectUnref(priv); + return NULL; + } + return priv; } @@ -311,6 +438,7 @@ libxlDomainObjPrivateDispose(void *obj) if (priv->deathW) libxl_evdisable_domain_death(priv->ctx, priv->deathW); + libxlDomainObjFreeJob(priv); virChrdevFree(priv->devs); libxl_ctx_free(priv->ctx); if (priv->logger_file) diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 8565820..0a29d38 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -30,6 +30,31 @@ # include "libxl_conf.h" # include "virchrdev.h" +# define JOB_MASK(job) (1 << (job - 1)) +# define DEFAULT_JOB_MASK \ + (JOB_MASK(LIBXL_JOB_DESTROY) | \ + JOB_MASK(LIBXL_JOB_ABORT)) + +/* Only 1 job is allowed at any time + * A job includes *all* libxl.so api, even those just querying + * information, not merely actions */ +enum libxlDomainJob { + LIBXL_JOB_NONE = 0, /* Always set to 0 for easy if (jobActive) conditions */ + LIBXL_JOB_QUERY, /* Doesn't change any state */ + LIBXL_JOB_DESTROY, /* Destroys the domain (cannot be masked out) */ + LIBXL_JOB_MODIFY, /* May change state */ + + LIBXL_JOB_LAST +}; +VIR_ENUM_DECL(libxlDomainJob) + + +struct libxlDomainJobObj { + virCond cond; /* Use to coordinate jobs */ + enum libxlDomainJob active; /* Currently running job */ + int owner; /* Thread which set current job */ +}; + typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate; typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr; struct _libxlDomainObjPrivate { @@ -43,6 +68,8 @@ struct _libxlDomainObjPrivate { /* console */ virChrdevsPtr devs; libxl_evgen_domain_death *deathW; + + struct libxlDomainJobObj job; }; @@ -53,4 +80,15 @@ extern virDomainDefParserConfig libxlDomainDefParserConfig; int libxlDomainObjPrivateInitCtx(virDomainObjPtr vm); +int +libxlDomainObjBeginJob(libxlDriverPrivatePtr driver, + virDomainObjPtr obj, + enum libxlDomainJob job) + ATTRIBUTE_RETURN_CHECK; + +bool +libxlDomainObjEndJob(libxlDriverPrivatePtr driver, + virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; + #endif /* LIBXL_DOMAIN_H */ -- 1.8.1.4

Creating a large domain could potentially be time consuming. Use the recently added job functions and unlock the virDomainObj while the create operation is in progress. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Defer getting libxlDriverConfig until aquiring a job, simplifying cleanup handling a bit. Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 66 +++++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 342ad3b..ecd1b8d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -573,26 +573,30 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virDomainDefPtr def = NULL; virObjectEventPtr event = NULL; libxlSavefileHeader hdr; - int ret; + int ret = -1; uint32_t domid = 0; char *dom_xml = NULL; char *managed_save_path = NULL; int managed_save_fd = -1; libxlDomainObjPrivatePtr priv = vm->privateData; - libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + libxlDriverConfigPtr cfg; #ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS libxl_domain_restore_params params; #endif if (libxlDomainObjPrivateInitCtx(vm) < 0) - goto error; + return ret; + + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + return ret; + cfg = libxlDriverConfigGet(driver); /* If there is a managed saved state restore it instead of starting * from scratch. The old state is removed once the restoring succeeded. */ if (restore_fd < 0) { managed_save_path = libxlDomainManagedSavePath(driver, vm); if (managed_save_path == NULL) - goto error; + goto endjob; if (virFileExists(managed_save_path)) { @@ -600,7 +604,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, managed_save_path, &def, &hdr); if (managed_save_fd < 0) - goto error; + goto endjob; restore_fd = managed_save_fd; @@ -614,7 +618,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, _("cannot restore domain '%s' uuid %s from a file" " which belongs to domain '%s' uuid %s"), vm->def->name, vm_uuidstr, def->name, def_uuidstr); - goto error; + goto endjob; } virDomainObjAssignDef(vm, def, true, NULL); @@ -632,17 +636,17 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxl_domain_config_init(&d_config); if (libxlBuildDomainConfig(driver, vm, &d_config) < 0) - goto error; + goto endjob; if (cfg->autoballoon && libxlFreeMem(priv, &d_config) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to get free memory for domain '%s'"), d_config.c_info.name); - goto error; + goto endjob; } - /* use as synchronous operations => ao_how = NULL and no intermediate reports => ao_progress = NULL */ - + /* Unlock virDomainObj while creating the domain */ + virObjectUnlock(vm); if (restore_fd < 0) { ret = libxl_domain_create_new(priv->ctx, &d_config, &domid, NULL, NULL); @@ -656,6 +660,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, restore_fd, NULL, NULL); #endif } + virObjectLock(vm); if (ret) { if (restore_fd < 0) @@ -666,25 +671,29 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to restore domain '%s'"), d_config.c_info.name); - goto error; + goto endjob; } + /* + * The domain has been successfully created with libxl, so it should + * be cleaned up if there are any subsequent failures. + */ vm->def->id = domid; if (libxlDomEventsRegister(vm) < 0) - goto error; + goto cleanup_dom; if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) - goto error; + goto cleanup_dom; if (libxl_userdata_store(priv->ctx, domid, "libvirt-xml", (uint8_t *)dom_xml, strlen(dom_xml) + 1)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxenlight failed to store userdata")); - goto error; + goto cleanup_dom; } if (libxlDomainSetVcpuAffinities(driver, vm) < 0) - goto error; + goto cleanup_dom; if (!start_paused) { libxl_domain_unpause(priv->ctx, domid); @@ -693,9 +702,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); } - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto error; + goto cleanup_dom; if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); @@ -707,25 +715,25 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, if (event) libxlDomainEventQueue(driver, event); - libxl_domain_config_dispose(&d_config); - VIR_FREE(dom_xml); - VIR_FORCE_CLOSE(managed_save_fd); - virObjectUnref(cfg); - return 0; + ret = 0; + goto endjob; + +cleanup_dom: + libxl_domain_destroy(priv->ctx, domid, NULL); + vm->def->id = -1; + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); + +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; -error: - if (domid > 0) { - libxl_domain_destroy(priv->ctx, domid, NULL); - vm->def->id = -1; - virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); - } libxl_domain_config_dispose(&d_config); VIR_FREE(dom_xml); VIR_FREE(managed_save_path); virDomainDefFree(def); VIR_FORCE_CLOSE(managed_save_fd); virObjectUnref(cfg); - return -1; + return ret; } -- 1.8.1.4

Large balloon operation can be time consuming. Use the recently added job functions and unlock the virDomainObj while ballooning. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ecd1b8d..87771f7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1632,6 +1632,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (virDomainSetMemoryFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + isActive = virDomainObjIsActive(vm); if (flags == VIR_DOMAIN_MEM_CURRENT) { @@ -1650,19 +1653,19 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot set memory on an inactive domain")); - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_MEM_CONFIG) { if (!vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot change persistent config of a transient domain")); - goto cleanup; + goto endjob; } if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm))) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_MEM_MAXIMUM) { @@ -1674,7 +1677,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set maximum memory for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } } @@ -1685,7 +1688,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; ret = virDomainSaveConfig(cfg->configDir, persistentDef); - goto cleanup; + goto endjob; } } else { @@ -1694,17 +1697,23 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (newmem > vm->def->mem.max_balloon) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_MEM_LIVE) { + int res; + priv = vm->privateData; - if (libxl_set_memory_target(priv->ctx, dom->id, newmem, 0, - /* force */ 1) < 0) { + /* Unlock virDomainObj while ballooning memory */ + virObjectUnlock(vm); + res = libxl_set_memory_target(priv->ctx, dom->id, newmem, 0, + /* force */ 1); + virObjectLock(vm); + if (res < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set memory for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } } @@ -1712,12 +1721,16 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, sa_assert(persistentDef); persistentDef->mem.cur_balloon = newmem; ret = virDomainSaveConfig(cfg->configDir, persistentDef); - goto cleanup; + goto endjob; } } ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.1.4

These operations aren't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 87771f7..36fc9f5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1342,9 +1342,12 @@ libxlDomainSuspend(virDomainPtr dom) if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -1354,7 +1357,7 @@ libxlDomainSuspend(virDomainPtr dom) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to suspend domain '%d' with libxenlight"), dom->id); - goto cleanup; + goto endjob; } virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); @@ -1364,10 +1367,14 @@ libxlDomainSuspend(virDomainPtr dom) } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto cleanup; + goto endjob; ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -1394,9 +1401,12 @@ libxlDomainResume(virDomainPtr dom) if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -1406,7 +1416,7 @@ libxlDomainResume(virDomainPtr dom) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to resume domain '%d' with libxenlight"), dom->id); - goto cleanup; + goto endjob; } virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, @@ -1417,10 +1427,14 @@ libxlDomainResume(virDomainPtr dom) } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto cleanup; + goto endjob; ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.1.4

When explicitly destroying a domain (libxlDomainDestroyFlags), or handling an out-of-band domain shutdown event, cleanup the domain in the context of a job. Introduce libxlVmCleanupJob to wrap libxlVmCleanup in a job block. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Introduce libxlVmCleanupJob and call it when needing libxlVmCleanup in a job block. Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 36fc9f5..0c2c1d7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -317,6 +317,26 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, } /* + * Cleanup function for domain that has reached shutoff state. + * Executed in the context of a job. + * + * virDomainObjPtr should be locked on invocation + * Returns true if references remain on virDomainObjPtr, false otherwise. + */ +static bool +libxlVmCleanupJob(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + virDomainShutoffReason reason) +{ + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY) < 0) + return true; + + libxlVmCleanup(driver, vm, reason); + + return libxlDomainObjEndJob(driver, vm); +} + +/* * Handle previously registered event notification from libxenlight. * * Note: Xen 4.3 removed the const from the event handler signature. @@ -364,10 +384,11 @@ libxlDomainShutdownThread(void *opaque) reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; } libxl_domain_destroy(ctx, vm->def->id, NULL); - libxlVmCleanup(driver, vm, reason); - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; + if (libxlVmCleanupJob(driver, vm, reason)) { + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } } break; case LIBXL_SHUTDOWN_REASON_REBOOT: @@ -1561,10 +1582,11 @@ libxlDomainDestroyFlags(virDomainPtr dom, goto cleanup; } - libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; + if (libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } } ret = 0; -- 1.8.1.4

Saving domain memory and cpu state can take considerable time. Use the recently added job functions and unlock the virDomainObj while saving the domain. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 56 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0c2c1d7..839754d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1902,10 +1902,16 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup; } - if (libxl_domain_suspend(priv->ctx, vm->def->id, fd, 0, NULL) != 0) { + /* Unlock virDomainObj while saving domain */ + virObjectUnlock(vm); + ret = libxl_domain_suspend(priv->ctx, vm->def->id, fd, 0, NULL); + virObjectLock(vm); + + if (ret != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to save domain '%d' with libxenlight"), vm->def->id); + ret = -1; goto cleanup; } @@ -1938,6 +1944,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; + bool remove_dom = false; virCheckFlags(0, -1); if (dxml) { @@ -1952,22 +1959,31 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (libxlDoDomainSave(driver, vm, to) < 0) - goto cleanup; + goto endjob; - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } + if (!vm->persistent) + remove_dom = true; ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: + if (remove_dom && vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } if (vm) virObjectUnlock(vm); return ret; @@ -2129,6 +2145,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm = NULL; char *name = NULL; int ret = -1; + bool remove_dom = false; virCheckFlags(0, -1); @@ -2138,33 +2155,42 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (!vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot do managed save for transient domain")); - goto cleanup; + goto endjob; } name = libxlDomainManagedSavePath(driver, vm); if (name == NULL) - goto cleanup; + goto endjob; VIR_INFO("Saving state to %s", name); if (libxlDoDomainSave(driver, vm, name) < 0) - goto cleanup; + goto endjob; - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } + if (!vm->persistent) + remove_dom = true; ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: + if (remove_dom && vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } if (vm) virObjectUnlock(vm); VIR_FREE(name); -- 1.8.1.4

Dumping a domain's core can take considerable time. Use the recently added job functions and unlock the virDomainObj while dumping core. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 839754d..9c42e28 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2063,6 +2063,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; virObjectEventPtr event = NULL; + bool remove_dom = false; bool paused = false; int ret = -1; @@ -2074,9 +2075,12 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -2088,39 +2092,42 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) _("Before dumping core, failed to suspend domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_DUMP); paused = true; } - if (libxl_domain_core_dump(priv->ctx, dom->id, to, NULL) != 0) { + /* Unlock virDomainObj while dumping core */ + virObjectUnlock(vm); + ret = libxl_domain_core_dump(priv->ctx, dom->id, to, NULL); + virObjectLock(vm); + if (ret != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to dump core of domain '%d' with libxenlight"), dom->id); - goto cleanup_unpause; + ret = -1; + goto unpause; } if (flags & VIR_DUMP_CRASH) { if (libxl_domain_destroy(priv->ctx, dom->id, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), dom->id); - goto cleanup_unpause; + goto unpause; } libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } + if (!vm->persistent) + remove_dom = true; } ret = 0; -cleanup_unpause: - if (vm && virDomainObjIsActive(vm) && paused) { +unpause: + if (virDomainObjIsActive(vm) && paused) { if (libxl_domain_unpause(priv->ctx, dom->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("After dumping core, failed to resume domain '%d' with" @@ -2130,7 +2137,16 @@ cleanup_unpause: VIR_DOMAIN_RUNNING_UNPAUSED); } } + +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: + if (remove_dom && vm) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } if (vm) virObjectUnlock(vm); if (event) -- 1.8.1.4

These operations aren't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 48 +++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9c42e28..1f3ea51 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2329,22 +2329,25 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_VCPU_LIVE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot set vcpus on an inactive domain")); - goto cleanup; + goto endjob; } if (!vm->persistent && (flags & VIR_DOMAIN_VCPU_CONFIG)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot change persistent config of a transient domain")); - goto cleanup; + goto endjob; } if ((max = libxlConnectGetMaxVcpus(dom->conn, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine max vcpus for the domain")); - goto cleanup; + goto endjob; } if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && vm->def->maxvcpus < max) { @@ -2355,17 +2358,17 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" " vcpus for the domain: %d > %d"), nvcpus, max); - goto cleanup; + goto endjob; } priv = vm->privateData; if (!(def = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm))) - goto cleanup; + goto endjob; maplen = VIR_CPU_MAPLEN(nvcpus); if (VIR_ALLOC_N(bitmask, maplen) < 0) - goto cleanup; + goto endjob; for (i = 0; i < nvcpus; ++i) { pos = i / 8; @@ -2391,7 +2394,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } break; @@ -2400,7 +2403,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } def->vcpus = nvcpus; break; @@ -2411,6 +2414,10 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_VCPU_CONFIG) ret = virDomainSaveConfig(cfg->configDir, def); +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: VIR_FREE(bitmask); if (vm) @@ -2502,15 +2509,18 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is inactive")); - goto cleanup; + goto endjob; } if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm, &flags, &targetDef) < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_LIVE) { targetDef = vm->def; @@ -2521,7 +2531,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, pcpumap = virBitmapNewData(cpumap, maplen); if (!pcpumap) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_LIVE) { libxl_bitmap map = { .size = maplen, .map = cpumap }; @@ -2532,7 +2542,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to pin vcpu '%d' with libxenlight"), vcpu); - goto cleanup; + goto endjob; } } @@ -2542,14 +2552,14 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to delete vcpupin xml for vcpu '%d'"), vcpu); - goto cleanup; + goto endjob; } - goto out; + goto done; } if (!targetDef->cputune.vcpupin) { if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0) - goto cleanup; + goto endjob; targetDef->cputune.nvcpupin = 0; } if (virDomainVcpuPinAdd(&targetDef->cputune.vcpupin, @@ -2559,10 +2569,10 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml")); - goto cleanup; + goto endjob; } -out: +done: ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -2571,6 +2581,10 @@ out: ret = virDomainSaveConfig(cfg->configDir, targetDef); } +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.1.4

These operations aren't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1f3ea51..f19d551 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3423,6 +3423,9 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (virDomainObjIsActive(vm)) { if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT) flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE; @@ -3433,14 +3436,14 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } } if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot modify device on transient domain")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -3449,15 +3452,15 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, VIR_DOMAIN_XML_INACTIVE))) - goto cleanup; + goto endjob; /* Make a copy for updated domain. */ if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps, driver->xmlopt))) - goto cleanup; + goto endjob; if ((ret = libxlDomainAttachDeviceConfig(vmdef, dev)) < 0) - goto cleanup; + goto endjob; } else { ret = 0; } @@ -3468,10 +3471,10 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, VIR_DOMAIN_XML_INACTIVE))) - goto cleanup; + goto endjob; if ((ret = libxlDomainAttachDeviceLive(priv, vm, dev)) < 0) - goto cleanup; + goto endjob; /* * update domain status forcibly because the domain status may be @@ -3490,6 +3493,10 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, } } +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); @@ -3527,6 +3534,9 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (virDomainObjIsActive(vm)) { if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT) flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE; @@ -3537,14 +3547,14 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } } if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot modify device on transient domain")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -3553,15 +3563,15 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, VIR_DOMAIN_XML_INACTIVE))) - goto cleanup; + goto endjob; /* Make a copy for updated domain. */ if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps, driver->xmlopt))) - goto cleanup; + goto endjob; if ((ret = libxlDomainDetachDeviceConfig(vmdef, dev)) < 0) - goto cleanup; + goto endjob; } else { ret = 0; } @@ -3572,10 +3582,10 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, VIR_DOMAIN_XML_INACTIVE))) - goto cleanup; + goto endjob; if ((ret = libxlDomainDetachDeviceLive(priv, vm, dev)) < 0) - goto cleanup; + goto endjob; /* * update domain status forcibly because the domain status may be @@ -3594,6 +3604,10 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, } } +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); -- 1.8.1.4

Setting autostart is a modify operation that needs to wait in the queue of modify jobs. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index f19d551..9741b3a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3866,40 +3866,43 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart) if (virDomainSetAutostartEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot set autostart for transient domain")); - goto cleanup; + goto endjob; } autostart = (autostart != 0); if (vm->autostart != autostart) { if (!(configFile = virDomainConfigFile(cfg->configDir, vm->def->name))) - goto cleanup; + goto endjob; if (!(autostartLink = virDomainConfigFile(cfg->autostartDir, vm->def->name))) - goto cleanup; + goto endjob; if (autostart) { if (virFileMakePath(cfg->autostartDir) < 0) { virReportSystemError(errno, _("cannot create autostart directory %s"), cfg->autostartDir); - goto cleanup; + goto endjob; } if (symlink(configFile, autostartLink) < 0) { virReportSystemError(errno, _("Failed to create symlink '%s to '%s'"), autostartLink, configFile); - goto cleanup; + goto endjob; } } else { if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { virReportSystemError(errno, _("Failed to delete symlink '%s'"), autostartLink); - goto cleanup; + goto endjob; } } @@ -3907,6 +3910,10 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart) } ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); -- 1.8.1.4

Modify operation that needs to wait in the queue of modify jobs. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Check libxlDomainObjEndJob() return value src/libxl/libxl_driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9741b3a..f7ca91c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4057,6 +4057,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, int nparams, unsigned int flags) { + libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; libxl_domain_sched_params sc_info; @@ -4079,9 +4080,12 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virDomainSetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -4091,14 +4095,14 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, if (sched_id != LIBXL_SCHEDULER_CREDIT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Only 'credit' scheduler is supported")); - goto cleanup; + goto endjob; } if (libxl_domain_sched_params_get(priv->ctx, dom->id, &sc_info) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get scheduler parameters for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } for (i = 0; i < nparams; ++i) { @@ -4114,11 +4118,15 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set scheduler parameters for domain '%d'" " with libxenlight"), dom->id); - goto cleanup; + goto endjob; } ret = 0; +endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.1.4