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

[Kishen Maloor <kishen.maloor@xxxxxxxxx>]

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);