| Message ID | 1441182199-8328-6-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
* zhanghailiang (zhang.zhanghailiang@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.zhanghailiang@huawei.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > Signed-off-by: Gonglei <arei.gonglei@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 / dgilbert@redhat.com / Manchester, UK
On 2015/10/10 0:53, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@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.zhanghailiang@huawei.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> Signed-off-by: Gonglei <arei.gonglei@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. > Of course, it is really a good idea to reuse migration thread, i will fix it like that. and the helper function colo_thread will be renamed :) Thanks, zhanghailiang > 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 / dgilbert@redhat.com / Manchester, UK > > . >
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"