[libvirt] [PATCH 05/11] libxl: use job functions in libxlDomainDestroyFlags

[Daniel P. Berrange]

On Thu, Feb 06, 2014 at 08:53:09PM -0700, Jim Fehlig wrote:
> Modify operation that needs to wait in the queue of modify jobs.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
>  src/libxl/libxl_driver.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index caabb44..bb574bc 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1549,6 +1549,7 @@ libxlDomainDestroyFlags(virDomainPtr dom,
>      libxlDriverPrivatePtr driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      int ret = -1;
> +    bool remove_dom = false;
>      virObjectEventPtr event = NULL;
>  
>      virCheckFlags(0, -1);
> @@ -1559,10 +1560,13 @@ libxlDomainDestroyFlags(virDomainPtr dom,
>      if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
>  
> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY) < 0)
> +        goto cleanup;

So there's one complication in the destroy method. You really,
really want destroy to succeed no matter what. In the QEMU driver
we will kill -9 the QEMU process from outside the job condition.
This should unblock any currently active jobs, so when we then
begin the destroy job there's no delay. In the libvirtd daemon
RPC dispatch code, the destroy method is also marked as high
priority so it gets processed in a dedicated thread pool for
high priority RPC calls. Again we don't want high priority
calls to be blocked by low priority calls.

With this it looks like you're just going to wait indefinitely
for any pending job to complete before attempting to kill the
guest.

Is there anything you can safely do with the libxl APIs which
would be equivalent to kill -9, outside the job block. Then
you just need the job to protect cleanup of the internal state
on virDomainObjPtr.


> +
>      if (!virDomainObjIsActive(vm)) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
>                         "%s", _("Domain is not running"));
> -        goto cleanup;
> +        goto endjob;
>      }
>  
>      event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> @@ -1571,17 +1575,22 @@ libxlDomainDestroyFlags(virDomainPtr dom,
>      if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED) != 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Failed to destroy domain '%d'"), dom->id);
> -        goto cleanup;
> +        goto endjob;
>      }
>  
> -    if (!vm->persistent) {
> -        virDomainObjListRemove(driver->domains, vm);
> -        vm = NULL;
> -    }
> +    if (!vm->persistent)
> +        remove_dom = true;
>  
>      ret = 0;
>  
> +endjob:
> +    libxlDomainObjEndJob(driver, vm);
> +
>  cleanup:
> +    if (remove_dom) {
> +        virDomainObjListRemove(driver->domains, vm);
> +        vm = NULL;
> +    }
>      if (vm)
>          virObjectUnlock(vm);
>      if (event)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|