From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9tjZ-00020u-KH for qemu-devel@nongnu.org; Fri, 18 Dec 2015 07:02:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9tjT-0000fs-QX for qemu-devel@nongnu.org; Fri, 18 Dec 2015 07:02:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43868) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9tjT-0000fo-Ju for qemu-devel@nongnu.org; Fri, 18 Dec 2015 07:01:59 -0500 Date: Fri, 18 Dec 2015 12:01:53 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20151218120152.GC2459@work-vm> References: <1450167779-9960-1-git-send-email-zhang.zhanghailiang@huawei.com> <1450167779-9960-33-git-send-email-zhang.zhanghailiang@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1450167779-9960-33-git-send-email-zhang.zhanghailiang@huawei.com> Subject: Re: [Qemu-devel] [PATCH COLO-Frame v12 32/38] COLO: Split qemu_savevm_state_begin out of checkpoint process List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang 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 * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: > It is unnecessary to call qemu_savevm_state_begin() in every checkponit process. > It mainly sets up devices and does the first device state pass. These data will > not change during the later checkpoint process. So, we split it out of > colo_do_checkpoint_transaction(), in this way, we can reduce these data > transferring in the later checkpoint. > > Signed-off-by: zhanghailiang > Signed-off-by: Li Zhijian > --- > migration/colo.c | 51 +++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 37 insertions(+), 14 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index d253d64..4571359 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -276,15 +276,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s, > if (ret < 0) { > goto out; > } > - /* Disable block migration */ > - s->params.blk = 0; > - s->params.shared = 0; > - qemu_savevm_state_begin(s->to_dst_file, &s->params); > - ret = qemu_file_get_error(s->to_dst_file); > - if (ret < 0) { > - error_report("save vm state begin error\n"); > - goto out; > - } > > qemu_mutex_lock_iothread(); > /* Note: device state is saved into buffer */ > @@ -348,6 +339,21 @@ out: > return ret; > } > > +static int colo_prepare_before_save(MigrationState *s) > +{ > + int ret; > + /* Disable block migration */ > + s->params.blk = 0; > + s->params.shared = 0; > + qemu_savevm_state_begin(s->to_dst_file, &s->params); > + ret = qemu_file_get_error(s->to_dst_file); > + if (ret < 0) { > + error_report("save vm state begin error\n"); '\n' again not needed. > + return ret; > + } > + return 0; > +} > + > static void colo_process_checkpoint(MigrationState *s) > { > QEMUSizedBuffer *buffer = NULL; > @@ -363,6 +369,11 @@ static void colo_process_checkpoint(MigrationState *s) > goto out; > } > > + ret = colo_prepare_before_save(s); > + if (ret < 0) { > + goto out; > + } > + > /* > * Wait for Secondary finish loading vm states and enter COLO > * restore. > @@ -484,6 +495,18 @@ static int colo_wait_handle_cmd(QEMUFile *f, int *checkpoint_request) > } > } > > +static int colo_prepare_before_load(QEMUFile *f) > +{ > + int ret; > + > + ret = qemu_loadvm_state_begin(f); > + if (ret < 0) { > + error_report("load vm state begin error, ret=%d", ret); > + return ret; You can simplify these returns; remove this line. > + } > + return 0; and make this return ret; same in a few places. Other than those minor issues; Reviewed-by: Dr. David Alan Gilbert > +} > + > void *colo_process_incoming_thread(void *opaque) > { > MigrationIncomingState *mis = opaque; > @@ -522,6 +545,11 @@ void *colo_process_incoming_thread(void *opaque) > goto out; > } > > + ret = colo_prepare_before_load(mis->from_src_file); > + if (ret < 0) { > + goto out; > + } > + > ret = colo_put_cmd(mis->to_src_file, COLO_COMMAND_CHECKPOINT_READY); > if (ret < 0) { > goto out; > @@ -556,11 +584,6 @@ void *colo_process_incoming_thread(void *opaque) > goto out; > } > > - ret = qemu_loadvm_state_begin(mis->from_src_file); > - if (ret < 0) { > - error_report("load vm state begin error, ret=%d", ret); > - goto out; > - } > ret = qemu_load_ram_state(mis->from_src_file); > if (ret < 0) { > error_report("load ram state error"); > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK