On 07.04.25 09: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.qi...@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.
---

[...]

-typedef int (*ReplayRamPopulate)(MemoryRegionSection *section, void *opaque);
-typedef void (*ReplayRamDiscard)(MemoryRegionSection *section, void *opaque);
+typedef int (*ReplayStateChange)(MemoryRegionSection *section, void *opaque);

But it's not a state change.

ReplayRamState maybe?

[...]
  /*
diff --git a/system/memory.c b/system/memory.c
index 62d6b410f0..b5ab729e13 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2147,7 +2147,7 @@ bool ram_discard_manager_is_populated(const 
RamDiscardManager *rdm,
int ram_discard_manager_replay_populated(const RamDiscardManager *rdm,
                                           MemoryRegionSection *section,
-                                         ReplayRamPopulate replay_fn,
+                                         ReplayStateChange replay_fn,
                                           void *opaque)
  {
      RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_GET_CLASS(rdm);
@@ -2156,15 +2156,15 @@ int ram_discard_manager_replay_populated(const 
RamDiscardManager *rdm,
      return rdmc->replay_populated(rdm, section, replay_fn, 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,
+                                         ReplayStateChange replay_fn,
+                                         void *opaque)
  {
      RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_GET_CLASS(rdm);
g_assert(rdmc->replay_discarded);
-    rdmc->replay_discarded(rdm, section, replay_fn, opaque);
+    return rdmc->replay_discarded(rdm, section, replay_fn, opaque);
  }

The idea was that ram_discard_manager_replay_discarded() would never be able to fail. But I don't think this really matters, because the function is provided by the caller, that can just always return 0 -- like we do in dirty_bitmap_clear_section() now.

So yeah, this looks fine to me, given that we don't call it a "state change" when we are merely replaying a selected state.

--
Cheers,

David / dhildenb


Reply via email to