* [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes
@ 2026-06-11 6:35 Akihiko Odaki
2026-06-11 6:35 ` [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks Akihiko Odaki
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Akihiko Odaki @ 2026-06-11 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu,
Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Peter Xu,
Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti,
Alex Williamson, Cédric Le Goater, qemu-block, xen-devel,
kvm, Akihiko Odaki
Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp>
("[PATCH] system/physmem: Assert migration invariants")
ram_mig_ram_block_resized() already aborts migration when migratable RAM
is resized. Extend the same handling to other unsupported changes to the
migratable RAMBlock set, such as removing a migratable RAMBlock or
changing a RAMBlock's migratable state.
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Akihiko Odaki (3):
system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks
system/physmem: Notify RAMBlock migratable and idstr changes
migration/ram: Abort on unsupported migratable RAM changes
include/migration/misc.h | 2 +-
include/system/ramlist.h | 24 ++++++---
block/block-ram-registrar.c | 8 +--
hw/core/numa.c | 54 ++++++++++++++++---
hw/xen/xen-mapcache.c | 6 +--
migration/ram.c | 125 ++++++++++++++++++++++++++++++++++++--------
system/physmem.c | 16 ++++--
target/i386/nvmm/nvmm-all.c | 4 +-
target/i386/sev.c | 8 +--
util/vfio-helpers.c | 7 +--
10 files changed, 194 insertions(+), 60 deletions(-)
---
base-commit: 2db91528542672cf0db78b3f2cc0e22b36302b38
change-id: 20260606-ram-dcef14f001fb
Best regards,
--
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks 2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki @ 2026-06-11 6:35 ` Akihiko Odaki 2026-06-11 6:35 ` [PATCH 2/3] system/physmem: Notify RAMBlock migratable and idstr changes Akihiko Odaki ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Akihiko Odaki @ 2026-06-11 6:35 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Peter Xu, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm, Akihiko Odaki The RAM save/restore code needs the RAMBlock in RAMBlockNotifier callbacks to decide whether migration must be aborted and to update RAMBlock state for postcopy. Looking up the RAMBlock inside callbacks is hazardous: with Xen mapcache it can deadlock, and it also leaves the callback contract unclear. Pass the RAMBlock explicitly to RAMBlockNotifier callbacks. The ram_block_resized callback takes a non-const RAMBlock because it may modify the RAMBlock, while the other callbacks take const RAMBlock. Xen mapcache passes NULL because mapcache entries do not have a one-to-one relationship with RAMBlocks. ram_block_resized() is never called from Xen mapcache, so its RAMBlock argument is always non-NULL. Drop redundant parameters from ram_block_resized() because they can be derived from the RAMBlock. Keep them for the other callbacks because their RAMBlock argument may be NULL. Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> --- include/system/ramlist.h | 18 ++++++++++-------- block/block-ram-registrar.c | 8 ++++---- hw/core/numa.c | 18 ++++++++++-------- hw/xen/xen-mapcache.c | 6 +++--- migration/ram.c | 12 +++--------- system/physmem.c | 7 +++---- target/i386/nvmm/nvmm-all.c | 4 ++-- target/i386/sev.c | 8 ++++---- util/vfio-helpers.c | 7 ++++--- 9 files changed, 43 insertions(+), 45 deletions(-) diff --git a/include/system/ramlist.h b/include/system/ramlist.h index c7f388f487d7..32157cc84305 100644 --- a/include/system/ramlist.h +++ b/include/system/ramlist.h @@ -62,11 +62,11 @@ void qemu_mutex_lock_ramlist(void); void qemu_mutex_unlock_ramlist(void); struct RAMBlockNotifier { - void (*ram_block_added)(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size); - void (*ram_block_removed)(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size); - void (*ram_block_resized)(RAMBlockNotifier *n, void *host, size_t old_size, + void (*ram_block_added)(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size); + void (*ram_block_removed)(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size); + void (*ram_block_resized)(RAMBlockNotifier *n, RAMBlock *rb, size_t new_size); QLIST_ENTRY(RAMBlockNotifier) next; }; @@ -77,9 +77,11 @@ int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque); void ram_block_notifier_add(RAMBlockNotifier *n); void ram_block_notifier_remove(RAMBlockNotifier *n); -void ram_block_notify_add(void *host, size_t size, size_t max_size); -void ram_block_notify_remove(void *host, size_t size, size_t max_size); -void ram_block_notify_resize(void *host, size_t old_size, size_t new_size); +void ram_block_notify_add(const RAMBlock *rb, + void *host, size_t size, size_t max_size); +void ram_block_notify_remove(const RAMBlock *rb, + void *host, size_t size, size_t max_size); +void ram_block_notify_resize(RAMBlock *rb, size_t new_size); GString *ram_block_format(void); diff --git a/block/block-ram-registrar.c b/block/block-ram-registrar.c index fcda2b86afb2..5b938de22587 100644 --- a/block/block-ram-registrar.c +++ b/block/block-ram-registrar.c @@ -9,8 +9,8 @@ #include "system/block-ram-registrar.h" #include "qapi/error.h" -static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size) +static void ram_block_added(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { BlockRAMRegistrar *r = container_of(n, BlockRAMRegistrar, notifier); Error *err = NULL; @@ -26,8 +26,8 @@ static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size, } } -static void ram_block_removed(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size) +static void ram_block_removed(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { BlockRAMRegistrar *r = container_of(n, BlockRAMRegistrar, notifier); blk_unregister_buf(r->blk, host, max_size); diff --git a/hw/core/numa.c b/hw/core/numa.c index f462883c87cf..40acb98bdd0b 100644 --- a/hw/core/numa.c +++ b/hw/core/numa.c @@ -824,7 +824,7 @@ static int ram_block_notify_add_single(RAMBlock *rb, void *opaque) RAMBlockNotifier *notifier = opaque; if (host) { - notifier->ram_block_added(notifier, host, size, max_size); + notifier->ram_block_added(notifier, rb, host, size, max_size); } return 0; } @@ -837,7 +837,7 @@ static int ram_block_notify_remove_single(RAMBlock *rb, void *opaque) RAMBlockNotifier *notifier = opaque; if (host) { - notifier->ram_block_removed(notifier, host, size, max_size); + notifier->ram_block_removed(notifier, rb, host, size, max_size); } return 0; } @@ -861,38 +861,40 @@ void ram_block_notifier_remove(RAMBlockNotifier *n) } } -void ram_block_notify_add(void *host, size_t size, size_t max_size) +void ram_block_notify_add(const RAMBlock *rb, + void *host, size_t size, size_t max_size) { RAMBlockNotifier *notifier; RAMBlockNotifier *next; QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { if (notifier->ram_block_added) { - notifier->ram_block_added(notifier, host, size, max_size); + notifier->ram_block_added(notifier, rb, host, size, max_size); } } } -void ram_block_notify_remove(void *host, size_t size, size_t max_size) +void ram_block_notify_remove(const RAMBlock *rb, + void *host, size_t size, size_t max_size) { RAMBlockNotifier *notifier; RAMBlockNotifier *next; QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { if (notifier->ram_block_removed) { - notifier->ram_block_removed(notifier, host, size, max_size); + notifier->ram_block_removed(notifier, rb, host, size, max_size); } } } -void ram_block_notify_resize(void *host, size_t old_size, size_t new_size) +void ram_block_notify_resize(RAMBlock *rb, size_t new_size) { RAMBlockNotifier *notifier; RAMBlockNotifier *next; QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { if (notifier->ram_block_resized) { - notifier->ram_block_resized(notifier, host, old_size, new_size); + notifier->ram_block_resized(notifier, rb, new_size); } } } diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c index 85cf0cf359ca..f321c65b630e 100644 --- a/hw/xen/xen-mapcache.c +++ b/hw/xen/xen-mapcache.c @@ -222,7 +222,7 @@ static void xen_remap_bucket(MapCache *mc, if (entry->vaddr_base != NULL) { if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) { - ram_block_notify_remove(entry->vaddr_base, entry->size, + ram_block_notify_remove(NULL, entry->vaddr_base, entry->size, entry->size); } @@ -308,7 +308,7 @@ static void xen_remap_bucket(MapCache *mc, } if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) { - ram_block_notify_add(vaddr_base, size, size); + ram_block_notify_add(NULL, vaddr_base, size, size); } entry->vaddr_base = vaddr_base; @@ -601,7 +601,7 @@ static void xen_invalidate_map_cache_entry_unlocked(MapCache *mc, return; } - ram_block_notify_remove(entry->vaddr_base, entry->size, entry->size); + ram_block_notify_remove(NULL, entry->vaddr_base, entry->size, entry->size); if (entry->flags & XEN_MAPCACHE_ENTRY_GRANT) { rc = xengnttab_unmap(xen_region_gnttabdev, entry->vaddr_base, entry->size >> mc->bucket_shift); diff --git a/migration/ram.c b/migration/ram.c index fc38ffbf8af1..6bc7f705d31a 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -4699,18 +4699,12 @@ static SaveVMHandlers savevm_ram_handlers = { .save_postcopy_prepare = ram_save_postcopy_prepare, }; -static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host, - size_t old_size, size_t new_size) +static void ram_mig_ram_block_resized(RAMBlockNotifier *n, RAMBlock *rb, + size_t new_size) { PostcopyState ps = postcopy_state_get(); - ram_addr_t offset; - RAMBlock *rb = qemu_ram_block_from_host(host, false, &offset); Error *err = NULL; - - if (!rb) { - error_report("RAM block not found"); - return; - } + ram_addr_t old_size = qemu_ram_get_used_length(rb); if (migrate_ram_is_ignored(rb)) { return; diff --git a/system/physmem.c b/system/physmem.c index 7bcbf8757361..6d00e99270c7 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -2019,7 +2019,6 @@ static int memory_try_enable_merging(void *addr, size_t len) */ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) { - const ram_addr_t oldsize = block->used_length; const ram_addr_t unaligned_size = newsize; newsize = TARGET_PAGE_ALIGN(newsize); @@ -2057,7 +2056,7 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) /* Notify before modifying the ram block and touching the bitmaps. */ if (block->host) { - ram_block_notify_resize(block->host, oldsize, newsize); + ram_block_notify_resize(block, newsize); } physical_memory_clear_dirty_range(block->offset, block->used_length); @@ -2283,7 +2282,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK); } - ram_block_notify_add(new_block->host, new_block->used_length, + ram_block_notify_add(new_block, new_block->host, new_block->used_length, new_block->max_length); } return; @@ -2600,7 +2599,7 @@ void qemu_ram_free(RAMBlock *block) } if (block->host) { - ram_block_notify_remove(block->host, block->used_length, + ram_block_notify_remove(block, block->host, block->used_length, block->max_length); } diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c index 8a1af35ed32b..f29b9c504ea8 100644 --- a/target/i386/nvmm/nvmm-all.c +++ b/target/i386/nvmm/nvmm-all.c @@ -1134,8 +1134,8 @@ static MemoryListener nvmm_memory_listener = { }; static void -nvmm_ram_block_added(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size) +nvmm_ram_block_added(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { struct nvmm_machine *mach = get_nvmm_mach(); uintptr_t hva = (uintptr_t)host; diff --git a/target/i386/sev.c b/target/i386/sev.c index b44b5a1c2b94..63d8d36b6d41 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -329,8 +329,8 @@ sev_set_guest_state(SevCommonState *sev_common, SevState new_state) } static void -sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size) +sev_ram_block_added(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { int r; struct kvm_enc_region range; @@ -359,8 +359,8 @@ sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t size, } static void -sev_ram_block_removed(RAMBlockNotifier *n, void *host, size_t size, - size_t max_size) +sev_ram_block_removed(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { int r; struct kvm_enc_region range; diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index aab0bf9d485d..5059ed44b8af 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -465,8 +465,8 @@ fail_container: return ret; } -static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host, - size_t size, size_t max_size) +static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier); Error *local_err = NULL; @@ -481,7 +481,8 @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host, } } -static void qemu_vfio_ram_block_removed(RAMBlockNotifier *n, void *host, +static void qemu_vfio_ram_block_removed(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) { QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier); -- 2.54.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] system/physmem: Notify RAMBlock migratable and idstr changes 2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki 2026-06-11 6:35 ` [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks Akihiko Odaki @ 2026-06-11 6:35 ` Akihiko Odaki 2026-06-11 6:35 ` [PATCH 3/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki 2026-06-22 20:23 ` [PATCH 0/3] " Peter Xu 3 siblings, 0 replies; 9+ messages in thread From: Akihiko Odaki @ 2026-06-11 6:35 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Peter Xu, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm, Akihiko Odaki The RAM migration code assumes that the set of migratable RAMBlocks and their idstr values do not change while migration is running. Add RAMBlockNotifier callbacks for RAM_MIGRATABLE flag and idstr changes so migration can detect attempts to make such changes at runtime and abort with a clear error. Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> --- include/system/ramlist.h | 6 ++++++ hw/core/numa.c | 36 ++++++++++++++++++++++++++++++++++++ system/physmem.c | 9 +++++++++ 3 files changed, 51 insertions(+) diff --git a/include/system/ramlist.h b/include/system/ramlist.h index 32157cc84305..e96de5573eed 100644 --- a/include/system/ramlist.h +++ b/include/system/ramlist.h @@ -66,6 +66,9 @@ struct RAMBlockNotifier { void *host, size_t size, size_t max_size); void (*ram_block_removed)(RAMBlockNotifier *n, const RAMBlock *rb, void *host, size_t size, size_t max_size); + void (*ram_block_set_migratable)(RAMBlockNotifier *n, const RAMBlock *rb); + void (*ram_block_unset_migratable)(RAMBlockNotifier *n, const RAMBlock *rb); + void (*ram_block_set_idstr)(RAMBlockNotifier *n, const RAMBlock *rb); void (*ram_block_resized)(RAMBlockNotifier *n, RAMBlock *rb, size_t new_size); QLIST_ENTRY(RAMBlockNotifier) next; @@ -81,6 +84,9 @@ void ram_block_notify_add(const RAMBlock *rb, void *host, size_t size, size_t max_size); void ram_block_notify_remove(const RAMBlock *rb, void *host, size_t size, size_t max_size); +void ram_block_notify_set_migratable(const RAMBlock *rb); +void ram_block_notify_unset_migratable(const RAMBlock *rb); +void ram_block_notify_set_idstr(const RAMBlock *rb); void ram_block_notify_resize(RAMBlock *rb, size_t new_size); GString *ram_block_format(void); diff --git a/hw/core/numa.c b/hw/core/numa.c index 40acb98bdd0b..0685cd1c6ccc 100644 --- a/hw/core/numa.c +++ b/hw/core/numa.c @@ -887,6 +887,42 @@ void ram_block_notify_remove(const RAMBlock *rb, } } +void ram_block_notify_set_migratable(const RAMBlock *rb) +{ + RAMBlockNotifier *notifier; + RAMBlockNotifier *next; + + QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { + if (notifier->ram_block_set_migratable) { + notifier->ram_block_set_migratable(notifier, rb); + } + } +} + +void ram_block_notify_unset_migratable(const RAMBlock *rb) +{ + RAMBlockNotifier *notifier; + RAMBlockNotifier *next; + + QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { + if (notifier->ram_block_unset_migratable) { + notifier->ram_block_unset_migratable(notifier, rb); + } + } +} + +void ram_block_notify_set_idstr(const RAMBlock *rb) +{ + RAMBlockNotifier *notifier; + RAMBlockNotifier *next; + + QLIST_FOREACH_SAFE(notifier, &ram_list.ramblock_notifiers, next, next) { + if (notifier->ram_block_set_idstr) { + notifier->ram_block_set_idstr(notifier, rb); + } + } +} + void ram_block_notify_resize(RAMBlock *rb, size_t new_size) { RAMBlockNotifier *notifier; diff --git a/system/physmem.c b/system/physmem.c index 6d00e99270c7..83c9243236a5 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -1911,11 +1911,17 @@ bool qemu_ram_is_migratable(const RAMBlock *rb) void qemu_ram_set_migratable(RAMBlock *rb) { + /* Notify before modifying the ram block. */ + ram_block_notify_set_migratable(rb); + rb->flags |= RAM_MIGRATABLE; } void qemu_ram_unset_migratable(RAMBlock *rb) { + /* Notify before modifying the ram block. */ + ram_block_notify_unset_migratable(rb); + rb->flags &= ~RAM_MIGRATABLE; } @@ -1937,6 +1943,9 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev) assert(new_block); assert(!new_block->idstr[0]); + /* Notify before modifying the ram block. */ + ram_block_notify_set_idstr(new_block); + if (dev) { char *id = qdev_get_dev_path(dev); if (id) { -- 2.54.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki 2026-06-11 6:35 ` [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks Akihiko Odaki 2026-06-11 6:35 ` [PATCH 2/3] system/physmem: Notify RAMBlock migratable and idstr changes Akihiko Odaki @ 2026-06-11 6:35 ` Akihiko Odaki 2026-06-22 20:23 ` [PATCH 0/3] " Peter Xu 3 siblings, 0 replies; 9+ messages in thread From: Akihiko Odaki @ 2026-06-11 6:35 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Peter Xu, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm, Akihiko Odaki ram_mig_ram_block_resized() already aborts migration when migratable RAM is resized. Extend the same handling to other unsupported changes to the migratable RAMBlock set, such as removing a migratable RAMBlock, changing a RAMBlock's migratable state, or setting a RAMBlock's idstr. Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> --- include/migration/misc.h | 2 +- migration/ram.c | 117 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/include/migration/misc.h b/include/migration/misc.h index 3159a5e53c3c..91015524a83d 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -40,7 +40,7 @@ void precopy_remove_notifier(NotifierWithReturn *n); int precopy_notify(PrecopyNotifyReason reason, Error **errp); void qemu_guest_free_page_hint(void *addr, size_t len); -bool migrate_ram_is_ignored(RAMBlock *block); +bool migrate_ram_is_ignored(const RAMBlock *block); /* migration/block.c */ diff --git a/migration/ram.c b/migration/ram.c index 6bc7f705d31a..eb761644520f 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -224,7 +224,7 @@ static bool postcopy_preempt_active(void) return migrate_postcopy_preempt() && migration_in_postcopy(); } -bool migrate_ram_is_ignored(RAMBlock *block) +bool migrate_ram_is_ignored(const RAMBlock *block) { MigMode mode = migrate_mode(); return !qemu_ram_is_migratable(block) || @@ -364,6 +364,12 @@ struct RAMSrcPageRequest { QSIMPLEQ_ENTRY(RAMSrcPageRequest) next_req; }; +typedef enum RAMStateStage { + RAM_STATE_STAGE_ITERATING, + RAM_STATE_STAGE_COMPLETING, + RAM_STATE_STAGE_COMPLETED, +} RAMStateStage; + /* State of RAM for migration */ struct RAMState { /* @@ -398,8 +404,8 @@ struct RAMState { uint64_t xbzrle_bytes_prev; /* Are we really using XBZRLE (e.g., after the first round). */ bool xbzrle_started; - /* Are we on the last stage of migration */ - bool last_stage; + /* Migration stage */ + RAMStateStage stage; /* total handled target pages at the beginning of period */ uint64_t target_page_count_prev; @@ -635,7 +641,7 @@ static int save_xbzrle_page(RAMState *rs, PageSearchStatus *pss, if (!cache_is_cached(XBZRLE.cache, current_addr, generation)) { xbzrle_counters.cache_miss++; - if (!rs->last_stage) { + if (rs->stage == RAM_STATE_STAGE_ITERATING) { if (cache_insert(XBZRLE.cache, current_addr, *current_data, generation) == -1) { return -1; @@ -674,7 +680,7 @@ static int save_xbzrle_page(RAMState *rs, PageSearchStatus *pss, * Update the cache contents, so that it corresponds to the data * sent, in all cases except where we skip the page. */ - if (!rs->last_stage && encoded_len != 0) { + if (rs->stage == RAM_STATE_STAGE_ITERATING && encoded_len != 0) { memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE); /* * In the case where we couldn't compress, ensure that the caller @@ -840,7 +846,8 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs, * * Do the same for postcopy due to the same reason. */ - if (!rs->last_stage && !migration_in_postcopy()) { + if (rs->stage == RAM_STATE_STAGE_ITERATING && + !migration_in_postcopy()) { /* * Clear dirty bitmap if needed. This _must_ be called before we * send any of the page in the chunk because we need to make sure @@ -1316,7 +1323,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss) if (rs->xbzrle_started && !migration_in_postcopy()) { pages = save_xbzrle_page(rs, pss, &p, current_addr, block, offset); - if (!rs->last_stage) { + if (rs->stage == RAM_STATE_STAGE_ITERATING) { /* Can't send this cached data async, since the cache page * might get updated before it gets to the wire */ @@ -2920,6 +2927,8 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out) RAMBlock *block; uint64_t pages = 0; + rs->stage = RAM_STATE_STAGE_ITERATING; + /* * Postcopy is not using xbzrle/compression, so no need for that. * Also, since source are already halted, we don't need to care @@ -3373,7 +3382,9 @@ static int ram_save_complete(QEMUFile *f, void *opaque) trace_ram_save_complete(rs->migration_dirty_pages, 0); - rs->last_stage = !migration_in_colo_state(); + if (!migration_in_colo_state()) { + rs->stage = RAM_STATE_STAGE_COMPLETING; + } WITH_RCU_READ_LOCK_GUARD() { if (!migration_in_postcopy()) { @@ -3383,7 +3394,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque) ret = rdma_registration_start(f, RAM_CONTROL_FINISH); if (ret < 0) { qemu_file_set_error(f, ret); - return ret; + goto err; } /* try transferring iterative blocks of memory */ @@ -3400,7 +3411,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque) } if (pages < 0) { qemu_mutex_unlock(&rs->bitmap_mutex); - return pages; + ret = pages; + goto err; } } qemu_mutex_unlock(&rs->bitmap_mutex); @@ -3408,7 +3420,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque) ret = rdma_registration_stop(f, RAM_CONTROL_FINISH); if (ret < 0) { qemu_file_set_error(f, ret); - return ret; + goto err; } } @@ -3419,7 +3431,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque) */ ret = multifd_ram_flush_and_sync(f); if (ret < 0) { - return ret; + goto err; } } @@ -3428,10 +3440,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque) if (qemu_file_get_error(f)) { Error *local_err = NULL; - int err = qemu_file_get_error_obj(f, &local_err); + ret = -qemu_file_get_error_obj(f, &local_err); error_reportf_err(local_err, "Failed to write bitmap to file: "); - return -err; + goto err; } } @@ -3439,7 +3451,14 @@ static int ram_save_complete(QEMUFile *f, void *opaque) trace_ram_save_complete(rs->migration_dirty_pages, 1); - return qemu_fflush(f); + ret = qemu_fflush(f); + +err: + if (!migration_in_colo_state()) { + rs->stage = RAM_STATE_STAGE_COMPLETED; + } + + return ret; } static void ram_state_pending(void *opaque, MigPendingData *pending, @@ -4699,6 +4718,68 @@ static SaveVMHandlers savevm_ram_handlers = { .save_postcopy_prepare = ram_save_postcopy_prepare, }; +static bool ram_migration_is_running(void) +{ + return ram_state && ram_state->stage != RAM_STATE_STAGE_COMPLETED; +} + +static void ram_mig_ram_block_set_migratable(RAMBlockNotifier *n, + const RAMBlock *rb) +{ + Error *err = NULL; + + if (!ram_migration_is_running()) { + return; + } + + error_setg(&err, "RAM block '%s' set migratable during precopy.", + rb->idstr); + migrate_error_propagate(migrate_get_current(), err); + migration_cancel(); +} + +static void ram_mig_ram_block_unset_migratable(RAMBlockNotifier *n, + const RAMBlock *rb) +{ + Error *err = NULL; + + if (!ram_migration_is_running()) { + return; + } + + error_setg(&err, "RAM block '%s' unset migratable during precopy.", + rb->idstr); + migrate_error_propagate(migrate_get_current(), err); + migration_cancel(); +} + +static void ram_mig_ram_block_set_idstr(RAMBlockNotifier *n, const RAMBlock *rb) +{ + Error *err = NULL; + + if (!ram_migration_is_running() || migrate_ram_is_ignored(rb)) { + return; + } + + error_setg(&err, "RAM block idstr set during precopy."); + migrate_error_propagate(migrate_get_current(), err); + migration_cancel(); +} + +static void ram_mig_ram_block_removed(RAMBlockNotifier *n, const RAMBlock *rb, + void *host, size_t size, size_t max_size) +{ + Error *err = NULL; + + if (!rb || !ram_migration_is_running() || migrate_ram_is_ignored(rb)) { + return; + } + + error_setg(&err, "RAM block '%s' removed during precopy.", rb->idstr); + migrate_error_propagate(migrate_get_current(), err); + migration_cancel(); +} + static void ram_mig_ram_block_resized(RAMBlockNotifier *n, RAMBlock *rb, size_t new_size) { @@ -4710,7 +4791,7 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, RAMBlock *rb, return; } - if (migration_is_running()) { + if (ram_migration_is_running()) { /* * Precopy code on the source cannot deal with the size of RAM blocks * changing at random points in time - especially after sending the @@ -4754,6 +4835,10 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, RAMBlock *rb, } static RAMBlockNotifier ram_mig_ram_notifier = { + .ram_block_removed = ram_mig_ram_block_removed, + .ram_block_set_migratable = ram_mig_ram_block_set_migratable, + .ram_block_unset_migratable = ram_mig_ram_block_unset_migratable, + .ram_block_set_idstr = ram_mig_ram_block_set_idstr, .ram_block_resized = ram_mig_ram_block_resized, }; -- 2.54.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki ` (2 preceding siblings ...) 2026-06-11 6:35 ` [PATCH 3/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki @ 2026-06-22 20:23 ` Peter Xu 2026-06-23 12:05 ` Akihiko Odaki 3 siblings, 1 reply; 9+ messages in thread From: Peter Xu @ 2026-06-22 20:23 UTC (permalink / raw) To: Akihiko Odaki Cc: qemu-devel, Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote: > Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp> > ("[PATCH] system/physmem: Assert migration invariants") > > ram_mig_ram_block_resized() already aborts migration when migratable RAM > is resized. Extend the same handling to other unsupported changes to the > migratable RAMBlock set, such as removing a migratable RAMBlock or > changing a RAMBlock's migratable state. > > Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> > --- > Akihiko Odaki (3): > system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks > system/physmem: Notify RAMBlock migratable and idstr changes > migration/ram: Abort on unsupported migratable RAM changes Thanks for looking at this, Akihiko. I understand this is a protection to the system to trap error use cases. The question I have is do we have any possible way to trigger these. I worry we add a bunch of code and notifiers, and then there's zero way to trigger, essentially add dead code. Logically we could already add assert() on things we don't expect to happen. This case might be slightly risky, but still I think we can also consider things like error_report_once() instead of introducing slightly complex notifiers just to cover what we think shouldn't happen. Or do you have way to trigger any of these notifiers? PS: today I went back and I wanted to try how the existing resize() notifier would trigger, I can't even reproduce it with David's example here: https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size, so it didn't really notify a thing. I don't really know how to trigger ram_block_notify_resize(). If you know, please share. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-22 20:23 ` [PATCH 0/3] " Peter Xu @ 2026-06-23 12:05 ` Akihiko Odaki 2026-06-23 15:45 ` Peter Xu 0 siblings, 1 reply; 9+ messages in thread From: Akihiko Odaki @ 2026-06-23 12:05 UTC (permalink / raw) To: Peter Xu Cc: qemu-devel, Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm On 2026/06/23 5:23, Peter Xu wrote: > On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote: >> Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp> >> ("[PATCH] system/physmem: Assert migration invariants") >> >> ram_mig_ram_block_resized() already aborts migration when migratable RAM >> is resized. Extend the same handling to other unsupported changes to the >> migratable RAMBlock set, such as removing a migratable RAMBlock or >> changing a RAMBlock's migratable state. >> >> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> >> --- >> Akihiko Odaki (3): >> system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks >> system/physmem: Notify RAMBlock migratable and idstr changes >> migration/ram: Abort on unsupported migratable RAM changes > > Thanks for looking at this, Akihiko. > > I understand this is a protection to the system to trap error use cases. > The question I have is do we have any possible way to trigger these. > > I worry we add a bunch of code and notifiers, and then there's zero way to > trigger, essentially add dead code. > > Logically we could already add assert() on things we don't expect to > happen. This case might be slightly risky, but still I think we can also > consider things like error_report_once() instead of introducing slightly > complex notifiers just to cover what we think shouldn't happen. > > Or do you have way to trigger any of these notifiers? I simply followed what's already done for resize(), expecting resize() does the correct thing and following it won't introduce a regression. > > PS: today I went back and I wanted to try how the existing resize() > notifier would trigger, I can't even reproduce it with David's example > here: > > https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t > > I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size, > so it didn't really notify a thing. I don't really know how to trigger > ram_block_notify_resize(). If you know, please share. I made an LLM amend the reproducer. Below is its output. Regards, Akihiko Odaki LLM output: A synthetic but effective variant is to add custom ACPI filler tables so the initial `etc/acpi/tables` blob is just under the 128 KiB alignment bucket, then let the normal boot-time fw_cfg ACPI rebuild push it over. I tested this shape: ```sh truncate -s 65000 /tmp/fill1 truncate -s 50600 /tmp/fill2 ``` Then add to the original-ish command: ```sh -device pcie-root-port,id=rp0,chassis=1,slot=1 \ -acpitable sig=FI1A,data=/tmp/fill1 \ -acpitable sig=FI2A,data=/tmp/fill2 ``` Observed via `info ramblock`: ```text before cont: /rom@etc/acpi/tables Used 0x0000000000020000 after cont: /rom@etc/acpi/tables Used 0x0000000000040000 ``` So this does produce a real RAMBlock used-size growth during boot in the current tree. With migration started before `cont` using a stalled `exec:` target, `info migrate` moved to `cancelling`, which is consistent with the current resize-during-precopy abort path. The key is not the root port itself; the key is making the ACPI table rebuild cross `ACPI_BUILD_TABLE_SIZE` alignment. The filler is a bit artificial, but it is a good stress variant for the exact class of bug. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-23 12:05 ` Akihiko Odaki @ 2026-06-23 15:45 ` Peter Xu 2026-06-23 16:38 ` Akihiko Odaki 0 siblings, 1 reply; 9+ messages in thread From: Peter Xu @ 2026-06-23 15:45 UTC (permalink / raw) To: Akihiko Odaki Cc: qemu-devel, Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm On Tue, Jun 23, 2026 at 09:05:22PM +0900, Akihiko Odaki wrote: > On 2026/06/23 5:23, Peter Xu wrote: > > On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote: > > > Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp> > > > ("[PATCH] system/physmem: Assert migration invariants") > > > > > > ram_mig_ram_block_resized() already aborts migration when migratable RAM > > > is resized. Extend the same handling to other unsupported changes to the > > > migratable RAMBlock set, such as removing a migratable RAMBlock or > > > changing a RAMBlock's migratable state. > > > > > > Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> > > > --- > > > Akihiko Odaki (3): > > > system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks > > > system/physmem: Notify RAMBlock migratable and idstr changes > > > migration/ram: Abort on unsupported migratable RAM changes > > > > Thanks for looking at this, Akihiko. > > > > I understand this is a protection to the system to trap error use cases. > > The question I have is do we have any possible way to trigger these. > > > > I worry we add a bunch of code and notifiers, and then there's zero way to > > trigger, essentially add dead code. > > > > Logically we could already add assert() on things we don't expect to > > happen. This case might be slightly risky, but still I think we can also > > consider things like error_report_once() instead of introducing slightly > > complex notifiers just to cover what we think shouldn't happen. > > > > Or do you have way to trigger any of these notifiers? > > I simply followed what's already done for resize(), expecting resize() does > the correct thing and following it won't introduce a regression. > > > > > PS: today I went back and I wanted to try how the existing resize() > > notifier would trigger, I can't even reproduce it with David's example > > here: > > > > https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t > > > > I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size, > > so it didn't really notify a thing. I don't really know how to trigger > > ram_block_notify_resize(). If you know, please share. > I made an LLM amend the reproducer. Below is its output. > > Regards, > Akihiko Odaki > > LLM output: > > A synthetic but effective variant is to add custom ACPI filler tables so the > initial `etc/acpi/tables` blob is just under the 128 KiB alignment bucket, > then let the normal boot-time fw_cfg ACPI rebuild push it over. > > I tested this shape: > > ```sh > truncate -s 65000 /tmp/fill1 > truncate -s 50600 /tmp/fill2 > ``` > > Then add to the original-ish command: > > ```sh > -device pcie-root-port,id=rp0,chassis=1,slot=1 \ > -acpitable sig=FI1A,data=/tmp/fill1 \ > -acpitable sig=FI2A,data=/tmp/fill2 > ``` These lines should inject some sections into ACPI, but I don't see why the acpi table would change: that should be appended right at QEMU boots, so I expect the ACPI table to grow indeed comparing to when without these lines, but not resize during VM running. I wonder if below is hallucinations from the AI. > > Observed via `info ramblock`: > > ```text > before cont: > /rom@etc/acpi/tables Used 0x0000000000020000 > > after cont: > /rom@etc/acpi/tables Used 0x0000000000040000 > ``` > > So this does produce a real RAMBlock used-size growth during boot in the > current tree. With migration started before `cont` using a stalled `exec:` > target, `info migrate` moved to `cancelling`, which is consistent with the > current resize-during-precopy abort path. > > The key is not the root port itself; the key is making the ACPI table > rebuild cross `ACPI_BUILD_TABLE_SIZE` alignment. The filler is a bit > artificial, but it is a good stress variant for the exact class of bug. I did have a closer look on this whole "MR size can change" thing. We have two users: ACPI (rom_add_blob()) and other firmwares (most of them rom_add_file() users, very little used rom_add_blob()). AFAIU, the real resize should only happen at the 2nd user, not ACPI. ACPI seems to be able to change ROM size (PS: this is tricky to call it ROM in the first place: I believe it's only a data blob in fw_cfg) when e.g. it scans the pci bus and things changed, only happen during reboot, but it can't happen during migration because qdev_add is forbidden. Device ROMs can really change size if dest host has newer firmware packages than source, but that's another use case and I _think_ we support fine, except that firmwares can only grow not shrink, guarded by qemu_ram_resize() check on max_length. That's a pretty niche use case and nothing I can think of that on change of flipping migratable and so on. So IMHO we will need to understand the problem better before having more notifiers. PS: I wished ACPI three use cases of ROM can be part of device states already, then it is out of question on MR resize complexity: the max size is 128K as far as I know; it doesn't need iterability... we migrate devices sometimes much larger than 128KB on device states. It can be a VMSD field. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-23 15:45 ` Peter Xu @ 2026-06-23 16:38 ` Akihiko Odaki 2026-06-29 18:21 ` Peter Xu 0 siblings, 1 reply; 9+ messages in thread From: Akihiko Odaki @ 2026-06-23 16:38 UTC (permalink / raw) To: Peter Xu Cc: qemu-devel, Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm On 2026/06/24 0:45, Peter Xu wrote: > On Tue, Jun 23, 2026 at 09:05:22PM +0900, Akihiko Odaki wrote: >> On 2026/06/23 5:23, Peter Xu wrote: >>> On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote: >>>> Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp> >>>> ("[PATCH] system/physmem: Assert migration invariants") >>>> >>>> ram_mig_ram_block_resized() already aborts migration when migratable RAM >>>> is resized. Extend the same handling to other unsupported changes to the >>>> migratable RAMBlock set, such as removing a migratable RAMBlock or >>>> changing a RAMBlock's migratable state. >>>> >>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> >>>> --- >>>> Akihiko Odaki (3): >>>> system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks >>>> system/physmem: Notify RAMBlock migratable and idstr changes >>>> migration/ram: Abort on unsupported migratable RAM changes >>> >>> Thanks for looking at this, Akihiko. >>> >>> I understand this is a protection to the system to trap error use cases. >>> The question I have is do we have any possible way to trigger these. >>> >>> I worry we add a bunch of code and notifiers, and then there's zero way to >>> trigger, essentially add dead code. >>> >>> Logically we could already add assert() on things we don't expect to >>> happen. This case might be slightly risky, but still I think we can also >>> consider things like error_report_once() instead of introducing slightly >>> complex notifiers just to cover what we think shouldn't happen. >>> >>> Or do you have way to trigger any of these notifiers? >> >> I simply followed what's already done for resize(), expecting resize() does >> the correct thing and following it won't introduce a regression. >> >>> >>> PS: today I went back and I wanted to try how the existing resize() >>> notifier would trigger, I can't even reproduce it with David's example >>> here: >>> >>> https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t >>> >>> I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size, >>> so it didn't really notify a thing. I don't really know how to trigger >>> ram_block_notify_resize(). If you know, please share. >> I made an LLM amend the reproducer. Below is its output. >> >> Regards, >> Akihiko Odaki >> >> LLM output: >> >> A synthetic but effective variant is to add custom ACPI filler tables so the >> initial `etc/acpi/tables` blob is just under the 128 KiB alignment bucket, >> then let the normal boot-time fw_cfg ACPI rebuild push it over. >> >> I tested this shape: >> >> ```sh >> truncate -s 65000 /tmp/fill1 >> truncate -s 50600 /tmp/fill2 >> ``` >> >> Then add to the original-ish command: >> >> ```sh >> -device pcie-root-port,id=rp0,chassis=1,slot=1 \ >> -acpitable sig=FI1A,data=/tmp/fill1 \ >> -acpitable sig=FI2A,data=/tmp/fill2 >> ``` > > These lines should inject some sections into ACPI, but I don't see why the > acpi table would change: that should be appended right at QEMU boots, so I > expect the ACPI table to grow indeed comparing to when without these lines, > but not resize during VM running. I wonder if below is hallucinations from > the AI. The resize happens because the ACPI fw_cfg blobs are built lazily when the guest firmware selects them. acpi_add_rom_blob() registers acpi_build_update() as the fw_cfg select callback; after `cont`, firmware reads the fw_cfg ACPI entries, QEMU builds the tables, and acpi_ram_update() calls memory_region_ram_resize(). Below is the reprouction case (LLM-generated): #!/bin/sh set -eu QEMU=${QEMU:-build/qemu-system-x86_64} tmp=$(mktemp -d) trap 'rm -rf "$tmp"' EXIT qmp_migrate() { printf '%s%s%s\n' \ '{"execute":"migrate","arguments":{"channels":[{' \ '"channel-type":"main","addr":{"transport":"exec",' \ '"args":["/bin/sleep","1000"]}}]}}' } truncate -s 65000 "$tmp/fill1" truncate -s 50600 "$tmp/fill2" truncate -s 256M "$tmp/nvdimm" { echo '{"execute":"qmp_capabilities"}' echo '{"execute":"x-query-ramblock"}' qmp_migrate sleep 1 echo '{"execute":"query-migrate"}' echo '{"execute":"cont"}' sleep 3 echo '{"execute":"query-migrate"}' echo '{"execute":"x-query-ramblock"}' echo '{"execute":"quit"}' } | "$QEMU" \ -S \ -machine q35,nvdimm=on,accel=tcg \ -smp 1 \ -cpu max \ -m size=20G,slots=8,maxmem=22G \ -object \ memory-backend-file,id=mem0,mem-path="$tmp/nvdimm",size=256M \ -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \ -nodefaults \ -qmp stdio \ -serial none \ -device vmgenid \ -device intel-iommu \ -acpitable sig=FI1A,data="$tmp/fill1" \ -acpitable sig=FI2A,data="$tmp/fill2" \ -display none Expected markers in the output: /rom@etc/acpi/tables ... Used 0x0000000000020000 "status": "active" "status": "cancelling", "error-desc": "RAM block '/rom@etc/acpi/tables' resized during precopy." /rom@etc/acpi/tables ... Used 0x0000000000040000 Regards, Akihiko Odaki > >> >> Observed via `info ramblock`: >> >> ```text >> before cont: >> /rom@etc/acpi/tables Used 0x0000000000020000 >> >> after cont: >> /rom@etc/acpi/tables Used 0x0000000000040000 >> ``` >> >> So this does produce a real RAMBlock used-size growth during boot in the >> current tree. With migration started before `cont` using a stalled `exec:` >> target, `info migrate` moved to `cancelling`, which is consistent with the >> current resize-during-precopy abort path. >> >> The key is not the root port itself; the key is making the ACPI table >> rebuild cross `ACPI_BUILD_TABLE_SIZE` alignment. The filler is a bit >> artificial, but it is a good stress variant for the exact class of bug. > > I did have a closer look on this whole "MR size can change" thing. > > We have two users: ACPI (rom_add_blob()) and other firmwares (most of them > rom_add_file() users, very little used rom_add_blob()). > > AFAIU, the real resize should only happen at the 2nd user, not ACPI. > > ACPI seems to be able to change ROM size (PS: this is tricky to call it ROM > in the first place: I believe it's only a data blob in fw_cfg) when e.g. it > scans the pci bus and things changed, only happen during reboot, but it > can't happen during migration because qdev_add is forbidden. > > Device ROMs can really change size if dest host has newer firmware packages > than source, but that's another use case and I _think_ we support fine, > except that firmwares can only grow not shrink, guarded by > qemu_ram_resize() check on max_length. > > That's a pretty niche use case and nothing I can think of that on change of > flipping migratable and so on. So IMHO we will need to understand the > problem better before having more notifiers. > > PS: I wished ACPI three use cases of ROM can be part of device states > already, then it is out of question on MR resize complexity: the max size > is 128K as far as I know; it doesn't need iterability... we migrate devices > sometimes much larger than 128KB on device states. It can be a VMSD field. > > Thanks, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes 2026-06-23 16:38 ` Akihiko Odaki @ 2026-06-29 18:21 ` Peter Xu 0 siblings, 0 replies; 9+ messages in thread From: Peter Xu @ 2026-06-29 18:21 UTC (permalink / raw) To: Akihiko Odaki Cc: qemu-devel, Kevin Wolf, Hanna Reitz, Philippe Mathieu-Daudé, Zhao Liu, Stefano Stabellini, Anthony PERARD, Edgar E. Iglesias, Fabiano Rosas, Paolo Bonzini, Reinoud Zandijk, Marcelo Tosatti, Alex Williamson, Cédric Le Goater, qemu-block, xen-devel, kvm On Wed, Jun 24, 2026 at 01:38:32AM +0900, Akihiko Odaki wrote: > On 2026/06/24 0:45, Peter Xu wrote: > > On Tue, Jun 23, 2026 at 09:05:22PM +0900, Akihiko Odaki wrote: > > > On 2026/06/23 5:23, Peter Xu wrote: > > > > On Thu, Jun 11, 2026 at 03:35:47PM +0900, Akihiko Odaki wrote: > > > > > Supersedes: <20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp> > > > > > ("[PATCH] system/physmem: Assert migration invariants") > > > > > > > > > > ram_mig_ram_block_resized() already aborts migration when migratable RAM > > > > > is resized. Extend the same handling to other unsupported changes to the > > > > > migratable RAMBlock set, such as removing a migratable RAMBlock or > > > > > changing a RAMBlock's migratable state. > > > > > > > > > > Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> > > > > > --- > > > > > Akihiko Odaki (3): > > > > > system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks > > > > > system/physmem: Notify RAMBlock migratable and idstr changes > > > > > migration/ram: Abort on unsupported migratable RAM changes > > > > > > > > Thanks for looking at this, Akihiko. > > > > > > > > I understand this is a protection to the system to trap error use cases. > > > > The question I have is do we have any possible way to trigger these. > > > > > > > > I worry we add a bunch of code and notifiers, and then there's zero way to > > > > trigger, essentially add dead code. > > > > > > > > Logically we could already add assert() on things we don't expect to > > > > happen. This case might be slightly risky, but still I think we can also > > > > consider things like error_report_once() instead of introducing slightly > > > > complex notifiers just to cover what we think shouldn't happen. > > > > > > > > Or do you have way to trigger any of these notifiers? > > > > > > I simply followed what's already done for resize(), expecting resize() does > > > the correct thing and following it won't introduce a regression. > > > > > > > > > > > PS: today I went back and I wanted to try how the existing resize() > > > > notifier would trigger, I can't even reproduce it with David's example > > > > here: > > > > > > > > https://lore.kernel.org/qemu-devel/20210429112708.12291-1-david@redhat.com/#t > > > > > > > > I can trap a qemu_ram_resize(), but that's invoked with newsize==rb->size, > > > > so it didn't really notify a thing. I don't really know how to trigger > > > > ram_block_notify_resize(). If you know, please share. > > > I made an LLM amend the reproducer. Below is its output. > > > > > > Regards, > > > Akihiko Odaki > > > > > > LLM output: > > > > > > A synthetic but effective variant is to add custom ACPI filler tables so the > > > initial `etc/acpi/tables` blob is just under the 128 KiB alignment bucket, > > > then let the normal boot-time fw_cfg ACPI rebuild push it over. > > > > > > I tested this shape: > > > > > > ```sh > > > truncate -s 65000 /tmp/fill1 > > > truncate -s 50600 /tmp/fill2 > > > ``` > > > > > > Then add to the original-ish command: > > > > > > ```sh > > > -device pcie-root-port,id=rp0,chassis=1,slot=1 \ > > > -acpitable sig=FI1A,data=/tmp/fill1 \ > > > -acpitable sig=FI2A,data=/tmp/fill2 > > > ``` > > > > These lines should inject some sections into ACPI, but I don't see why the > > acpi table would change: that should be appended right at QEMU boots, so I > > expect the ACPI table to grow indeed comparing to when without these lines, > > but not resize during VM running. I wonder if below is hallucinations from > > the AI. > > The resize happens because the ACPI fw_cfg blobs are built lazily when the > guest firmware selects them. acpi_add_rom_blob() registers > acpi_build_update() as the fw_cfg select callback; after `cont`, firmware > reads the fw_cfg ACPI entries, QEMU builds the tables, and > acpi_ram_update() calls memory_region_ram_resize(). > > Below is the reprouction case (LLM-generated): > > #!/bin/sh > set -eu > > QEMU=${QEMU:-build/qemu-system-x86_64} > tmp=$(mktemp -d) > trap 'rm -rf "$tmp"' EXIT > > qmp_migrate() > { > printf '%s%s%s\n' \ > '{"execute":"migrate","arguments":{"channels":[{' \ > '"channel-type":"main","addr":{"transport":"exec",' \ > '"args":["/bin/sleep","1000"]}}]}}' > } > > truncate -s 65000 "$tmp/fill1" > truncate -s 50600 "$tmp/fill2" > truncate -s 256M "$tmp/nvdimm" > > { > echo '{"execute":"qmp_capabilities"}' > echo '{"execute":"x-query-ramblock"}' > qmp_migrate > sleep 1 > echo '{"execute":"query-migrate"}' > echo '{"execute":"cont"}' > sleep 3 > echo '{"execute":"query-migrate"}' > echo '{"execute":"x-query-ramblock"}' > echo '{"execute":"quit"}' > } | "$QEMU" \ > -S \ > -machine q35,nvdimm=on,accel=tcg \ > -smp 1 \ > -cpu max \ > -m size=20G,slots=8,maxmem=22G \ > -object \ > memory-backend-file,id=mem0,mem-path="$tmp/nvdimm",size=256M \ > -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \ > -nodefaults \ > -qmp stdio \ > -serial none \ > -device vmgenid \ > -device intel-iommu \ > -acpitable sig=FI1A,data="$tmp/fill1" \ > -acpitable sig=FI2A,data="$tmp/fill2" \ > -display none > > Expected markers in the output: > > /rom@etc/acpi/tables ... Used 0x0000000000020000 > "status": "active" > "status": "cancelling", "error-desc": "RAM block '/rom@etc/acpi/tables' > resized during precopy." > /rom@etc/acpi/tables ... Used 0x0000000000040000 Yes, this script did trigger this. I didn't check why -acpitable is special, but still, it isn't a real-life use case to me, and after "cont" it won't change. It's just still too special so far so less of a concern to me. The other thing is, it also didn't further prove there's any possible update of e.g. migratable flag or idstr that would justify the new notifiers are not dead code.. On the ACPI code alone, I kept thinking we shouldn't use resize() callback on its own; we already have vmstate_fw_cfg_acpi_mr, I think we could have marked all three regions non-migratable then migrate them in the same VMSD.. It saves all these complexities on thinking about what could happen. But the same applies here, if there's no real-life issue with it, we don't need to bother either.. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-06-29 18:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-11 6:35 [PATCH 0/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki 2026-06-11 6:35 ` [PATCH 1/3] system/physmem: Pass RAMBlock to RAMBlockNotifier callbacks Akihiko Odaki 2026-06-11 6:35 ` [PATCH 2/3] system/physmem: Notify RAMBlock migratable and idstr changes Akihiko Odaki 2026-06-11 6:35 ` [PATCH 3/3] migration/ram: Abort on unsupported migratable RAM changes Akihiko Odaki 2026-06-22 20:23 ` [PATCH 0/3] " Peter Xu 2026-06-23 12:05 ` Akihiko Odaki 2026-06-23 15:45 ` Peter Xu 2026-06-23 16:38 ` Akihiko Odaki 2026-06-29 18:21 ` Peter Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox