Thanks Zhao for your review!
On 1/9/2025 4:14 PM, Zhao Liu wrote:
>> #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
>> @@ -1885,6 +1886,9 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
>> qemu_mutex_unlock_ramlist();
>> goto out_free;
>> }
>> +
>> + GuestMemfdManager *gmm = GUEST_MEMFD_MANAGER(object_new(TYPE_GUEST_MEMFD_MANAGER));
>> + guest_memfd_manager_realize(gmm, new_block->mr, new_block->mr->size);
>
> realize & unrealize are usually used for QDev. I think it's not good to use
> *realize and *unrealize here.
>
> Why about "guest_memfd_manager_attach_ram"?
>
> In addition, it seems the third parameter is unnecessary and we can access
> MemoryRegion.size directly in guest_memfd_manager_realize().
LGTM. Will follow your suggestion if we still wrap the operations in one
function. (We may change to the HostMemoryBackend RDM then unpack the
operations).
>
>> }
>>
>> ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS;
>> @@ -2139,6 +2143,9 @@ static void reclaim_ramblock(RAMBlock *block)
>>
>> if (block->guest_memfd >= 0) {
>> close(block->guest_memfd);
>> + GuestMemfdManager *gmm = GUEST_MEMFD_MANAGER(block->mr->rdm);
>> + guest_memfd_manager_unrealize(gmm);
>
> Similiarly, what about "guest_memfd_manager_unattach_ram"?
Ditto. thanks.
>
>> + object_unref(OBJECT(gmm));
>> ram_block_discard_require(false);
>> }
>>
>
> Regards,
> Zhao
>
