[libvirt] [Xen-devel] [PATCH 2/3] libxl: acquire a job when destroying a domain
Jim Fehlig
jfehlig at suse.com
Thu Mar 26 21:29:51 UTC 2015
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 at suse.com>
>> ---
>> 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 at oracle.com>
> 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
More information about the libvir-list
mailing list