| Message ID | 20200219161725.115218-7-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migrate/ram: Fix resizing RAM blocks while migrating | expand |
On 19.02.20 17:17, David Hildenbrand wrote: > In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when > synchronizing the RAM block state with the migration source), the resized > part would not get discarded. Let's perform that when being notified > about a resize while postcopy has been advised and the guest is not > running yet. > > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > Cc: Juan Quintela <quintela@redhat.com> > Cc: Peter Xu <peterx@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > migration/ram.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/migration/ram.c b/migration/ram.c > index 57f32011a3..cbd54947fb 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -3722,6 +3722,25 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, > return; > } > > + /* > + * Especially at the start of precopy on the migration target, before > + * starting postcopy, we synchronize the RAM block sizes. Let's make sure > + * that any resizes before starting the guest are properly handled by > + * postcopy. Note: All other postcopy handling (e.g., registering handlers, > + * disabling THP) happens after all resizes (e.g., during precopy) were > + * performed. > + */ I think it would be clearer to do a ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_RUNNING We really only want to do something when psotcopy has been advised but the guest is not running yet. Will look into that as I find ways to actually test this :)
* David Hildenbrand (david@redhat.com) wrote: > On 19.02.20 17:17, David Hildenbrand wrote: > > In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when > > synchronizing the RAM block state with the migration source), the resized > > part would not get discarded. Let's perform that when being notified > > about a resize while postcopy has been advised and the guest is not > > running yet. > > > > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Cc: Juan Quintela <quintela@redhat.com> > > Cc: Peter Xu <peterx@redhat.com> > > Signed-off-by: David Hildenbrand <david@redhat.com> > > --- > > migration/ram.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/migration/ram.c b/migration/ram.c > > index 57f32011a3..cbd54947fb 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -3722,6 +3722,25 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, > > return; > > } > > > > + /* > > + * Especially at the start of precopy on the migration target, before > > + * starting postcopy, we synchronize the RAM block sizes. Let's make sure > > + * that any resizes before starting the guest are properly handled by > > + * postcopy. Note: All other postcopy handling (e.g., registering handlers, > > + * disabling THP) happens after all resizes (e.g., during precopy) were > > + * performed. > > + */ > > I think it would be clearer to do a > > ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_RUNNING > > We really only want to do something when psotcopy has been advised but > the guest is not running yet. > > Will look into that as I find ways to actually test this :) Should that be < POSTCOPY_INCOMING_LISTENING - i.e. before the userfaultfd has been enabled on the region? Dave > > -- > Thanks, > > David / dhildenb -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 21.02.20 16:51, Dr. David Alan Gilbert wrote: > * David Hildenbrand (david@redhat.com) wrote: >> On 19.02.20 17:17, David Hildenbrand wrote: >>> In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when >>> synchronizing the RAM block state with the migration source), the resized >>> part would not get discarded. Let's perform that when being notified >>> about a resize while postcopy has been advised and the guest is not >>> running yet. >>> >>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >>> Cc: Juan Quintela <quintela@redhat.com> >>> Cc: Peter Xu <peterx@redhat.com> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> migration/ram.c | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/migration/ram.c b/migration/ram.c >>> index 57f32011a3..cbd54947fb 100644 >>> --- a/migration/ram.c >>> +++ b/migration/ram.c >>> @@ -3722,6 +3722,25 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, >>> return; >>> } >>> >>> + /* >>> + * Especially at the start of precopy on the migration target, before >>> + * starting postcopy, we synchronize the RAM block sizes. Let's make sure >>> + * that any resizes before starting the guest are properly handled by >>> + * postcopy. Note: All other postcopy handling (e.g., registering handlers, >>> + * disabling THP) happens after all resizes (e.g., during precopy) were >>> + * performed. >>> + */ >> >> I think it would be clearer to do a >> >> ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_RUNNING >> >> We really only want to do something when psotcopy has been advised but >> the guest is not running yet. >> >> Will look into that as I find ways to actually test this :) > > Should that be < POSTCOPY_INCOMING_LISTENING - i.e. before the > userfaultfd has been enabled on the region? > We can be even stricter. I have in my current patch: diff --git a/migration/ram.c b/migration/ram.c index 39c7d1c4a6..1ccb40970e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3714,6 +3714,7 @@ static SaveVMHandlers savevm_ram_handlers = { static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, size_t old_size, size_t new_size) { + PostcopyState ps = postcopy_state_get(); ram_addr_t offset; Error *err = NULL; RAMBlock *rb = qemu_ram_block_from_host(host, false, &offset); @@ -3734,6 +3735,34 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, error_free(err); migration_cancel(); } + + switch (ps) { + case POSTCOPY_INCOMING_ADVISE: + /* + * Update what ram_postcopy_incoming_init()->init_range() does at the + * time postcopy was advised. Syncing RAM blocks with the source will + * result in RAM resizes. + */ + if (old_size < new_size) { + if (ram_discard_range(rb->idstr, old_size, new_size - old_size)) { + error_report("RAM block '%s' discard of resized RAM failed", + rb->idstr); + } + } + break; + case POSTCOPY_INCOMING_NONE: + case POSTCOPY_INCOMING_RUNNING: + case POSTCOPY_INCOMING_END: + /* + * Once our guest is running, postcopy does no longer care about + * resizes. When growing, the new memory was not available on the + * source, no handler needed. + */ + break; + default: + error_report("Unexpected RAM resize during postcopy state: %d", ps); + exit(-1); + } }
diff --git a/migration/ram.c b/migration/ram.c index 57f32011a3..cbd54947fb 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3722,6 +3722,25 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, return; } + /* + * Especially at the start of precopy on the migration target, before + * starting postcopy, we synchronize the RAM block sizes. Let's make sure + * that any resizes before starting the guest are properly handled by + * postcopy. Note: All other postcopy handling (e.g., registering handlers, + * disabling THP) happens after all resizes (e.g., during precopy) were + * performed. + */ + if (postcopy_is_advised() && !runstate_is_running()) { + /* Update what ram_postcopy_incoming_init()->init_range() does. */ + if (old_size < new_size) { + if (ram_discard_range(rb->idstr, old_size, new_size - old_size)) { + error_report("RAM block '%s' discard of resized RAM failed", + rb->idstr); + } + } + return; + } + /* * Some resizes are triggered on the migration target by precopy code, * when synchronizing RAM block sizes. In these cases, the VM is not
In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when synchronizing the RAM block state with the migration source), the resized part would not get discarded. Let's perform that when being notified about a resize while postcopy has been advised and the guest is not running yet. Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Cc: Juan Quintela <quintela@redhat.com> Cc: Peter Xu <peterx@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- migration/ram.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)