Freeing the error at migration_connect() is redundant in the normal
migration case. The freeing already happened at migrate_init():
qmp_migrate()
-> migrate_prepare()
-> migrate_init()
-> qmp_migrate_finish()
-> *_start_outgoing_migration()
-> migration_channel_connect()
-> migration_connect()
For the resume case, migrate_prepare() returns early and doesn't reach
migrate_init(). Move the extra migrate_error_free() call to
migrate_prepare() along with the resume check.
Also change migrate_init() to use migrate_error_free(), so it's easier
to see where are the places the error gets freed.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-8-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
The TLS hostname is doing a tour around the world just to be cached
into s->hostname. We're already abusing MigrationState by doing that,
so incorporate the s->hostname into migration_tls_hostname() and stop
passing the string around.
The old route was roughly:
-transport code (socket.c, fd.c, etc):
if (SOCKET_ADDRESS_TYPE_INET)
hostname = saddr->u.inet.host
else
hostname = NULL
migration_channel_connect(..., hostname)
s->hostname = hostname;
migration_tls_client_create(..., hostname)
if (migrate_tls_hostname())
qio_channel_tls_new_client(migrate_tls_hostname())
else
qio_channel_tls_new_client(hostname)
-postcopy_preempt_setup:
postcopy_preempt_send_channel_new
migration_tls_client_create(..., s->hostname)
New route is:
-socket.c only:
if SOCKET_ADDRESS_TYPE_INET
s->hostname = saddr->u.inet.host
migration_channel_connect()
migration_tls_client_create()
qio_channel_tls_new_client(migrate_tls_hostname())
-postcopy_preempt_setup:
postcopy_preempt_send_channel_new
migration_tls_client_create()
qio_channel_tls_new_client(migrate_tls_hostname())
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-5-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Use error_setg_errno() instead of passing the value of strerror() or
g_strerror() to error_setg().
The separator between the error message proper and the value of
strerror() changes from " : ", "", " - ", "- " to ": " in places.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20251121121438.1249498-14-armbru@redhat.com>
Acked-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
A few error messages show numeric errno codes. Use error_setg_errno()
to show human-readable text instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20251121121438.1249498-13-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[Trivial fixup to riscv_kvm_cpu_finalize_features()]
* cleanup include/hw headers
* cleanup memory headers
* rust: preludes
* rust: support for dtrace
* rust/hpet: first part of reorganization
* meson: small cleanups
* target/i386: Diamond Rapids CPU model including CET, APX, AVX10.2
# -----BEGIN PGP SIGNATURE-----
#
# iQFIBAABCgAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmlPov8UHHBib256aW5p
# QHJlZGhhdC5jb20ACgkQv/vSX3jHroPN1wf9HCceQ1273g7HbNeamay2bSaqypyM
# sEUBk4ipwO0dp7AYaaX5MeJ8NxeYcK82oFgm35WLY1tMOv0BZG5ez02dLoh5C4fb
# Bmy3kV1aY9cxF0IwTyD4dIADlZoaMnGgMElUKFY2/EixjxOUMLe90b1MO2KczqFa
# jvC4gmjx5PC1r+BHycSEdKm2Rbunueb/5eSkKeyTX7rjxQ/Eij0uGjrWrZkMWtgs
# ERJ2xo+D6a38w/uJ88KuqUV1BqYxNNwKmvOwVBU2xFB9o9bm20TNOJZ3+D+Ki8Aj
# idv+rU0XY1bWseo4USuozsqxfkjLJ5lj2YYUkSVO/I1wJmuO7Bq6xzrCxg==
# =/nIt
# -----END PGP SIGNATURE-----
# gpg: Signature made Sat 27 Dec 2025 08:12:31 PM AEDT
# gpg: using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg: issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [unknown]
# gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" [unknown]
# gpg: WARNING: The key's User ID is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1
# Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83
* tag 'for-upstream' of https://gitlab.com/bonzini/qemu: (152 commits)
block: rename block/aio-wait.h to qemu/aio-wait.h
block: rename block/aio.h to qemu/aio.h
block: reduce files included by block/aio.h
block: extract include/qemu/aiocb.h out of include/block/aio.h
hw: add missing includes hidden by block/aio.h
qmp: Fix thread race
thread-pool: Fix thread race
dosc/cpu-models-x86: Add documentation for DiamondRapids
i386/cpu: Add CPU model for Diamond Rapids
i386/cpu: Define dependency for VMX_VM_ENTRY_LOAD_IA32_FRED
i386/cpu: Add an option in X86CPUDefinition to control CPUID 0x1f
i386/cpu: Allow cache to be shared at thread level
i386/cpu: Allow unsupported avx10_version with x-force-features
i386/cpu: Add a helper to get host avx10 version
i386/cpu: Support AVX10.2 with AVX10 feature models
i386/cpu: Add support for AVX10_VNNI_INT in CPUID enumeration
i386/cpu: Add CPUID.0x1E.0x1 subleaf for AMX instructions
i386/cpu: Add support for MOVRS in CPUID enumeration
run: introduce a script for running devel commands
gitlab-ci: enable rust for msys2-64bit
...
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Move RAMBlock functions out of ram_addr.h and cpu-common.h;
move memory API headers out of include/exec and into include/system.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In our long-term experience in Bytedance, we've found that under
the same load, live migration of larger VMs with more devices is
often more difficult to converge (requiring a larger downtime limit).
Through some testing and calculations, we conclude that bitmap sync time
affects the calculation of live migration bandwidth.
When the addresses processed are not aligned, a large number of
clear_dirty ioctl occur (e.g. a 4MB misaligned memory can generate
2048 clear_dirty ioctls from two different memory_listener),
which increases the time required for bitmap_sync and makes it
more difficult for dirty pages to converge.
For a 64C256G vm with 8 vhost-user-net(32 queue per nic) and
16 vhost-user-blk(4 queue per blk), the sync time is as high as *73ms*
(tested with 10GBps dirty rate, the sync time increases as the dirty
page rate increases), Here are each part of the sync time:
- sync from kvm to ram_list: 2.5ms
- vhost_log_sync:3ms
- sync aligned memory from ram_list to RAMBlock: 5ms
- sync misaligned memory from ram_list to RAMBlock: 61ms
Attempt to merge those fragmented clear_dirty ioctls, then syncing
misaligned memory from ram_list to RAMBlock takes only about 1ms,
and the total sync time is only *12ms*.
Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251218114220.83354-1-xuchuangxclwt@bytedance.com
[peterx: drop var "offset" in physical_memory_sync_dirty_bitmap]
Signed-off-by: Peter Xu <peterx@redhat.com>
QAPI_CLONE_MEMBERS is a better option than copying parameters one by
one because it operates on the entire struct and follows pointers. It
also avoids the need to alter this function every time a new parameter
is added.
For this to work, the has_* fields of s->parameters need to be already
set beforehand, so move migrate_mark_all_params_present() to the init
routine.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-14-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
MigrationParameters needs to have all of its has_* fields marked as
true when used as the return of query_migrate_parameters because the
corresponding QMP command has all of its members non-optional by
design, despite them being marked as optional in migration.json.
Extract this code into a function and make it assert if any field is
missing. With this we ensure future changes will not inadvertently
leave any parameters missing.
Note that the block-bitmap-mapping is a special case because the empty
list is considered a valid value, so it has historically not been
present in the command's output if it has never been set.
CC: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-13-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
The migration parameters validation produces a temporary structure
which is the merge of the current parameter values (s->parameters,
MigrationParameters) with the new parameters set by the user
(former MigrateSetParameters).
When copying the values from s->parameters into the temporary
structure, the has_* fields are copied along, but when merging the
user-input values they are not.
During migrate_params_check(), only the parameters that have the
corresponding has_* field will be checked, so only the parameters that
were initialized in migrate_params_init() will be validated.
This causes (almost) all of the migration parameters to be validated
every time a parameter is set, regardless of which fields the user
touched, but it also skips validation of any values that are not set
in migrate_params_init().
It's not clear what was the intention of the original code, whether to
validate all fields always, or only validate what the user input
changed. Since the current situation is closer to the former option,
make the choice of validating all parameters by removing the checks
for the has_* fields when validating.
Note that bringing the user input into the temporary structure for
validation still needs to look at the has_* fields, otherwise any
parameters not set by the user (i.e. 0) would override the
corresponding value in s->parameters.
The empty migrate_params_init() will be kept because subsequent
patches will add code to it.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-11-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
The QAPI converts an empty list on the block-bitmap-mapping input into
a NULL BitmapMigrationNodeAliasList. The empty list is a valid input
for the block-bitmap-mapping option, so commit 3cba22c9ad ("migration:
Fix block_bitmap_mapping migration") started using the
s->parameters.has_block_bitmap_mapping field to tell when the user has
passed in an empty list vs. when no list has been passed at all.
Using s->parameters.has_block_bitmap_mapping field is only possible
because MigrationParameters has had its members made optional due to
historical reasons.
In order to make improvements to the way configuration options are set
for a migration, we'd like to reduce the open-coded usage of the has_*
fields of the global configuration object (s->parameters).
Add a separate boolean to track the status of the block_bitmap_mapping
option.
No functional change intended.
(this was verified to not regress iotest 300, which is the test that
3cba22c9ad refers to)
CC: Kevin Wolf <kwolf@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-10-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
Some migration parameters are updated immediately once they are set
via migrate-set-parameters. Move that work outside of
migrate_params_apply() and leave that function with the single
responsibility of setting s->parameters and not doing any
side-effects.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-9-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
Now that the TLS options have been made the same between
migrate-set-parameters and query-migrate-parameters, a single type can
be used. Remove MigrateSetParameters.
The TLS options documentation from MigrationParameters were replaced
with the ones from MigrateSetParameters which was more complete.
Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-7-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
The migration parameters tls_creds, tls_authz and tls_hostname
currently have a non-uniform handling. When used as arguments to
migrate-set-parameters, their type is StrOrNull and when used as
return value from query-migrate-parameters their type is a plain
string.
Not only having to convert between the types is cumbersome, but it
also creates the issue of requiring two different QAPI types to be
used, one for each command. MigrateSetParameters is used for
migrate-set-parameters with the TLS arguments as StrOrNull while
MigrationParameters is used for query-migrate-parameters with the TLS
arguments as str.
Since StrOrNull could be considered a superset of str, change the type
of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
that QTYPE_QNULL is never used.
1) migrate-set-parameters will always write QTYPE_QSTRING to
s->parameters, either an empty or non-empty string.
2) query-migrate-parameters will always return a QTYPE_QSTRING, either
empty or non-empty.
3) the migrate_tls_* helpers will always return a non-empty string or
NULL, for the internal migration code's consumption.
Points (1) and (2) above help simplify the parameters validation and
the query command handling because s->parameters is already kept in
the format that query-migrate-parameters (and info migrate_paramters)
expect. Point (3) is so people don't need to care about StrOrNull in
migration code.
This will allow the type duplication to be removed in the next
patches.
Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
from str to StrOrNull in introspection of the query-migrate-parameters
command. We accept this imprecision to enable de-duplication.
There's no need to free the TLS options in
migration_instance_finalize() because they're freed by the qdev
properties .release method.
Temporary in this patch:
migrate_params_test_apply() copies s->parameters into a temporary
structure, so it's necessary to drop the references to the TLS options
if they were not set by the user to avoid double-free. This is fixed
in the next patches.
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-6-farosas@suse.de
[peterx: in hmp_info_migrate_parameters(), remove an extra dump of
max_postcopy_bandwidth, introduced likely by accident]
Signed-off-by: Peter Xu <peterx@redhat.com>
The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
was done about eight years ago so the migration code could make use of
qdev properties to define the defaults for the migration parameters
and to be able to expose migration knobs for debugging via the
'-global migration' command line option.
Due to unrelated historical reasons, three of the migration parameters
(TLS options) received different types when used via the
query-migrate-parameters QMP command than with the
migrate-set-parameters command. This has created a lot of duplication
in the migration code and in the QAPI documentation because the whole
of MigrationParameters had to be duplicated as well.
The migration code is now being fixed to remove the duplication and
for that to happen the offending fields need to be reconciled into a
single type. The StrOrNull type is going to be used.
To keep the command line compatibility, the parameters need to
continue being exposed via qdev properties accessible from the command
line. Introduce a qdev property StrOrNull just for that.
Note that this code is being kept in migration/options.c as this
version of StrOrNull doesn't need to handle QNULL because it was never
a valid option in the previous command line, which took a string.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20251215220041.12657-4-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
Caught by inspection, but ASAN also reports:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 in malloc
#1 in g_malloc
#2 in g_memdup
#3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
#4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
#5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
#6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
#7 in migrate_params_apply ../migration/options.c:1407:13
#8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
#9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
#10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5
Note that this is entirely harmless because the migration object which
contains the MigrationParameters structure is kept until the QEMU
process exits.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-2-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
migrate_set_error() currently doesn't take ownership of the error being
passed in. It's not aligned with the error API and meanwhile it also
makes most of the caller free the error explicitly.
Change the API to take the ownership of the Error object instead. This
should save a lot of error_copy() invocations.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-8-peterx@redhat.com
[peterx: break line for qemu_savevm_send_packaged, per markus]
Signed-off-by: Peter Xu <peterx@redhat.com>
Make multifd_send_set_error() take ownership of the error always. Paving
way for making migrate_set_error() to take ownership.
When at it, rename it to multifd_send_error_propagate() to imply the
ownership transition following Error API's naming style.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-6-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Make migration_connect_set_error() take ownership of the error always.
Paving way for making migrate_set_error() to take ownership.
When at it, renaming it to migration_connect_error_propagate(), following
Error API, to imply the Error object ownership transition.
NOTE: this patch also makes migration_connect() to take ownership of the
Error passed in.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-5-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
There're only two use cases of g_autoptr to free Error objects in migration
code paths.
Due to the nature of how Error should be used (normally ownership will be
passed over to Error APIs, like error_report_err), auto-free functions may
be error prone on its own. The auto cleanup function was merged without
proper review, as pointed out by Dan and Markus:
https://lore.kernel.org/r/aSWSLMi6ZhTCS_p2@redhat.com
Remove the two use cases so that we can remove the auto cleanup function,
hence suggest to not use auto frees for Errors.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Currently if you set these flags and have any shared memory object, saving
a snapshot will fail with:
Failed to write bitmap to file: Unable to write to file: Bad address
We need to skip writing RAMBlocks that are backed by shared objects.
Also, we should mark these RAMBlocks as skipped, so the snapshot format stays
readable to tools that later don't know QEMU's command line (for example
scripts/analyze-migration.py). I used bitmap_offset=0 pages_offset=0 for this.
This minor change to snapshot format should be safe, as offset=0 should not
have ever been possible.
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Link: https://lore.kernel.org/r/20251126154734.940066-1-pzmarzly0@gmail.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Snapshots made with mapped-ram and x-ignore-shared flags are
not parsed properly.
The ignore-shared feature adds and extra field in the stream, which
needs to be consumed on the destination side. Even though mapped-ram has
a fixed header format, the ignore-shared is part of the "generic" stream
infomation so the mapped-ram code is currently skipping that be64 read
which incorrectly offsets every subsequent read from the stream.
The current ignore-shared handling can simply be moved earlier in the code
to encompass mapped-ram as well since the ignore-shared doubleword is the
first one read when parsing the ramblock section of the stream.
Co-authored-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251126121233.542473-1-pzmarzly0@gmail.com
[peterx: enhance commit log per fabiano]
Signed-off-by: Peter Xu <peterx@redhat.com>
