Re: [Qemu-devel] [PATCH COLO-Frame v8 11/34] COLO: Save VM state to slave when do checkpoint

[Dr. David Alan Gilbert]

* zhanghailiang (address@hidden) wrote:
> We should save PVM's RAM/device to slave when needed.
> 
> For VM state, we will cache them in slave, we use QEMUSizedBuffer
> to store the data, we need know the data size of VM state, so in master,
> we use qsb to store VM state temporarily, and then migrate the data to
> slave.
> 
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Yang Hongyang <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> Signed-off-by: Li Zhijian <address@hidden>

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> ---
>  include/migration/migration.h |  7 +++++
>  migration/colo.c              | 60 
> ++++++++++++++++++++++++++++++++++++++++---
>  migration/ram.c               | 46 +++++++++++++++++++++++++--------
>  migration/savevm.c            |  2 +-
>  4 files changed, 101 insertions(+), 14 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 0548371..f14c321 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -66,6 +66,11 @@ MigrationIncomingState 
> *migration_incoming_get_current(void);
>  MigrationIncomingState *migration_incoming_state_new(QEMUFile *f);
>  void migration_incoming_state_destroy(void);
>  
> +typedef struct COLOState COLOState;
> +struct COLOState {
> +    QEMUSizedBuffer *buffer;
> +};
> +
>  struct MigrationState
>  {
>      int64_t bandwidth_limit;
> @@ -88,6 +93,8 @@ struct MigrationState
>      int64_t xbzrle_cache_size;
>      int64_t setup_time;
>      int64_t dirty_sync_count;
> +
> +    COLOState colo_state;
>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/colo.c b/migration/colo.c
> index 4ba6f65..a77f23b 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -65,6 +65,8 @@ const char * const COLOCommand_lookup[] = {
>  };
>  
>  static QEMUBH *colo_bh;
> +/* colo buffer */
> +#define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
>  
>  bool colo_supported(void)
>  {
> @@ -139,6 +141,8 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
>  static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile 
> *control)
>  {
>      int ret;
> +    size_t size;
> +    QEMUFile *trans = NULL;
>  
>      ret = colo_ctl_put(s->file, COLO_CHECKPOINT_NEW);
>      if (ret < 0) {
> @@ -149,15 +153,47 @@ static int 
> colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control)
>      if (ret < 0) {
>          goto out;
>      }
> +    /* Reset colo buffer and open it for write */
> +    qsb_set_length(s->colo_state.buffer, 0);
> +    trans = qemu_bufopen("w", s->colo_state.buffer);
> +    if (!trans) {
> +        error_report("Open colo buffer for write failed");
> +        goto out;
> +    }
> +
> +    /* suspend and save vm state to colo buffer */
> +    qemu_mutex_lock_iothread();
> +    vm_stop_force_state(RUN_STATE_COLO);
> +    qemu_mutex_unlock_iothread();
> +    trace_colo_vm_state_change("run", "stop");
> +
> +    /* Disable block migration */
> +    s->params.blk = 0;
> +    s->params.shared = 0;
> +    qemu_savevm_state_begin(trans, &s->params);
> +    qemu_mutex_lock_iothread();
> +    qemu_savevm_state_complete(trans);
> +    qemu_mutex_unlock_iothread();
>  
> -    /* TODO: suspend and save vm state to colo buffer */
> +    qemu_fflush(trans);
>  
>      ret = colo_ctl_put(s->file, COLO_CHECKPOINT_SEND);
>      if (ret < 0) {
>          goto out;
>      }
> +    /* we send the total size of the vmstate first */
> +    size = qsb_get_length(s->colo_state.buffer);
> +    ret = colo_ctl_put(s->file, size);
> +    if (ret < 0) {
> +        goto out;
> +    }
>  
> -    /* TODO: send vmstate to Secondary */
> +    qsb_put_buffer(s->file, s->colo_state.buffer, size);
> +    qemu_fflush(s->file);
> +    ret = qemu_file_get_error(s->file);
> +    if (ret < 0) {
> +        goto out;
> +    }
>  
>      ret = colo_ctl_get(control, COLO_CHECKPOINT_RECEIVED);
>      if (ret < 0) {
> @@ -169,9 +205,18 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s, QEMUFile *control)
>          goto out;
>      }
>  
> -    /* TODO: resume Primary */
> +    ret = 0;
> +    /* resume master */
> +    qemu_mutex_lock_iothread();
> +    vm_start();
> +    qemu_mutex_unlock_iothread();
> +    trace_colo_vm_state_change("stop", "run");
>  
>  out:
> +    if (trans) {
> +        qemu_fclose(trans);
> +    }
> +
>      return ret;
>  }
>  
> @@ -196,6 +241,12 @@ static void *colo_thread(void *opaque)
>          goto out;
>      }
>  
> +    s->colo_state.buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
> +    if (s->colo_state.buffer == NULL) {
> +        error_report("Failed to allocate colo buffer!");
> +        goto out;
> +    }
> +
>      qemu_mutex_lock_iothread();
>      vm_start();
>      qemu_mutex_unlock_iothread();
> @@ -212,6 +263,9 @@ out:
>      migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
>                        MIGRATION_STATUS_COMPLETED);
>  
> +    qsb_free(s->colo_state.buffer);
> +    s->colo_state.buffer = NULL;
> +
>      if (colo_control) {
>          qemu_fclose(colo_control);
>      }
> diff --git a/migration/ram.c b/migration/ram.c
> index 7f007e6..68980be 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -38,6 +38,7 @@
>  #include "trace.h"
>  #include "exec/ram_addr.h"
>  #include "qemu/rcu_queue.h"
> +#include "migration/colo.h"
>  
>  #ifdef DEBUG_MIGRATION_RAM
>  #define DPRINTF(fmt, ...) \
> @@ -1090,15 +1091,8 @@ void migration_bitmap_extend(ram_addr_t old, 
> ram_addr_t new)
>      }
>  }
>  
> -/* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> - * long-running RCU critical section.  When rcu-reclaims in the code
> - * start to become numerous it will be necessary to reduce the
> - * granularity of these critical sections.
> - */
> -
> -static int ram_save_setup(QEMUFile *f, void *opaque)
> +static int ram_save_init_globals(void)
>  {
> -    RAMBlock *block;
>      int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
>  
>      mig_throttle_on = false;
> @@ -1158,6 +1152,31 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      migration_bitmap_sync();
>      qemu_mutex_unlock_ramlist();
>      qemu_mutex_unlock_iothread();
> +    rcu_read_unlock();
> +
> +    return 0;
> +}
> +
> +/* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> + * long-running RCU critical section.  When rcu-reclaims in the code
> + * start to become numerous it will be necessary to reduce the
> + * granularity of these critical sections.
> + */
> +
> +static int ram_save_setup(QEMUFile *f, void *opaque)
> +{
> +    RAMBlock *block;
> +
> +    /*
> +     * migration has already setup the bitmap, reuse it.
> +     */
> +    if (!migration_in_colo_state()) {
> +        if (ram_save_init_globals() < 0) {
> +            return -1;
> +         }
> +    }
> +
> +    rcu_read_lock();
>  
>      qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
>  
> @@ -1257,7 +1276,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>      while (true) {
>          int pages;
>  
> -        pages = ram_find_and_save_block(f, true, &bytes_transferred);
> +        pages = ram_find_and_save_block(f, !migration_in_colo_state(),
> +                                        &bytes_transferred);
>          /* no more blocks to sent */
>          if (pages == 0) {
>              break;
> @@ -1268,8 +1288,14 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>      ram_control_after_iterate(f, RAM_CONTROL_FINISH);
>  
>      rcu_read_unlock();
> +    /*
> +     * Since we need to reuse dirty bitmap in colo,
> +     * don't cleanup the bitmap.
> +     */
> +    if (!migrate_enable_colo() || 
> migration_has_failed(migrate_get_current())) {
> +        migration_end();
> +    }
>  
> -    migration_end();
>      qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>  
>      return 0;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 81dbe58..7f91136 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -48,7 +48,7 @@
>  #include "qemu/iov.h"
>  #include "block/snapshot.h"
>  #include "block/qapi.h"
> -
> +#include "migration/colo.h"
>  
>  #ifndef ETH_P_RARP
>  #define ETH_P_RARP 0x8035
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK