Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [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