[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.