[libvirt] [PATCH V3 0/3] libxl: domain destroy fixes

V3 of a small series to fix issues wrt domain destroy V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper. Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain src/libxl/libxl_domain.c | 106 ++++++++++++++++++++++---------------------- src/libxl/libxl_domain.h | 8 ++-- src/libxl/libxl_driver.c | 81 ++++++++++++++++++++++++--------- src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-) -- 1.8.4.5

Let callers of libxlDomainStart decide when it is appropriate to acquire a job on the associated virDomainObj. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- Job handling in the migration code is currently broken/incomplete, so fixing it is deferred to a follow-up series I'm working on. src/libxl/libxl_domain.c | 24 ++++++++------------- src/libxl/libxl_driver.c | 55 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 37b2b58..391291d 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -903,16 +903,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxl_domain_config_init(&d_config); - 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 endjob; + goto cleanup; if (virFileExists(managed_save_path)) { @@ -920,7 +917,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, managed_save_path, &def, &hdr); if (managed_save_fd < 0) - goto endjob; + goto cleanup; restore_fd = managed_save_fd; @@ -934,7 +931,7 @@ libxlDomainStart(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 endjob; + goto cleanup; } virDomainObjAssignDef(vm, def, true, NULL); @@ -951,14 +948,14 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, cfg->ctx, &d_config) < 0) - goto endjob; + goto cleanup; if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) - goto endjob; + goto cleanup; if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI) < 0) - goto endjob; + goto cleanup; /* Unlock virDomainObj while creating the domain */ virObjectUnlock(vm); @@ -990,7 +987,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to restore domain '%s'"), d_config.c_info.name); - goto endjob; + goto cleanup; } /* @@ -1037,7 +1034,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxlDomainEventQueue(driver, event); ret = 0; - goto endjob; + goto cleanup; cleanup_dom: if (priv->deathW) { @@ -1048,10 +1045,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); - endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; - + cleanup: libxl_domain_config_dispose(&d_config); VIR_FREE(dom_xml); VIR_FREE(managed_save_path); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 05f6eb1..e315b32 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -303,18 +303,26 @@ libxlAutostartDomain(virDomainObjPtr vm, virObjectLock(vm); virResetLastError(); + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { + virObjectUnlock(vm); + return ret; + } + if (vm->autostart && !virDomainObjIsActive(vm) && libxlDomainStart(driver, vm, false, -1) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, err ? err->message : _("unknown error")); - goto cleanup; + goto endjob; } ret = 0; - cleanup: - virObjectUnlock(vm); + + endjob: + if (libxlDomainObjEndJob(driver, vm)) + virObjectUnlock(vm); + return ret; } @@ -885,17 +893,28 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; def = NULL; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + goto cleanup; + } + if (libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1) < 0) { virDomainObjListRemove(driver->domains, vm); - vm = NULL; - goto cleanup; + goto endjob; } dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: virDomainDefFree(def); if (vm) @@ -1681,7 +1700,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr); if (fd < 0) - return -1; + goto cleanup; if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) goto cleanup; @@ -1695,11 +1714,20 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, def = NULL; + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + goto cleanup; + } + ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd); - if (ret < 0 && !vm->persistent) { + if (ret < 0 && !vm->persistent) virDomainObjListRemove(driver->domains, vm); + + if (!libxlDomainObjEndJob(driver, vm)) vm = NULL; - } cleanup: if (VIR_CLOSE(fd) < 0) @@ -2567,17 +2595,24 @@ libxlDomainCreateWithFlags(virDomainPtr dom, if (virDomainCreateWithFlagsEnsureACL(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 already running")); - goto cleanup; + goto endjob; } ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1); if (ret < 0) - goto cleanup; + goto endjob; dom->id = vm->def->id; + endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.4.5

On 04.04.2015 00:49, Jim Fehlig wrote:
Let callers of libxlDomainStart decide when it is appropriate to acquire a job on the associated virDomainObj.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
Job handling in the migration code is currently broken/incomplete, so fixing it is deferred to a follow-up series I'm working on.
src/libxl/libxl_domain.c | 24 ++++++++------------- src/libxl/libxl_driver.c | 55 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 37b2b58..391291d 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -903,16 +903,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
Not to be seen in this chunk, but there's a comment before the function saying that the @vm must be locked. Extend it please to denote the fact callers should acquire a job before calling this function. Not doing so is acceptable only for a few exceptions which are bing fixed later in the series anyway.
libxl_domain_config_init(&d_config);
- 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 endjob; + goto cleanup;
if (virFileExists(managed_save_path)) {
Michal

A job should be acquired at the beginning of a domain destroy operation, not at the end when cleaning up the domain. Fix two occurrences of this late job acquisition in the libxl driver. Doing so renders libxlDomainCleanupJob unused, so it is removed. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- Unchanged from V2. src/libxl/libxl_domain.c | 53 ++++++++++++++++-------------------------------- src/libxl/libxl_domain.h | 4 ---- src/libxl/libxl_driver.c | 20 ++++++++++-------- 3 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 391291d..37ed082 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -371,6 +371,9 @@ libxlDomainShutdownThread(void *opaque) cfg = libxlDriverConfigGet(driver); + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; + if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) { dom_event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -384,7 +387,7 @@ libxlDomainShutdownThread(void *opaque) goto restart; case VIR_DOMAIN_LIFECYCLE_PRESERVE: case VIR_DOMAIN_LIFECYCLE_LAST: - goto cleanup; + goto endjob; } } else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) { dom_event = virDomainEventLifecycleNewFromObj(vm, @@ -399,7 +402,7 @@ libxlDomainShutdownThread(void *opaque) goto restart; case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE: case VIR_DOMAIN_LIFECYCLE_CRASH_LAST: - goto cleanup; + goto endjob; case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY: libxlDomainAutoCoreDump(driver, vm); goto destroy; @@ -420,11 +423,11 @@ libxlDomainShutdownThread(void *opaque) goto restart; case VIR_DOMAIN_LIFECYCLE_PRESERVE: case VIR_DOMAIN_LIFECYCLE_LAST: - goto cleanup; + goto endjob; } } else { VIR_INFO("Unhandled shutdown_reason %d", xl_reason); - goto cleanup; + goto endjob; } destroy: @@ -433,13 +436,11 @@ libxlDomainShutdownThread(void *opaque) dom_event = NULL; } libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); - if (libxlDomainCleanupJob(driver, vm, reason)) { - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } - } - goto cleanup; + libxlDomainCleanup(driver, vm, reason); + if (!vm->persistent) + virDomainObjListRemove(driver->domains, vm); + + goto endjob; restart: if (dom_event) { @@ -447,13 +448,17 @@ libxlDomainShutdownThread(void *opaque) dom_event = NULL; } libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); - libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); + libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); if (libxlDomainStart(driver, vm, false, -1) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to restart VM '%s': %s"), vm->def->name, err ? err->message : _("unknown error")); } + endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -691,26 +696,6 @@ libxlDomainCleanup(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. - */ -bool -libxlDomainCleanupJob(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, - virDomainShutoffReason reason) -{ - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY) < 0) - return true; - - libxlDomainCleanup(driver, vm, reason); - - return libxlDomainObjEndJob(driver, vm); -} - -/* * Core dump domain to default dump path. * * virDomainObjPtr must be locked on invocation @@ -735,15 +720,11 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, timestr) < 0) goto cleanup; - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) - goto cleanup; - /* Unlock virDomainObj while dumping core */ virObjectUnlock(vm); libxl_domain_core_dump(cfg->ctx, vm->def->id, dumpfile, NULL); virObjectLock(vm); - ignore_value(libxlDomainObjEndJob(driver, vm)); ret = 0; cleanup: diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index a032e46..30855a2 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -108,10 +108,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virDomainShutoffReason reason); -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 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index e315b32..c9623ef 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1240,10 +1240,13 @@ libxlDomainDestroyFlags(virDomainPtr dom, if (virDomainDestroyFlagsEnsureACL(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; } event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1252,18 +1255,19 @@ libxlDomainDestroyFlags(virDomainPtr dom, 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; + goto endjob; } - if (libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { - if (!vm->persistent) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } - } + libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + if (!vm->persistent) + virDomainObjListRemove(driver->domains, vm); ret = 0; + endjob: + if (!libxlDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.4.5

A destroy operation can take considerable time on large memory domains due to scrubbing the domain's memory. Unlock the virDomainObj while libxl_domain_destroy is executing. Implement libxlDomainDestroyInternal wrapper to handle unlocking, calling destroy, and locking. Change all callers of libxl_domain_destroy to use the wrapper. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V3: Provide a wrapper to unlock, destroy and lock domain instead of do so at each call site of libxl_domain_destroy. src/libxl/libxl_domain.c | 29 ++++++++++++++++++++++++++--- src/libxl/libxl_domain.h | 4 ++++ src/libxl/libxl_driver.c | 6 +++--- src/libxl/libxl_migration.c | 4 ++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 37ed082..1b809ca 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -435,7 +435,7 @@ libxlDomainShutdownThread(void *opaque) libxlDomainEventQueue(driver, dom_event); dom_event = NULL; } - libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); + libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm, reason); if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); @@ -447,7 +447,7 @@ libxlDomainShutdownThread(void *opaque) libxlDomainEventQueue(driver, dom_event); dom_event = NULL; } - libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); + libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); if (libxlDomainStart(driver, vm, false, -1) < 0) { virErrorPtr err = virGetLastError(); @@ -626,6 +626,29 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver, } /* + * Internal domain destroy function. + * + * virDomainObjPtr must be locked on invocation + */ +int +libxlDomainDestroyInternal(libxlDriverPrivatePtr driver, + virDomainObjPtr vm) +{ + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + int ret = -1; + + /* Unlock virDomainObj during destroy, which can take considerable + * time on large memory domains. + */ + virObjectUnlock(vm); + ret = libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); + virObjectLock(vm); + + virObjectUnref(cfg); + return ret; +} + +/* * Cleanup function for domain that has reached shutoff state. * * virDomainObjPtr must be locked on invocation @@ -1022,7 +1045,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxl_evdisable_domain_death(cfg->ctx, priv->deathW); priv->deathW = NULL; } - libxl_domain_destroy(cfg->ctx, domid, NULL); + libxlDomainDestroyInternal(driver, vm); 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 30855a2..aa647b8 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -103,6 +103,10 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver, libxlSavefileHeaderPtr ret_hdr) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); +int +libxlDomainDestroyInternal(libxlDriverPrivatePtr driver, + virDomainObjPtr vm); + void libxlDomainCleanup(libxlDriverPrivatePtr driver, virDomainObjPtr vm, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index c9623ef..9eb071e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1252,7 +1252,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { + if (libxlDomainDestroyInternal(driver, vm) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto endjob; @@ -1595,7 +1595,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); - if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { + if (libxlDomainDestroyInternal(driver, vm) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto cleanup; @@ -1802,7 +1802,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) } if (flags & VIR_DUMP_CRASH) { - if (libxl_domain_destroy(cfg->ctx, vm->def->id, NULL) < 0) { + if (libxlDomainDestroyInternal(driver, vm) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), vm->def->id); goto unpause; diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 3d0c96e..4010506 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -575,7 +575,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn, cleanup: if (dom == NULL) { - libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); + libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); @@ -614,7 +614,7 @@ libxlDomainMigrationConfirm(libxlDriverPrivatePtr driver, goto cleanup; } - libxl_domain_destroy(cfg->ctx, vm->def->id, NULL); + libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); -- 1.8.4.5

On Fri, Apr 03, 2015 at 04:49:01PM -0600, Jim Fehlig wrote:
V3 of a small series to fix issues wrt domain destroy
V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html
V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html
In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain
src/libxl/libxl_domain.c | 106 ++++++++++++++++++++++---------------------- src/libxl/libxl_domain.h | 8 ++-- src/libxl/libxl_driver.c | 81 ++++++++++++++++++++++++--------- src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-)
-- 1.8.4.5
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel

On 04.04.2015 00:49, Jim Fehlig wrote:
V3 of a small series to fix issues wrt domain destroy
V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html
V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html
In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper.
Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain
src/libxl/libxl_domain.c | 106 ++++++++++++++++++++++---------------------- src/libxl/libxl_domain.h | 8 ++-- src/libxl/libxl_driver.c | 81 ++++++++++++++++++++++++--------- src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-)
ACK series, but see my comment to the first patch. Michal

On 04/09/2015 09:03 AM, Michal Privoznik wrote:
On 04.04.2015 00:49, Jim Fehlig wrote:
V3 of a small series to fix issues wrt domain destroy
V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html
V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html
In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper.
Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain
src/libxl/libxl_domain.c | 106 ++++++++++++++++++++++---------------------- src/libxl/libxl_domain.h | 8 ++-- src/libxl/libxl_driver.c | 81 ++++++++++++++++++++++++--------- src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-)
ACK series, but see my comment to the first patch.
I fixed the comment for libxlDomainStart as you suggested and pushed the series. Thanks for the review Michal! Regards, Jim
participants (3)
-
Jim Fehlig
-
Konrad Rzeszutek Wilk
-
Michal Privoznik