* Wei Yang ([email protected]) 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 <[email protected]> > > 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 <[email protected]> > --- > 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 / [email protected] / Manchester, UK
