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 <bjzh...@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 ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
