From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44164) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIR63-0005uW-JQ for qemu-devel@nongnu.org; Sun, 10 Jan 2016 20:16:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIR5z-0003wT-Bi for qemu-devel@nongnu.org; Sun, 10 Jan 2016 20:16:35 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:40742) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIR5y-0003u8-Gt for qemu-devel@nongnu.org; Sun, 10 Jan 2016 20:16:31 -0500 References: <1451372975-5048-1-git-send-email-zhang.zhanghailiang@huawei.com> <1451372975-5048-18-git-send-email-zhang.zhanghailiang@huawei.com> <20160104190011.GC12368@work-vm> From: Hailiang Zhang Message-ID: <56930251.7010806@huawei.com> Date: Mon, 11 Jan 2016 09:16:01 +0800 MIME-Version: 1.0 In-Reply-To: <20160104190011.GC12368@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v13 17/39] COLO: Load VMState into qsb before restore it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: xiecl.fnst@cn.fujitsu.com, 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, zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 2016/1/5 3:00, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> We should not destroy the state of SVM (Secondary VM) until we receive the whole >> state from the PVM (Primary VM), in case the primary fails in the middle of sending >> the state, so, here we cache the device state in Secondary before restore it. >> >> Besides, we should call qemu_system_reset() before load VM state, >> which can ensure the data is intact. >> >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian >> Signed-off-by: Gonglei >> Reviewed-by: Dr. David Alan Gilbert >> Cc: Dr. David Alan Gilbert >> --- >> v13: >> - Fix the define of colo_get_cmd_value() to use 'Error **errp' instead of >> return value. >> v12: >> - Use the new helper colo_get_cmd_value() instead of colo_ctl_get() >> --- >> migration/colo.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 72 insertions(+), 2 deletions(-) >> >> diff --git a/migration/colo.c b/migration/colo.c >> index 925eb3c..8414feb 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -114,6 +114,28 @@ static void colo_get_check_cmd(QEMUFile *f, COLOCommand expect_cmd, >> } >> } >> >> +static uint64_t colo_get_cmd_value(QEMUFile *f, uint32_t expect_cmd, >> + Error **errp) >> +{ >> + Error *local_err = NULL; >> + uint64_t value; >> + int ret; >> + >> + colo_get_check_cmd(f, expect_cmd, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return 0; >> + } >> + >> + value = qemu_get_be64(f); >> + ret = qemu_file_get_error(f); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Failed to get value for COlO commnd: %s", > > Tiny typo; you've used 'CO*l*O' rather than 'COLO' - actually all your other errors say 'colo' > so it's probably best to standardise. > OK, i will fix them all in next version, thanks. > Dave > >> + COLOCommand_lookup[expect_cmd]); >> + } >> + return value; >> +} >> + >> static int colo_do_checkpoint_transaction(MigrationState *s, >> QEMUSizedBuffer *buffer) >> { >> @@ -297,6 +319,10 @@ static void colo_wait_handle_cmd(QEMUFile *f, int *checkpoint_request, >> void *colo_process_incoming_thread(void *opaque) >> { >> MigrationIncomingState *mis = opaque; >> + QEMUFile *fb = NULL; >> + QEMUSizedBuffer *buffer = NULL; /* Cache incoming device state */ >> + uint64_t total_size; >> + uint64_t value; >> Error *local_err = NULL; >> int ret; >> >> @@ -320,6 +346,12 @@ void *colo_process_incoming_thread(void *opaque) >> goto out; >> } >> >> + buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE); >> + if (buffer == NULL) { >> + error_report("Failed to allocate colo buffer!"); >> + goto out; >> + } >> + >> colo_put_cmd(mis->to_src_file, COLO_COMMAND_CHECKPOINT_READY, >> &local_err); >> if (local_err) { >> @@ -347,7 +379,21 @@ void *colo_process_incoming_thread(void *opaque) >> goto out; >> } >> >> - /* TODO: read migration data into colo buffer */ >> + /* read the VM state total size first */ >> + value = colo_get_cmd_value(mis->from_src_file, >> + COLO_COMMAND_VMSTATE_SIZE, &local_err); >> + if (local_err) { >> + goto out; >> + } >> + >> + /* read vm device state into colo buffer */ >> + total_size = qsb_fill_buffer(buffer, mis->from_src_file, value); >> + if (total_size != value) { >> + error_report("Got %lu VMState data, less than expected %lu", >> + total_size, value); >> + ret = -EINVAL; >> + goto out; >> + } >> >> colo_put_cmd(mis->to_src_file, COLO_COMMAND_VMSTATE_RECEIVED, >> &local_err); >> @@ -355,13 +401,32 @@ void *colo_process_incoming_thread(void *opaque) >> goto out; >> } >> >> - /* TODO: load vm state */ >> + /* open colo buffer for read */ >> + fb = qemu_bufopen("r", buffer); >> + if (!fb) { >> + error_report("Can't open colo buffer for read"); >> + goto out; >> + } >> + >> + qemu_mutex_lock_iothread(); >> + qemu_system_reset(VMRESET_SILENT); >> + if (qemu_loadvm_state(fb) < 0) { >> + error_report("COLO: loadvm failed"); >> + qemu_mutex_unlock_iothread(); >> + goto out; >> + } >> + qemu_mutex_unlock_iothread(); >> + >> + /* TODO: flush vm state */ >> >> colo_put_cmd(mis->to_src_file, COLO_COMMAND_VMSTATE_LOADED, >> &local_err); >> if (local_err) { >> goto out; >> } >> + >> + qemu_fclose(fb); >> + fb = NULL; >> } >> >> out: >> @@ -370,6 +435,11 @@ out: >> error_report_err(local_err); >> } >> >> + if (fb) { >> + qemu_fclose(fb); >> + } >> + qsb_free(buffer); >> + >> qemu_mutex_lock_iothread(); >> colo_release_ram_cache(); >> qemu_mutex_unlock_iothread(); >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >