Re: [PATCH v3 3/7] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard()

[Kishen Maloor]

On 3/10/25 1:18 AM, Chenyi Qiang wrote:
...
diff --git a/migration/ram.c b/migration/ram.c
index ce28328141..053730367b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -816,8 +816,8 @@ static inline bool migration_bitmap_clear_dirty(RAMState 
*rs,
      return ret;
  }
  
-static void dirty_bitmap_clear_section(MemoryRegionSection *section,
-                                       void *opaque)
+static int dirty_bitmap_clear_section(MemoryRegionSection *section,
+                                      void *opaque)
  {
      const hwaddr offset = section->offset_within_region;
      const hwaddr size = int128_get64(section->size);
@@ -836,6 +836,7 @@ static void dirty_bitmap_clear_section(MemoryRegionSection 
*section,
      }
      *cleared_bits += bitmap_count_one_with_offset(rb->bmap, start, npages);
      bitmap_clear(rb->bmap, start, npages);

It appears that the ram_discard_manager_replay_discarded() path would clear all 
private pages
from the dirty bitmap over here, and thus break migration.

Perhaps the MemoryAttributeManager should be excluded from this flow, something 
like below?

@@ -910,6 +909,9 @@ static uint64_t 
ramblock_dirty_bitmap_clear_discarded_pages(RAMBlock *rb)
             .size = int128_make64(qemu_ram_get_used_length(rb)),
         };
 
+        if (object_dynamic_cast(OBJECT(rdm), TYPE_MEMORY_ATTRIBUTE_MANAGER))
+            return 0;
+
         ram_discard_manager_replay_discarded(rdm, &section,
                                              dirty_bitmap_clear_section,
                                              &cleared_bits);