[libvirt] [PATCH 2/2] Add lock in libxl api

[Jim Fehlig]

Eric Blake wrote:
> On 10/22/2012 05:10 PM, Jim Fehlig wrote:
>   
>> Bamvor Jian Zhang wrote:
>>     
>>> Add long-running jobs for save, dump. Add normal job for the
>>> api maybe modify the domain.
>>>
>>> Signed-off-by: Bamvor Jian Zhang <bjzhang at suse.com>
>>>       
>
>   
>>> +
>>> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>>> +        goto cleanup;
>>> +
>>>      if (!virDomainObjIsActive(vm)) {
>>>          virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
>>> -        goto cleanup;
>>> +        goto endjob;
>>>      }
>>>   
>>>       
>> IMO, we should check if the domain is active before calling
>> libxlDomainObjBeginJob().
>>     
>
> That's a possible optimization to avoid contending for the lock, but the
> point remains that libxlDomainObjBeginJob() temporarily drops locks, and
> while locks are down, the domain can stop.

Oh, right.  Thanks for catching that.

>   Therefore, you must ALWAYS
> check if the domain is active after obtaining the job, even if you
> already checked prior to requesting the job.  This particular section of
> code looks correct to me as-is.
>   

Agreed.  Bamvor, ignore my comments about changing this pattern
throughout the patch.

Regards,
Jim