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

Eric Blake eblake at redhat.com
Tue Oct 23 16:06:23 UTC 2012


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.  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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121023/7d4f0392/attachment-0001.sig>


More information about the libvir-list mailing list