Re: [PATCH v4 03/13] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard()

[David Hildenbrand <david@redhat.com>]

On 09.04.25 07:43, Alexey Kardashevskiy wrote:
> 
> 
> On 7/4/25 17:49, Chenyi Qiang wrote:
>> Update ReplayRamDiscard() function to return the result and unify the
>> ReplayRamPopulate() and ReplayRamDiscard() to ReplayStateChange() at
>> the same time due to their identical definitions. This unification
>> simplifies related structures, such as VirtIOMEMReplayData, which makes
>> it more cleaner and maintainable.
>>
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>> ---
>> Changes in v4:
>>       - Modify the commit message. We won't use Replay() operation when
>>         doing the attribute change like v3.
>>
>> Changes in v3:
>>       - Newly added.
>> ---
>>    hw/virtio/virtio-mem.c | 20 ++++++++++----------
>>    include/exec/memory.h  | 31 ++++++++++++++++---------------
>>    migration/ram.c        |  5 +++--
>>    system/memory.c        | 12 ++++++------
>>    4 files changed, 35 insertions(+), 33 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>> index d0d3a0240f..1a88d649cb 100644
>> --- a/hw/virtio/virtio-mem.c
>> +++ b/hw/virtio/virtio-mem.c
>> @@ -1733,7 +1733,7 @@ static bool virtio_mem_rdm_is_populated(const RamDiscardManager *rdm,
>>    }
>>    
>>    struct VirtIOMEMReplayData {
>> -    void *fn;
>> +    ReplayStateChange fn;
> 
> 
> s/ReplayStateChange/ReplayRamStateChange/
> 
> Just "State" is way too generic imho.

Right, but raised in my review, the "Change" is wrong, it's not a change.

ReplayRamState ... ReplayRamDiscardState or sth like that ?

After all, it's the "RAM Discard manager".

-- 
Cheers,

David / dhildenb