[libvirt] [PATCH 1/3] libxl: Move job acquisition in libxlDomainStart to callers
Jim Fehlig
jfehlig at suse.com
Wed Apr 1 17:29:14 UTC 2015
Martin Kletzander wrote:
> On Wed, Mar 25, 2015 at 02:08:34PM -0600, Jim Fehlig wrote:
>> Let callers of libxlDomainStart decide when it is appropriate to
>> acquire a job on the associated virDomainObj.
>>
>
> This makes sense, I see many bugs this fixes, but how come
> libxlDomainShutdownThread(), libxlDomainRestoreFlags() and
> libxlDoMigrateReceive() don't need to start the job when they all call
> libxlDomainStart()?
Commit 0521d658 starts a job for libxlDomainShutdownThread. This patch
adds starting a job in libxlDomainRestoreFlags. For migration, I'll
need to work on a follow-up series that fixes job handling in general.
>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>> src/libxl/libxl_domain.c | 24 ++++++++--------------
>> src/libxl/libxl_driver.c | 53
>> +++++++++++++++++++++++++++++++++++++++---------
>> 2 files changed, 52 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 05f6eb1..da4c1c7 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -885,17 +893,26 @@ libxlDomainCreateXML(virConnectPtr conn, const
>> char *xml,
>> goto cleanup;
>> def = NULL;
>>
>> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
>> + virDomainObjListRemove(driver->domains, vm);
>> + vm = NULL;
>> + goto cleanup;
>> + }
>> +
>
> This should be acquired before virDomainObjListAdd() since you need to
> check whether it's active after creating the job.
Acquiring the job requires a virDomainObj, which we get from the call to
virDomainObjListAdd().
> If I'm wrong, then
> virDomainObjListRemove() should be only called if the vm is not
> persistent (as CreateXML can be called on persistent ones as well),
> shouldn't it?
Yes, I think you are right. This was coded up assuming CreateXML only
handled transient domains.
>
> [...]
>> @@ -1695,11 +1712,20 @@ libxlDomainRestoreFlags(virConnectPtr conn,
>> const char *from,
>>
>> def = NULL;
>>
>> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
>> + if (!vm->persistent) {
>> + virDomainObjListRemove(driver->domains, vm);
>> + vm = NULL;
>> + }
>> + goto cleanup;
>> + }
>> +
>
> Same here, I guess.
Yes :-). A virDomainObj is needed to acquire a job.
Thanks for the review. I'll fix calling virDomainObjListRemove() on
persistent domains in V2.
Regards,
Jim
More information about the libvir-list
mailing list