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