Re: [Qemu-devel] [RFC 13/29] migration: allow src return path to pause

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Xu (peterx@redhat.com) wrote:
> Let the thread pause for network issues.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c  | 35 +++++++++++++++++++++++++++++++++--
>  migration/migration.h  |  1 +
>  migration/trace-events |  2 ++
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index c729c5a..d0b9a86 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -996,6 +996,7 @@ static void migrate_fd_cleanup(void *opaque)
>      block_cleanup_parameters(s);
>  
>      qemu_sem_destroy(&s->postcopy_pause_sem);
> +    qemu_sem_destroy(&s->postcopy_pause_rp_sem);
>  }
>  
>  void migrate_fd_error(MigrationState *s, const Error *error)
> @@ -1140,6 +1141,7 @@ MigrationState *migrate_init(void)
>      error_free(s->error);
>      s->error = NULL;
>      qemu_sem_init(&s->postcopy_pause_sem, 0);
> +    qemu_sem_init(&s->postcopy_pause_rp_sem, 0);
>  
>      migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
>  
> @@ -1527,6 +1529,18 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
>      }
>  }
>  
> +/* Return true to retry, false to quit */
> +static bool postcopy_pause_return_path_thread(MigrationState *s)
> +{
> +    trace_postcopy_pause_return_path();
> +
> +    qemu_sem_wait(&s->postcopy_pause_rp_sem);
> +
> +    trace_postcopy_pause_return_path_continued();
> +
> +    return true;
> +}
> +
>  /*
>   * Handles messages sent on the return path towards the source VM
>   *
> @@ -1543,6 +1557,8 @@ static void *source_return_path_thread(void *opaque)
>      int res;
>  
>      trace_source_return_path_thread_entry();
> +
> +retry:
>      while (!ms->rp_state.error && !qemu_file_get_error(rp) &&
>             migration_is_setup_or_active(ms->state)) {
>          trace_source_return_path_thread_loop_top();
> @@ -1634,13 +1650,28 @@ static void *source_return_path_thread(void *opaque)
>              break;
>          }
>      }
> -    if (qemu_file_get_error(rp)) {
> +
> +out:
> +    res = qemu_file_get_error(rp);
> +    if (res) {
> +        if (res == -EIO) {
> +            /*
> +             * Maybe there is something we can do: it looks like a
> +             * network down issue, and we pause for a recovery.
> +             */
> +            if (postcopy_pause_return_path_thread(ms)) {
> +                /* Reload rp, reset the rest */
> +                rp = ms->rp_state.from_dst_file;
> +                ms->rp_state.error = false;
> +                goto retry;

The recursion trick I suggested in the previous patch might
also work here.

but it's OK, so

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +            }
> +        }
> +
>          trace_source_return_path_thread_bad_end();
>          mark_source_rp_bad(ms);
>      }
>  
>      trace_source_return_path_thread_end();
> -out:
>      ms->rp_state.from_dst_file = NULL;
>      qemu_fclose(rp);
>      return NULL;
> diff --git a/migration/migration.h b/migration/migration.h
> index 08b90e8..7aaab13 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -157,6 +157,7 @@ struct MigrationState
>  
>      /* Needed by postcopy-pause state */
>      QemuSemaphore postcopy_pause_sem;
> +    QemuSemaphore postcopy_pause_rp_sem;
>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/trace-events b/migration/trace-events
> index 22a629e..a269eec 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -98,6 +98,8 @@ migration_thread_setup_complete(void) ""
>  open_return_path_on_source(void) ""
>  open_return_path_on_source_continue(void) ""
>  postcopy_start(void) ""
> +postcopy_pause_return_path(void) ""
> +postcopy_pause_return_path_continued(void) ""
>  postcopy_pause_continued(void) ""
>  postcopy_pause_incoming(void) ""
>  postcopy_pause_incoming_continued(void) ""
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK