[PATCH] system/physmem: Assert migration invariants

Akihiko Odaki posted 1 patch 3 days, 15 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260604-migration-v1-1-cef4a5b1bbdd@rsg.ci.i.u-tokyo.ac.jp
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@mailo.com>
system/physmem.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] system/physmem: Assert migration invariants
Posted by Akihiko Odaki 3 days, 15 hours ago
The migration stream assumes that the set of migratable RAMBlocks does
not change while migration is running. Assert that RAMBlocks are not
made migratable or non-migratable during migration, and that migratable
RAMBlocks are not freed during migration.

Non-migratable RAMBlocks may still be allocated or freed during
migration; for example, QMP object-add of memory-backend-ram creates a
non-migratable RAMBlock without exposing it to the guest.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 system/physmem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/system/physmem.c b/system/physmem.c
index 7bcbf8757361..42a39141645e 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -39,6 +39,7 @@
 #include "hw/core/qdev.h"
 #include "hw/core/qdev-properties.h"
 #include "hw/core/boards.h"
+#include "migration/misc.h"
 #include "system/xen.h"
 #include "system/kvm.h"
 #include "system/tcg.h"
@@ -1911,11 +1912,13 @@ bool qemu_ram_is_migratable(const RAMBlock *rb)
 
 void qemu_ram_set_migratable(RAMBlock *rb)
 {
+    assert(!migration_is_running());
     rb->flags |= RAM_MIGRATABLE;
 }
 
 void qemu_ram_unset_migratable(RAMBlock *rb)
 {
+    assert(!migration_is_running());
     rb->flags &= ~RAM_MIGRATABLE;
 }
 
@@ -2599,6 +2602,8 @@ void qemu_ram_free(RAMBlock *block)
         return;
     }
 
+    assert(!migration_is_running() || !qemu_ram_is_migratable(block));
+
     if (block->host) {
         ram_block_notify_remove(block->host, block->used_length,
                                 block->max_length);

---
base-commit: 2db91528542672cf0db78b3f2cc0e22b36302b38
change-id: 20260602-migration-15128dbe4036

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH] system/physmem: Assert migration invariants
Posted by Peter Xu 3 days, 7 hours ago
On Thu, Jun 04, 2026 at 02:26:13PM +0900, Akihiko Odaki wrote:
> The migration stream assumes that the set of migratable RAMBlocks does
> not change while migration is running. Assert that RAMBlocks are not
> made migratable or non-migratable during migration, and that migratable
> RAMBlocks are not freed during migration.
> 
> Non-migratable RAMBlocks may still be allocated or freed during
> migration; for example, QMP object-add of memory-backend-ram creates a
> non-migratable RAMBlock without exposing it to the guest.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  system/physmem.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/system/physmem.c b/system/physmem.c
> index 7bcbf8757361..42a39141645e 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -39,6 +39,7 @@
>  #include "hw/core/qdev.h"
>  #include "hw/core/qdev-properties.h"
>  #include "hw/core/boards.h"
> +#include "migration/misc.h"
>  #include "system/xen.h"
>  #include "system/kvm.h"
>  #include "system/tcg.h"
> @@ -1911,11 +1912,13 @@ bool qemu_ram_is_migratable(const RAMBlock *rb)
>  
>  void qemu_ram_set_migratable(RAMBlock *rb)
>  {
> +    assert(!migration_is_running());
>      rb->flags |= RAM_MIGRATABLE;
>  }
>  
>  void qemu_ram_unset_migratable(RAMBlock *rb)
>  {
> +    assert(!migration_is_running());
>      rb->flags &= ~RAM_MIGRATABLE;
>  }
>  
> @@ -2599,6 +2602,8 @@ void qemu_ram_free(RAMBlock *block)
>          return;
>      }
>  
> +    assert(!migration_is_running() || !qemu_ram_is_migratable(block));
> +

The intention makes sense, however these asserts are slightly risky to me
on a few things..

Firstly, migration_is_running() covers a wide range, the tricky part is
when CANCELLING and FAILING: when QEMU shutdowns with qemu_cleanup(), it
only kicks the migration out with migration_cancel(), it won't wait for
migration to finish completely.  I didn't verify the possibility, but IIUC
it means during QEMU tearing down we may have ramblock freed while
migration_is_running() returns true.

Above will also include possibility of changing migration migratable. In
this specific case, it's about marking ramblock to be unmigratable during
tearing down with a call to vmstate_unregister_ram().

The other thing is, strictly speaking the migration ABI is not ramblock
pointers, but a combination of ramblock ID+page_size+addr+... info, per
ram_save_setup().  I don't know if there's legal user (I'm having some form
of ROMs in mind..) that may silently replace a ramblock pointer but kept
everything else intact..

Thanks,

>      if (block->host) {
>          ram_block_notify_remove(block->host, block->used_length,
>                                  block->max_length);
> 
> ---
> base-commit: 2db91528542672cf0db78b3f2cc0e22b36302b38
> change-id: 20260602-migration-15128dbe4036
> 
> Best regards,
> --  
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 

-- 
Peter Xu
Re: [PATCH] system/physmem: Assert migration invariants
Posted by Philippe Mathieu-Daudé 3 days, 15 hours ago
On 4/6/26 07:26, Akihiko Odaki wrote:
> The migration stream assumes that the set of migratable RAMBlocks does
> not change while migration is running. Assert that RAMBlocks are not
> made migratable or non-migratable during migration, and that migratable
> RAMBlocks are not freed during migration.
> 
> Non-migratable RAMBlocks may still be allocated or freed during
> migration; for example, QMP object-add of memory-backend-ram creates a
> non-migratable RAMBlock without exposing it to the guest.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   system/physmem.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>