On 1/21/2025 5:26 PM, David Hildenbrand wrote:
> On 21.01.25 10:00, Chenyi Qiang wrote:
>> Thanks Peter for your review!
>>
>> On 1/21/2025 2:09 AM, Peter Xu wrote:
>>> Two trivial comments I spot:
>>>
>>> On Fri, Dec 13, 2024 at 03:08:44PM +0800, Chenyi Qiang wrote:
>>>> +struct GuestMemfdManager {
>>>> + Object parent;
>>>> +
>>>> + /* Managed memory region. */
>>>> + MemoryRegion *mr;
>>>> +
>>>> + /*
>>>> + * 1-setting of the bit represents the memory is populated
>>>> (shared).
>>>> + */
>>>> + int32_t bitmap_size;
>>>> + unsigned long *bitmap;
>>>
>>> Might be clearer to name the bitmap directly as what it represents.
>>> E.g.,
>>> shared_bitmap?
>>
>> Make sense.
>>
>
> BTW, I was wondering if this information should be stored/linked from
> the RAMBlock, where we already store the guest_memdfd "int guest_memfd;".
>
> For example, having a "struct guest_memfd_state", and either embedding
> it in the RAMBlock or dynamically allocating and linking it.
>
> Alternatively, it would be such an object that we would simply link from
> the RAMBlock. (depending on which object will implement the manager
> interface)
>
> In any case, having all guest_memfd state that belongs to a RAMBlock at
> a single location might be cleanest.
Good suggestion. Follow the design of this series, we can add link to
the guest_memfd_manager object in RAMBlock.
>
