Message ID | 1450167779-9960-33-git-send-email-zhang.zhanghailiang@huawei.com |
---|---|
State | New |
Headers | show |
* 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 <zhang.zhanghailiang@huawei.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- > 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 <dgilbert@redhat.com> > +} > + > 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
On 2015/12/18 20:01, Dr. David Alan Gilbert wrote: > * 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 <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> 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; > I will fix them all in next version. > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Thanks. Hailiang > >> +} >> + >> 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 > > . >
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"); + 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; + } + return 0; +} + 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");