Operations that read guest RAM (dump-guest-memory, memsave, pmemsave)
must refuse to run while the destination of a migration is still
receiving that RAM: during precopy it is incomplete, and during postcopy
a read faults the page in from the source. Provide a single predicate
they can share instead of open-coding the runstate and postcopy checks.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20260619101834.228432-2-den@openvz.org>
Alex Bennée reported a ThreadSanitizer warning about a plain concurrent
access to ram_list [1]. Ensure the concurrent accesses to ram_list are
properly synchronized with atomic accesses, mutexes, or RCU.
First, the plain assignments of ram_list.mru_block are replaced with
qatomic_set(). A comment in qemu_get_ram_block() explains why the
ordering requirement is relaxed, but it still needs to be atomically
accessed. include/qemu/atomic.h says:
> The C11 memory model says that variables that are accessed from
> different threads should at least be done with __ATOMIC_RELAXED
> primitives or the result is undefined. Generally this has little to
> no effect on the generated code but not using the atomic primitives
> will get flagged by sanitizers as a violation.
Second, ram_list.version accesses are replaced with atomic operations
or protected with a mutex. Unlike ram_list.mru_block, ram_list.version
has tighter ordering requirements for one of its goals: ensuring that
the reader-held rs->last_seen_block value is invalidated whenever a RAM
block is reclaimed between two RCU reader critical sections. Below are
steps a reader and an updater follow:
Reader:
R-1. Enter the first RCU read-side critical section:
R-1-1. rs->last_version = qatomic_load_acquire(&ram_list.version)
R-1-2. rs->last_seen_block = an element of ram_list.blocks
R-2. Enter the second RCU read-side critical section:
R-2-1. if (qatomic_read(&ram_list.version) != rs->last_version)
R-2-2. rs->last_seen_block = NULL
Updater:
W-1. Enter a ram_list.mutex critical section
W-1-1. Update ram_list.blocks
W-1-2. qatomic_store_release(&ram_list.version, ram_list.version + 1)
W-2. Enter another ram_list.mutex critical section
W-2-1. QLIST_REMOVE_RCU(block, next)
W-2-2. qatomic_store_release(&ram_list.version, ram_list.version + 1)
W-2-3. call_rcu(block, reclaim_ramblock, rcu)
W-1-2 represents the write observed by R-1-1.
ram_list.version is read non-atomically on the update side because the
update side is serialized with ram_list.mutex. The other ram_list
accesses in these steps are reasoned about in two cases.
When the grace period of W-2-3 contains R-2:
qatomic_load_acquire() at R-1-1 and qatomic_store_release() at W-1-2
enforce the following ordering:
W-1-1 -> W-1-2 -> R-1-1 -> R-1-2
The value of ram_list.blocks stored by W-1-1 or a newer value that
was loaded by R-1-2 is still valid because of the grace period.
When the grace period of W-2-3 ends before R-2:
call_rcu() at W-2-3 and the read-side critical section at R-2 ensure
the following ordering:
W-2-2 -> W-2-3 -> the grace period -> R-2 -> R-2-1
The value of ram_list.version stored by W-2-2 or a newer value that
was loaded by R-2-1 differs from rs->last_version and the reader
invalidates rs->last_seen_block.
Together, these steps ensure that rs->last_seen_block is invalidated
whenever necessary. With added atomic operations, pre-existing memory
barriers are no longer necessary and are removed.
Any other ram_list accesses are already properly synchronized.
[1] https://lore.kernel.org/qemu-devel/878q9fbmap.fsf@draig.linaro.org/
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20260523-tsan-v1-1-07d5eb9dcaa2@rsg.ci.i.u-tokyo.ac.jp
Signed-off-by: Peter Xu <peterx@redhat.com>
In GLib, GByteArray is an object managed by the library. Currently,
migrating a GByteArray requires treating it as a raw C struct and using
VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in
ui/vdagent.c
QEMU cannot pretend that GByteArray is a C struct and simply use
VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly
overwrites the data pointer with a newly allocated buffer, thereby
leaking the previous memory. Besides, GLib tracks the array's capacity
in a hidden alloc field. Bypassing GLib APIs leave this capacity out of
sync with the newly allocated buffer, potentially leading to heap buffer
overflows during subsequent g_byte_array_append() calls.
This commit introduces VMSTATE_GBYTEARRAY which uses specific library
API calls (g_byte_array_set_size()) to safely resize and populate the
buffer.
Signed-off-by: Arun Menon <armenon@redhat.com>
Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260423105733.113046-2-armenon@redhat.com
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Use a GHashTable to store cpr fds to reduce the time
consumption of `cpr_find_fd` in scenarios with a large
number of fds. The time complexity for `cpr_find_fd` is
reduced from O(N) to O(1).
Keep cpr fds lookups in a GHashTable during normal runtime
while preserving the existing QLIST migration ABI. Build a
temporary QLIST from the hash table in pre_save and rebuild
the hash table from the loaded QLIST in post_load.
To demonstrate the performance improvement, we tested the total time
consumed by `cpr_find_fd` (called N times for N fds) under our real-world
business scenarios with different numbers of file descriptors. The results
are measured in nanoseconds:
| Number of FDs | Total time with QLIST (ns) | Total time with GHashTable (ns) |
|---------------|----------------------------|---------------------------------|
| 540 | 936,753 | 393,358 |
| 2,870 | 24,102,342 | 2,212,113 |
| 7,530 | 152,715,916 | 5,474,310 |
As shown in the data, the lookup time grows exponentially with the QLIST
as the number of fds increases. With the GHashTable, the time consumption
remains linear (O(1) per lookup), significantly reducing the downtime during
the CPR process.
Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
Link: https://lore.kernel.org/r/20260519134315.27997-1-hongmianquan@bytedance.com
Signed-off-by: Peter Xu <peterx@redhat.com>
multifd_send_sync_main() is called once per RAM synchronization round
during live migration. It iterates over all multifd channels twice
(signal loop + wait loop), calling migrate_multifd_channels()
independently in each loop header.
Cache migrate_multifd_channels() in a local thread_count variable at
function entry, matching the pattern already used in
multifd_send_setup() and multifd_recv_setup(). This eliminates 2
redundant config lookups per sync call.
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260518110112.21395-9-guobin@linux.alibaba.com
Signed-off-by: Peter Xu <peterx@redhat.com>
multifd_send() and multifd_recv() are on the per-page-batch hot path
of live migration. Both functions call migrate_multifd_channels()
multiple times (3-4 calls each) for modulo arithmetic in the
round-robin channel selection loop.
Each call goes through migrate_get_current() -> dereference
MigrationState -> read parameters.multifd_channels. While each
individual call is cheap, these functions execute for every page
batch during the entire migration, easily millions of times.
Cache the return value in a local variable at function entry. The
channel count is fixed for the duration of a migration and cannot
change mid-flight.
For multifd_send(): 3 calls reduced to 1.
For multifd_recv(): 4 calls reduced to 1.
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260518110112.21395-8-guobin@linux.alibaba.com
Signed-off-by: Peter Xu <peterx@redhat.com>
multifd_recv_initial_packet() validates the channel ID received from
the source against the configured number of channels. The current
check uses '>' which allows msg.id == N to pass through. This ID is
then used to index multifd_recv_state->params[msg.id], which was
allocated with g_new0(MultiFDRecvParams, N) -- an out-of-bounds
access.
A malicious or buggy source could send id == N and cause heap
corruption on the destination.
Fix by changing '>' to '>='. Also fix the error message to say
"exceeds channel count" for accuracy.
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260518110112.21395-6-guobin@linux.alibaba.com
Signed-off-by: Peter Xu <peterx@redhat.com>
configuration_validate_capabilities() allocates a bitmap on the heap
to track source capabilities via bitmap_new()/g_free(). Since
MIGRATION_CAPABILITY__MAX is a small compile-time constant (< 64),
a heap allocation for a bitmap this small is wasteful: it adds
malloc/free overhead and a potential cache miss for a transient
8-byte allocation.
Replace with DECLARE_BITMAP() on the stack and bitmap_zero() to
initialize. This eliminates the heap round-trip entirely.
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260518110112.21395-5-guobin@linux.alibaba.com
Signed-off-by: Peter Xu <peterx@redhat.com>
For every NULL slot in a VMS_ARRAY_OF_POINTER (or every entry of a
dynamic array), the saver allocates a 1-element fake VMStateField via
g_new0 and frees it again right after the save. For arrays of
thousands of entries this is thousands of malloc/free pairs on the
hot save path.
Replace the heap-allocated marker with a stack-resident field
populated by an init helper. The caller passes a pointer to a local
VMStateField, the helper fills it in (still asserting the
precondition), and no g_free is needed.
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260518110112.21395-4-guobin@linux.alibaba.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Replaces the direct accesses to global variable `current_migration`
with `migrate_get_current()` to ensure consistency across systems.
Note: Following this only direct access to `current_migration` will be
* `migrate_get_current()` itself
* `migration_object_init()` initializes `current_migration`
* `migration_shutdown()` to pair up with initialization
* `migration_is_running()`, as there might be a case where this function
is called by a thread before object initialization
Signed-off-by: Aadeshveer Singh <aadeshveer07@gmail.com>
Link: https://lore.kernel.org/r/20260513063513.250911-1-aadeshveer07@gmail.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Commit dd4fe8844b changed the reporting of expected downtime behavior, so
that the value will be calculated on-demand. One side effect on the change
is QEMU will allow the calculation to happen anytime even if there's no
transfer happening for a short while.
PeterM reported an ubsan report from clang when running migration-test with
aarch64 binary on x86_64 hosts. I can also reproduce if I run the test
concurrently so some of the src QEMU may not get chance to push any data,
causing mbps to be 0:
../migration/migration.c:1051:12: runtime error: -nan is outside the range of representable values of type 'long'
Fix it by properly handle both Inf and Nan to return INT64_MAX.
Add a rich comment, per PeterM's suggestion.
Link: https://lore.kernel.org/r/CAFEAcA-MYH6C39xO0OLx4-M5pKurJpurwRsMqZe9q=W-NShAbw@mail.gmail.com
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: dd4fe8844b ("migration: Calculate expected downtime on demand")
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20260511182432.1333467-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Marc-André reported an issue on QEMU crash when retrying a cancelled
migration during early setup phase, see "Link:" for more information, and
also easy way to reproduce.
This patch is a replacement of the prior fix proposed by not only switching
to migration_cleanup(), but also fixing it from CPR side, so that we track
hup_source properly to know if src QEMU is waiting or the HUP signal.
To put it simple: this chunk of special casing in migration_cancel() should
not affect normal migration, but only cpr-transfer migration to cover the
small window when the src QEMU is waiting for a HUP signal on cpr
channel (so that src QEMU can continue the migration on the main channel).
To achieve that, we'll also need to remember to detach the hup_source
whenenver invoked: after that point, we should always be able to cleanup
the migration.
It's not a generic operation to explicitly detach a gsource from its
context while in its dispatch() function. But it should be safe, because
gsource disptch() will only happen with a boosted refcount for the
dispatcher so that the gsource will not be freed until the callback
completes. It's also safe to return G_SOURCE_REMOVE after the gsource is
detached, as glib will simply ignore the G_SOURCE_REMOVE.
One can refer to latest 2.86.5 glib code in g_main_dispatch() for that:
https://github.com/GNOME/glib/blob/2.86.5/glib/gmain.c#L3592
When at this, add a bunch of assertions to make sure nothing surprises us.
After this patch applied, the 2nd migration will not crash QEMU, instead
it'll be in CANCELLING until the socket connection times out (it will take
~2min on my Fedora default kernel). During this process no 2nd migration
will be allowed, and after it timed out migration can be restarted.
It's because so far we don't have control over socket_connect_outgoing(),
or anything yet managed by a task executed in qio_task_run_in_thread().
Speeding up the cancellation to be left for future.
I also tested cpr-transfer by only providing cpr channel not the main
channel (with -incoming defer), kickoff migration on source, then cancel it
on source directly without providing the main channel. It keeps working.
I wanted to add an unit test for that but it'll need to refactor current
cpr-transfer tests first; let's leave it for later.
Link: https://lore.kernel.org/r/20260417184742.293061-1-marcandre.lureau@redhat.com
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260421175820.302795-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Various patches related to single binary effort:
- Reduce "exec/cpu-defs.h" inclusions
- Build various target specific files once
- Remove need of per-target monitor handlers
- Reduce target_ulong uses in migration code
- Reduce uses of legacy native endianness & ld/st_phys APIs
- Removed MIPSCPU::mvp memory leak
- Clear dangling GLib event source tag
- Remove pointless variable initialization in *FOREACH*() macro uses
- Few checkpatch.pl updates
# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAmn7S/EACgkQ4+MsLN6t
# wN5bCBAAu05saLnGWy3s9aJ7mrrs2yqSOHRMskrV8/ULr9beJhqLCH7xobuHoSO0
# c3o6BSU9QnBB04Ps3uM2SKcWx2JrhN0em09CjxUW6QRk7pLhvyRWD+JK/JBc8mU+
# kew5ul9udpPeRo8JjyQQzB8qI8ZZZtFQl5iRp4vLxWWjaf3qHOtgnfl+86L9OLCy
# KLuzt2ppppwBOQuOl/i/yZ5JXyx+2cy21m9CmYIX0ApWYC8FngmNTtSgFCTFVu5a
# BZ21EaPGwvvw7OiE0pzY1424BvYKcR6EFQ5NOS1WNl1YvYsq8XeTONRFAFlVR6XC
# OVsaLnQpIMOhi5V7kTCsS5/OH5iNJNK5LVPF2R3e6F9ShcWiNVhR0RONkjpu3e/5
# OIXBfw4swO7rCDZvHg5dSW/up1KRs3XN+jGgvj0CONzxEcXJ/2VJTVdVdv6TqQUn
# dg8q0yfwxHbQZ/lfNH8wRhOmQw35gI3sZk/rGtV+0dTwK2ohQFBewCq6x5aLTKkY
# gMrZ5UvFntpTUBJUEw6L+56qLp3yQk3t5Wn43oQ04iRO6rAeGmHEw9IiVbF/jkxo
# ohe5DzZiUNErd8jbuFNgd/xMRvjKvLgxfwdqnos8yT0wyhhNa88pBUDcFP0diE0j
# sJGFjziBhfLhmrs+2VEsknY1I/Y0yJGN7ENyA5/+VnrW3Hlaa8A=
# =rwvm
# -----END PGP SIGNATURE-----
# gpg: Signature made Wed 06 May 2026 10:10:57 EDT
# gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* tag 'single-binary-20260506' of https://github.com/philmd/qemu: (110 commits)
system/vl: inline qemu_opts_parse_noisily() result checks
scripts/checkpatch: Avoid false positive on empty blocks
cocci: Do not initialize variable used by RAMBLOCK_FOREACH* macro
cocci: Do not initialize variable used by QTAILQ_FOREACH macro
cocci: Do not initialize variable used by QSIMPLEQ_FOREACH macro
cocci: Do not initialize variable used by QSLIST_FOREACH macro
cocci: Do not initialize variable used by QLIST_FOREACH macro
scripts/checkpatch: Reject another license boilerplate pattern
io: use g_clear_handle_id() for GSource cleanup
io: Clear dangling GLib event source tag
target/xtensa/core: register types using type_init
target/s390x: Do not compile KVM stubs for linux-user binary
target/mips: Do not initialize variable used by CPU_FOREACH macro
target/mips: Reduce CPUState scope when used with CPU_FOREACH()
target/riscv: Iterate vCPUs using CPU_FOREACH() macro
target/s390x: Replace cpu_stb_data_ra -> cpu_stb_mmu in STFLE opcode
target/s390x: Compile crypto_helper.c as common unit
target/s390x: Have MSA helper pass a mmu_idx argument
target/s390x: Compile vec_helper.c as common unit
target/s390x: Compile translate.c as common unit
...
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The RAMBLOCK_FOREACH_MIGRATABLE() macro, defined in
migration/ram.h, ends up calling QLIST_FOREACH_RCU()
which always assigns its iterator variable when entering
the loop. Remove the pointless and possibly misleading
assignment.
Mechanical patch using the following coccinelle spatch:
@@
type T;
identifier e;
iterator FOREACH_MACRO =~ ".*_FOREACH.*";
statement S;
@@
- T *e = ...;
+ T *e;
... when != e
FOREACH_MACRO(e, ...) S
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Message-Id: <20260415215539.92629-6-philmd@linaro.org>
The QSIMPLEQ_FOREACH() macro, defined in "qemu/queue.h",
always assigns its iterator variable when entering the
loop. Remove the pointless and possibly misleading
assignment.
Mechanical patch using the following coccinelle spatch:
@@
type T;
identifier e;
iterator FOREACH_MACRO =~ ".*_FOREACH.*";
statement S;
@@
- T *e = ...;
+ T *e;
... when != e
FOREACH_MACRO(e, ...) S
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Message-Id: <20260415215539.92629-4-philmd@linaro.org>
Currently, mgmt can only query for remaining RAM using QMP command
"query-migrate" and monitor the "ram" section. There's no way to report
system-wide remaining data including VFIO devices. It was not a problem
before, because for a very long time RAM was the only part that matters.
After VFIO migrations landed upstream, it may not be enough. There can be
GPU devices that contain GBs of device states. Mgmt may want to know how
much remaining for special devices like VFIO, because all of them will be
accounted as VM data to migrate and will contribute to downtime in the
switchover phase.
Add a new "remaining" field in query-migrate results on the top level,
reflecting system-wide remaining data, which will include everything like
VFIO devices.
This information will be useful for mgmt to implement generic way of stall
detection that covers all system resources. For example, when system-wide
remaining data (especially, if sampled right after each migration
iteration) does not decrease anymore for a relatively long period of time,
then it may imply there is a challenge of converging, mgmt can react based
on how this value changes over time.
Before this patch, "expected_downtime" almost played this role. For
example, by monitoring "expected_downtime" at the beginning of each
iteration can in most cases also reflect the progress of migration
system-wide.
Said that, "expected_downtime" was always calculated based on a bandwidth
value that can fluctuate if avail-switchover-bandwidth is not used. This
new "remaining" field will remove that part of uncertainty for mgmt no
matter if avail-switchover-bandwidth is used by the mgmt.
With the new field, HMP "info migrate" now reports this:
(qemu) info migrate
Status: active
Time (ms): total=12080, setup=14, exp_down=300
Remaining: 1.36 GiB <--- this is the new line
RAM info:
Throughput (Mbps): 840.50
Sizes: pagesize=4 KiB, total=4.02 GiB
Transfers: transferred=1.18 GiB, remain=1.36 GiB
Channels: precopy=1.18 GiB, multifd=0 B, postcopy=0 B
Page Types: normal=307923, zero=388148
Page Rates (pps): transfer=25660
Others: dirty_syncs=1
When VFIO is not involved, the value reported in the new field should be
approximately the same as reported in the "remaining" field of the RAM
section. It is only approximately because the system-wide remaining data
is a cached value, which gets frequently updated by migration core. OTOH,
the RAM's remaining data is accurate.
When VFIO is involved, the new value reported should normally be larger,
because it will include the size of VFIO remaining data too.
Cc: Aseef Imran <aimran@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Acked-by: Markus Armbruster <armbru@redhat.com> # QAPI schema
Link: https://lore.kernel.org/r/20260421202110.306051-15-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Introduce this new counter to remember the total dirty bytes for the whole
system. It will be used for query-migrate command to fetch system-wise
remaining data.
A prior attempt was made to not use this counter but query directly from
all the modules in a QMP handler, but it exposed some complexity not only
on migration state machine race conditions (where the query may be invoked
anytime of the state machine), or on locking implications (where some of
the query hooks may take BQL, which is illegal at least in a QMP handler).
For more information, see:
https://lore.kernel.org/r/aeZMtxqrKWAMKzdN@x1.local
This oneliner will resolve everything, except that it is not as accurate.
The hope is it is a worthwhile trade-off solution, after knowing above
challenges.
Now, there is one more reason we should make each invocation of
save_live_iterate() to be lightweight, because this counter will only get
updated once for each loop over all save_live_iterate() hooks when present.
But that's always the goal.
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-14-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
QEMU will provide an expected downtime for the whole system during
migration, by remembering the total dirty RAM that we synced the last time,
divides the estimated switchover bandwidth.
That was flawed when VFIO is taking into account: consider there is a VFIO
GPU device that contains GBs of data to migrate during stop phase. Those
will not be accounted in this math.
Fix it by updating dirty_bytes_last_sync properly only when we go to the
next iteration, rather than hide this update in the RAM code. Meanwhile,
fetch the total (rather than RAM-only) portion of dirty bytes, so as to
include GPU device states too.
Update the comment of the field to reflect its new meaning.
Now after this change, the expected-downtime to be read from query-migrate
should be very accurate even with VFIO devices involved.
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-13-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
This value does not need to be calculated as frequent. Only calculate it
on demand when query-migrate happened. With that we can remove the
variable in MigrationState.
This paves way for fixing this value to include all modules (not only RAM
but others too).
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-12-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Add a helper migration_get_switchover_bw() to return an estimate of
switchover bandwidth. Use it to simplify the current code.
This will be used in later to remove expected_downtime.
When at it, remove two qatomic_read() to shrink the lines because atomic
ops are not needed when it's always the same thread who does the updates.
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-11-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
It used to hide in RAM dirty sync path. Now with more modules being able
to slow sync on dirty information, keeping it there may not be good anymore
because it's not RAM's own concept for iterations: all modules should
follow.
More importantly, mgmt may try to query dirty info (to make policy
decisions like adjusting downtime) by listening to iteration count changes
via QMP events. So we must make sure the boost of iterations only happens
_after_ the dirty sync operations with whatever form (RAM's dirty bitmap
sync, or VFIO's different ioctls to fetch latest dirty info from kernel).
Move this to core migration path to manage, together with the event
generation, so that it can be well ordered with the sync operations for all
modules.
This brings a good side effect that we should have an old issue regarding
to cpu_throttle_dirty_sync_timer_tick() which can randomly boost iteration
counts (because it invokes sync ops). Now it won't, which is actually the
right behavior.
Said that, we have code (not only QEMU, but likely mgmt too) assuming the
1st iteration will always shows dirty count to 1. Make it initialized with
1 this time, because we'll miss the dirty sync for setup() on boosting this
counter now.
Reviewed-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-10-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Allow modules to report data that can only be migrated after VM is stopped.
When this concept is introduced, we will need to account stopcopy size to
be part of pending_size as before.
However, when there're data only can be migrated in stopcopy phase, it
means the old "pending_size" may not always be able to reach low enough to
kickoff an slow version of query sync.
It used to be almost guaranteed to happen as all prior iterative modules
doesn't have stopcopy only data. VFIO may change that fact by having some
data that must be copied during stop phase.
So we need to make sure QEMU will kickoff a synchronized version of query
pending when all precopy data is migrated. This might be important to VFIO
to keep making progress even if the downtime cannot yet be satisfied.
So far, this patch should introduce no functional change, as no module yet
report stopcopy size.
This paves way for VFIO to properly report its pending data sizes, which
will start to include stop-only data.
Reviewed-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-8-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
This stats is only about RAM, make it accurate. This paves way for
statistics for all devices.
Thanks to Markus, who pointed out that docs/devel/qapi-code-gen.rst has a
section "Compatibility considerations" stated:
Since type names are not visible in the Client JSON Protocol, types
may be freely renamed. Even certain refactorings are invisible, such
as splitting members from one type into a common base type.
Hence this change is not ABI violation according to the document.
While at it, touch up the lines to make it read better, correct the
restriction on migration status being 'active' or 'completed': over time we
grew too many new status that will also report "ram" section.
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: devel@lists.libvirt.org
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-4-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
When QEMU queried the estimated version of pending data and thinks it's
ready to converge, it'll send another accurate query to make sure of it.
It is needed to make sure we collect the latest reports and that equation
still holds true.
However we missed one tiny little difference here on "<" v.s. "<=" when
comparing pending_size (A) to threshold_size (B)..
QEMU src only re-query if A<B, but will kickoff switchover if A<=B.
I think it means it is possible to happen if A (as an estimate only so far)
accidentally equals to B, then re-query won't happen and switchover will
proceed without considering new dirtied data.
It turns out it was an accident in my commit 7aaa1fc072 when refactoring
the code around. Fix this by using the same equation in both places.
Fixes: 7aaa1fc072 ("migration: Rewrite the migration complete detect logic")
Cc: qemu-stable@nongnu.org
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-3-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Use QAPI_CLONE_MEMBERS instead of making an assignment. The QAPI
method makes the handling of the TLS strings more intuitive because it
clones them as well.
This also fixes a segfault when a NULL TLS option is accessed as part
of a validation check for another option (e.g. in the zero-copy +
multifd compression case). Details follow:
Currently, after copying s->parameters to the temporary
MigrationParameters object before migrate_params_check(), the
references in temporary object to the TLS options are dropped, either
because:
a) the user set a new option, in which case that's fine as
s->parameters still holds the reference to the old memory or,
b) the user did not set a new option, in which case keeping the
references in the temporary object would later cause them to be
freed along with it, leading to double-free when s->parameters is
also freed later on.
In this second case, it was overlooked that the TLS options can be
accessed already during migrate_params_check() as part of validation
of another option. Those pointers should not have been cleared.
Using QAPI_CLONE_MEMBERS fixes the issue because the temporary object
is not stealing a reference from s->parameters anymore.
Cc: qemu-stable <qemu-stable@nongnu.org>
Fixes: aed97f0563 ("migration: Normalize tls arguments")
Reported-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Link: https://lore.kernel.org/r/a65a1049-9f19-460a-8e27-a62bb30d2727@maciej.szmigiero.name
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Tested-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Link: https://lore.kernel.org/r/20260414223718.23965-1-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
The package_loaded event is not set in case MIG_RP_MSG_PONG does not
arrive on the source from the destination in the return path thread. The
migration thread would then be blocked waiting for package_loaded event
indefinitely in POSTCOPY_DEVICE state. Where as, in such a condition the
source VM can safely resume as the destination has not yet started. The
pong message can get lost in case of a network failure or destination
crash before sending the pong.
This patch removes the package_loaded event and uses rp_sem, instead of
kicking multiple events. The error is detected in case of network
failure or destination crash and rp_sem is set in the out path of the
return path thread. This will kick the migration thread out from a
condition of indefinitely waiting for rp_sem. The migration thread then
fails early and breaks from the migration loop to resume the vm on the
source side.
Fixes: 7b842fe354 ("migration: Introduce POSTCOPY_DEVICE state")
Signed-off-by: Pranav Tyagi <prtyagi@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20260423094438.43556-1-prtyagi@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
file_write_ramblock_iov() uses single-shot qio_channel_pwritev() and
only checks for ret < 0. A short write (0 <= ret < requested) would be
treated as success.
Switch to qio_channel_pwritev_all() which retries until all bytes are
written or an error occurs.
Fixes: f427d90b98 ("migration/multifd: Support outgoing mapped-ram stream format")
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260420201317.30199-3-junjie.cao@intel.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
qemu_put_buffer_at() and qemu_get_buffer_at() have the same pattern as
the bug fixed in multifd_file_recv_data(): the ssize_t return value from
the channel layer is stored in a size_t variable, and a short transfer
would be mishandled rather than retried.
Switch to qio_channel_pwrite_all() / qio_channel_pread_all() which
handle short transfers internally and make the code more robust and
consistent with the rest of the positioned I/O call sites.
Fixes: 7f5b50a401 ("migration/qemu-file: add utility methods for working with seekable channels")
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260420201317.30199-2-junjie.cao@intel.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
multifd_file_recv_data() stores the return value of qio_channel_pread()
(ssize_t) in a size_t variable. On I/O error the -1 return value wraps
to SIZE_MAX, producing a nonsensical read size in the error message.
More critically, a short read (0 <= ret < data->size) is possible when
the migration file is truncated. In that case qio_channel_pread()
returns a non-negative value without setting *errp. The function then
calls error_prepend(errp, ...) which dereferences *errp -- a NULL
pointer -- crashing QEMU.
Fix both issues by switching to qio_channel_pread_all() introduced in
a previous patch, which retries on short reads and treats end-of-file
as an error, so the caller no longer needs to check the byte count
manually. Add ERRP_GUARD() so that error_prepend() works correctly
even when errp is &error_fatal or NULL.
Fixes: a49d15a38d ("migration/multifd: Support incoming mapped-ram stream format")
Suggested-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/qemu-devel/20260413214549.926435-4-junjie.cao@intel.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
mapped_ram_read_header() reads page_size from the migration stream and
stores it in MappedRamHeader, but does not validate that the value is
non-zero before it is later used in parse_ramblock_mapped_ram():
num_pages = length / header.page_size;
If a corrupted or malformed migration stream provides invalid, guest
resumes either with corrupted memory or crashes unexpectedly (eg.
page_size = 0)
Add validation in mapped_ram_read_header() to reject invalid page_size
values early and return an error instead of continuing with an invalid
header.
Steps to reproduce:
Create a migration snapshot with mapped-ram enabled:
(qemu) migrate_set_capability mapped-ram on
(qemu) migrate file:/tmp/qemu-snapshots/snapshot.bin
Modify the snapshot so that MappedRamHeader.page_size becomes diff with
target psize. (0/512/8192/1GB).
Restore the snapshot:
(qemu) migrate_set_capability mapped-ram on
(qemu) migrate_incoming file:/tmp/qemu-snapshots/snapshot.bin
As-is:
* [0]: Floating point exception (core dumped)
* [512/8192]: Silent corruption
* [1GB]: "post load hook failed for: kvm-tpr-opt" (EPERM)
To-be:
* All: qemu-system-x86_64: Migration mapped-ram header has invalid
page_size [val] (expected 4096)
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260405094447.11347-1-viking4@gmail.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Introduce a new flag, VMS_ARRAY_OF_POINTER_AUTO_ALLOC, for VMSD field. It
must be used together with VMS_ARRAY_OF_POINTER.
It can be used to allow migration of an array of pointers where the
pointers may point to NULLs.
Note that we used to allow migration of a NULL pointer within an array that
is being migrated. That corresponds to the code around vmstate_info_nullptr
where we may get/put one byte showing that the element of an array is NULL.
That usage is fine but very limited, it's because even if it will migrate a
NULL pointer with a marker, it still works in a way that both src and dest
QEMUs must know exactly which elements of the array are non-NULL, so
instead of dynamically loading an array (which can have NULL pointers), it
actually only verifies the known NULL pointers are still NULL pointers
after migration.
Also, in that case since dest QEMU knows exactly which element is NULL,
which is not NULL, dest QEMU's device code will manage all allocations for
the elements before invoking vmstate_load_vmsd().
That's not enough per evolving needs of new device states that may want to
provide real dynamic array of pointers, like what Alexander proposed here
with the NVMe device migration:
https://lore.kernel.org/r/20260317102708.126725-1-alexander@mihalicyn.com
This patch is an alternative approach to address the problem.
Along with the flag, introduce two new macros:
VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_UINT{8|32}_ALLOC()
Which will be used very soon in the NVMe series.
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260401202844.673494-10-peterx@redhat.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
The loader side of ptr marker is pretty straightforward, instead of playing
the inner_field trick, just do the load manually assuming the marker layout
is a stable ABI (which it is true already).
This will remove some logic while loading VMSD, and hopefully it makes it
slightly easier to read. Unfortunately, we still need to keep the sender
side because of the JSON blob we're maintaining..
This paves way for future processing of non-NULL markers as well.
When at it, not check "size" anymore for existing NULL markers, and move it
under the same VMS_ARRAY_OF_POINTER section because that's the only place
that NULL marker can happen (which guarantess size==host ptr size, which is
non-zero).
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260401202844.673494-9-peterx@redhat.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
We used to have one vmstate called "nullptr" which is only used to generate
one-byte hint to say one pointer is NULL.
Let's extend its use so that it will generate another byte to say the
pointer is non-NULL.
With that, the name of the info struct (or functions) do not apply anymore.
Update correspondingly.
Update analyze-migration.py to work with the new layout.
No functional change intended yet.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260401202844.673494-8-peterx@redhat.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
QEMU has a trick in vmstate_save_vmsd_v(), where it will try to compress
multiple JSON entries into one with a count to avoid duplicated entries.
That only applies to the cases where vmsd_can_compress() should return
true. For example, vmsd_desc_field_start() later (who will take the
updated max_elems as the last parameter) will ignore the value passed in
when vmsd_can_compress() returns false.
Do that check once at the start of loop, and use it to update max_elems, so
that max_elems keeps 1 for uncompressable VMSD fields, which is more
straightforward.
This also paves way to make this counter work for ptr marker VMSD fields
too.
No functional change intended in this patch alone.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260401202844.673494-5-peterx@redhat.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
When VMS_ARRAY_OF_POINTER is specified, it means the vmstate field is an
array of pointers.
The size of the element is not relevant to whatever it is stored inside: it
is always the host pointer size.
Let's reserve the "size" field in this case for future use, update
vmstate_size() so as to make it still work for array of pointers properly.
When at this, provide rich documentation on how size / size_offset works in
vmstate.
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260401202844.673494-4-peterx@redhat.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Commit 03a680c978 changed g_autoptr(QIOChannelFile) to a plain pointer
but failed to restore the necessary object_unref() calls on error paths.
Previously, these were handled implicitly by the g_autoptr cleanup
mechanism.
Two error paths currently leak the QIOChannelFile object and its
underlying file descriptor:
1. When ftruncate() fails (e.g., on character or block devices).
2. When qio_channel_io_seek() fails after the channel is created.
In environments that retry migration automatically (e.g., libvirt),
these FDs accumulate until QEMU hits RLIMIT_NOFILE and fails with
EMFILE (Too many open files).
Add the missing object_unref() calls to both error paths to ensure
resources are properly released.
Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260328121215.159532-1-vikingtc4@gmail.com
Signed-off-by: Fabiano Rosas <farosas@suse.de>
