From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a94qs-0005NY-UC for qemu-devel@nongnu.org; Wed, 16 Dec 2015 00:42:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a94qo-00012F-Nm for qemu-devel@nongnu.org; Wed, 16 Dec 2015 00:42:14 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:36094) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a94qn-00011j-NM for qemu-devel@nongnu.org; Wed, 16 Dec 2015 00:42:10 -0500 References: <1450167779-9960-1-git-send-email-zhang.zhanghailiang@huawei.com> <1450167779-9960-6-git-send-email-zhang.zhanghailiang@huawei.com> <20151215173632.GI2500@work-vm> From: Hailiang Zhang Message-ID: <5670F89C.5040800@huawei.com> Date: Wed, 16 Dec 2015 13:37:32 +0800 MIME-Version: 1.0 In-Reply-To: <20151215173632.GI2500@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v12 05/38] migration: Add state records for migration incoming 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/12/16 1:36, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> For migration destination, we also need to know its state, >> we will use it in COLO. >> >> Here we add a new member 'state' for MigrationIncomingState, >> and also use migrate_set_state() to modify its value. >> >> Signed-off-by: zhanghailiang >> Reviewed-by: Dr. David Alan Gilbert > > Actually note there is a bug here; see below > >> --- >> v11: >> - Split exporting migrate_set_state() part into a new patch (Juan's suggestion) >> >> Signed-off-by: zhanghailiang >> --- >> include/migration/migration.h | 1 + >> migration/migration.c | 14 +++++++++----- >> 2 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index 4b19e80..99dfa92 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -105,6 +105,7 @@ struct MigrationIncomingState { >> QemuMutex rp_mutex; /* We send replies from multiple threads */ >> void *postcopy_tmp_page; >> >> + int state; >> /* See savevm.c */ >> LoadStateEntry_Head loadvm_handlers; >> }; >> diff --git a/migration/migration.c b/migration/migration.c >> index c9cd80d..d58ce98 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -112,6 +112,7 @@ MigrationIncomingState *migration_incoming_state_new(QEMUFile* f) >> { >> mis_current = g_new0(MigrationIncomingState, 1); >> mis_current->from_src_file = f; >> + mis_current->state = MIGRATION_STATUS_NONE; >> QLIST_INIT(&mis_current->loadvm_handlers); >> qemu_mutex_init(&mis_current->rp_mutex); >> qemu_event_init(&mis_current->main_thread_load_event, false); >> @@ -332,8 +333,8 @@ static void process_incoming_migration_co(void *opaque) >> >> mis = migration_incoming_state_new(f); >> postcopy_state_set(POSTCOPY_INCOMING_NONE); >> - migrate_generate_event(MIGRATION_STATUS_ACTIVE); >> - >> + migrate_set_state(&mis->state, MIGRATION_STATUS_NONE, >> + MIGRATION_STATUS_ACTIVE); >> ret = qemu_loadvm_state(f); >> >> ps = postcopy_state_get(); >> @@ -362,7 +363,8 @@ static void process_incoming_migration_co(void *opaque) >> migration_incoming_state_destroy(); > > We're freeing mis now - we can't use the state later! > >> >> if (ret < 0) { >> - migrate_generate_event(MIGRATION_STATUS_FAILED); >> + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> + MIGRATION_STATUS_FAILED); >> error_report("load of migration failed: %s", strerror(-ret)); >> migrate_decompress_threads_join(); >> exit(EXIT_FAILURE); >> @@ -371,7 +373,8 @@ static void process_incoming_migration_co(void *opaque) >> /* Make sure all file formats flush their mutable metadata */ >> bdrv_invalidate_cache_all(&local_err); >> if (local_err) { >> - migrate_generate_event(MIGRATION_STATUS_FAILED); >> + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> + MIGRATION_STATUS_FAILED); >> error_report_err(local_err); >> migrate_decompress_threads_join(); >> exit(EXIT_FAILURE); >> @@ -403,7 +406,8 @@ static void process_incoming_migration_co(void *opaque) >> * observer sees this event they might start to prod at the VM assuming >> * it's ready to use. >> */ >> - migrate_generate_event(MIGRATION_STATUS_COMPLETED); >> + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> + MIGRATION_STATUS_COMPLETED); > > So I moved the migration_incoming_state_destroy() to here in my world. > Yes, it is bug, thank you for fixing it. > Dave > >> } >> >> void process_incoming_migration(QEMUFile *f) >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >