* zhanghailiang (zhang.zhanghaili...@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.zhanghaili...@huawei.com>
> Signed-off-by: Li Zhijian <lizhij...@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 <dgilb...@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 / dgilb...@redhat.com / Manchester, UK
