From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56383) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGAMh-0006OB-Og for qemu-devel@nongnu.org; Mon, 04 Jan 2016 14:00:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGAMd-0000go-HW for qemu-devel@nongnu.org; Mon, 04 Jan 2016 14:00:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39456) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGAMd-0000gK-Af for qemu-devel@nongnu.org; Mon, 04 Jan 2016 14:00:19 -0500 Date: Mon, 4 Jan 2016 19:00:11 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20160104190011.GC12368@work-vm> References: <1451372975-5048-1-git-send-email-zhang.zhanghailiang@huawei.com> <1451372975-5048-18-git-send-email-zhang.zhanghailiang@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1451372975-5048-18-git-send-email-zhang.zhanghailiang@huawei.com> 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: zhanghailiang 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 * 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. 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