Re: [Qemu-devel] [PATCH v7 28/42] Postcopy: Postcopy startup in migration thread

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Rework the migration thread to setup and start postcopy.
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  include/migration/migration.h |   3 +
>  migration/migration.c         | 166 
> ++++++++++++++++++++++++++++++++++++++++--
>  trace-events                  |   4 +
>  3 files changed, 167 insertions(+), 6 deletions(-)
>
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e6585c5..68a1731 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -120,6 +120,9 @@ struct MigrationState
>      /* Flag set once the migration has been asked to enter postcopy */
>      bool start_postcopy;
>  
> +    /* Flag set once the migration thread is running (and needs joining) */
> +    bool started_migration_thread;
> +

migration_thread_started?

> +
> +    /*
> +     * send rest of state - note things that are doing postcopy
> +     * will notice we're in POSTCOPY_ACTIVE and not actually
> +     * wrap their state up here
> +     */
> +    qemu_file_set_rate_limit(ms->file, INT64_MAX);

Do we undo this?  or, are we sure that it is ok to maximize network
output?

> +    /* Ping just for debugging, helps line traces up */
> +    qemu_savevm_send_ping(ms->file, 2);

Change the values 1, 2, 3 to constants?

> +     * We need to leave the fd free for page transfers during the
> +     * loading of the device state, so wrap all the remaining
> +     * commands and state into a package that gets sent in one go
> +     */
> +    QEMUFile *fb = qemu_bufopen("w", NULL);
> +    if (!fb) {
> +        error_report("Failed to create buffered file");
> +        goto fail;
> +    }
> +
> +    qemu_savevm_state_complete_precopy(fb);
> +    qemu_savevm_send_ping(fb, 3);
> +
> +    qemu_savevm_send_postcopy_run(fb);
> +
> +    /* <><> end of stuff going into the package */
> +    qsb = qemu_buf_get(fb);
> +
> +    /* Now send that blob */
> +    if (qemu_savevm_send_packaged(ms->file, qsb)) {
> +        goto fail_closefb;
> +    }
> +    qemu_fclose(fb);

Why can't we send this directly without the extra copy?
I guess that there are some missing/extra section starts/end whatever?
Anything specific?

> +    ms->downtime =  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - time_at_stop;

Now, that we are here, is there a counter of the time that takes the
postcopy stage?  Just curious.
> +/*
>   * Master migration thread on the source VM.
>   * It drives the migration and pumps the data down the outgoing channel.
>   */
>  static void *migration_thread(void *opaque)
>  {
>      MigrationState *s = opaque;
> +    /* Used by the bandwidth calcs, updated later */
>      int64_t initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>      int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>      int64_t initial_bytes = 0;
>      int64_t max_size = 0;
>      int64_t start_time = initial_time;
>      bool old_vm_running = false;
> +    bool entered_postcopy = false;
> +    /* The active state we expect to be in; ACTIVE or POSTCOPY_ACTIVE */
> +    enum MigrationStatus current_active_type = MIGRATION_STATUS_ACTIVE;

current_active_state?