Re: [libvirt] [PATCH] qemu: check for vm after starting a job

On Tue, Oct 26, 2010 at 05:32:09PM -0600, Eric Blake wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=638285 - when migrating > a guest, it was very easy to provoke a race where an application > could query block information on a VM that had just been migrated > away. Any time qemu code obtains a job lock, it must also check > that the VM was not taken down in the time where it was waiting > for the lock. > > @@ -4948,6 +4949,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { > if (qemuDomainObjBeginJob(vm)< 0) > goto cleanup; > > + if (!virDomainObjIsActive(vm)) { > + qemuReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("domain is not running")); > + goto endjob; > + } > + There is already an IsActive check in this method, it is simply in the wrong place wrt to the BeginJob call, so it is better to just move the existing call, rather than duplicate it i reckon.

> @@ -10350,6 +10368,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, > &info->allocation); > qemuDomainObjExitMonitor(vm); > > + endjob: > if (qemuDomainObjEndJob(vm) == 0) > vm = NULL; > } else { I'm not much of a fan of adding goto jump targets which are in the middle of long methods. Could we just invert the IsActive checks in these two methods& thus avoid the 'endjob' in the middle of the method.