[1/2] migration/postcopy: allocate tmp_page in setup stage

During migration, a tmp page is allocated so that we could place a whole
host page during postcopy.

Currently the page is allocated during load stage, this is a little bit
late. And more important, if we failed to allocate it, the error is not
checked properly. Even it is NULL, we would still use it.

This patch moves the allocation to setup stage and if failed error
message would be printed and caller would notice it.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 migration/postcopy-ram.c | 40 ++++++++++------------------------------
 migration/postcopy-ram.h |  7 -------
 migration/ram.c          |  2 +-
 3 files changed, 11 insertions(+), 38 deletions(-)

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> During migration, a tmp page is allocated so that we could place a whole
> host page during postcopy.
> 
> Currently the page is allocated during load stage, this is a little bit
> late. And more important, if we failed to allocate it, the error is not
> checked properly. Even it is NULL, we would still use it.

Oops, yes.


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

> 
> This patch moves the allocation to setup stage and if failed error
> message would be printed and caller would notice it.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  migration/postcopy-ram.c | 40 ++++++++++------------------------------
>  migration/postcopy-ram.h |  7 -------
>  migration/ram.c          |  2 +-
>  3 files changed, 11 insertions(+), 38 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 51dc164693..e554f93eec 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1132,6 +1132,16 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
>          return -1;
>      }
>  
> +    mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size,
> +                                  PROT_READ | PROT_WRITE, MAP_PRIVATE |
> +                                  MAP_ANONYMOUS, -1, 0);
> +    if (mis->postcopy_tmp_page == MAP_FAILED) {
> +        mis->postcopy_tmp_page = NULL;
> +        error_report("%s: Failed to map postcopy_tmp_page %s",
> +                     __func__, strerror(errno));
> +        return -1;
> +    }
> +
>      /*
>       * Ballooning can mark pages as absent while we're postcopying
>       * that would cause false userfaults.
> @@ -1258,30 +1268,6 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>      }
>  }
>  
> -/*
> - * Returns a target page of memory that can be mapped at a later point in time
> - * using postcopy_place_page
> - * The same address is used repeatedly, postcopy_place_page just takes the
> - * backing page away.
> - * Returns: Pointer to allocated page
> - *
> - */
> -void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> -{
> -    if (!mis->postcopy_tmp_page) {
> -        mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size,
> -                             PROT_READ | PROT_WRITE, MAP_PRIVATE |
> -                             MAP_ANONYMOUS, -1, 0);
> -        if (mis->postcopy_tmp_page == MAP_FAILED) {
> -            mis->postcopy_tmp_page = NULL;
> -            error_report("%s: %s", __func__, strerror(errno));
> -            return NULL;
> -        }
> -    }
> -
> -    return mis->postcopy_tmp_page;
> -}
> -
>  #else
>  /* No target OS support, stubs just fail */
>  void fill_destination_postcopy_migration_info(MigrationInfo *info)
> @@ -1339,12 +1325,6 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>      return -1;
>  }
>  
> -void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> -{
> -    assert(0);
> -    return NULL;
> -}
> -
>  int postcopy_wake_shared(struct PostCopyFD *pcfd,
>                           uint64_t client_addr,
>                           RAMBlock *rb)
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index e3dde32155..c0ccf64a96 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -100,13 +100,6 @@ typedef enum {
>      POSTCOPY_INCOMING_END
>  } PostcopyState;
>  
> -/*
> - * Allocate a page of memory that can be mapped at a later point in time
> - * using postcopy_place_page
> - * Returns: Pointer to allocated page
> - */
> -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,
> diff --git a/migration/ram.c b/migration/ram.c
> index 4c15162bd6..adbaf0b11a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4048,7 +4048,7 @@ static int ram_load_postcopy(QEMUFile *f)
>      bool matches_target_page_size = false;
>      MigrationIncomingState *mis = migration_incoming_get_current();
>      /* Temporary page that is later 'placed' */
> -    void *postcopy_host_page = postcopy_get_tmp_page(mis);
> +    void *postcopy_host_page = mis->postcopy_tmp_page;
>      void *last_host = NULL;
>      bool all_zero = false;
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK