Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating

[David Hildenbrand]

> Am 24.02.2020 um 21:04 schrieb Peter Xu <address@hidden>:
> 
> On Mon, Feb 24, 2020 at 08:34:16PM +0100, David Hildenbrand wrote:
>> 
>> 
>>>> Am 24.02.2020 um 20:19 schrieb Peter Xu <address@hidden>:
>>> 
>>> On Mon, Feb 24, 2020 at 07:59:10PM +0100, David Hildenbrand wrote:
>>>>> On 24.02.20 19:44, David Hildenbrand wrote:
>>>>> On 24.02.20 18:45, Peter Xu wrote:
>>>>>> On Mon, Feb 24, 2020 at 10:09:19AM +0100, David Hildenbrand wrote:
>>>>>>> On 21.02.20 19:04, Peter Xu wrote:
>>>>>>>> On Fri, Feb 21, 2020 at 05:41:51PM +0100, David Hildenbrand wrote:
>>>>>>>>> I was now able to actually test resizing while migrating. I am using 
>>>>>>>>> the
>>>>>>>>> prototype of virtio-mem to test (which also makes use of resizable
>>>>>>>>> allocations). Things I was able to reproduce:
>>>>>>>> 
>>>>>>>> The test cases cover quite a lot.  Thanks for doing that.
>>>>>>>> 
>>>>>>>>> - Resize while still running on the migration source. Migration is 
>>>>>>>>> canceled
>>>>>>>>> -- Test case for "migraton/ram: Handle RAM block resizes during 
>>>>>>>>> precopy"
>>>>>>>> 
>>>>>>>>> - Resize (grow+shrink) on the migration target during postcopy 
>>>>>>>>> migration
> 
> [2]
> 
>>>>>>>>> (when syncing RAM blocks), while not yet running on the target
>>>>>>>>> -- Test case for "migration/ram: Discard new RAM when growing RAM 
>>>>>>>>> blocks
>>>>>>>>>  and the VM is stopped", and overall RAM size synchronization. Seems 
>>>>>>>>> to
>>>>>>>>>  work just fine.
>>>>>>>> 
>>>>>>>> This won't be able to trigger without virtio-mem, right?
>>>>>>> 
>>>>>>> AFAIK all cases can also be triggered without virtio-mem (not just that
>>>>>>> easily :) ). This case would be "RAM block is bigger on source than on
>>>>>>> destination.".
>>>>>>> 
>>>>>>>> 
>>>>>>>> And I'm also curious on how to test this even with virtio-mem.  Is
>>>>>>>> that a QMP command to extend/shrink virtio-mem?
>>>>>>> 
>>>>>>> Currently, there is a single qom property that can be modifed via
>>>>>>> QMP/HMP - "requested-size". With resizable resizable memory backends,
>>>>>>> increasing the requested size will also implicitly grow the RAM block.
>>>>>>> Shrinking the requested size will currently result in shrinking the RAM
>>>>>>> block on the next reboot.
>>>>>>> 
>>>>>>> So, to trigger growing of a RAM block (assuming requested-size was
>>>>>>> smaller before, e.g., 1000M)
>>>>>>> 
>>>>>>> echo "qom-set vm1 requested-size 6000M" | sudo nc -U $MON
>>>>>>> 
>>>>>>> To trigger shrinking (assuming requested-size was bigger before)
>>>>>>> 
>>>>>>> echo "qom-set vm1 requested-size 100M" | sudo nc -U $MON
>>>>>>> echo 'system_reset' | sudo nc -U $MON
>>>>>>> 
>>>>>>> 
>>>>>>> Placing these at the right spots during a migration allows to test this
>>>>>>> very reliably.
>>>>>> 
>>>>>> I see, thanks for the context.  The question was majorly about when
>>>>>> you say "during postcopy migration (when syncing RAM blocks), while
>>>>>> not yet running on the target" - it's not easy to do so imho, because:
>>>>> 
>>>>> This case is very easy to trigger, even with acpi. Simply have a ram
>>>>> block on the source be bigger than one on the target. The sync code
>>>>> (migration/ram.c:qemu_ram_resize()) will perform the resize during
> 
> [1]
> 
>>>>> precopy. Postcopy misses to discard the additional memory.
>>> 
>>> But when resizing happens during precopy, we should cancel this
>>> migration directly?  Hmm?...
>> 
>> ?
>> 
>> We are talking about the migration target, not the source. Please have a 
>> look at the RAM block size sync code I mentioned. That‘s probably faster 
>> than me having to explain it (and obviously failing to do so :) ).
> 
> OK finally I noticed you meant migration/ram.c:ram_load_precopy() [1]
> not qemu_ram_resize().

Right, the single invocation of qemu_ram_resize() in that file/function.

> And at [2] I think you meant during precopy
> migration, not postcopy.

The precopy stage when postcopy was advised. Yes, it‘s confusing :)

> Those are probably the things that made me
> confused.  And yes we need to consider this case.  Thanks,

Thanks for having a look!

> 
> -- 
> Peter Xu
>