Introduce internal memory_region_do_init_ram() function to remove
duplicated code from different memory_region_init_*ram functions.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
system/memory.c | 123 ++++++++++++++++--------------------------------
1 file changed, 41 insertions(+), 82 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index e15f931a8a..96a09d7cf3 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1579,19 +1579,12 @@ void memory_region_init_io(MemoryRegion *mr,
memory_region_set_ops(mr, ops, opaque);
}
-bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- uint32_t ram_flags,
- Error **errp)
+static bool memory_region_do_init_ram(MemoryRegion *mr,
+ Error *err, Error **errp)
{
- Error *err = NULL;
- memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
- mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
@@ -1601,6 +1594,17 @@ bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
return true;
}
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t size,
+ uint32_t ram_flags, Error **errp)
+{
+ Error *err = NULL;
+
+ memory_region_init(mr, owner, name, size);
+ mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
+ return memory_region_do_init_ram(mr, err, errp);
+}
+
bool memory_region_init_resizeable_ram(MemoryRegion *mr,
Object *owner,
const char *name,
@@ -1612,108 +1616,66 @@ bool memory_region_init_resizeable_ram(MemoryRegion *mr,
Error **errp)
{
Error *err = NULL;
+
memory_region_init(mr, owner, name, size);
- mr->ram = true;
- mr->terminates = true;
- mr->destructor = memory_region_destructor_ram;
- mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
- mr, &err);
- if (err) {
- mr->size = int128_zero();
- object_unparent(OBJECT(mr));
- error_propagate(errp, err);
- return false;
- }
- return true;
+ mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized, mr,
+ &err);
+ return memory_region_do_init_ram(mr, err, errp);
}
#if defined(CONFIG_POSIX) && !defined(EMSCRIPTEN)
-bool memory_region_init_ram_from_file(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- uint64_t align,
- uint32_t ram_flags,
- const char *path,
- ram_addr_t offset,
+bool memory_region_init_ram_from_file(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t size,
+ uint64_t align, uint32_t ram_flags,
+ const char *path, ram_addr_t offset,
Error **errp)
{
Error *err = NULL;
+
memory_region_init(mr, owner, name, size);
- mr->ram = true;
mr->readonly = !!(ram_flags & RAM_READONLY);
- mr->terminates = true;
- mr->destructor = memory_region_destructor_ram;
mr->align = align;
- mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path,
- offset, &err);
- if (err) {
- mr->size = int128_zero();
- object_unparent(OBJECT(mr));
- error_propagate(errp, err);
- return false;
- }
- return true;
+ mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, offset,
+ &err);
+ return memory_region_do_init_ram(mr, err, errp);
}
-bool memory_region_init_ram_from_fd(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- uint32_t ram_flags,
- int fd,
- ram_addr_t offset,
- Error **errp)
+bool memory_region_init_ram_from_fd(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t size,
+ uint32_t ram_flags, int fd,
+ ram_addr_t offset, Error **errp)
{
Error *err = NULL;
+
memory_region_init(mr, owner, name, size);
- mr->ram = true;
mr->readonly = !!(ram_flags & RAM_READONLY);
- mr->terminates = true;
- mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd,
offset, false, &err);
- if (err) {
- mr->size = int128_zero();
- object_unparent(OBJECT(mr));
- error_propagate(errp, err);
- return false;
- }
- return true;
+ return memory_region_do_init_ram(mr, err, errp);
}
#endif
-void memory_region_init_ram_ptr(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- void *ptr)
+void memory_region_init_ram_ptr(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t size, void *ptr)
{
memory_region_init(mr, owner, name, size);
- mr->ram = true;
- mr->terminates = true;
- mr->destructor = memory_region_destructor_ram;
-
/* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL. */
assert(ptr != NULL);
mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+ memory_region_do_init_ram(mr, NULL, NULL);
}
-void memory_region_init_ram_device_ptr(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
+void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t size,
void *ptr)
{
memory_region_init(mr, owner, name, size);
- mr->ram = true;
- mr->ram_device = true;
memory_region_set_ops(mr, &ram_device_mem_ops, mr);
- mr->destructor = memory_region_destructor_ram;
-
/* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL. */
assert(ptr != NULL);
mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+ memory_region_do_init_ram(mr, NULL, NULL);
+ mr->ram_device = true;
}
void memory_region_init_alias(MemoryRegion *mr,
@@ -3764,15 +3726,12 @@ bool memory_region_init_rom_device(MemoryRegion *mr,
assert(ops);
memory_region_init(mr, owner, name, size);
memory_region_set_ops(mr, ops, opaque);
- mr->rom_device = true;
- mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
- if (err) {
- mr->size = int128_zero();
- object_unparent(OBJECT(mr));
- error_propagate(errp, err);
+ if (!memory_region_do_init_ram(mr, err, errp)) {
return false;
}
+ mr->ram = false;
+ mr->rom_device = true;
/* This will assert if owner is neither NULL nor a DeviceState.
* We only want the owner here for the purposes of defining a
* unique name for migration. TODO: Ideally we should implement
--
2.41.3On Mon, Feb 02, 2026 at 03:28:17PM +0100, BALATON Zoltan wrote:
> -bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
> - Object *owner,
> - const char *name,
> - uint64_t size,
> - uint32_t ram_flags,
> - Error **errp)
> +static bool memory_region_do_init_ram(MemoryRegion *mr,
> + Error *err, Error **errp)
I keep thinking taking two Errors here is almost not readable for an init
function. It only makes sense to me if it's something like
error_propagate(). E.g. we also have migrate_error_propagate(). I can't
think of anything that should pass in two Errors..
I'm not sure how bad it is we keep the old code as-is.. but if we really
want to provide some helpers, IMHO we should still avoid this, maybe:
memory_region_ram_setup(mr)
{
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
}
memory_region_error_propagate(mr, err, errp)
{
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
Then taking one example:
memory_region_init_ram_flags_nomigrate()
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
memory_region_ram_setup(mr);
mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
if (err) {
memory_region_error_propagate(mr, err, errp);
return false;
}
return true;
}
Thanks,
--
Peter Xu
© 2016 - 2026 Red Hat, Inc.