* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: > Add a migrate state: MIGRATION_STATUS_COLO, enter this migration state > after the first live migration successfully finished. > > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> > Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com> > Signed-off-by: Gonglei <arei.gong...@huawei.com>
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 / dgilb...@redhat.com / Manchester, UK