Re: [PATCH v1 06/13] migrate/ram: Discard new RAM when growing RAM blocks and the VM is stopped

[David Hildenbrand]

On 21.02.20 16:51, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (address@hidden) 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" <address@hidden>
>>> Cc: Juan Quintela <address@hidden>
>>> Cc: Peter Xu <address@hidden>
>>> Signed-off-by: David Hildenbrand <address@hidden>
>>> ---
>>>  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);
+    }
 }



-- 
Thanks,

David / dhildenb