[RFC,17/29] migration: rebuild channel on source

This patch detects the "resume" flag of migration command, rebuild the
channels only if the flag is set.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 52 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 11 deletions(-)

* Peter Xu (peterx@redhat.com) wrote:
> This patch detects the "resume" flag of migration command, rebuild the
> channels only if the flag is set.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 52 ++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 36ff8c3..64de0ee 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1244,6 +1244,15 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      MigrationState *s = migrate_get_current();
>      const char *p;
>  
> +    if (has_resume && resume) {
> +        if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) {
> +            error_setg(errp, "Cannot resume if there is no "
> +                       "paused migration");
> +            return;
> +        }
> +        goto do_resume;
> +    }
> +
>      if (migration_is_setup_or_active(s->state) ||
>          s->state == MIGRATION_STATUS_CANCELLING ||
>          s->state == MIGRATION_STATUS_COLO) {
> @@ -1279,6 +1288,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>  
>      s = migrate_init();
>  
> +do_resume:

Can we find a way to avoid this label?
Perhaps split the bottom half of this function out into a separate
function?

Dave

>      if (strstart(uri, "tcp:", &p)) {
>          tcp_start_outgoing_migration(s, p, &local_err);
>  #ifdef CONFIG_RDMA
> @@ -1700,7 +1710,8 @@ out:
>      return NULL;
>  }
>  
> -static int open_return_path_on_source(MigrationState *ms)
> +static int open_return_path_on_source(MigrationState *ms,
> +                                      bool create_thread)
>  {
>  
>      ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
> @@ -1709,6 +1720,12 @@ static int open_return_path_on_source(MigrationState *ms)
>      }
>  
>      trace_open_return_path_on_source();
> +
> +    if (!create_thread) {
> +        /* We're done */
> +        return 0;
> +    }
> +
>      qemu_thread_create(&ms->rp_state.rp_thread, "return path",
>                         source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
>  
> @@ -2249,15 +2266,24 @@ static void *migration_thread(void *opaque)
>  
>  void migrate_fd_connect(MigrationState *s)
>  {
> -    s->expected_downtime = s->parameters.downtime_limit;
> -    s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
> +    int64_t rate_limit;
> +    bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
>  
> -    qemu_file_set_blocking(s->to_dst_file, true);
> -    qemu_file_set_rate_limit(s->to_dst_file,
> -                             s->parameters.max_bandwidth / XFER_LIMIT_RATIO);
> +    if (resume) {
> +        /* This is a resumed migration */
> +        rate_limit = INT64_MAX;
> +    } else {
> +        /* This is a fresh new migration */
> +        rate_limit = s->parameters.max_bandwidth / XFER_LIMIT_RATIO;
> +        s->expected_downtime = s->parameters.downtime_limit;
> +        s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
>  
> -    /* Notify before starting migration thread */
> -    notifier_list_notify(&migration_state_notifiers, s);
> +        /* Notify before starting migration thread */
> +        notifier_list_notify(&migration_state_notifiers, s);
> +    }
> +
> +    qemu_file_set_rate_limit(s->to_dst_file, rate_limit);
> +    qemu_file_set_blocking(s->to_dst_file, true);
>  
>      /*
>       * Open the return path. For postcopy, it is used exclusively. For
> @@ -2265,15 +2291,19 @@ void migrate_fd_connect(MigrationState *s)
>       * QEMU uses the return path.
>       */
>      if (migrate_postcopy_ram() || migrate_use_return_path()) {
> -        if (open_return_path_on_source(s)) {
> +        if (open_return_path_on_source(s, !resume)) {
>              error_report("Unable to open return-path for postcopy");
> -            migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> -                              MIGRATION_STATUS_FAILED);
> +            migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
>              migrate_fd_cleanup(s);
>              return;
>          }
>      }
>  
> +    if (resume) {
> +        /* TODO: do the resume logic */
> +        return;
> +    }
> +
>      qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
>                         QEMU_THREAD_JOINABLE);
>      s->migration_thread_running = true;
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Tue, Aug 01, 2017 at 11:59:07AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > This patch detects the "resume" flag of migration command, rebuild the
> > channels only if the flag is set.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/migration.c | 52 ++++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 41 insertions(+), 11 deletions(-)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 36ff8c3..64de0ee 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1244,6 +1244,15 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >      MigrationState *s = migrate_get_current();
> >      const char *p;
> >  
> > +    if (has_resume && resume) {
> > +        if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) {
> > +            error_setg(errp, "Cannot resume if there is no "
> > +                       "paused migration");
> > +            return;
> > +        }
> > +        goto do_resume;
> > +    }
> > +
> >      if (migration_is_setup_or_active(s->state) ||
> >          s->state == MIGRATION_STATUS_CANCELLING ||
> >          s->state == MIGRATION_STATUS_COLO) {
> > @@ -1279,6 +1288,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >  
> >      s = migrate_init();
> >  
> > +do_resume:
> 
> Can we find a way to avoid this label?
> Perhaps split the bottom half of this function out into a separate
> function?

Yes this label can indeed be avoided (sorry for my laziness). Will
take the suggestion.  Thanks,