Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state

[Juan Quintela <quintela@redhat.com>]

Peter Xu <peterx@redhat.com> wrote:
> Firstly, it was passed around.  Let's just move it into MigrationState
> just like many other variables as state of migration.
>
> One thing to mention is that for postcopy, we actually don't need this
> knowledge at all since postcopy can't resume a VM even if it fails (we
> can see that from the old code too: when we try to resume we also check
> against "entered_postcopy" variable).  So further we do this:
>
> - in postcopy_start(), we don't update vm_old_running since useless
> - in migration_thread(), we don't need to check entered_postcopy when
>   resume, since it's only used for precopy.
>
> Comment this out too for that variable definition.

Reviewed-by: Juan Quintela <quintela@redhat.com>

But I wonder if we can came with a better name.  Best one that I can
think is
   vm_was_running

Any other name that I came is bad for precopy or colo.

i.e. restart_vm_on_cancel_error

is meaningful for precopy, but not for colo.

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 17 +++++++----------
>  migration/migration.h |  6 ++++++
>  2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/migration/migration.h b/migration/migration.h
> index ac74a12713..0f5df2367c 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -111,6 +111,12 @@ struct MigrationState
>      int64_t expected_downtime;
>      bool enabled_capabilities[MIGRATION_CAPABILITY__MAX];
>      int64_t setup_time;
> +    /*
> +     * Whether the old VM is running for the last migration.  This is
> +     * used to resume the VM when precopy failed or cancelled somehow.
> +     * It's never used for postcopy.
> +     */
> +    bool old_vm_running;

I think this comment is right for precopy, but not for colo.  BTW, I
think that I would put the postcopy comment on its use, not here.

/me tries to improve the comment

  Guest was running when we enter the completion stage.  If migration don't
  sucess, we need to continue running guest on source.

What do you think?

Later, Juan.


>  
>      /* Flag set once the migration has been asked to enter postcopy */
>      bool start_postcopy;