Re: [Xen-devel] [PATCH 2/3] libxl: acquire a job when destroying a domain

[Jim Fehlig <jfehlig@xxxxxxxx>]

Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 25, 2015 at 02:08:35PM -0600, Jim Fehlig wrote:
>   
>> A job should be acquired at the beginning of a domain destroy operation,
>> not at the end when cleaning up the domain.  Fix two occurances of this
>> late job acquisition in the libxl driver.  Doing so renders
>> libxlDomainCleanup unused, so it is removed.
>>     

Just noticed this should be libxlDomainCleanupJob.

>> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
>> ---
>>  src/libxl/libxl_domain.c | 49 
>> +++++++++++++++++-------------------------------
>>  src/libxl/libxl_domain.h |  4 ----
>>  src/libxl/libxl_driver.c | 20 ++++++++++++--------
>>  3 files changed, 29 insertions(+), 44 deletions(-)
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index eca1ff0..e240eae 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;
>> +
>>     
>
> Won't this a deadlock with 'libxlDomainAutoCoreDump'  (line 410) (which also
> calls :
>
>  727     if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>  728         goto cleanup;
>
> well, not deadlock - but spin up to 30 seconds? and then give up?
>   

Yes, another good catch!

> Perhaps this patch should be folded in?
>
> From 9f2bac0c28815fc51850290c4b1962881691bfca Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Wed, 25 Mar 2015 22:34:51 -0400
> Subject: [PATCH] squash
>
> ---
>  src/libxl/libxl_domain.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index aef0157..774b070 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -723,15 +723,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));
>   

I've squashed this in my branch and fixed the commit message typo.

Regards,
Jim


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