Re: [PATCH v2 4/4] virtio-mem: Support "x-ignore-shared" migration

[David Hildenbrand <david@redhat.com>]

On 06.07.23 13:06, Juan Quintela wrote:
> David Hildenbrand <david@redhat.com> wrote:
>> To achieve desired "x-ignore-shared" functionality, we should not
>> discard all RAM when realizing the device and not mess with
>> preallocation/postcopy when loading device state. In essence, we should
>> not touch RAM content.
>>
>> As "x-ignore-shared" gets set after realizing the device, we cannot
>> rely on that. Let's simply skip discarding of RAM on incoming migration.
>> Note that virtio_mem_post_load() will call
>> virtio_mem_restore_unplugged() -- unless "x-ignore-shared" is set. So
>> once migration finished we'll have a consistent state.
>>
>> The initial system reset will also not discard any RAM, because
>> virtio_mem_unplug_all() will not call virtio_mem_unplug_all() when no
>> memory is plugged (which is the case before loading the device state).
>>
>> Note that something like VM templating -- see commit b17fbbe55cba
>> ("migration: allow private destination ram with x-ignore-shared")
> 
> And here I am, I reviewed the patch, and 4 years later I don't remember
> anything about it O:-)

:)

[...]

>> +    /*
>> +     * If shared RAM is migrated using the file content and not using QEMU,
>> +     * don't mess with preallocation and postcopy.
>> +     */
>> +    if (migrate_ram_is_ignored(vmem->memdev->mr.ram_block)) {
>> +        return 0;
>> +    }
>> +
>> +    if (vmem->prealloc && !vmem->early_migration) {
>> +        warn_report("Proper preallocation with migration requires a newer QEMU machine");
>> +    }
>> +
> 
> Could you explain why you are putting the check after calling
> virtio_mem_notify_populate_cb()?
> 
> What is it expected to for file memory backed RAM?  I got lost when I
> saw that it just calls:
> 
> static int virtio_mem_notify_populate_cb(MemoryRegionSection *s, void *arg)
> {
>      RamDiscardListener *rdl = arg;
> 
>      return rdl->notify_populate(rdl, s);
> }
> 
> 
> I end in vfio, and got completely confused about what is going on there.


:)


Once we reached virtio_mem_post_load(), we restored the bitmap that 
contains the state of all device blocks (plugged vs. unplugged).

Whenever we modify the bitmap (plug / unplug), we have to notify 
(RamDiscardManager) listeners, such that they are aware of the state 
change and can perform according action.

For example, vfio will go ahead and register the newly plugged blocks 
with the kernel (DMA map it into the vfio), where the kernel will end up 
long-term pinning these pages. Effectively, we only end up DMA-mapping 
plugged memory blocks, so only these get pinned by the kernel (and we 
can actually release the memory of unplugged blocks).


So here (virtio_mem_post_load()), we just restored the bitmap from the 
migration stream and effectively went from 0 plugged blocks (bitmap 
empty) before migration to "maybe some plugged blocks in the bitmap".

So we go over the bitmap and tell the world (vfio) to go ahead and 
DMA-map these blocks that are suddenly plugged.


And that part is independent of the actual RAM migration / 
x-ignore-shared, sow have to do it unconditional.



Thanks for the thorough review!

-- 
Cheers,

David / dhildenb