Re: [PATCH] memory: Set mr->ram before RAM Block allocation

[BALATON Zoltan]

On Thu, 12 Mar 2026, Peter Xu wrote:
On Thu, Mar 12, 2026 at 02:34:20PM +0800, Xiaoyao Li wrote:
Commit 2fb627ef2f48 ("memory: Factor out common ram region initialization")
introduced a helper function memory_region_set_ram_block(), which causes
mr->ram to be set to true after the RAM Block allocation by
qemu_ram_alloc_*().
It leads to the assertion

  g_assert(memory_region_is_ram(mr));

in memory_region_set_ram_discard_manager() being triggered when creating
RAM Block with the RAM_GUEST_MEMFD flag.

Fix this by restoring the original behavior of setting mr->ram before
RAM Block allocation.

Closes: https://gitlab.com/qemu-project/qemu/-/work_items/3330
Reported-by: Farrah Chen <farrah.c...@intel.com>
Fixes: 2fb627ef2f48 ("memory: Factor out common ram region initialization")
Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>

Thanks for the report.  This is fast..

Almost agreed with the fix, except that it duplicates the lines all over
the places.  Would it be better to introduce memory_region_init_ram()?

First sorry for breaking this, I checked that qemu_alloc_ram did not refer 
to these fields but missed this use much deeper in the call stack. The 
memory_region_init_ram name is already taken so can't call it like that. 
Since all the different memory_region_init variants call different 
qemu_ram_alloc variants there does not seem to be a way to set this 
without addining an additional line. Previous versions of the series had 
memory_region_setup_ram() that set these mr fields then qemu_alloc_ram and 
then error_propagate was called last. We could bring back 
memory_region_setup_ram but it's the same additional line everywhere so 
this seems to be a simpler fix. I don't see a way to avoid this 
duplication other than maybe changing qemu_ram_alloc to put it somehow 
within that but that does not seem to be simple. I can try to think about 
it some more but so far I could not find a simpler fix.

Regards,
BALATON Zoltan

---
 system/memory.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 17a7bcd9af7c..56f3225b21ad 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1578,7 +1578,6 @@ void memory_region_init_io(MemoryRegion *mr, Object 
*owner,

 static bool memory_region_set_ram_block(MemoryRegion *mr, RAMBlock *rb)
 {
-    mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = rb;
@@ -1597,6 +1596,7 @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion 
*mr, Object *owner,
     RAMBlock *rb;

     memory_region_init(mr, owner, name, size);
+    mr->ram = true;
     rb = qemu_ram_alloc(size, ram_flags, mr, errp);
     return memory_region_set_ram_block(mr, rb);
 }
@@ -1614,6 +1614,7 @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
     RAMBlock *rb;

     memory_region_init(mr, owner, name, size);
+    mr->ram = true;
     rb = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
     return memory_region_set_ram_block(mr, rb);
 }
@@ -1628,6 +1629,7 @@ bool memory_region_init_ram_from_file(MemoryRegion *mr, 
Object *owner,
     RAMBlock *rb;

     memory_region_init(mr, owner, name, size);
+    mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
     mr->align = align;
     rb = qemu_ram_alloc_from_file(size, mr, ram_flags, path, offset, errp);
@@ -1642,6 +1644,7 @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr, 
Object *owner,
     RAMBlock *rb;

     memory_region_init(mr, owner, name, size);
+    mr->ram = true;
     mr->readonly = !!(ram_flags & RAM_READONLY);
     rb = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd, offset,
                                 false, errp);
@@ -1663,6 +1666,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, Object 
*owner,
                                 void *ptr)
 {
     memory_region_init(mr, owner, name, size);
+    mr->ram = true;
     memory_region_set_ram_ptr(mr, size, ptr);
 }

@@ -1671,6 +1675,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, 
Object *owner,
                                        void *ptr)
 {
     memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size);
+    mr->ram = true;
     mr->ram_device = true;
     memory_region_set_ram_ptr(mr, size, ptr);
 }
@@ -3699,7 +3704,6 @@ bool memory_region_init_rom_device(MemoryRegion *mr, 
Object *owner,
     memory_region_init_io(mr, owner, ops, opaque, name, size);
     rb = qemu_ram_alloc(size, 0, mr, errp);
     if (memory_region_set_ram_block(mr, rb)) {
-        mr->ram = false;
         mr->rom_device = true;
         memory_region_register_ram(mr, owner);
         return true;
--
2.43.0