[libvirt] [PATCH]: Fix non-live migration failure case

[Chris Lalancette]

Daniel Veillard wrote:
> On Wed, Feb 25, 2009 at 02:30:35PM +0100, Chris Lalancette wrote:
>> All,
>> There was a logic error in the Qemu driver when doing a non-live migrate.
>> During a non-live migrate, on the source host during the Perform step, we
>> pause the domain; however, if there was ever a failure, we were forgetting
>> to unpause the domain, meaning that the domain was paused forever.  Add a
>> flag to tell us when we should unpause the domain after a failure.
> 
>   Princile sounds fine ... but 
> 
>> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> 
>> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
>> index a8a2ae7..bcc4690 100644
>> --- a/src/qemu_driver.c
>> +++ b/src/qemu_driver.c
>> @@ -4331,6 +4331,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
>>      char cmd[HOST_NAME_MAX+50];
>>      char *info = NULL;
>>      int ret = -1;
>> +    int paused = 0;
>>  
>>      qemuDriverLock(driver);
>>      vm = virDomainFindByID(&driver->domains, dom->id);
>> @@ -4352,6 +4353,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
>>          qemudMonitorCommand (vm, cmd, &info);
>>          DEBUG ("stop reply: %s", info);
>>          VIR_FREE(info);
>> +        paused = 1;
>>  
>>          event = virDomainEventNewFromObj(vm,
>>                                           VIR_DOMAIN_EVENT_SUSPENDED,
>> @@ -4407,6 +4409,21 @@ qemudDomainMigratePerform (virDomainPtr dom,
>>      ret = 0;
>>  
>>  cleanup:
>> +    if (ret != 0 && paused) {
>> +        /* we got here through some sort of failure; start the domain again */
>> +        snprintf(cmd, sizeof cmd, "%s", "cont");
> 
>   sizeof without braces ?
> 
>> +        qemudMonitorCommand (vm, cmd, &info);
> 
>    and why not just doing 
>            qemudMonitorCommand (vm, "cont", &info);
> and avoiding this ?
>    and do we care about error handling there ?
> 
>> +        DEBUG ("cont reply: %s", info);
>> +        VIR_FREE(info);
> 
>   no need to do VIR_FREE(info) there since it's done below
> 
>> +        event = virDomainEventNewFromObj(vm,
>> +                                         VIR_DOMAIN_EVENT_RESUMED,
>> +                                         VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
>> +        if (event)
>> +            qemuDomainEventQueue(driver, event);
>> +        event = NULL;
>> +    }
>> +
>>      VIR_FREE(info);

As we discussed on IRC, this was a quick cut-n-paste job, and I should have
looked at it better.  The place where I copied it from is also needlessly using
"snprintf" (I think), so I'll spin a new patch that fixes that too.  Your other
comments are dead-on, and also make me realize I introduced a memory leak here.
 I'll fix all of this up and re-post.

Thanks,
-- 
Chris Lalancette