Re: [Qemu-devel] [PATCH 31/41] migration: Move postcopy stuff to postcopy-ram.c

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

* Juan Quintela (quintela@redhat.com) wrote:
> Yes, we don't have a good place to put that stuff.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  include/migration/migration.h | 25 -------------------------
>  migration/migration.c         | 18 ------------------
>  migration/postcopy-ram.c      | 18 ++++++++++++++++++
>  migration/postcopy-ram.h      | 26 ++++++++++++++++++++++++++
>  4 files changed, 44 insertions(+), 43 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 9063c32..5c27fe7 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -54,28 +54,6 @@ enum mig_rp_message_type {
>  
>  typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
>  
> -/* The current postcopy state is read/set by postcopy_state_get/set
> - * which update it atomically.
> - * The state is updated as postcopy messages are received, and
> - * in general only one thread should be writing to the state at any one
> - * time, initially the main thread and then the listen thread;
> - * Corner cases are where either thread finishes early and/or errors.
> - * The state is checked as messages are received to ensure that
> - * the source is sending us messages in the correct order.
> - * The state is also used by the RAM reception code to know if it
> - * has to place pages atomically, and the cleanup code at the end of
> - * the main thread to know if it has to delay cleanup until the end
> - * of postcopy.
> - */
> -typedef enum {
> -    POSTCOPY_INCOMING_NONE = 0,  /* Initial state - no postcopy */
> -    POSTCOPY_INCOMING_ADVISE,
> -    POSTCOPY_INCOMING_DISCARD,
> -    POSTCOPY_INCOMING_LISTENING,
> -    POSTCOPY_INCOMING_RUNNING,
> -    POSTCOPY_INCOMING_END
> -} PostcopyState;
> -
>  /* State for the incoming migration */
>  struct MigrationIncomingState {
>      QEMUFile *from_src_file;
> @@ -253,7 +231,4 @@ void savevm_skip_configuration(void);
>  int global_state_store(void);
>  void global_state_store_running(void);
>  
> -PostcopyState postcopy_state_get(void);
> -/* Set the state and return the old state */
> -PostcopyState postcopy_state_set(PostcopyState new_state);
>  #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index ff32086..d2f3c5e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -85,13 +85,6 @@ static NotifierList migration_state_notifiers =
>  
>  static bool deferred_incoming;
>  
> -/*
> - * Current state of incoming postcopy; note this is not part of
> - * MigrationIncomingState since it's state is used during cleanup
> - * at the end as MIS is being freed.
> - */
> -static PostcopyState incoming_postcopy_state;
> -
>  /* When we add fault tolerance, we could have several
>     migrations at once.  For now we don't need to add
>     dynamic creation of migration */
> @@ -2085,14 +2078,3 @@ void migrate_fd_connect(MigrationState *s)
>      s->migration_thread_running = true;
>  }
>  
> -PostcopyState  postcopy_state_get(void)
> -{
> -    return atomic_mb_read(&incoming_postcopy_state);
> -}
> -
> -/* Set the state and return the old state */
> -PostcopyState postcopy_state_set(PostcopyState new_state)
> -{
> -    return atomic_xchg(&incoming_postcopy_state, new_state);
> -}
> -
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 3359224..cae5d48 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -787,3 +787,21 @@ void postcopy_discard_send_finish(MigrationState *ms, PostcopyDiscardState *pds)
>  
>      g_free(pds);
>  }
> +
> +/*
> + * Current state of incoming postcopy; note this is not part of
> + * MigrationIncomingState since it's state is used during cleanup
> + * at the end as MIS is being freed.
> + */
> +static PostcopyState incoming_postcopy_state;
> +
> +PostcopyState  postcopy_state_get(void)
> +{
> +    return atomic_mb_read(&incoming_postcopy_state);
> +}
> +
> +/* Set the state and return the old state */
> +PostcopyState postcopy_state_set(PostcopyState new_state)
> +{
> +    return atomic_xchg(&incoming_postcopy_state, new_state);
> +}
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 4c25f03..52d51e8 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -81,6 +81,28 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>  int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>                               size_t pagesize);
>  
> +/* The current postcopy state is read/set by postcopy_state_get/set
> + * which update it atomically.
> + * The state is updated as postcopy messages are received, and
> + * in general only one thread should be writing to the state at any one
> + * time, initially the main thread and then the listen thread;
> + * Corner cases are where either thread finishes early and/or errors.
> + * The state is checked as messages are received to ensure that
> + * the source is sending us messages in the correct order.
> + * The state is also used by the RAM reception code to know if it
> + * has to place pages atomically, and the cleanup code at the end of
> + * the main thread to know if it has to delay cleanup until the end
> + * of postcopy.
> + */
> +typedef enum {
> +    POSTCOPY_INCOMING_NONE = 0,  /* Initial state - no postcopy */
> +    POSTCOPY_INCOMING_ADVISE,
> +    POSTCOPY_INCOMING_DISCARD,
> +    POSTCOPY_INCOMING_LISTENING,
> +    POSTCOPY_INCOMING_RUNNING,
> +    POSTCOPY_INCOMING_END
> +} PostcopyState;
> +
>  /*
>   * Allocate a page of memory that can be mapped at a later point in time
>   * using postcopy_place_page
> @@ -88,4 +110,8 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>   */
>  void *postcopy_get_tmp_page(MigrationIncomingState *mis);
>  
> +PostcopyState postcopy_state_get(void);
> +/* Set the state and return the old state */
> +PostcopyState postcopy_state_set(PostcopyState new_state);
> +
>  #endif
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK