Re: [PATCH 2/7] guest_memfd: Introduce an object to manage the guest-memfd with RamDiscardManager

[David Hildenbrand]

On 21.01.25 11:16, Chenyi Qiang wrote:
> 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.

Or we'll move / link that to the RAM memory region, because that's what 
the object actually controls.

It starts getting a bit blury what should be part of the RAMBlock and 
what should be part of the "owning" RAM memory region :(

--
Cheers,

David / dhildenb