[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into globa
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state |
Date: |
Wed, 03 Jan 2018 10:05:07 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Peter Xu <address@hidden> 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 <address@hidden>
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 <address@hidden>
> ---
> 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;
- [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, (continued)
- [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Peter Xu, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Juan Quintela, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Peter Xu, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Juan Quintela, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Peter Xu, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Juan Quintela, 2018/01/03
- Re: [Qemu-devel] [PATCH 02/11] migration: qemu_savevm_state_cleanup() in cleanup, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 03/11] migration: remove "enable_colo" var, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state, Peter Xu, 2018/01/03
- Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state,
Juan Quintela <=
[Qemu-devel] [PATCH 04/11] migration: split use of MigrationState.total_time, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 06/11] migration: introduce vm_down_start_time, Peter Xu, 2018/01/03