[06/30] migration: stop all cpus correctly

You can only stop all cpus from the iothread or an vcpu.  As we want
to do it from the migration_thread, we need to do this dance with the
botton handlers.

This patch is a request for ideas.  I can move this function to cpus.c, but
wondered if there is an easy way of doing this?

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Il 18/10/2012 09:30, Juan Quintela ha scritto:
> You can only stop all cpus from the iothread or an vcpu.  As we want
> to do it from the migration_thread, we need to do this dance with the
> botton handlers.
> 
> This patch is a request for ideas.  I can move this function to cpus.c, but
> wondered if there is an easy way of doing this?

The commit message here is stale (I think I had pointed this out
already).  Also, first_time should be a member of MigrationState,
initialized in migrate_fd_connect.

Paolo

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 02f4ffa..05ef1a3 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -20,6 +20,7 @@
>  #include "sysemu.h"
>  #include "block.h"
>  #include "qemu_socket.h"
> +#include "qemu-thread.h"
>  #include "block-migration.h"
>  #include "qmp-commands.h"
> 
> @@ -322,11 +323,22 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
>  void migrate_fd_put_ready(MigrationState *s)
>  {
>      int ret;
> +    static bool first_time = true;
> 
>      if (s->state != MIG_STATE_ACTIVE) {
>          DPRINTF("put_ready returning because of non-active state\n");
>          return;
>      }
> +    if (first_time) {
> +        first_time = false;
> +        DPRINTF("beginning savevm\n");
> +        ret = qemu_savevm_state_begin(s->file, &s->params);
> +        if (ret < 0) {
> +            DPRINTF("failed, %d\n", ret);
> +            migrate_fd_error(s);
> +            return;
> +        }
> +    }
> 
>      DPRINTF("iterate\n");
>      ret = qemu_savevm_state_iterate(s->file);
> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
>          DPRINTF("done iterating\n");
>          start_time = qemu_get_clock_ms(rt_clock);
>          qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> -        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +        if (old_vm_running) {
> +            vm_stop(RUN_STATE_FINISH_MIGRATE);
> +        } else {
> +            vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +        }
> 
>          if (qemu_savevm_state_complete(s->file) < 0) {
>              migrate_fd_error(s);
> @@ -428,19 +444,8 @@ bool migration_has_failed(MigrationState *s)
> 
>  void migrate_fd_connect(MigrationState *s)
>  {
> -    int ret;
> -
>      s->state = MIG_STATE_ACTIVE;
>      qemu_fopen_ops_buffered(s);
> -
> -    DPRINTF("beginning savevm\n");
> -    ret = qemu_savevm_state_begin(s->file, &s->params);
> -    if (ret < 0) {
> -        DPRINTF("failed, %d\n", ret);
> -        migrate_fd_error(s);
> -        return;
> -    }
> -    migrate_fd_put_ready(s);
>  }
> 
>  static MigrationState *migrate_init(const MigrationParams *params)
>

Il 12/11/2012 12:44, Paolo Bonzini ha scritto:
>> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
>>          DPRINTF("done iterating\n");
>>          start_time = qemu_get_clock_ms(rt_clock);
>>          qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>> -        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>> +        if (old_vm_running) {
>> +            vm_stop(RUN_STATE_FINISH_MIGRATE);
>> +        } else {
>> +            vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>> +        }
>> 
>>          if (qemu_savevm_state_complete(s->file) < 0) {
>>              migrate_fd_error(s);

This hunk also seems to be useless nowadays.

Paolo

Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 12/11/2012 12:44, Paolo Bonzini ha scritto:
>>> @@ -339,7 +351,11 @@ void migrate_fd_put_ready(MigrationState *s)
>>>          DPRINTF("done iterating\n");
>>>          start_time = qemu_get_clock_ms(rt_clock);
>>>          qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>>> -        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>> +        if (old_vm_running) {
>>> +            vm_stop(RUN_STATE_FINISH_MIGRATE);
>>> +        } else {
>>> +            vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>> +        }
>>> 
>>>          if (qemu_savevm_state_complete(s->file) < 0) {
>>>              migrate_fd_error(s);
>
> This hunk also seems to be useless nowadays.

it is needed for when we are migrating in suspended state.  We need to
fix it some other way, but it needs to remaing for now.

Il 14/12/2012 13:36, Juan Quintela ha scritto:
>>>> >>> -        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>>> >>> +        if (old_vm_running) {
>>>> >>> +            vm_stop(RUN_STATE_FINISH_MIGRATE);
>>>> >>> +        } else {
>>>> >>> +            vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>>>> >>> +        }
>>>> >>> 
>>>> >>>          if (qemu_savevm_state_complete(s->file) < 0) {
>>>> >>>              migrate_fd_error(s);
>> >
>> > This hunk also seems to be useless nowadays.
> it is needed for when we are migrating in suspended state.

That was the case when the line read

    vm_stop(RUN_STATE_FINISH_MIGRATE);

but it reads vm_stop_force_state.  So you're being kind and not forcing
the change if running, but that's not necessary.  Just always using
vm_stop_force_state works now, and would work with the migration thread too.

Paolo