Re: [RFC PATCH 4/5] migration: write up code to run fast snapshot load in qemu_loadvm_state

[Fabiano Rosas <farosas@suse.de>]

Aadeshveer Singh <aadeshveer07@gmail.com> writes:

> When both mapped-ram and postcopy-ram are set, divert from
> qemu_loadvm_state to run fast snapshot load
>
> Initialize postcopy RAM state and register RAM Blocks with userfaultfd
> via ram_postcopy_incoming_init() and postcopy_ram_incoming_setup().
> Launch fault thread before VM to serve faults for some hardwares
> emulation that need to read RAM (like vapic devices). Populate bitmaps
> and offset tables while reading file in qemu_loadvm_state_main. Call to
> qemu_loadvm_state_postcopy() which starts the VM using
> loadvm_postcopy_handle_run_bh() and launches eager load thread.
>
> Skip scheduling process_incoming_migration_bh() in
> process_incoming_migration_co(), for fast snapshot load as the state
> cleanup is managed by eager load thread on completion.
>
> Skip setting migration status to ACTIVE in process_incoming_migration_co
> and set set it to POSTCOPY_DEVICE in qemu_loadvm_state() itself.
>
> Remove the capability check that rejected mapped-ram and postcopy-ram
> being set simultaneously, as this combination now corresponds to fast
> snapshot load. The corresponding test will be updated in following
> patch.
>
> Signed-off-by: Aadeshveer Singh <aadeshveer07@gmail.com>
> ---
>  migration/migration.c | 10 ++++++---
>  migration/options.c   |  6 -----
>  migration/savevm.c    | 52 +++++++++++++++++++++++++++++++++++++++++--
>  migration/savevm.h    |  2 ++
>  4 files changed, 59 insertions(+), 11 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 074d3f2c69..e1ac310e20 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -756,8 +756,10 @@ process_incoming_migration_co(void *opaque)
>  
>      mis->largest_page_size = qemu_ram_pagesize_largest();
>      postcopy_state_set(POSTCOPY_INCOMING_NONE);
> -    migrate_set_state(&mis->state, MIGRATION_STATUS_SETUP,
> -                      MIGRATION_STATUS_ACTIVE);
> +    if (!migrate_fast_snapshot_load()) {
> +        migrate_set_state(&mis->state, MIGRATION_STATUS_SETUP,
> +                          MIGRATION_STATUS_ACTIVE);
> +    }
>  
>      mis->loadvm_co = qemu_coroutine_self();
>      ret = qemu_loadvm_state(mis->from_src_file, &local_err);
> @@ -786,7 +788,9 @@ process_incoming_migration_co(void *opaque)
>          colo_incoming_co();
>      }
>  
> -    migration_bh_schedule(process_incoming_migration_bh, mis);
> +    if (!migrate_fast_snapshot_load()) {
> +        migration_bh_schedule(process_incoming_migration_bh, mis);
> +    }

This inconspicuous 4 line hunk will cause bugs for years to come. =D

We should try to include the fast_snapshot actions within the incoming
BH so the paths are the same in both types of migration. Ideally we'd
merge all the BHs in a single one.

>      goto out;
>  
>  fail:
> diff --git a/migration/options.c b/migration/options.c
> index 5f80dd5b42..3f447cf7b2 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -732,12 +732,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>                         "Mapped-ram migration is incompatible with xbzrle");
>              return false;
>          }
> -
> -        if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
> -            error_setg(errp,
> -                       "Mapped-ram migration is incompatible with postcopy");
> -            return false;
> -        }

Just checking: Does this allow any type of migration that is not covered
by this series?

>      }
>  
>      /*
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 23adaf9dd9..f10cc3c2fc 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2959,6 +2959,32 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
>      return true;
>  }
>  
> +/*
> + * Starts the VM and launches the eager thread for fast snapshot load
> + */
> +int qemu_loadvm_state_postcopy(QEMUFile *f, MigrationIncomingState *mis,
> +                               Error **errp)
> +{
> +    ERRP_GUARD();
> +    int ret = 0;
> +
> +    postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
> +
> +    migration_bh_schedule(loadvm_postcopy_handle_run_bh, mis);
> +
> +    migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_DEVICE,
> +                      MIGRATION_STATUS_POSTCOPY_ACTIVE);
> +
> +    ret = postcopy_ram_eager_load_setup(mis);
> +    if (ret) {
> +        error_prepend(errp,
> +                      "Failed to setup eager load for fast snapshot load: ");
> +        return ret;
> +    }
> +
> +    return ret;
> +}
> +
>  int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
>                             Error **errp)
>  {
> @@ -3067,8 +3093,30 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
>  
>      cpu_synchronize_all_pre_loadvm();
>  
> -    ret = qemu_loadvm_state_main(f, mis, errp);
> -    qemu_event_set(&mis->main_thread_load_event);
> +    if (migrate_fast_snapshot_load()) {
> +        migrate_set_state(&mis->state, MIGRATION_STATUS_SETUP,
> +                          MIGRATION_STATUS_POSTCOPY_DEVICE);
> +
> +        if (ram_postcopy_incoming_init(mis, errp)) {
> +            return -EINVAL;
> +        }
> +
> +        postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
> +        if (postcopy_ram_incoming_setup(mis)) {
> +            return -EINVAL;
> +        }
> +
> +        ret = qemu_loadvm_state_main(f, mis, errp);
> +
> +        qemu_event_set(&mis->main_thread_load_event);
> +
> +        if (ret == 0) {
> +            ret = qemu_loadvm_state_postcopy(f, mis, errp);
> +        }
> +    } else {
> +        ret = qemu_loadvm_state_main(f, mis, errp);
> +        qemu_event_set(&mis->main_thread_load_event);
> +    }
>  
>      trace_qemu_loadvm_state_post_main(ret);
>  
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 96fdf96d4e..9656acd7fe 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -67,6 +67,8 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>  int qemu_save_device_state(QEMUFile *f, Error **errp);
>  int qemu_loadvm_state(QEMUFile *f, Error **errp);
>  void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> +int qemu_loadvm_state_postcopy(QEMUFile *f, MigrationIncomingState *mis,
> +                               Error **errp);
>  int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
>                             Error **errp);
>  int qemu_load_device_state(QEMUFile *f, Error **errp);