Re: [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread

[Amit Shah <amit.shah@redhat.com>]

On (Thu) 13 Aug 2015 [11:51:31], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The code that gets run at the end of the migration process
> is getting large, and I'm about to add more for postcopy.
> Split it into a separate function.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

> ---
>  migration/migration.c | 75 ++++++++++++++++++++++++++++++++-------------------
>  trace-events          |  2 ++
>  2 files changed, 49 insertions(+), 28 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 662e77e..46bb410 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -913,6 +913,50 @@ int64_t migrate_xbzrle_cache_size(void)
>      return s->xbzrle_cache_size;
>  }
>  
> +/**
> + * migration_completion: Used by migration_thread when there's not much left.
> + *   The caller 'breaks' the loop when this returns.
> + *
> + * @s: Current migration state
> + * @*old_vm_running: Pointer to old_vm_running flag
> + * @*start_time: Pointer to time to update
> + */
> +static void migration_completion(MigrationState *s, bool *old_vm_running,
> +                                 int64_t *start_time)
> +{
> +    int ret;
> +
> +    qemu_mutex_lock_iothread();
> +    *start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> +    *old_vm_running = runstate_is_running();
> +
> +    ret = global_state_store();
> +    if (!ret) {
> +        ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +        if (ret >= 0) {
> +            qemu_file_set_rate_limit(s->file, INT64_MAX);
> +            qemu_savevm_state_complete(s->file);
> +        }
> +    }

The case where ret >= 0 and where s->file has an error didn't set
status to failed nor break'ed in the removed context below.  But it
happened a few lines outside the context anyway.

Now it just happens in this function, which is cleaner and easier to
understand.

		Amit