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

[Eric Blake]

On 10/27/2010 08:08 AM, Daniel P. Berrange wrote:
> 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.

Okay, I'll see about swapping things around.

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

Yes, I'll submit a v2 that avoids extra goto labels (or at least sinks 
the endjob label to the end of the function rather than the middle).

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org