Re: [PATCH v4 02/13] memory: Change memory_region_set_ram_discard_manager() to return the result

[David Hildenbrand <david@redhat.com>]

                               RamDiscardManager *rdm);>>>      /**
>>>     * memory_region_find: translate an address/size relative to a
>>> diff --git a/system/memory.c b/system/memory.c
>>> index b17b5538ff..62d6b410f0 100644
>>> --- a/system/memory.c
>>> +++ b/system/memory.c
>>> @@ -2115,12 +2115,16 @@ RamDiscardManager
>>> *memory_region_get_ram_discard_manager(MemoryRegion *mr)
>>>        return mr->rdm;
>>>    }
>>>    -void memory_region_set_ram_discard_manager(MemoryRegion *mr,
>>> -                                           RamDiscardManager *rdm)
>>> +int memory_region_set_ram_discard_manager(MemoryRegion *mr,
>>> +                                          RamDiscardManager *rdm)
>>>    {
>>>        g_assert(memory_region_is_ram(mr));
>>> -    g_assert(!rdm || !mr->rdm);
>>> +    if (mr->rdm && rdm) {
>>> +        return -EBUSY;
>>> +    }
>>> +
>>>        mr->rdm = rdm;
>>> +    return 0;
>>
>> This is a change which can potentially break something, or currently
>> there is no way to trigger -EBUSY? Thanks,
> 
> Before this series, virtio-mem is the only user to
> set_ram_discard_manager(), there's no way to trigger -EBUSY. With this
> series, guest_memfd-backed RAMBlocks become the second user. It can be
> triggered if we try to use virtio-mem in confidential VMs.

Right. I have plans on looking into using virtio-mem for confidential 
VMs, so far it's not compatible.

One challenge will be resolving how to deal with two sources of information.

Assume virtio-mem says "memory is now plugged", we would have to check 
with guest_memfd if it is also in the "shared" state. If not, no need to 
notify anybody.

Similarly, if guest_memfd says "memory is now shared", we would have to 
check with guest_memfd if it is in the "plugged" state.


I don't know yet how exactly the solution would look like.

Possibly, we have a list of such "populate/discard" information sources, 
and a real "manager" on top, that gets notified by these sources.

That real "manager" would then collect information from other sources to 
make a decision whether to propagate the populate / shared notification.

-- 
Cheers,

David / dhildenb