Re: [PATCH v5 07/10] RAMBlock: Make guest_memfd require coordinate discard

[Chenyi Qiang <chenyi.qiang@intel.com>]

On 5/26/2025 5:08 PM, David Hildenbrand wrote:
> On 20.05.25 12:28, Chenyi Qiang wrote:
>> As guest_memfd is now managed by RamBlockAttribute with
>> RamDiscardManager, only block uncoordinated discard.
>>
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>> ---
>> Changes in v5:
>>      - Revert to use RamDiscardManager.
>>
>> Changes in v4:
>>      - Modify commit message (RamDiscardManager->PrivateSharedManager).
>>
>> Changes in v3:
>>      - No change.
>>
>> Changes in v2:
>>      - Change the ram_block_discard_require(false) to
>>        ram_block_coordinated_discard_require(false).
>> ---
>>   system/physmem.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/system/physmem.c b/system/physmem.c
>> index f05f7ff09a..58b7614660 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -1916,7 +1916,7 @@ static void ram_block_add(RAMBlock *new_block,
>> Error **errp)
>>           }
>>           assert(new_block->guest_memfd < 0);
>>   -        ret = ram_block_discard_require(true);
>> +        ret = ram_block_coordinated_discard_require(true);
>>           if (ret < 0) {
>>               error_setg_errno(errp, -ret,
>>                                "cannot set up private guest memory:
>> discard currently blocked");
>> @@ -1939,7 +1939,7 @@ static void ram_block_add(RAMBlock *new_block,
>> Error **errp)
>>                * ever develops a need to check for errors.
>>                */
>>               close(new_block->guest_memfd);
>> -            ram_block_discard_require(false);
>> +            ram_block_coordinated_discard_require(false);
>>               qemu_mutex_unlock_ramlist();
>>               goto out_free;
>>           }
>> @@ -2302,7 +2302,7 @@ static void reclaim_ramblock(RAMBlock *block)
>>       if (block->guest_memfd >= 0) {
>>           ram_block_attribute_destroy(block->ram_shared);
>>           close(block->guest_memfd);
>> -        ram_block_discard_require(false);
>> +        ram_block_coordinated_discard_require(false);
>>       }
>>         g_free(block);
> 
> 
> I think this patch should be squashed into the previous one, then the
> story in that single patch is consistent.

I think this patch is a gate to allow device assignment with guest_memfd
and want to make it separately. Can we instead add some commit message
in previous one? like:

"Using guest_memfd with vfio is still blocked via
ram_block_discard_disable()/ram_block_discard_require()."

>