[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH COLO-Frame v9 05/32] migration: Integrate COLO c
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH COLO-Frame v9 05/32] migration: Integrate COLO checkpoint process into migration |
Date: |
Fri, 9 Oct 2015 17:53:36 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* zhanghailiang (address@hidden) wrote:
> Add a migrate state: MIGRATION_STATUS_COLO, enter this migration state
> after the first live migration successfully finished.
>
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Li Zhijian <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
If I understand this correctly, I think what you're doing is:
migration_thread
does initial migrate
init_checkpointer
creates a bh
main thread
bh
start_checkpointer
creates colo_thread
colo thread
outbound side loop
Why not just keep the migration thread going and reuse that?
Couldn't you just call the colo_thread() function at the place you
currently call colo_init_checkpointer, and add the init code
to the start of colo_thread?
It seems simpler.
Other than that it's OK.
Dave
> ---
> include/migration/colo.h | 3 +++
> migration/colo.c | 58
> ++++++++++++++++++++++++++++++++++++++++++++++++
> migration/migration.c | 23 ++++++++++++++-----
> qapi-schema.json | 2 +-
> stubs/migration-colo.c | 9 ++++++++
> trace-events | 3 +++
> 6 files changed, 92 insertions(+), 6 deletions(-)
>
> diff --git a/include/migration/colo.h b/include/migration/colo.h
> index 9b6662d..dface19 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -19,4 +19,7 @@
> bool colo_supported(void);
> void colo_info_mig_init(void);
>
> +void colo_init_checkpointer(MigrationState *s);
> +bool migration_in_colo_state(void);
> +
> #endif
> diff --git a/migration/colo.c b/migration/colo.c
> index 2c40d2e..97e64a3 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -10,9 +10,67 @@
> * later. See the COPYING file in the top-level directory.
> */
>
> +#include "sysemu/sysemu.h"
> #include "migration/colo.h"
> +#include "trace.h"
> +
> +static QEMUBH *colo_bh;
>
> bool colo_supported(void)
> {
> return true;
> }
> +
> +bool migration_in_colo_state(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return (s->state == MIGRATION_STATUS_COLO);
> +}
> +
> +static void *colo_thread(void *opaque)
> +{
> + MigrationState *s = opaque;
> +
> + qemu_mutex_lock_iothread();
> + vm_start();
> + qemu_mutex_unlock_iothread();
> + trace_colo_vm_state_change("stop", "run");
> +
> + /*TODO: COLO checkpoint savevm loop*/
> +
> + migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
> + MIGRATION_STATUS_COMPLETED);
> +
> + qemu_mutex_lock_iothread();
> + qemu_bh_schedule(s->cleanup_bh);
> + qemu_mutex_unlock_iothread();
> +
> + return NULL;
> +}
> +
> +static void colo_start_checkpointer(void *opaque)
> +{
> + MigrationState *s = opaque;
> +
> + if (colo_bh) {
> + qemu_bh_delete(colo_bh);
> + colo_bh = NULL;
> + }
> +
> + qemu_mutex_unlock_iothread();
> + qemu_thread_join(&s->thread);
> + qemu_mutex_lock_iothread();
> +
> + migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> + MIGRATION_STATUS_COLO);
> +
> + qemu_thread_create(&s->thread, "colo", colo_thread, s,
> + QEMU_THREAD_JOINABLE);
> +}
> +
> +void colo_init_checkpointer(MigrationState *s)
> +{
> + colo_bh = qemu_bh_new(colo_start_checkpointer, s);
> + qemu_bh_schedule(colo_bh);
> +}
> diff --git a/migration/migration.c b/migration/migration.c
> index 98133f1..bee61aa 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -446,6 +446,10 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>
> get_xbzrle_cache_stats(info);
> break;
> + case MIGRATION_STATUS_COLO:
> + info->has_status = true;
> + /* TODO: display COLO specific information (checkpoint info etc.) */
> + break;
> case MIGRATION_STATUS_COMPLETED:
> get_xbzrle_cache_stats(info);
>
> @@ -731,7 +735,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>
> if (s->state == MIGRATION_STATUS_ACTIVE ||
> s->state == MIGRATION_STATUS_SETUP ||
> - s->state == MIGRATION_STATUS_CANCELLING) {
> + s->state == MIGRATION_STATUS_CANCELLING ||
> + s->state == MIGRATION_STATUS_COLO) {
> error_setg(errp, QERR_MIGRATION_ACTIVE);
> return;
> }
> @@ -948,6 +953,7 @@ static void *migration_thread(void *opaque)
> int64_t max_size = 0;
> int64_t start_time = initial_time;
> bool old_vm_running = false;
> + bool enable_colo = migrate_enable_colo();
>
> rcu_register_thread();
>
> @@ -992,8 +998,10 @@ static void *migration_thread(void *opaque)
> }
>
> if (!qemu_file_get_error(s->file)) {
> - migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> - MIGRATION_STATUS_COMPLETED);
> + if (!enable_colo) {
> + migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> + MIGRATION_STATUS_COMPLETED);
> + }
> break;
> }
> }
> @@ -1044,11 +1052,16 @@ static void *migration_thread(void *opaque)
> }
> runstate_set(RUN_STATE_POSTMIGRATE);
> } else {
> - if (old_vm_running) {
> + if (s->state == MIGRATION_STATUS_ACTIVE && enable_colo) {
> + colo_init_checkpointer(s);
> + } else if (old_vm_running) {
> vm_start();
> }
> }
> - qemu_bh_schedule(s->cleanup_bh);
> +
> + if (!enable_colo || s->state != MIGRATION_STATUS_ACTIVE) {
> + qemu_bh_schedule(s->cleanup_bh);
> + }
> qemu_mutex_unlock_iothread();
>
> rcu_unregister_thread();
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b14d1f4..6dd5c7c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -433,7 +433,7 @@
> ##
> { 'enum': 'MigrationStatus',
> 'data': [ 'none', 'setup', 'cancelling', 'cancelled',
> - 'active', 'completed', 'failed' ] }
> + 'active', 'completed', 'failed', 'colo' ] }
>
> ##
> # @MigrationInfo
> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c
> index 3d817df..51b8f66 100644
> --- a/stubs/migration-colo.c
> +++ b/stubs/migration-colo.c
> @@ -16,3 +16,12 @@ bool colo_supported(void)
> {
> return false;
> }
> +
> +bool migration_in_colo_state(void)
> +{
> + return false;
> +}
> +
> +void colo_init_checkpointer(MigrationState *s)
> +{
> +}
> diff --git a/trace-events b/trace-events
> index 8f9614a..487d1c7 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1472,6 +1472,9 @@ rdma_start_incoming_migration_after_rdma_listen(void) ""
> rdma_start_outgoing_migration_after_rdma_connect(void) ""
> rdma_start_outgoing_migration_after_rdma_source_init(void) ""
>
> +# migration/colo.c
> +colo_vm_state_change(const char *old, const char *new) "Change '%s' => '%s'"
> +
> # kvm-all.c
> kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
> kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
> --
> 1.8.3.1
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH COLO-Frame v9 05/32] migration: Integrate COLO checkpoint process into migration,
Dr. David Alan Gilbert <=