Re: [Qemu-devel] [RFC PATCH 15/17] COLO save: reuse migration bitmap under colo checkpoint

[Dr. David Alan Gilbert]

* Yang Hongyang (address@hidden) wrote:
> reuse migration bitmap under colo checkpoint, only send dirty pages
> per-checkpoint.
> 
> Signed-off-by: Yang Hongyang <address@hidden>
> ---
>  arch_init.c                        | 20 +++++++++++++++++++-
>  include/migration/migration-colo.h |  2 ++
>  migration-colo.c                   |  6 ++----
>  stubs/migration-colo.c             | 10 ++++++++++
>  4 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 8ddaf35..c84e6c8 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -52,6 +52,7 @@
>  #include "exec/ram_addr.h"
>  #include "hw/acpi/acpi.h"
>  #include "qemu/host-utils.h"
> +#include "migration/migration-colo.h"
>  
>  #ifdef DEBUG_ARCH_INIT
>  #define DPRINTF(fmt, ...) \
> @@ -769,6 +770,15 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      RAMBlock *block;
>      int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
>  
> +    /*
> +     * migration has already setup the bitmap, reuse it.
> +     */
> +    if (is_master()) {
> +        qemu_mutex_lock_ramlist();
> +        reset_ram_globals();
> +        goto out_setup;
> +    }
> +
>      mig_throttle_on = false;
>      dirty_rate_high_cnt = 0;
>      bitmap_sync_count = 0;
> @@ -828,6 +838,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      migration_bitmap_sync();
>      qemu_mutex_unlock_iothread();
>  
> +out_setup:
>      qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
>  
>      QTAILQ_FOREACH(block, &ram_list.blocks, next) {

Is it necessary to send the block list for each of your snapshots?

Dave

> @@ -937,7 +948,14 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>      }
>  
>      ram_control_after_iterate(f, RAM_CONTROL_FINISH);
> -    migration_end();
> +
> +    /*
> +     * Since we need to reuse dirty bitmap in colo,
> +     * don't cleanup the bitmap.
> +     */
> +    if (!migrate_use_colo() || migration_has_failed(migrate_get_current())) {
> +        migration_end();
> +    }
>  
>      qemu_mutex_unlock_ramlist();
>      qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> diff --git a/include/migration/migration-colo.h 
> b/include/migration/migration-colo.h
> index 861fa27..c286a60 100644
> --- a/include/migration/migration-colo.h
> +++ b/include/migration/migration-colo.h
> @@ -21,10 +21,12 @@ bool colo_supported(void);
>  /* save */
>  bool migrate_use_colo(void);
>  void colo_init_checkpointer(MigrationState *s);
> +bool is_master(void);
>  
>  /* restore */
>  bool restore_use_colo(void);
>  void restore_exit_colo(void);
> +bool is_slave(void);
>  
>  void colo_process_incoming_checkpoints(QEMUFile *f);
>  
> diff --git a/migration-colo.c b/migration-colo.c
> index 8596845..13a6a57 100644
> --- a/migration-colo.c
> +++ b/migration-colo.c
> @@ -222,8 +222,6 @@ static const QEMUFileOps colo_read_ops = {
>  };
>  
>  /* colo checkpoint control helper */
> -static bool is_master(void);
> -static bool is_slave(void);
>  
>  static void ctl_error_handler(void *opaque, int err)
>  {
> @@ -295,7 +293,7 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
>  
>  /* save */
>  
> -static bool is_master(void)
> +bool is_master(void)
>  {
>      MigrationState *s = migrate_get_current();
>      return (s->state == MIG_STATE_COLO);
> @@ -499,7 +497,7 @@ void colo_init_checkpointer(MigrationState *s)
>  
>  static Coroutine *colo;
>  
> -static bool is_slave(void)
> +bool is_slave(void)
>  {
>      return colo != NULL;
>  }
> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c
> index 55f0d37..ef65be6 100644
> --- a/stubs/migration-colo.c
> +++ b/stubs/migration-colo.c
> @@ -22,3 +22,13 @@ void colo_init_checkpointer(MigrationState *s)
>  void colo_process_incoming_checkpoints(QEMUFile *f)
>  {
>  }
> +
> +bool is_master(void)
> +{
> +    return false;
> +}
> +
> +bool is_slave(void)
> +{
> +    return false;
> +}
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK