[libvirt] [Xen-devel] [PATCH 3/3] libxl: drop virDomainObj lock when destroying a domain

Ian Campbell ian.campbell at citrix.com
Thu Mar 26 15:14:29 UTC 2015


On Wed, 2015-03-25 at 14:08 -0600, Jim Fehlig wrote:
> A destroy operation can take considerable time on large memory
> domains due to scrubbing the domain' memory.  The operation is
> running in the context of a job, so unlocking the domain and
> allowing query operations is safe.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>

I don't really know enough about the libvirt job/obj locking to comment
on the previous patches or that aspect of this one, but in general the
idea of dropping the lock before calling libxl_destroy seems reasonable
to me.

In principal you could use the async stuff (the final NULL to the call),
but you'd still be wanting to drop the lock for that period, so it's not
clear it's any better from your PoV.

Ian.

> ---
>  src/libxl/libxl_domain.c | 4 ++++
>  src/libxl/libxl_driver.c | 5 ++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index e240eae..aef0157 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -435,7 +435,9 @@ libxlDomainShutdownThread(void *opaque)
>          libxlDomainEventQueue(driver, dom_event);
>          dom_event = NULL;
>      }
> +    virObjectUnlock(vm);
>      libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
> +    virObjectLock(vm);
>      libxlDomainCleanup(driver, vm, reason);
>      if (!vm->persistent)
>          virDomainObjListRemove(driver->domains, vm);
> @@ -447,7 +449,9 @@ libxlDomainShutdownThread(void *opaque)
>          libxlDomainEventQueue(driver, dom_event);
>          dom_event = NULL;
>      }
> +    virObjectUnlock(vm);
>      libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
> +    virObjectLock(vm);
>      libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
>      if (libxlDomainStart(driver, vm, false, -1) < 0) {
>          virErrorPtr err = virGetLastError();
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a1977c2..21e20bb 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1250,7 +1250,10 @@ 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) {
> +    virObjectUnlock(vm);
> +    ret = libxl_domain_destroy(cfg->ctx, vm->def->id, NULL);
> +    virObjectLock(vm);
> +    if (ret < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Failed to destroy domain '%d'"), vm->def->id);
>          goto endjob;





More information about the libvir-list mailing list