From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZknbH-0002A4-U5 for qemu-devel@nongnu.org; Sat, 10 Oct 2015 02:25:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZknbD-0001Yy-NY for qemu-devel@nongnu.org; Sat, 10 Oct 2015 02:25:47 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:59839) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZknbC-0001XO-SP for qemu-devel@nongnu.org; Sat, 10 Oct 2015 02:25:43 -0400 References: <1441182199-8328-1-git-send-email-zhang.zhanghailiang@huawei.com> <1441182199-8328-6-git-send-email-zhang.zhanghailiang@huawei.com> <20151009165336.GI2702@work-vm> From: zhanghailiang Message-ID: <5618AF48.9010405@huawei.com> Date: Sat, 10 Oct 2015 14:25:12 +0800 MIME-Version: 1.0 In-Reply-To: <20151009165336.GI2702@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v9 05/32] migration: Integrate COLO checkpoint process into migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com, yanghy@cn.fujitsu.com 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 >> Signed-off-by: Li Zhijian >> Signed-off-by: Gonglei > > 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 > > . >