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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxx>

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;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel