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(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 617 bytes)