Re: [Qemu-devel] [PATCH COLO-Frame v15 27/38] migration/savevm: Add new helpers to process the different stages of loadvm

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> There are several stages during loadvm process. In different stage,
> migration incoming processes different section.
> We want to control these stages more accuracy, to optimize the COLO
> capability.
> 
> Here we add two new helper functions: qemu_loadvm_state_begin()
> and qemu_load_device_state().
> Besides, we make qemu_loadvm_state_main() API public.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

It's interesting; there's not that much difference between the two
routines, and they only wrap a little around the loadvm_state_main,
but I can see it makes it clearer at the place they're called.

Dave

> ---
> v14:
> - Split from patch 'COLO: Separate the process of saving/loading
>   ram and device state
> ---
>  include/sysemu/sysemu.h |  3 +++
>  migration/savevm.c      | 38 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 91eeda3..c0694a1 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -134,6 +134,9 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>                                             uint64_t *length_list);
>  
>  int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state_begin(QEMUFile *f);
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> +int qemu_load_device_state(QEMUFile *f);
>  
>  typedef enum DisplayType
>  {
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 9e3c18a..954e0a7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1249,8 +1249,6 @@ enum LoadVMExitCodes {
>      LOADVM_QUIT     =  1,
>  };
>  
> -static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> -
>  /* ------ incoming postcopy messages ------ */
>  /* 'advise' arrives before any transfers just to tell us that a postcopy
>   * *might* happen - it might be skipped if precopy transferred everything
> @@ -1832,7 +1830,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
>      return 0;
>  }
>  
> -static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
>  {
>      uint8_t section_type;
>      int ret;
> @@ -1965,6 +1963,40 @@ int qemu_loadvm_state(QEMUFile *f)
>      return ret;
>  }
>  
> +int qemu_loadvm_state_begin(QEMUFile *f)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    if (qemu_savevm_state_blocked(&local_err)) {
> +        error_report_err(local_err);
> +        return -EINVAL;
> +    }
> +    /* Load QEMU_VM_SECTION_START section */
> +    ret = qemu_loadvm_state_main(f, mis);
> +    if (ret < 0) {
> +        error_report("Failed to loadvm begin work: %d", ret);
> +    }
> +    return ret;
> +}
> +
> +int qemu_load_device_state(QEMUFile *f)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    int ret;
> +
> +    /* Load QEMU_VM_SECTION_FULL section */
> +    ret = qemu_loadvm_state_main(f, mis);
> +    if (ret < 0) {
> +        error_report("Failed to load device state: %d", ret);
> +        return ret;
> +    }
> +
> +    cpu_synchronize_all_post_init();
> +    return 0;
> +}
> +
>  void hmp_savevm(Monitor *mon, const QDict *qdict)
>  {
>      BlockDriverState *bs, *bs1;
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK