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@xxxxxxxxxx>
>
>> 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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list