[libvirt] [PATCH 2/2] check whether the vm is paused before setting it offline

[Wen Congyang]

At 05/20/2011 04:34 PM, Daniel P. Berrange Write:
> On Fri, May 20, 2011 at 03:09:42PM +0800, Wen Congyang wrote:
>> If we do not performe a live migration, the vm is always paused when we go to
>> doPeer2PeerMigrate2() and doPeer2PeerMigrate3(), because we stop the vm in
>> qemuMigrationPerform(). So we should check it before setting it offline.
> 
> The only code which checks for 'if (flags & VIR_MIGRATE_PAUSED)'
> is the qemuMigrateFinished method. This is called from
> doPeer2PeerMigrate2 or doPeer2PeerMigrate3.  So putting the
> change 'flags |= VIR_MIGRATED_PAUSED' does not make any
> difference to peer2peer migration, and for non-peer2peer
> migration, this will have already been set in libvirt.c.
> 
> So I'm not seeing what bug is being fixed by this change.

Without this patch, vm is still paused after migration when
we do p2p migration.

The vm should be paused only when:
1. the vm is paused before migration
2. we use the option --suspend.

We decide whether to restart the vm on the dest host accoring to whether
VIR_MIGRATED_PAUSED is set.

If we do not use the option --live and --suspend, and the vm is running
before migration, the vm should be also running after migration. But
the vm will be set offline in the function qemuMigrationPerform().
When we goto to doPeer2PeerMigrate2() or doPeer2PeerMigrate3(),
the vm is paused and VIR_MIGRATED_PAUSED is set. So the vm is not
restarted in the function qemuMigrateFinished().

The way to solute this bug is that: we should check the origin status of vm
before setting it offline. This patch does this.

> The doPeer2PeerMigrate2 and doPeer2PeerMigrate3 methods
> are intended to be structured (near) identically to the
> virDomainMigrate2 and virDomainMigrate3 methods in libvirt.c,
> so I think the code is correct as it already is.
> 
>>
>> ---
>>  src/qemu/qemu_migration.c |    9 +++------
>>  1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index ab28416..30167f2 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -1614,9 +1614,6 @@ static int doPeer2PeerMigrate2(struct qemud_driver *driver,
>>                                          VIR_DOMAIN_XML_UPDATE_CPU)))
>>          return -1;
>>  
>> -    if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED)
>> -        flags |= VIR_MIGRATE_PAUSED;
>> -
>>      VIR_DEBUG("Prepare2 %p", dconn);
>>      if (flags & VIR_MIGRATE_TUNNELLED) {
>>          /*
>> @@ -1748,9 +1745,6 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver,
>>      if (!dom_xml)
>>          goto cleanup;
>>  
>> -    if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED)
>> -        flags |= VIR_MIGRATE_PAUSED;
>> -
>>      VIR_DEBUG("Prepare3 %p", dconn);
>>      cookiein = cookieout;
>>      cookieinlen = cookieoutlen;
>> @@ -1985,6 +1979,9 @@ int qemuMigrationPerform(struct qemud_driver *driver,
>>      memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
>>      priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED;
>>  
>> +    if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED)
>> +        flags |= VIR_MIGRATE_PAUSED;
>> +
>>      resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING;
>>      if (!(flags & VIR_MIGRATE_LIVE) &&
>>          virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
> 
> 
> Daniel