[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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.  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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]