On 5/26/2025 5:35 PM, Philippe Mathieu-Daudé wrote:
> Hi Chenyi Qiang,
>
> On 20/5/25 12:28, Chenyi Qiang wrote:
>> Update ReplayRamDiscard() function to return the result and unify the
>> ReplayRamPopulate() and ReplayRamDiscard() to ReplayRamDiscardState() at
>> the same time due to their identical definitions. This unification
>> simplifies related structures, such as VirtIOMEMReplayData, which makes
>> it cleaner.
>>
>> Signed-off-by: Chenyi Qiang <chenyi.qi...@intel.com>
>> ---
>> Changes in v5:
>> - Rename ReplayRamStateChange to ReplayRamDiscardState (David)
>> - return data->fn(s, data->opaque) instead of 0 in
>> virtio_mem_rdm_replay_discarded_cb(). (Alexey)
>>
>> 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 | 21 ++++++++++-----------
>> include/system/memory.h | 36 +++++++++++++++++++-----------------
>> migration/ram.c | 5 +++--
>> system/memory.c | 12 ++++++------
>> 4 files changed, 38 insertions(+), 36 deletions(-)
>
>
>> diff --git a/include/system/memory.h b/include/system/memory.h
>> index 896948deb1..83b28551c4 100644
>> --- a/include/system/memory.h
>> +++ b/include/system/memory.h
>> @@ -575,8 +575,8 @@ static inline void
>> ram_discard_listener_init(RamDiscardListener *rdl,
>> rdl->double_discard_supported = double_discard_supported;
>> }
>> -typedef int (*ReplayRamPopulate)(MemoryRegionSection *section, void
>> *opaque);
>> -typedef void (*ReplayRamDiscard)(MemoryRegionSection *section, void
>> *opaque);
>> +typedef int (*ReplayRamDiscardState)(MemoryRegionSection *section,
>> + void *opaque);
>
> While changing this prototype, please add a documentation comment.
[...]
>
>> /*
>> * RamDiscardManagerClass:
>> @@ -650,36 +650,38 @@ struct RamDiscardManagerClass {
>> /**
>> * @replay_populated:
>> *
>> - * Call the #ReplayRamPopulate callback for all populated parts
>> within the
>> - * #MemoryRegionSection via the #RamDiscardManager.
>> + * Call the #ReplayRamDiscardState callback for all populated
>> parts within
>> + * the #MemoryRegionSection via the #RamDiscardManager.
>> *
>> * In case any call fails, no further calls are made.
>> *
>> * @rdm: the #RamDiscardManager
>> * @section: the #MemoryRegionSection
>> - * @replay_fn: the #ReplayRamPopulate callback
>> + * @replay_fn: the #ReplayRamDiscardState callback
>> * @opaque: pointer to forward to the callback
>> *
>> * Returns 0 on success, or a negative error if any notification
>> failed.
>> */
>> int (*replay_populated)(const RamDiscardManager *rdm,
>> MemoryRegionSection *section,
>> - ReplayRamPopulate replay_fn, void *opaque);
>> + ReplayRamDiscardState replay_fn, void
>> *opaque);
>> /**
>> * @replay_discarded:
>> *
>> - * Call the #ReplayRamDiscard callback for all discarded parts
>> within the
>> - * #MemoryRegionSection via the #RamDiscardManager.
>> + * Call the #ReplayRamDiscardState callback for all discarded
>> parts within
>> + * the #MemoryRegionSection via the #RamDiscardManager.
>> *
>> * @rdm: the #RamDiscardManager
>> * @section: the #MemoryRegionSection
>> - * @replay_fn: the #ReplayRamDiscard callback
>> + * @replay_fn: the #ReplayRamDiscardState callback
>> * @opaque: pointer to forward to the callback
>> + *
>> + * Returns 0 on success, or a negative error if any notification
>> failed.
>> */
>> - void (*replay_discarded)(const RamDiscardManager *rdm,
>> - MemoryRegionSection *section,
>> - ReplayRamDiscard replay_fn, void *opaque);
>> + int (*replay_discarded)(const RamDiscardManager *rdm,
>> + MemoryRegionSection *section,
>> + ReplayRamDiscardState replay_fn, void
>> *opaque);
>> /**
>> * @register_listener:
>> @@ -722,13 +724,13 @@ bool ram_discard_manager_is_populated(const
>> RamDiscardManager *rdm,
>> int ram_discard_manager_replay_populated(const RamDiscardManager
>> *rdm,
>> MemoryRegionSection *section,
>> - ReplayRamPopulate replay_fn,
>> + ReplayRamDiscardState
>> replay_fn,
>> void *opaque);
>> -void ram_discard_manager_replay_discarded(const RamDiscardManager
>> *rdm,
>> - MemoryRegionSection *section,
>> - ReplayRamDiscard replay_fn,
>> - void *opaque);
>> +int ram_discard_manager_replay_discarded(const RamDiscardManager *rdm,
>> + MemoryRegionSection *section,
>> + ReplayRamDiscardState
>> replay_fn,
>> + void *opaque);
>
> Similar for ram_discard_manager_replay_populated() and
> ram_discard_manager_replay_discarded(), since you understood
> what they do :)
Sure, will do. Thanks!
>
> Thanks!
>
> Phil.
>
