From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48109) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1TkY-0001Fv-RM for qemu-devel@nongnu.org; Wed, 25 Nov 2015 01:40:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1TkV-0006zk-I2 for qemu-devel@nongnu.org; Wed, 25 Nov 2015 01:40:18 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:32308) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1TkT-0006z2-Ny for qemu-devel@nongnu.org; Wed, 25 Nov 2015 01:40:15 -0500 References: <1448357149-17572-1-git-send-email-zhang.zhanghailiang@huawei.com> <1448357149-17572-8-git-send-email-zhang.zhanghailiang@huawei.com> <20151124181439.GH2490@work-vm> From: zhanghailiang Message-ID: <565557AB.6030503@huawei.com> Date: Wed, 25 Nov 2015 14:39:39 +0800 MIME-Version: 1.0 In-Reply-To: <20151124181439.GH2490@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v11 07/39] migration: Integrate COLO checkpoint process into loadvm 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, hongyang.yang@easystack.cn On 2015/11/25 2:14, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> Switch from normal migration loadvm process into COLO checkpoint process if >> COLO mode is enabled. >> We add three new members to struct MigrationIncomingState, 'have_colo_incoming_thread' >> and 'colo_incoming_thread' record the colo related threads for secondary VM, >> 'migration_incoming_co' records the original migration incoming coroutine. >> >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian > > Minor comment that needs fixing, see below, but otherwise: > > Reviewed-by: Dr. David Alan Gilbert > >> --- >> v11: >> - We moved the place of bdrv_invalidate_cache_all(), but done the deleting work >> in other patch. Fix it. >> - Add documentation for colo in 'MigrationStatus' (Eric's review comment) >> v10: >> - fix a bug about fd leak which is found by Dave. >> --- >> include/migration/colo.h | 7 +++++++ >> include/migration/migration.h | 7 +++++++ >> migration/colo-comm.c | 10 ++++++++++ >> migration/colo.c | 22 ++++++++++++++++++++++ >> migration/migration.c | 31 +++++++++++++++++++++---------- >> qapi-schema.json | 2 ++ >> stubs/migration-colo.c | 10 ++++++++++ >> 7 files changed, 79 insertions(+), 10 deletions(-) >> >> diff --git a/include/migration/colo.h b/include/migration/colo.h >> index f462f06..2676c4a 100644 >> --- a/include/migration/colo.h >> +++ b/include/migration/colo.h >> @@ -15,6 +15,8 @@ >> >> #include "qemu-common.h" >> #include "migration/migration.h" >> +#include "qemu/coroutine_int.h" >> +#include "qemu/thread.h" >> >> bool colo_supported(void); >> void colo_info_mig_init(void); >> @@ -22,4 +24,9 @@ void colo_info_mig_init(void); >> void migrate_start_colo_process(MigrationState *s); >> bool migration_in_colo_state(void); >> >> +/* loadvm */ >> +bool migration_incoming_enable_colo(void); >> +void migration_incoming_exit_colo(void); >> +void *colo_process_incoming_thread(void *opaque); >> +bool migration_incoming_in_colo_state(void); >> #endif >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index 99dfa92..a57a734 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -22,6 +22,7 @@ >> #include "migration/vmstate.h" >> #include "qapi-types.h" >> #include "exec/cpu-common.h" >> +#include "qemu/coroutine_int.h" >> >> #define QEMU_VM_FILE_MAGIC 0x5145564d >> #define QEMU_VM_FILE_VERSION_COMPAT 0x00000002 >> @@ -106,6 +107,12 @@ struct MigrationIncomingState { >> void *postcopy_tmp_page; >> >> int state; >> + >> + bool have_colo_incoming_thread; >> + QemuThread colo_incoming_thread; >> + /* The coroutine we should enter (back) after failover */ >> + Coroutine *migration_incoming_co; >> + >> /* See savevm.c */ >> LoadStateEntry_Head loadvm_handlers; >> }; >> diff --git a/migration/colo-comm.c b/migration/colo-comm.c >> index fb407e0..30df3d3 100644 >> --- a/migration/colo-comm.c >> +++ b/migration/colo-comm.c >> @@ -48,3 +48,13 @@ void colo_info_mig_init(void) >> { >> vmstate_register(NULL, 0, &colo_state, &colo_info); >> } >> + >> +bool migration_incoming_enable_colo(void) >> +{ >> + return colo_info.colo_requested; >> +} >> + >> +void migration_incoming_exit_colo(void) >> +{ >> + colo_info.colo_requested = 0; >> +} >> diff --git a/migration/colo.c b/migration/colo.c >> index cf0ccb8..6880aa0 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -13,6 +13,7 @@ >> #include "sysemu/sysemu.h" >> #include "migration/colo.h" >> #include "trace.h" >> +#include "qemu/error-report.h" >> >> bool colo_supported(void) >> { >> @@ -26,6 +27,13 @@ bool migration_in_colo_state(void) >> return (s->state == MIGRATION_STATUS_COLO); >> } >> >> +bool migration_incoming_in_colo_state(void) >> +{ >> + MigrationIncomingState *mis = migration_incoming_get_current(); >> + >> + return mis && (mis->state == MIGRATION_STATUS_COLO); >> +} >> + >> static void colo_process_checkpoint(MigrationState *s) >> { >> qemu_mutex_lock_iothread(); >> @@ -47,3 +55,17 @@ void migrate_start_colo_process(MigrationState *s) >> colo_process_checkpoint(s); >> qemu_mutex_lock_iothread(); >> } >> + >> +void *colo_process_incoming_thread(void *opaque) >> +{ >> + MigrationIncomingState *mis = opaque; >> + >> + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> + MIGRATION_STATUS_COLO); >> + >> + /* TODO: COLO checkpoint restore loop */ >> + >> + migration_incoming_exit_colo(); >> + >> + return NULL; >> +} >> diff --git a/migration/migration.c b/migration/migration.c >> index 46fe8a9..41eac0d 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -358,6 +358,27 @@ static void process_incoming_migration_co(void *opaque) >> /* Else if something went wrong then just fall out of the normal exit */ >> } >> >> + if (!ret) { >> + /* Make sure all file formats flush their mutable metadata */ >> + bdrv_invalidate_cache_all(&local_err); >> + if (local_err) { >> + error_report_err(local_err); >> + migrate_decompress_threads_join(); >> + exit(EXIT_FAILURE); >> + } >> + } >> + /* we get colo info, and know if we are in colo mode */ >> + if (!ret && migration_incoming_enable_colo()) { >> + mis->migration_incoming_co = qemu_coroutine_self(); >> + qemu_thread_create(&mis->colo_incoming_thread, "colo incoming", >> + colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE); >> + mis->have_colo_incoming_thread = true; >> + qemu_coroutine_yield(); >> + >> + /* Wait checkpoint incoming thread exit before free resource */ >> + qemu_thread_join(&mis->colo_incoming_thread); >> + } >> + >> qemu_fclose(f); >> free_xbzrle_decoded_buf(); >> migration_incoming_state_destroy(); >> @@ -370,16 +391,6 @@ static void process_incoming_migration_co(void *opaque) >> exit(EXIT_FAILURE); >> } >> >> - /* Make sure all file formats flush their mutable metadata */ >> - bdrv_invalidate_cache_all(&local_err); >> - if (local_err) { >> - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> - MIGRATION_STATUS_FAILED); >> - error_report_err(local_err); >> - migrate_decompress_threads_join(); >> - exit(EXIT_FAILURE); >> - } >> - >> /* >> * This must happen after all error conditions are dealt with and >> * we're sure the VM is going to be running on this host. >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 24b35f3..c2f3b63 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -438,6 +438,8 @@ >> # >> # @failed: some error occurred during migration process. >> # >> +# @colo: VM is in the process of fault tolerance. (since 2.6) >> +# >> # Since: 2.3 >> # >> ## > > That belongs in the previous patch I think; the previous patch > added before the @failed, but this change is correct, it just > needs to go with the actual addition. > Good catch, i will fix it in next version. Thanks. zhanghailiang > >> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c >> index acddca6..c12516e 100644 >> --- a/stubs/migration-colo.c >> +++ b/stubs/migration-colo.c >> @@ -22,6 +22,16 @@ bool migration_in_colo_state(void) >> return false; >> } >> >> +bool migration_incoming_in_colo_state(void) >> +{ >> + return false; >> +} >> + >> void migrate_start_colo_process(MigrationState *s) >> { >> } >> + >> +void *colo_process_incoming_thread(void *opaque) >> +{ >> + return NULL; >> +} >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >