* [PATCH v3 01/20] checkpatch: Fix checking of newlines in error messages
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 02/20] migration/options.c: Don't export migrate_tls_opts_free Fabiano Rosas
` (19 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert, Chao Liu
Using newlines in error messages is not allowed as per qapi/error.h
documentation.
Checkpatch has been enforcing this for the error.h API, but also for
g_test_message() which doesn't fall under the qapi/error.h
prescription. Using newlines in that function's format string is fine
and can help with readability of test code when formatting test
results. Note that g_test_message() automatically adds the '#'
required by the TAP protocol to the start of each line.
Relax the regex for this function, but still forbid a trailing newline
because it's added automatically and usually not what the user wants.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
scripts/checkpatch.pl | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9838a64178..1e9499ed9b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3347,13 +3347,20 @@ sub process {
info_vreport|
error_report|
warn_report|
- info_report|
- g_test_message}x;
+ info_report}x;
if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
ERROR("Error messages should not contain newlines\n" . $herecurr);
}
+ # No newlines at the end
+ my $trail_newline_error_funcs = qr{g_test_message}x;
+
+ if ($rawline =~ /\b(?:$trail_newline_error_funcs)\s*\(.*\".*\\n\"/) {
+ ERROR("Error messages should not contain trailing " .
+ "newlines\n" . $herecurr);
+ }
+
# Continue checking for error messages that contains newlines.
# This check handles cases where string literals are spread
# over multiple lines.
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 02/20] migration/options.c: Don't export migrate_tls_opts_free
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 01/20] checkpatch: Fix checking of newlines in error messages Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 03/20] migration: Rename variables in qmp_migrate_set_parameters Fabiano Rosas
` (18 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
The migrate_tls_opts_free function was never used outside
options.c. Make it static.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 2 +-
migration/options.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 05b026f4bf..f4bba8ca26 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1063,7 +1063,7 @@ AnnounceParameters *migrate_announce_params(void)
return ≈
}
-void migrate_tls_opts_free(MigrationParameters *params)
+static void migrate_tls_opts_free(MigrationParameters *params)
{
qapi_free_StrOrNull(params->tls_creds);
qapi_free_StrOrNull(params->tls_hostname);
diff --git a/migration/options.h b/migration/options.h
index b46221998a..c272eb6208 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -93,5 +93,4 @@ uint64_t migrate_rdma_chunk_size(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
-void migrate_tls_opts_free(MigrationParameters *params);
#endif
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 03/20] migration: Rename variables in qmp_migrate_set_parameters
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 01/20] checkpatch: Fix checking of newlines in error messages Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 02/20] migration/options.c: Don't export migrate_tls_opts_free Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 04/20] migration: Use QAPI_CLONE_MEMBERS in migrate_params_apply Fabiano Rosas
` (17 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Give the variables in qmp_migrate_set_parameters more semantic
names.
s/params/input/
this is the user input from qapi
s/tmp/new/
this is the combination of the current parameters and the input
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index f4bba8ca26..3ce15497f8 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1599,9 +1599,9 @@ static void migrate_params_apply(MigrationParameters *params)
}
}
-void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
+void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
{
- MigrationParameters tmp;
+ MigrationParameters new;
/*
* Convert QTYPE_QNULL and NULL to the empty string (""). Even
@@ -1611,18 +1611,18 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
* the options to the rest of the migration code already use
* return NULL when the empty string is found.
*/
- tls_opt_to_str(params->tls_creds);
- tls_opt_to_str(params->tls_hostname);
- tls_opt_to_str(params->tls_authz);
+ tls_opt_to_str(input->tls_creds);
+ tls_opt_to_str(input->tls_hostname);
+ tls_opt_to_str(input->tls_authz);
- migrate_params_test_apply(params, &tmp);
+ migrate_params_test_apply(input, &new);
- if (migrate_params_check(&tmp, errp)) {
- migrate_params_apply(params);
- migrate_post_update_params(params, errp);
+ if (migrate_params_check(&new, errp)) {
+ migrate_params_apply(input);
+ migrate_post_update_params(input, errp);
}
- migrate_tls_opts_free(&tmp);
- qapi_free_BitmapMigrationNodeAliasList(tmp.block_bitmap_mapping);
- qapi_free_strList(tmp.cpr_exec_command);
+ migrate_tls_opts_free(&new);
+ qapi_free_BitmapMigrationNodeAliasList(new.block_bitmap_mapping);
+ qapi_free_strList(new.cpr_exec_command);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 04/20] migration: Use QAPI_CLONE_MEMBERS in migrate_params_apply
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (2 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 03/20] migration: Rename variables in qmp_migrate_set_parameters Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 05/20] migration: Merge parameter structs instead of assigning one by one Fabiano Rosas
` (16 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Prasad Pandit
Instead of setting parameters one by one, use the temporary object,
which already contains the current migration parameters plus the new
ones and was just validated by migration_params_check(). Use cloning
to overwrite it.
This avoids the need to alter this function every time a new parameter
is added.
Since parameters are not individually checked anymore, the setting of
s->has_block_bitmap_mapping moves into migrate_post_update_params().
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 145 +++++---------------------------------------
1 file changed, 14 insertions(+), 131 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 3ce15497f8..ac7e94ed62 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1168,6 +1168,10 @@ static void migrate_post_update_params(MigrationParameters *new, Error **errp)
migration_rate_set(new->max_postcopy_bandwidth);
}
}
+
+ if (new->has_block_bitmap_mapping) {
+ s->has_block_bitmap_mapping = true;
+ }
}
/*
@@ -1463,140 +1467,19 @@ static void migrate_params_test_apply(MigrationParameters *params,
}
}
+/*
+ * Caller must ensure the has_* fields of @params are true so they all
+ * get copied and the pointer members don't dangle.
+ */
static void migrate_params_apply(MigrationParameters *params)
{
MigrationState *s = migrate_get_current();
+ MigrationParameters *cur = &s->parameters;
- /* TODO use QAPI_CLONE() instead of duplicating it inline */
-
- if (params->has_throttle_trigger_threshold) {
- s->parameters.throttle_trigger_threshold = params->throttle_trigger_threshold;
- }
-
- if (params->has_cpu_throttle_initial) {
- s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
- }
-
- if (params->has_cpu_throttle_increment) {
- s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
- }
-
- if (params->has_cpu_throttle_tailslow) {
- s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
- }
-
- if (params->tls_creds) {
- qapi_free_StrOrNull(s->parameters.tls_creds);
- s->parameters.tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
- }
-
- if (params->tls_hostname) {
- qapi_free_StrOrNull(s->parameters.tls_hostname);
- s->parameters.tls_hostname = QAPI_CLONE(StrOrNull,
- params->tls_hostname);
- }
-
- if (params->tls_authz) {
- qapi_free_StrOrNull(s->parameters.tls_authz);
- s->parameters.tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
- }
-
- if (params->has_max_bandwidth) {
- s->parameters.max_bandwidth = params->max_bandwidth;
- }
-
- if (params->has_avail_switchover_bandwidth) {
- s->parameters.avail_switchover_bandwidth = params->avail_switchover_bandwidth;
- }
-
- if (params->has_downtime_limit) {
- s->parameters.downtime_limit = params->downtime_limit;
- }
-
- if (params->has_x_checkpoint_delay) {
- s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
- }
-
- if (params->has_multifd_channels) {
- s->parameters.multifd_channels = params->multifd_channels;
- }
- if (params->has_multifd_compression) {
- s->parameters.multifd_compression = params->multifd_compression;
- }
- if (params->has_multifd_qatzip_level) {
- s->parameters.multifd_qatzip_level = params->multifd_qatzip_level;
- }
- if (params->has_multifd_zlib_level) {
- s->parameters.multifd_zlib_level = params->multifd_zlib_level;
- }
- if (params->has_multifd_zstd_level) {
- s->parameters.multifd_zstd_level = params->multifd_zstd_level;
- }
- if (params->has_xbzrle_cache_size) {
- s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
- }
- if (params->has_max_postcopy_bandwidth) {
- s->parameters.max_postcopy_bandwidth = params->max_postcopy_bandwidth;
- }
- if (params->has_max_cpu_throttle) {
- s->parameters.max_cpu_throttle = params->max_cpu_throttle;
- }
- if (params->has_announce_initial) {
- s->parameters.announce_initial = params->announce_initial;
- }
- if (params->has_announce_max) {
- s->parameters.announce_max = params->announce_max;
- }
- if (params->has_announce_rounds) {
- s->parameters.announce_rounds = params->announce_rounds;
- }
- if (params->has_announce_step) {
- s->parameters.announce_step = params->announce_step;
- }
-
- if (params->has_block_bitmap_mapping) {
- qapi_free_BitmapMigrationNodeAliasList(
- s->parameters.block_bitmap_mapping);
-
- s->has_block_bitmap_mapping = true;
- s->parameters.block_bitmap_mapping =
- QAPI_CLONE(BitmapMigrationNodeAliasList,
- params->block_bitmap_mapping);
- }
-
- if (params->has_x_vcpu_dirty_limit_period) {
- s->parameters.x_vcpu_dirty_limit_period =
- params->x_vcpu_dirty_limit_period;
- }
- if (params->has_vcpu_dirty_limit) {
- s->parameters.vcpu_dirty_limit = params->vcpu_dirty_limit;
- }
-
- if (params->has_mode) {
- s->parameters.mode = params->mode;
- }
-
- if (params->has_zero_page_detection) {
- s->parameters.zero_page_detection = params->zero_page_detection;
- }
-
- if (params->has_direct_io) {
- s->parameters.direct_io = params->direct_io;
- }
-
- if (params->has_x_rdma_chunk_size) {
- s->parameters.x_rdma_chunk_size = params->x_rdma_chunk_size;
- }
-
- if (params->has_cpr_exec_command) {
- qapi_free_strList(s->parameters.cpr_exec_command);
- s->parameters.cpr_exec_command =
- QAPI_CLONE(strList, params->cpr_exec_command);
- }
-
- if (params->has_local) {
- s->parameters.local = params->local;
- }
+ migrate_tls_opts_free(cur);
+ qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
+ qapi_free_strList(cur->cpr_exec_command);
+ QAPI_CLONE_MEMBERS(MigrationParameters, cur, params);
}
void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
@@ -1618,7 +1501,7 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
migrate_params_test_apply(input, &new);
if (migrate_params_check(&new, errp)) {
- migrate_params_apply(input);
+ migrate_params_apply(&new);
migrate_post_update_params(input, errp);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 05/20] migration: Merge parameter structs instead of assigning one by one
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (3 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 04/20] migration: Use QAPI_CLONE_MEMBERS in migrate_params_apply Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 06/20] migration: Open code migrate_params_apply Fabiano Rosas
` (15 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Convert the code in migrate_params_test_apply() from an open-coded
copy of every migration parameter to a merge operation using QAPI
visitors and QDict.
The purpose of that routine is to update a temporary structure
(pre-populated with the current migration parameters), with the values
received from the user via QAPI. As a result, the temporary structure
will then contain the "to be applied" parameters and it's validated
before being used to overwrite the parameters currently in use.
The update is currently done as follows:
where 'params' is the user input from QAPI,
for each parameter:
a) check if the option is present
params->has_<name> == true
params-><name> != NULL // for strings
b) if the parameter is a pointer, free the to-be-assigned member and
allocate memory for the copy from params
c) assign the user provided value to the temporary structure.
Step (a) is the same in principle as what the QAPI visitors do at
visit_type_MigrationParameters_members().
Steps (b) and (c) are roughly the same as what the QDict
implementation does when qdict_del() and qdict_put_obj() are combined.
Therefore, replace the open-coded function with
migrate_params_merge(), which achieves the same goal, but uses
visitors and QDict. This hides the details of QAPI (has_*) from the
migration code and avoids the need to update
migrate_params_test_apply() every time a new migration parameter is
added.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 204 ++++++++++++++------------------------------
1 file changed, 65 insertions(+), 139 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index ac7e94ed62..0f7478aa6a 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -21,6 +21,9 @@
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
#include "qapi/qmp/qerror.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qobject/qdict.h"
#include "qobject/qnull.h"
#include "system/runstate.h"
#include "migration/colo.h"
@@ -1082,6 +1085,28 @@ static void tls_opt_to_str(StrOrNull *opt)
opt->u.s = g_strdup("");
}
+static QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
+{
+ QObject *obj = NULL;
+ Visitor *v = qobject_output_visitor_new(&obj);
+
+ if (visit_type_MigrationParameters(v, NULL, &p, errp)) {
+ visit_complete(v, &obj);
+ }
+ visit_free(v);
+ return qobject_to(QDict, obj);
+}
+
+static MigrationParameters *migrate_params_from_dict(QDict *d, Error **errp)
+{
+ Visitor *v = qobject_input_visitor_new(QOBJECT(d));
+ MigrationParameters *tmp = NULL;
+
+ visit_type_MigrationParameters(v, NULL, &tmp, errp);
+ visit_free(v);
+ return tmp;
+}
+
/*
* query-migrate-parameters expects all members of MigrationParameters
* to be present, but we cannot mark them non-optional in QAPI because
@@ -1174,6 +1199,38 @@ static void migrate_post_update_params(MigrationParameters *new, Error **errp)
}
}
+static bool migrate_params_merge(MigrationParameters *base,
+ MigrationParameters *updates,
+ MigrationParameters **new,
+ Error **errp)
+{
+ g_autoptr(QDict) d_base = NULL;
+ g_autoptr(QDict) d_upd = NULL;
+ const QDictEntry *e;
+
+ d_base = migrate_params_to_dict(base, errp);
+ if (!d_base) {
+ return false;
+ }
+
+ d_upd = migrate_params_to_dict(updates, errp);
+ if (!d_upd) {
+ return false;
+ }
+
+ for (e = qdict_first(d_upd); e; e = qdict_next(d_upd, e)) {
+ const char *key = qdict_entry_key(e);
+ QObject *value = qdict_entry_value(e);
+
+ qobject_ref(value);
+ qdict_put_obj(d_base, key, value);
+ }
+
+ *new = migrate_params_from_dict(d_base, errp);
+
+ return !!*new;
+}
+
/*
* Check whether the parameters are valid. Error will be put into errp
* (if provided). Return true if valid, otherwise false.
@@ -1336,137 +1393,6 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return true;
}
-static void migrate_params_test_apply(MigrationParameters *params,
- MigrationParameters *dest)
-{
- MigrationState *s = migrate_get_current();
-
- QAPI_CLONE_MEMBERS(MigrationParameters, dest, &s->parameters);
-
- if (params->has_throttle_trigger_threshold) {
- dest->throttle_trigger_threshold = params->throttle_trigger_threshold;
- }
-
- if (params->has_cpu_throttle_initial) {
- dest->cpu_throttle_initial = params->cpu_throttle_initial;
- }
-
- if (params->has_cpu_throttle_increment) {
- dest->cpu_throttle_increment = params->cpu_throttle_increment;
- }
-
- if (params->has_cpu_throttle_tailslow) {
- dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
- }
-
- if (params->tls_creds) {
- qapi_free_StrOrNull(dest->tls_creds);
- dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
- }
-
- if (params->tls_hostname) {
- qapi_free_StrOrNull(dest->tls_hostname);
- dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname);
- }
-
- if (params->tls_authz) {
- qapi_free_StrOrNull(dest->tls_authz);
- dest->tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
- }
-
- if (params->has_max_bandwidth) {
- dest->max_bandwidth = params->max_bandwidth;
- }
-
- if (params->has_avail_switchover_bandwidth) {
- dest->avail_switchover_bandwidth = params->avail_switchover_bandwidth;
- }
-
- if (params->has_downtime_limit) {
- dest->downtime_limit = params->downtime_limit;
- }
-
- if (params->has_x_checkpoint_delay) {
- dest->x_checkpoint_delay = params->x_checkpoint_delay;
- }
-
- if (params->has_multifd_channels) {
- dest->multifd_channels = params->multifd_channels;
- }
- if (params->has_multifd_compression) {
- dest->multifd_compression = params->multifd_compression;
- }
- if (params->has_multifd_qatzip_level) {
- dest->multifd_qatzip_level = params->multifd_qatzip_level;
- }
- if (params->has_multifd_zlib_level) {
- dest->multifd_zlib_level = params->multifd_zlib_level;
- }
- if (params->has_multifd_zstd_level) {
- dest->multifd_zstd_level = params->multifd_zstd_level;
- }
- if (params->has_xbzrle_cache_size) {
- dest->xbzrle_cache_size = params->xbzrle_cache_size;
- }
- if (params->has_max_postcopy_bandwidth) {
- dest->max_postcopy_bandwidth = params->max_postcopy_bandwidth;
- }
- if (params->has_max_cpu_throttle) {
- dest->max_cpu_throttle = params->max_cpu_throttle;
- }
- if (params->has_announce_initial) {
- dest->announce_initial = params->announce_initial;
- }
- if (params->has_announce_max) {
- dest->announce_max = params->announce_max;
- }
- if (params->has_announce_rounds) {
- dest->announce_rounds = params->announce_rounds;
- }
- if (params->has_announce_step) {
- dest->announce_step = params->announce_step;
- }
-
- if (params->has_block_bitmap_mapping) {
- qapi_free_BitmapMigrationNodeAliasList(dest->block_bitmap_mapping);
- dest->block_bitmap_mapping = QAPI_CLONE(BitmapMigrationNodeAliasList,
- params->block_bitmap_mapping);
- }
-
- if (params->has_x_vcpu_dirty_limit_period) {
- dest->x_vcpu_dirty_limit_period =
- params->x_vcpu_dirty_limit_period;
- }
- if (params->has_vcpu_dirty_limit) {
- dest->vcpu_dirty_limit = params->vcpu_dirty_limit;
- }
-
- if (params->has_mode) {
- dest->mode = params->mode;
- }
-
- if (params->has_zero_page_detection) {
- dest->zero_page_detection = params->zero_page_detection;
- }
-
- if (params->has_direct_io) {
- dest->direct_io = params->direct_io;
- }
-
- if (params->has_x_rdma_chunk_size) {
- dest->x_rdma_chunk_size = params->x_rdma_chunk_size;
- }
-
- if (params->has_cpr_exec_command) {
- qapi_free_strList(dest->cpr_exec_command);
- dest->cpr_exec_command = QAPI_CLONE(strList, params->cpr_exec_command);
- }
-
- if (params->has_local) {
- dest->local = params->local;
- }
-}
-
/*
* Caller must ensure the has_* fields of @params are true so they all
* get copied and the pointer members don't dangle.
@@ -1484,7 +1410,8 @@ static void migrate_params_apply(MigrationParameters *params)
void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
{
- MigrationParameters new;
+ MigrationParameters *cur = &migrate_get_current()->parameters;
+ g_autoptr(MigrationParameters) new = NULL;
/*
* Convert QTYPE_QNULL and NULL to the empty string (""). Even
@@ -1498,14 +1425,13 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
tls_opt_to_str(input->tls_hostname);
tls_opt_to_str(input->tls_authz);
- migrate_params_test_apply(input, &new);
+ /* merge input on top of current */
+ if (!migrate_params_merge(cur, input, &new, errp)) {
+ return;
+ }
- if (migrate_params_check(&new, errp)) {
- migrate_params_apply(&new);
+ if (migrate_params_check(new, errp)) {
+ migrate_params_apply(new);
migrate_post_update_params(input, errp);
}
-
- migrate_tls_opts_free(&new);
- qapi_free_BitmapMigrationNodeAliasList(new.block_bitmap_mapping);
- qapi_free_strList(new.cpr_exec_command);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 06/20] migration: Open code migrate_params_apply
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (4 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 05/20] migration: Merge parameter structs instead of assigning one by one Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 07/20] migration: Stop freeing s->parameters members individually Fabiano Rosas
` (14 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Prasad Pandit
Remove migrate_params_apply so the logic of setting migration
parameters is all in one spot.
Suggested-by: Prasad Pandit <ppandit@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 0f7478aa6a..12f884caa4 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1393,21 +1393,6 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return true;
}
-/*
- * Caller must ensure the has_* fields of @params are true so they all
- * get copied and the pointer members don't dangle.
- */
-static void migrate_params_apply(MigrationParameters *params)
-{
- MigrationState *s = migrate_get_current();
- MigrationParameters *cur = &s->parameters;
-
- migrate_tls_opts_free(cur);
- qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
- qapi_free_strList(cur->cpr_exec_command);
- QAPI_CLONE_MEMBERS(MigrationParameters, cur, params);
-}
-
void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
{
MigrationParameters *cur = &migrate_get_current()->parameters;
@@ -1430,8 +1415,15 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
return;
}
- if (migrate_params_check(new, errp)) {
- migrate_params_apply(new);
- migrate_post_update_params(input, errp);
+ if (!migrate_params_check(new, errp)) {
+ return;
}
+
+ migrate_tls_opts_free(cur);
+ qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
+ qapi_free_strList(cur->cpr_exec_command);
+
+ QAPI_CLONE_MEMBERS(MigrationParameters, cur, new);
+
+ migrate_post_update_params(input, errp);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 07/20] migration: Stop freeing s->parameters members individually
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (5 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 06/20] migration: Open code migrate_params_apply Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 08/20] migration: Use migrate_params_free during finalize Fabiano Rosas
` (13 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Expecting every pointer member from s->parameters to be freed
individually is prone to leave some fields forgotten when the code is
eventually updated. Use a dealloc visitor to free them all at once.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/options.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 12f884caa4..bdcff002db 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -17,6 +17,7 @@
#include "qapi/util.h"
#include "exec/target_page.h"
#include "qapi/clone-visitor.h"
+#include "qapi/dealloc-visitor.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
@@ -1066,11 +1067,21 @@ AnnounceParameters *migrate_announce_params(void)
return ≈
}
-static void migrate_tls_opts_free(MigrationParameters *params)
+static bool migrate_params_free(MigrationParameters *params, Error **errp)
{
- qapi_free_StrOrNull(params->tls_creds);
- qapi_free_StrOrNull(params->tls_hostname);
- qapi_free_StrOrNull(params->tls_authz);
+ Visitor *v = qapi_dealloc_visitor_new();
+ bool ret;
+
+ /*
+ * qapi_free_MigrationParameters can't be used here because
+ * MigrationParameters is embedded in MigrationState due to qdev
+ * needing to access the offset of the migration properties inside
+ * the migration object.
+ */
+ ret = visit_type_MigrationParameters_members(v, params, errp);
+ visit_free(v);
+
+ return ret;
}
/* normalize QTYPE_QNULL to QTYPE_QSTRING "" */
@@ -1419,9 +1430,9 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
return;
}
- migrate_tls_opts_free(cur);
- qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
- qapi_free_strList(cur->cpr_exec_command);
+ if (!migrate_params_free(cur, errp)) {
+ return;
+ }
QAPI_CLONE_MEMBERS(MigrationParameters, cur, new);
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 08/20] migration: Use migrate_params_free during finalize
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (6 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 07/20] migration: Stop freeing s->parameters members individually Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 09/20] tests/qtest/migration: Add a test for HMP Fabiano Rosas
` (12 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Use the recently introduced migrate_params_free routine at
migration_instance_finalize() so that newly added pointers are already
freed by default.
Special case: The TLS options are currently the only pointers that
also have a qdev property implementation, so they will be freed by
qdev using the .release method. Update the method so that a second
invocation of qapi_free_StrOrNull doesn't assert.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 3 +--
migration/options.c | 7 ++++---
migration/options.h | 1 +
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index d14beb5d22..fab7949fe1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4052,8 +4052,7 @@ static void migration_instance_finalize(Object *obj)
{
MigrationState *ms = MIGRATION(obj);
- qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
- qapi_free_strList(ms->parameters.cpr_exec_command);
+ migrate_params_free(&ms->parameters, NULL);
qemu_mutex_destroy(&ms->error_mutex);
qemu_mutex_destroy(&ms->qemu_file_lock);
qemu_sem_destroy(&ms->wait_unplug_sem);
diff --git a/migration/options.c b/migration/options.c
index bdcff002db..46d040fc3f 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -273,8 +273,9 @@ static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
static void release_StrOrNull(Object *obj, const char *name, void *opaque)
{
- const Property *prop = opaque;
- qapi_free_StrOrNull(*(StrOrNull **)object_field_prop_ptr(obj, prop));
+ StrOrNull **ptr = object_field_prop_ptr(obj, opaque);
+
+ g_clear_pointer(ptr, qapi_free_StrOrNull);
}
static void set_default_value_tls_opt(ObjectProperty *op, const Property *prop)
@@ -1067,7 +1068,7 @@ AnnounceParameters *migrate_announce_params(void)
return ≈
}
-static bool migrate_params_free(MigrationParameters *params, Error **errp)
+bool migrate_params_free(MigrationParameters *params, Error **errp)
{
Visitor *v = qapi_dealloc_visitor_new();
bool ret;
diff --git a/migration/options.h b/migration/options.h
index c272eb6208..c7da2d0b5b 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -93,4 +93,5 @@ uint64_t migrate_rdma_chunk_size(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
+bool migrate_params_free(MigrationParameters *params, Error **errp);
#endif
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 09/20] tests/qtest/migration: Add a test for HMP
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (7 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 08/20] migration: Use migrate_params_free during finalize Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 10/20] tests/qtest/migration: Add a test for HMP completion Fabiano Rosas
` (11 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
The following patches will change how parameters are set and shown in
HMP, add a test for it.
The test issues HMP migrate_set_parameters for each of the existing
migration parameters and queries them back with the info command. A
list is kept with the expected strings. A substring match function
inspired by glib's g_str_match_string is implemented for this test so
the test can produce a decent error output instead of just assert
failure. E.g:
# HMP output mismatch for entry at line 55:
# expected vs. found:
#
# max-bandwidth: 10356305952768 bytes/hour
# ---
# max-bandwidth: 10356305952768 bytes/second
(note that line 55 above is the source line where the test case for
max-bandwith is, which helps find the failing test in the list)
Usage:
QTEST_QEMU_BINARY=./qemu-system-x86_64 \
./tests/qtest/migration-test --full -p /x86_64/migration/hmp/parameters
Acked-by: Peter Xu <peterx@redhat.com>
Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
tests/qtest/migration/misc-tests.c | 200 +++++++++++++++++++++++++++++
1 file changed, 200 insertions(+)
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 4e0deb7f18..02469a92ff 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -22,6 +22,202 @@
static char *tmpfs;
+#ifdef CONFIG_HMP
+static int test_case_line;
+
+#define TEST(i1, i2, e) { i1, i2, e , .line = __LINE__, }
+#define SKIP(i1, i2, e) { i1, i2, e , .skip = true, }
+
+typedef struct HMPTestData {
+ const char *input1;
+ const char *input2;
+ const char *output1;
+ bool skip;
+ int line;
+} HMPTestData;
+
+/*
+ * Find a contiguous run of tokens in @larger that match the sequence
+ * of tokens in @smaller, ignoring mismatches due to sequences of
+ * empty tokens.
+ *
+ * Returns whether a match was found. @last is set if at least one
+ * token has matched.
+ */
+static bool token_list_is_substr(char **smaller, char **larger, int *last)
+{
+ int i, j, k = 0;
+ bool match = false;
+
+ for (i = 0; smaller[i]; i++) {
+ for (j = k; larger[j]; j++) {
+ if (!*larger[j]) {
+ continue;
+ }
+
+ /* readline adds several escape sequences */
+ if (*larger[j] == '\033') {
+ continue;
+ }
+
+ if (g_str_equal(larger[j], smaller[i])) {
+ match = true;
+ *last = j;
+ k = j + 1;
+ break;
+ }
+
+ if (match) {
+ return false;
+ } else {
+ match = false;
+ }
+ }
+ }
+
+ return match;
+}
+
+static void assert_hmp_match_line(const char *str, const char *text)
+{
+ g_auto(GStrv) tok_str = g_strsplit_set(str, " ", -1);
+ g_auto(GStrv) lines = g_strsplit_set(text, " \r\n", -1);
+ int i, idx = 0;
+
+ /*
+ * Note that the reason the 'str' above is split is to allow
+ * token_list_is_substr() to first match on the parameter name so
+ * matching can stop immediately after a mismatched value is
+ * found. This provides a better output for failing test cases
+ * than simply "str != line".
+ */
+
+ for (i = 0; lines[i]; i++) {
+ if (token_list_is_substr(tok_str, (char **)&lines[i], &idx)) {
+ return;
+ }
+
+ if (idx) {
+ break;
+ }
+ }
+
+ g_test_message("HMP output mismatch for entry at line %d:", test_case_line);
+ g_test_message("expected vs. found:\n\n%s\n---\n%s %s", str, lines[idx],
+ lines[idx + 1]);
+ g_assert_not_reached();
+}
+
+static void assert_hmp_success(const char *str)
+{
+ if (!g_str_equal(str, "")) {
+ g_test_message("HMP command failed:\n\n%s", str);
+ g_assert_not_reached();
+ }
+}
+
+static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
+{
+ QTestState *qts;
+ /*
+ * .input1: string to be used as parameter name
+ * .input2: string to be used as parameter value
+ * .output1: expected output of migrate_set_parameters
+ * E.g:
+ * (qemu) migrate_set_parameters .input1 .input2
+ * .output1
+ */
+ HMPTestData test_cases[] = {
+ TEST("", "", "migrate_set_parameter: string expected"),
+ TEST("foo", "", "migrate_set_parameter: string expected"),
+ TEST("foo", "on", "Error: invalid parameter value: foo"),
+
+ /* bool */
+ TEST("cpu-throttle-tailslow", "on", "on"),
+ TEST("direct-io", "on", "on"),
+ TEST("local", "on", "on"),
+
+ /* uint64_t */
+ TEST("announce-initial", "60", "60 ms"),
+ TEST("announce-max", "600", "600 ms"),
+ TEST("announce-rounds", "6", "6"),
+ TEST("announce-step", "15", "15 ms"),
+ TEST("downtime-limit", "400", "400 ms"),
+ TEST("avail-switchover-bandwidth", "2097152",
+ "2199023255552 bytes/second"),
+ TEST("max-bandwidth", "9876543", "10356305952768 bytes/second"),
+ TEST("max-postcopy-bandwidth", "1048576", "1048576 bytes/second"),
+ TEST("vcpu-dirty-limit", "20", "20 MB/s"),
+ TEST("x-rdma-chunk-size", "1048576", "1048576 bytes"),
+ TEST("x-vcpu-dirty-limit-period", "750", "750 ms"),
+ TEST("xbzrle-cache-size", "67108864", "67108864 bytes"),
+
+ /* uint32_t */
+ TEST("x-checkpoint-delay", "5000", "5000 ms"),
+
+ /* uint8_t */
+ TEST("cpu-throttle-increment", "15", "15"),
+ TEST("cpu-throttle-initial", "25", "25"),
+ TEST("max-cpu-throttle", "85", "85"),
+ TEST("multifd-channels", "8", "8"),
+ TEST("throttle-trigger-threshold", "65", "65"),
+
+ /* complex types */
+ TEST("mode", "cpr-exec", "cpr-exec"),
+ TEST("multifd-compression", "zlib", "zlib"),
+ TEST("zero-page-detection", "none", "none"),
+ TEST("tls-authz", "my_authz", "'my_authz'"),
+ TEST("tls-creds", "null", "'null'"),
+ TEST("tls-hostname", "localhost", "'localhost'"),
+ TEST("cpr-exec-command", "/bin/true foobar", "/bin/true foobar"),
+
+ /* can be set but are currently missing in the query output */
+ SKIP("multifd-qatzip-level", "5", "5"),
+ SKIP("multifd-zlib-level", "4", "4"),
+ SKIP("multifd-zstd-level", "6", "6"),
+
+ /* cannot be set */
+ TEST("block-bitmap-mapping", "[]",
+ "Error: The block-bitmap-mapping parameter "
+ "can only be set through QMP"),
+ };
+
+ /* force TCG so it can run in all targets */
+ qts = qtest_init("-accel tcg -nodefaults -S");
+
+ for (int i = 0; i < G_N_ELEMENTS(test_cases); i++) {
+ g_autofree char *resp = NULL;
+ g_autofree char *line = NULL;
+ struct HMPTestData *t = &test_cases[i];
+
+ if (t->skip) {
+ continue;
+ }
+
+ test_case_line = t->line;
+
+ resp = qtest_hmp(qts, "migrate_set_parameter %s %s", t->input1,
+ t->input2);
+
+ if (g_str_has_prefix(t->output1, "Error:") ||
+ g_str_has_prefix(resp, "migrate_set_parameter:")) {
+
+ assert_hmp_match_line(t->output1, resp);
+ continue;
+ }
+ assert_hmp_success(resp);
+ g_free(resp);
+
+ resp = qtest_hmp(qts, "info migrate_parameters");
+
+ line = g_strconcat(t->input1, ": ", t->output1, NULL);
+ assert_hmp_match_line(line, resp);
+ }
+
+ qtest_quit(qts);
+}
+#endif /* CONFIG_HMP */
+
static void test_baddest(char *name, MigrateCommon *args)
{
QTestState *from, *to;
@@ -260,4 +456,8 @@ void migration_test_add_misc(MigrationTestEnv *env)
test_validate_uri_channels_both_set);
migration_test_add("/migration/validate_uri/channels/none_set",
test_validate_uri_channels_none_set);
+#ifdef CONFIG_HMP
+ migration_test_add("/migration/hmp/parameters",
+ test_hmp_migration_parameters);
+#endif
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 10/20] tests/qtest/migration: Add a test for HMP completion
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (8 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 09/20] tests/qtest/migration: Add a test for HMP Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:01 ` [PATCH v3 11/20] migration: HMP: Change bandwidth parameters expected input unit Fabiano Rosas
` (10 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
The following patches will touch HMP readline completion for migration
parameters, add a test case.
The test puts the monitor on a chardev via socket and bypasses qtest
facilities because it needs to emit raw codes to readline. It
therefore requires a couple of new helpers to read/write to the
monitor socket.
Usage:
QTEST_QEMU_BINARY=./qemu-system-x86_64 \
./tests/qtest/migration-test --full -p /x86_64/migration/hmp/completion
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
tests/qtest/migration/misc-tests.c | 216 +++++++++++++++++++++++++++++
1 file changed, 216 insertions(+)
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 02469a92ff..41bf123359 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -11,6 +11,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qemu/sockets.h"
#include "qapi/error.h"
#include "qobject/qjson.h"
#include "libqtest.h"
@@ -78,6 +80,22 @@ static bool token_list_is_substr(char **smaller, char **larger, int *last)
return match;
}
+static void assert_hmp_match_text(const char *str, const char *text)
+{
+ g_auto(GStrv) tok_str = g_strsplit_set(str, " ", -1);
+ g_auto(GStrv) tok_txt = g_strsplit_set(text, " \r\n", -1);
+ int idx;
+
+ if (token_list_is_substr(tok_str, tok_txt, &idx)) {
+ return;
+ }
+
+ g_test_message("HMP output mismatch for entry at line %d:", test_case_line);
+ g_test_message("expected vs. found (whitespace ignored):\n\n%s\n---\n%s",
+ str, text);
+ g_assert_not_reached();
+}
+
static void assert_hmp_match_line(const char *str, const char *text)
{
g_auto(GStrv) tok_str = g_strsplit_set(str, " ", -1);
@@ -216,6 +234,202 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
qtest_quit(qts);
}
+
+static void hmp_sock_write(int fd, const char *buf)
+{
+ size_t sz = strlen(buf);
+
+ assert(fd > 0);
+ assert(write(fd, buf, sz) == sz);
+}
+
+static void hmp_sock_read(int fd, char *buf, size_t buf_sz)
+{
+ char *p = buf;
+ size_t sz = buf_sz - 1;
+
+ assert(fd >= 0);
+ memset(buf, 0, buf_sz);
+
+ while (sz > 0) {
+ ssize_t r = read(fd, p, sz);
+ char *prompt;
+
+ if (!r) {
+ break;
+ } else if (r < 0) {
+ if (errno == EINTR) {
+ continue;
+ }
+ g_assert_not_reached();
+ }
+
+ p += r;
+ sz -= r;
+
+ /* stop reading after the next prompt appears */
+ prompt = strstr(buf, "(qemu) ");
+ if (prompt) {
+ *prompt = '\0';
+ break;
+ }
+ }
+}
+
+static gint comp(gconstpointer a, gconstpointer b)
+{
+ return qemu_pstrcmp0((const char **)a, (const char **)b);
+}
+
+static char *get_migration_opts_sorted(QTestState *qts, const char *opt)
+{
+ GPtrArray *header = g_ptr_array_new_with_free_func(g_free);
+ GPtrArray *opts = g_ptr_array_new();
+ char *sorted_str;
+
+ if (g_str_equal(opt, "@params@")) {
+ const QDictEntry *e;
+ QDict *rsp;
+
+ g_ptr_array_add(header, g_strdup("migrate_set_parameter"));
+
+ rsp = qtest_qmp_assert_success_ref(
+ qts, "{ 'execute': 'query-migrate-parameters' }");
+
+ for (e = qdict_first(rsp); e; e = qdict_next(rsp, e)) {
+ g_ptr_array_add(opts, g_strdup(qdict_entry_key(e)));
+ }
+
+ qobject_unref(rsp);
+
+ /*
+ * query_migrate has this as optional, but for completion we want
+ * it present.
+ */
+ g_ptr_array_add(opts, g_strdup("block-bitmap-mapping"));
+
+ } else if (g_str_equal(opt, "@caps@")) {
+ int i;
+
+ g_ptr_array_add(header, g_strdup("migrate_set_capability"));
+
+ for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
+ g_ptr_array_add(opts,
+ g_strdup(MigrationCapability_lookup.array[i]));
+ }
+ } else {
+ g_assert_not_reached();
+ }
+
+ g_ptr_array_sort(opts, comp);
+ g_ptr_array_extend_and_steal(header, opts);
+
+ g_ptr_array_add(header, NULL);
+ sorted_str = g_strjoinv(" ", (char **)header->pdata);
+
+ g_ptr_array_unref(header);
+
+ return sorted_str;
+}
+
+static void hmp_completion_single(QTestState *qts, int fd,
+ const struct HMPTestData *t)
+{
+ g_autofree char *exp = NULL;
+ char buf[8192];
+ char *output;
+
+ test_case_line = t->line;
+
+ if (g_str_has_prefix(t->output1, "@")) {
+ exp = get_migration_opts_sorted(qts, t->output1);
+ } else {
+ exp = g_strdup(t->output1);
+ }
+
+ hmp_sock_write(fd, t->input1);
+ hmp_sock_read(fd, buf, sizeof(buf));
+
+ /*
+ * readline first rewrites the input to the common root of the
+ * completions, then outputs the completion suggestions:
+ *
+ * (qemu) info migr<TAB>
+ * (qemu) migrate migrate_parameters
+ * migrate_capabilities ...
+ */
+ output = strstr(buf, t->input2);
+ assert_hmp_match_text(exp, output);
+
+ /* ^U backward kill line */
+ hmp_sock_write(fd, "\x15");
+}
+
+static void test_hmp_completion(char *name, MigrateCommon *args)
+{
+ g_autofree char *cmdline;
+ char buf[1024];
+ QTestState *qts;
+ int sockfds[2];
+ /*
+ * .input1: partial string with an ending TAB (as if pressed by
+ * the user).
+ * .input2: common root of the completions, i.e. what the partial
+ * part of .input1 string completes to.
+ * .output1: full list of completion suggestions for the string
+ * in .input2.
+ * E.g:
+ * (qemu) .input1
+ * <after TAB>
+ * (qemu) .input2
+ * .output1
+ */
+ HMPTestData completion_cases[] = {
+ TEST("migra\t",
+ "migrate",
+ "migrate migrate_cancel migrate_continue migrate_incoming "
+ "migrate_pause migrate_recover migrate_set_capability "
+ "migrate_set_parameter migrate_start_postcopy"),
+
+ /*
+ * Note QEMU doesn't keep 'info' when offering the completions
+ * suggestions.
+ */
+ TEST("info migra\t",
+ "migrate",
+ "migrate migrate_capabilities migrate_parameters"),
+
+ TEST("migrate_se\t",
+ "migrate_set_",
+ "migrate_set_capability migrate_set_parameter"),
+
+ /*
+ * parameters and capabilities are not listed here to avoid having
+ * to enumerate them all, see test_hmp_completion().
+ */
+ TEST("migrate_set_parameter \t", "migrate_set_parameter ", "@params@"),
+ TEST("migrate_set_capability \t", "migrate_set_capability ", "@caps@"),
+ };
+
+ assert(!qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, sockfds));
+ qemu_clear_cloexec(sockfds[1]);
+
+ cmdline = g_strdup_printf("-chardev socket,id=mon0,fd=%d "
+ "-mon chardev=mon0,mode=readline -S",
+ sockfds[1]);
+ qts = qtest_init(cmdline);
+ close(sockfds[1]);
+
+ /* read HMP banner */
+ hmp_sock_read(sockfds[0], buf, sizeof(buf));
+
+ for (int i = 0; i < G_N_ELEMENTS(completion_cases); i++) {
+ hmp_completion_single(qts, sockfds[0], &completion_cases[i]);
+ }
+
+ close(sockfds[0]);
+ qtest_quit(qts);
+}
#endif /* CONFIG_HMP */
static void test_baddest(char *name, MigrateCommon *args)
@@ -459,5 +673,7 @@ void migration_test_add_misc(MigrationTestEnv *env)
#ifdef CONFIG_HMP
migration_test_add("/migration/hmp/parameters",
test_hmp_migration_parameters);
+ migration_test_add("/migration/hmp/completion",
+ test_hmp_completion);
#endif
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 11/20] migration: HMP: Change bandwidth parameters expected input unit
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (9 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 10/20] tests/qtest/migration: Add a test for HMP completion Fabiano Rosas
@ 2026-09-15 0:01 ` Fabiano Rosas
2026-09-15 0:02 ` [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output Fabiano Rosas
` (9 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
The max-bandwidth and avail-switchover-bandwidth parameters are
declared as bytes/second in migration.json and interpreted as such
throughout the code, except in HMP input where they're expected to be
provided as mebibytes.
Adjust the HMP code to take the values as bytes to make it uniform.
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration-hmp-cmds.c | 23 +++--------------------
tests/qtest/migration/misc-tests.c | 5 ++---
2 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 27b38d0e5f..67ea2eea15 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -596,10 +596,9 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const QDict *qdict)
const char *valuestr = qdict_get_str(qdict, "value");
Visitor *v = string_input_visitor_new(valuestr);
MigrationParameters *p = g_new0(MigrationParameters, 1);
- uint64_t valuebw = 0;
uint64_t cache_size;
Error *err = NULL;
- int val, ret;
+ int val;
val = qapi_enum_parse(&MigrationParameter_lookup, param, -1, &err);
if (val < 0) {
@@ -644,27 +643,11 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const QDict *qdict)
break;
case MIGRATION_PARAMETER_MAX_BANDWIDTH:
p->has_max_bandwidth = true;
- /*
- * Can't use visit_type_size() here, because it
- * defaults to Bytes rather than Mebibytes.
- */
- ret = qemu_strtosz_MiB(valuestr, NULL, &valuebw);
- if (ret < 0 || valuebw > INT64_MAX
- || (size_t)valuebw != valuebw) {
- error_setg(&err, "Invalid size %s", valuestr);
- break;
- }
- p->max_bandwidth = valuebw;
+ visit_type_size(v, param, &p->max_bandwidth, &err);
break;
case MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH:
p->has_avail_switchover_bandwidth = true;
- ret = qemu_strtosz_MiB(valuestr, NULL, &valuebw);
- if (ret < 0 || valuebw > INT64_MAX
- || (size_t)valuebw != valuebw) {
- error_setg(&err, "Invalid size %s", valuestr);
- break;
- }
- p->avail_switchover_bandwidth = valuebw;
+ visit_type_size(v, param, &p->avail_switchover_bandwidth, &err);
break;
case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
p->has_downtime_limit = true;
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 41bf123359..f70d35c658 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -161,9 +161,8 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
TEST("announce-rounds", "6", "6"),
TEST("announce-step", "15", "15 ms"),
TEST("downtime-limit", "400", "400 ms"),
- TEST("avail-switchover-bandwidth", "2097152",
- "2199023255552 bytes/second"),
- TEST("max-bandwidth", "9876543", "10356305952768 bytes/second"),
+ TEST("avail-switchover-bandwidth", "2097152", "2097152 bytes/second"),
+ TEST("max-bandwidth", "9876543", "9876543 bytes/second"),
TEST("max-postcopy-bandwidth", "1048576", "1048576 bytes/second"),
TEST("vcpu-dirty-limit", "20", "20 MB/s"),
TEST("x-rdma-chunk-size", "1048576", "1048576 bytes"),
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (10 preceding siblings ...)
2026-09-15 0:01 ` [PATCH v3 11/20] migration: HMP: Change bandwidth parameters expected input unit Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-15 11:57 ` Markus Armbruster
2026-09-17 21:08 ` Dr. David Alan Gilbert
2026-09-15 0:02 ` [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command Fabiano Rosas
` (8 subsequent siblings)
20 siblings, 2 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert, Kevin Wolf,
Hanna Reitz, Laurent Vivier, Paolo Bonzini
The output of 'info migrate_parameters' includes units of measurement
for a few parameters. This is convenient for a user. However, having
the units also means that every parameter must have its own separate
piece of code to handle the formatting.
From a development and maintenance perspective, having a list of
parameters explicitly written in several parts of the code brings
annoyances: conflicts during rebase, extra hits when grepping,
contributors need to mirror a change to various locations, etc.
As the HMP output is not part of any ABI, remove the units from the
output to avoid this requirement of having individualized code for
every parameter. Next patches will take benefit from that and use
generic code to output the parameters values in HMP.
Aside from the units, also remove quotes from around the TLS options
strings as this is inconsistent with all the other strings.
Similarly, change block-bitmap-mapping format to remove the arrows
(->) for the mappings as not all parameters represented as a list have
this semantics. This requires updating one of the iotests.
Before: After:
(unchanged entries omitted)
announce-initial: 50 ms announce-initial: 50
announce-max: 550 ms announce-max: 550
announce-rounds: 5 announce-rounds: 5
announce-step: 100 ms announce-step: 100
tls-creds: '' tls-creds:
tls-hostname: '' tls-hostname:
tls-authz: '' tls-authz:
max-bandwidth: 134217728 bytes/second max-bandwidth: 134217728
avail-switchover-bandwidth: 0 bytes/second avail-switchover-bandwidth: 0
max-postcopy-bandwidth: 0 bytes/second max-postcopy-bandwidth: 0
downtime-limit: 300 ms downtime-limit: 300
x-checkpoint-delay: 20000 ms x-checkpoint-delay: 20000
xbzrle-cache-size: 67108864 bytes xbzrle-cache-size: 67108864
x-vcpu-dirty-limit-period: 1000 ms x-vcpu-dirty-limit-period: 1000
vcpu-dirty-limit: 1 MB/s vcpu-dirty-limit: 1
x-rdma-chunk-size: 1048576 bytes x-rdma-chunk-size: 1048576
block-bitmap-mapping: block-bitmap-mapping:
'node-src' -> 'node-dst' bitmaps:
'bmap0' -> 'bmap0' name: bmap0 alias: bmap0
node-name: node-src alias: node-dst
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration-hmp-cmds.c | 79 +++++++++++++++++-------------
tests/qemu-iotests/300 | 20 +++++---
tests/qtest/migration/misc-tests.c | 30 ++++++------
3 files changed, 74 insertions(+), 55 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 67ea2eea15..b3422b848a 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -335,16 +335,16 @@ void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
params = qmp_query_migrate_parameters(NULL);
if (params) {
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " ms\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_INITIAL),
params->announce_initial);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " ms\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_MAX),
params->announce_max);
monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_ROUNDS),
params->announce_rounds);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " ms\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_STEP),
params->announce_step);
assert(params->has_throttle_trigger_threshold);
@@ -368,35 +368,35 @@ void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
params->max_cpu_throttle);
assert(params->tls_creds);
- monitor_hmp_printf(hmp, "%s: '%s'\n",
+ monitor_hmp_printf(hmp, "%s: %s\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
params->tls_creds->u.s);
assert(params->tls_hostname);
- monitor_hmp_printf(hmp, "%s: '%s'\n",
+ monitor_hmp_printf(hmp, "%s: %s\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_HOSTNAME),
params->tls_hostname->u.s);
assert(params->tls_authz);
- monitor_hmp_printf(hmp, "%s: '%s'\n",
+ monitor_hmp_printf(hmp, "%s: %s\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
params->tls_authz->u.s);
assert(params->has_max_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " bytes/second\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH),
params->max_bandwidth);
assert(params->has_avail_switchover_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " bytes/second\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH),
params->avail_switchover_bandwidth);
assert(params->has_max_postcopy_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " bytes/second\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
params->max_postcopy_bandwidth);
assert(params->has_downtime_limit);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " ms\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT),
params->downtime_limit);
assert(params->has_x_checkpoint_delay);
- monitor_hmp_printf(hmp, "%s: %u ms\n",
+ monitor_hmp_printf(hmp, "%s: %u\n",
MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
params->x_checkpoint_delay);
monitor_hmp_printf(hmp, "%s: %u\n",
@@ -410,41 +410,52 @@ void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
qapi_enum_lookup(&ZeroPageDetection_lookup,
params->zero_page_detection));
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " bytes\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
params->xbzrle_cache_size);
if (s->has_block_bitmap_mapping) {
- const BitmapMigrationNodeAliasList *bmnal;
+ BitmapMigrationNodeAliasList *nal;
+ BitmapMigrationNodeAlias *na;
+ BitmapMigrationBitmapAliasList *bal;
+ BitmapMigrationBitmapAlias *ba;
+ BitmapMigrationBitmapAliasTransform *bat;
monitor_hmp_printf(hmp, "%s:\n",
- MigrationParameter_str(
- MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING));
+ MigrationParameter_str(
+ MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING));
- for (bmnal = params->block_bitmap_mapping;
- bmnal;
- bmnal = bmnal->next)
+ for (nal = params->block_bitmap_mapping; nal; nal = nal->next)
{
- const BitmapMigrationNodeAlias *bmna = bmnal->value;
- const BitmapMigrationBitmapAliasList *bmbal;
+ na = nal->value;
+ monitor_hmp_printf(hmp, " bitmaps:\n");
+ for (bal = na->bitmaps; bal; bal = bal->next) {
+ ba = bal->value;
+ bat = ba->transform;
- monitor_hmp_printf(hmp, " '%s' -> '%s'\n",
- bmna->node_name, bmna->alias);
-
- for (bmbal = bmna->bitmaps; bmbal; bmbal = bmbal->next) {
- const BitmapMigrationBitmapAlias *bmba = bmbal->value;
-
- monitor_hmp_printf(hmp, " '%s' -> '%s'\n",
- bmba->name, bmba->alias);
+ monitor_hmp_printf(hmp, " name: %s", ba->name);
+ if (bat && bat->has_persistent) {
+ monitor_hmp_printf(hmp, " transform:");
+ if (bat->persistent) {
+ monitor_hmp_printf(hmp, " persistent: on");
+ } else {
+ monitor_hmp_printf(hmp, " persistent: off");
+ }
+ }
+ monitor_hmp_printf(hmp, " alias: %s\n", ba->alias);
}
+ monitor_hmp_printf(hmp, " node-name: %s alias: %s\n",
+ na->node_name, na->alias);
}
+
+ monitor_hmp_printf(hmp, "\n");
}
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " ms\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
params->x_vcpu_dirty_limit_period);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " MB/s\n",
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT),
params->vcpu_dirty_limit);
@@ -461,10 +472,10 @@ void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
}
if (params->has_x_rdma_chunk_size) {
- monitor_hmp_printf(hmp, "%s: %" PRIu64 " bytes\n",
- MigrationParameter_str(
- MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE),
- params->x_rdma_chunk_size);
+ monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
+ MigrationParameter_str(
+ MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE),
+ params->x_rdma_chunk_size);
}
assert(params->has_cpr_exec_command);
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index df2ba32632..7b97eef1fd 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -147,11 +147,9 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
result = vm.qmp('human-monitor-command',
command_line='info migrate_parameters')
-
- m = re.search(r'^block-bitmap-mapping:\r?(\n .*)*\n',
+ m = re.search(r'^block-bitmap-mapping:\r\n( .*\r\n)*',
result['return'], flags=re.MULTILINE)
hmp_mapping = m.group(0).replace('\r', '') if m else None
-
self.assertEqual(hmp_mapping, self.to_hmp_mapping(mapping))
else:
self.assert_qmp(result, 'error/desc', error)
@@ -161,11 +159,21 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
result = 'block-bitmap-mapping:\n'
for node in mapping:
- result += f" '{node['node-name']}' -> '{node['alias']}'\n"
-
assert isinstance(node['bitmaps'], list)
+ result += ' bitmaps:\n'
for bitmap in node['bitmaps']:
- result += f" '{bitmap['name']}' -> '{bitmap['alias']}'\n"
+ result += f" name: {bitmap['name']}"
+ if 'transform' in bitmap:
+ result += " transform:"
+ if 'persistent' in bitmap['transform']:
+ if bitmap['transform']['persistent']:
+ result += " persistent: on"
+ else:
+ result += " persistent: off"
+ result += f" alias: {bitmap['alias']}\n"
+
+ result += f" node-name: {node['node-name']} alias: {node['alias']}"
+ result += '\n'
return result
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index f70d35c658..182a8518a9 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -156,21 +156,21 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
TEST("local", "on", "on"),
/* uint64_t */
- TEST("announce-initial", "60", "60 ms"),
- TEST("announce-max", "600", "600 ms"),
+ TEST("announce-initial", "60", "60"),
+ TEST("announce-max", "600", "600"),
TEST("announce-rounds", "6", "6"),
- TEST("announce-step", "15", "15 ms"),
- TEST("downtime-limit", "400", "400 ms"),
- TEST("avail-switchover-bandwidth", "2097152", "2097152 bytes/second"),
- TEST("max-bandwidth", "9876543", "9876543 bytes/second"),
- TEST("max-postcopy-bandwidth", "1048576", "1048576 bytes/second"),
- TEST("vcpu-dirty-limit", "20", "20 MB/s"),
- TEST("x-rdma-chunk-size", "1048576", "1048576 bytes"),
- TEST("x-vcpu-dirty-limit-period", "750", "750 ms"),
- TEST("xbzrle-cache-size", "67108864", "67108864 bytes"),
+ TEST("announce-step", "15", "15"),
+ TEST("downtime-limit", "400", "400"),
+ TEST("avail-switchover-bandwidth", "2097152", "2097152"),
+ TEST("max-bandwidth", "9876543", "9876543"),
+ TEST("max-postcopy-bandwidth", "1048576", "1048576"),
+ TEST("vcpu-dirty-limit", "20", "20"),
+ TEST("x-rdma-chunk-size", "1048576", "1048576"),
+ TEST("x-vcpu-dirty-limit-period", "750", "750"),
+ TEST("xbzrle-cache-size", "67108864", "67108864"),
/* uint32_t */
- TEST("x-checkpoint-delay", "5000", "5000 ms"),
+ TEST("x-checkpoint-delay", "5000", "5000"),
/* uint8_t */
TEST("cpu-throttle-increment", "15", "15"),
@@ -183,9 +183,9 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
TEST("mode", "cpr-exec", "cpr-exec"),
TEST("multifd-compression", "zlib", "zlib"),
TEST("zero-page-detection", "none", "none"),
- TEST("tls-authz", "my_authz", "'my_authz'"),
- TEST("tls-creds", "null", "'null'"),
- TEST("tls-hostname", "localhost", "'localhost'"),
+ TEST("tls-authz", "my_authz", "my_authz"),
+ TEST("tls-creds", "null", "null"),
+ TEST("tls-hostname", "localhost", "localhost"),
TEST("cpr-exec-command", "/bin/true foobar", "/bin/true foobar"),
/* can be set but are currently missing in the query output */
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output
2026-09-15 0:02 ` [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output Fabiano Rosas
@ 2026-09-15 11:57 ` Markus Armbruster
2026-09-17 21:08 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-15 11:57 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Peter Xu, Dr . David Alan Gilbert, Kevin Wolf,
Hanna Reitz, Laurent Vivier, Paolo Bonzini
Fabiano Rosas <farosas@suse.de> writes:
> The output of 'info migrate_parameters' includes units of measurement
> for a few parameters. This is convenient for a user. However, having
> the units also means that every parameter must have its own separate
> piece of code to handle the formatting.
>
>>From a development and maintenance perspective, having a list of
> parameters explicitly written in several parts of the code brings
> annoyances: conflicts during rebase, extra hits when grepping,
> contributors need to mirror a change to various locations, etc.
>
> As the HMP output is not part of any ABI, remove the units from the
> output to avoid this requirement of having individualized code for
> every parameter. Next patches will take benefit from that and use
> generic code to output the parameters values in HMP.
>
> Aside from the units, also remove quotes from around the TLS options
> strings as this is inconsistent with all the other strings.
>
> Similarly, change block-bitmap-mapping format to remove the arrows
> (->) for the mappings as not all parameters represented as a list have
> this semantics. This requires updating one of the iotests.
>
> Before: After:
> (unchanged entries omitted)
> announce-initial: 50 ms announce-initial: 50
> announce-max: 550 ms announce-max: 550
> announce-rounds: 5 announce-rounds: 5
> announce-step: 100 ms announce-step: 100
> tls-creds: '' tls-creds:
> tls-hostname: '' tls-hostname:
> tls-authz: '' tls-authz:
> max-bandwidth: 134217728 bytes/second max-bandwidth: 134217728
> avail-switchover-bandwidth: 0 bytes/second avail-switchover-bandwidth: 0
> max-postcopy-bandwidth: 0 bytes/second max-postcopy-bandwidth: 0
> downtime-limit: 300 ms downtime-limit: 300
> x-checkpoint-delay: 20000 ms x-checkpoint-delay: 20000
> xbzrle-cache-size: 67108864 bytes xbzrle-cache-size: 67108864
> x-vcpu-dirty-limit-period: 1000 ms x-vcpu-dirty-limit-period: 1000
> vcpu-dirty-limit: 1 MB/s vcpu-dirty-limit: 1
> x-rdma-chunk-size: 1048576 bytes x-rdma-chunk-size: 1048576
> block-bitmap-mapping: block-bitmap-mapping:
> 'node-src' -> 'node-dst' bitmaps:
> 'bmap0' -> 'bmap0' name: bmap0 alias: bmap0
> node-name: node-src alias: node-dst
I think the value of block-bitmap-mapping could be more legible.
@block-bitmap-mapping has the following structure (using JSON as
notation):
[{"node-name": NODE-NAME1, "alias": NODE-ALIAS1,
"bitmaps": [{"name": BITMAP-NAME1.1, "alias": BITMAP-ALIAS1.1,
"transform": {"persistent": PERS1.1}}
{"name": BITMAP-NAME1.2, "alias": BITMAP-ALIAS1.2,
"transform": {"persistent": PERS1.2}},
...]},
{"node-name": NODE-NAME2, "alias": NODE-ALIAS2,
"bitmaps": [{"name": BITMAP-NAME2.1, ...}, ...]},
...]
As far as I can tell, it gets formatted like
block-bitmap-mapping:
bitmaps:
name: BITMAP-NAME1.1 transform: persistent: PERS1.1 alias: ALIAS-NAME1.1
name: BITMAP-NAME1.2 transform: persistent: PERS1.2 alias: ALIAS-NAME1.2
node-name: NODE-NAME1 alias: NODE-ALIAS1
name: BITMAP-NAME2.1 ...
node-name: NODE-NAME2 alias: NODE-ALIAS2
Suggest to do it more like this
block-bitmap-mapping:
node-name: NODE-NAME1 alias: NODE-ALIAS1
bitmaps:
name: BITMAP-NAME1.1 alias: BITMAP-ALIAS1.1 persistent: PERS1.1
name: BITMAP-NAME1.2 alias: BITMAP-ALIAS1.2 persistent: PERS1.2
node-name: NODE-NAME2 alias: NODE-ALIAS2
bitmaps:
name: BITMAP-NAME2.1 ...
Or just dump as pretty-printed JSON and call it good.
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output
2026-09-15 0:02 ` [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output Fabiano Rosas
2026-09-15 11:57 ` Markus Armbruster
@ 2026-09-17 21:08 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 37+ messages in thread
From: Dr. David Alan Gilbert @ 2026-09-17 21:08 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Peter Xu, Markus Armbruster, Kevin Wolf, Hanna Reitz,
Laurent Vivier, Paolo Bonzini
* Fabiano Rosas (farosas@suse.de) wrote:
>
> As the HMP output is not part of any ABI, remove the units from the
> output to avoid this requirement of having individualized code for
> every parameter. Next patches will take benefit from that and use
> generic code to output the parameters values in HMP.
Yes, it's allowed to remove them - although it was helpful.
IMHO the cause of what makes it hard is qapi not having any finer grain
typing, too many things are 'size' when they are in no way sizes!
While there's comments in migration.json - they're not structured enough
to allow the extraction of units, which is a shame.
Hopefully Rustification may someday lead to proper typing so that
times are compared to times and bandwidth rates naturally fall out of
divisions of sizes and times.
Dave
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (11 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 12/20] migration: Change HMP 'info migrate_parameters' output Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-17 11:08 ` Markus Armbruster
2026-09-15 0:02 ` [PATCH v3 14/20] migration: Use output visitor in info command Fabiano Rosas
` (7 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
Change the hmp_migrate_set_parameter command to use a keyval input
visitor.
Currently a string visitor is used and due to limitations of that
particular visitor's implementation it's necessary to consult the QAPI
type enum (MigrationParameter_lookup) and call each visit_type_*
function individually. Which makes using a visitor pointless.
Since there are other visitors implemented properly and generated code
to iterate the QAPI object, prefer using one of those. The keyval
input visitor is adequate because HMP provides basically one key and
one value for each migrate_set_parameter command.
To switch from string_input_visitor to keyval_input_visitor simply put
the parameter name and value into a dict and invoke
visit_type_MigrationParameters().
Note that it's not necessary to go through any of the keyval_* code
because due to the nature of HMP, there's no parsing to do (no '=', no
',', etc).
With this the migrate_set_parameters HMP commands will be
automatically updated anytime a new migration parameter is added.
Note that one parameter, "cpr-exec-command", takes the strList type so
the HMP string needs to be first converted to a QList.
A small side-effect improvement of this change is that by not using
the MigrationParameter enum as a means to verify whether the
user-provided key is a valid parameter name, the message when an
invalid parameter is given now changes from "invalid value x" to
"parameter x is unexpected", which is more accurate.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration-hmp-cmds.c | 187 ++++-------------------------
tests/qtest/migration/misc-tests.c | 2 +-
2 files changed, 26 insertions(+), 163 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index b3422b848a..7491c79a2e 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -25,7 +25,10 @@
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
+#include "qapi/qobject-input-visitor.h"
#include "qobject/qdict.h"
+#include "qobject/qlist.h"
+#include "qobject/qstring.h"
#include "qapi/string-input-visitor.h"
#include "qapi/string-output-visitor.h"
#include "qemu/cutils.h"
@@ -605,183 +608,43 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const QDict *qdict)
{
const char *param = qdict_get_str(qdict, "parameter");
const char *valuestr = qdict_get_str(qdict, "value");
- Visitor *v = string_input_visitor_new(valuestr);
- MigrationParameters *p = g_new0(MigrationParameters, 1);
- uint64_t cache_size;
+ g_autoptr(QDict) input = qdict_new();
+ g_autoptr(MigrationParameters) p = NULL;
+ QObject *value;
+ Visitor *v;
Error *err = NULL;
- int val;
- val = qapi_enum_parse(&MigrationParameter_lookup, param, -1, &err);
- if (val < 0) {
- goto cleanup;
- }
-
- switch (val) {
- case MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD:
- p->has_throttle_trigger_threshold = true;
- visit_type_uint8(v, param, &p->throttle_trigger_threshold, &err);
- break;
- case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
- p->has_cpu_throttle_initial = true;
- visit_type_uint8(v, param, &p->cpu_throttle_initial, &err);
- break;
- case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
- p->has_cpu_throttle_increment = true;
- visit_type_uint8(v, param, &p->cpu_throttle_increment, &err);
- break;
- case MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW:
- p->has_cpu_throttle_tailslow = true;
- visit_type_bool(v, param, &p->cpu_throttle_tailslow, &err);
- break;
- case MIGRATION_PARAMETER_MAX_CPU_THROTTLE:
- p->has_max_cpu_throttle = true;
- visit_type_uint8(v, param, &p->max_cpu_throttle, &err);
- break;
- case MIGRATION_PARAMETER_TLS_CREDS:
- p->tls_creds = g_new0(StrOrNull, 1);
- p->tls_creds->type = QTYPE_QSTRING;
- visit_type_str(v, param, &p->tls_creds->u.s, &err);
- break;
- case MIGRATION_PARAMETER_TLS_HOSTNAME:
- p->tls_hostname = g_new0(StrOrNull, 1);
- p->tls_hostname->type = QTYPE_QSTRING;
- visit_type_str(v, param, &p->tls_hostname->u.s, &err);
- break;
- case MIGRATION_PARAMETER_TLS_AUTHZ:
- p->tls_authz = g_new0(StrOrNull, 1);
- p->tls_authz->type = QTYPE_QSTRING;
- visit_type_str(v, param, &p->tls_authz->u.s, &err);
- break;
- case MIGRATION_PARAMETER_MAX_BANDWIDTH:
- p->has_max_bandwidth = true;
- visit_type_size(v, param, &p->max_bandwidth, &err);
- break;
- case MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH:
- p->has_avail_switchover_bandwidth = true;
- visit_type_size(v, param, &p->avail_switchover_bandwidth, &err);
- break;
- case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
- p->has_downtime_limit = true;
- visit_type_size(v, param, &p->downtime_limit, &err);
- break;
- case MIGRATION_PARAMETER_X_CHECKPOINT_DELAY:
- p->has_x_checkpoint_delay = true;
- visit_type_uint32(v, param, &p->x_checkpoint_delay, &err);
- break;
- case MIGRATION_PARAMETER_MULTIFD_CHANNELS:
- p->has_multifd_channels = true;
- visit_type_uint8(v, param, &p->multifd_channels, &err);
- break;
- case MIGRATION_PARAMETER_MULTIFD_COMPRESSION:
- p->has_multifd_compression = true;
- visit_type_MultiFDCompression(v, param, &p->multifd_compression,
- &err);
- break;
- case MIGRATION_PARAMETER_MULTIFD_ZLIB_LEVEL:
- p->has_multifd_zlib_level = true;
- visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
- break;
- case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
- p->has_multifd_qatzip_level = true;
- visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
- break;
- case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
- p->has_multifd_zstd_level = true;
- visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
- break;
- case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION:
- p->has_zero_page_detection = true;
- visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, &err);
- break;
- case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
- p->has_xbzrle_cache_size = true;
- if (!visit_type_size(v, param, &cache_size, &err)) {
- break;
- }
- if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
- error_setg(&err, "Invalid size %s", valuestr);
- break;
- }
- p->xbzrle_cache_size = cache_size;
- break;
- case MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH:
- p->has_max_postcopy_bandwidth = true;
- visit_type_size(v, param, &p->max_postcopy_bandwidth, &err);
- break;
- case MIGRATION_PARAMETER_ANNOUNCE_INITIAL:
- p->has_announce_initial = true;
- visit_type_size(v, param, &p->announce_initial, &err);
- break;
- case MIGRATION_PARAMETER_ANNOUNCE_MAX:
- p->has_announce_max = true;
- visit_type_size(v, param, &p->announce_max, &err);
- break;
- case MIGRATION_PARAMETER_ANNOUNCE_ROUNDS:
- p->has_announce_rounds = true;
- visit_type_size(v, param, &p->announce_rounds, &err);
- break;
- case MIGRATION_PARAMETER_ANNOUNCE_STEP:
- p->has_announce_step = true;
- visit_type_size(v, param, &p->announce_step, &err);
- break;
- case MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING:
- error_setg(&err, "The block-bitmap-mapping parameter can only be set "
- "through QMP");
- break;
- case MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD:
- p->has_x_vcpu_dirty_limit_period = true;
- visit_type_size(v, param, &p->x_vcpu_dirty_limit_period, &err);
- break;
- case MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT:
- p->has_vcpu_dirty_limit = true;
- visit_type_size(v, param, &p->vcpu_dirty_limit, &err);
- break;
- case MIGRATION_PARAMETER_MODE:
- p->has_mode = true;
- visit_type_MigMode(v, param, &p->mode, &err);
- break;
- case MIGRATION_PARAMETER_DIRECT_IO:
- p->has_direct_io = true;
- visit_type_bool(v, param, &p->direct_io, &err);
- break;
- case MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE:
- p->has_x_rdma_chunk_size = true;
- visit_type_size(v, param, &p->x_rdma_chunk_size, &err);
- break;
- case MIGRATION_PARAMETER_CPR_EXEC_COMMAND: {
- /*
- * NOTE: g_autofree will only auto g_free() the strv array when
- * needed, it will not free the strings within the array. It's
- * intentional: when strv is set, the ownership of the strings will
- * always be passed to p->cpr_exec_command via QAPI_LIST_APPEND().
- */
+ if (g_str_equal(param, "block-bitmap-mapping")) {
+ error_setg(&err, "The %s parameter can only be set through QMP", param);
+ goto err;
+ } else if (g_str_equal(param, "cpr-exec-command")) {
g_autofree char **strv = NULL;
g_autoptr(GError) gerr = NULL;
- strList **tail = &p->cpr_exec_command;
+ QList *list;
if (!g_shell_parse_argv(valuestr, NULL, &strv, &gerr)) {
error_setg(&err, "%s", gerr->message);
- break;
+ goto err;
}
+
+ list = qlist_new();
for (int i = 0; strv[i]; i++) {
- QAPI_LIST_APPEND(tail, strv[i]);
+ qlist_append_str(list, strv[i]);
+ g_free(strv[i]);
}
- p->has_cpr_exec_command = true;
- break;
- }
- default:
- g_assert_not_reached();
+ value = QOBJECT(list);
+ } else {
+ value = QOBJECT(qstring_from_str(valuestr));
}
- if (err) {
- goto cleanup;
+ qdict_put_obj(input, param, value);
+ v = qobject_input_visitor_new_keyval(QOBJECT(input));
+ if (visit_type_MigrationParameters(v, NULL, &p, &err)) {
+ qmp_migrate_set_parameters(p, &err);
}
- qmp_migrate_set_parameters(p, &err);
-
- cleanup:
- qapi_free_MigrationParameters(p);
visit_free(v);
+err:
hmp_handle_error(hmp, err);
}
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 182a8518a9..49280e5bc9 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -148,7 +148,7 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
HMPTestData test_cases[] = {
TEST("", "", "migrate_set_parameter: string expected"),
TEST("foo", "", "migrate_set_parameter: string expected"),
- TEST("foo", "on", "Error: invalid parameter value: foo"),
+ TEST("foo", "on", "Error: Parameter 'foo' is unexpected"),
/* bool */
TEST("cpu-throttle-tailslow", "on", "on"),
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command
2026-09-15 0:02 ` [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command Fabiano Rosas
@ 2026-09-17 11:08 ` Markus Armbruster
2026-09-17 18:28 ` Peter Xu
0 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 11:08 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Peter Xu, Dr . David Alan Gilbert, Laurent Vivier,
Paolo Bonzini
Fabiano Rosas <farosas@suse.de> writes:
> Change the hmp_migrate_set_parameter command to use a keyval input
> visitor.
>
> Currently a string visitor is used and due to limitations of that
> particular visitor's implementation it's necessary to consult the QAPI
> type enum (MigrationParameter_lookup) and call each visit_type_*
> function individually. Which makes using a visitor pointless.
>
> Since there are other visitors implemented properly and generated code
> to iterate the QAPI object, prefer using one of those. The keyval
> input visitor is adequate because HMP provides basically one key and
> one value for each migrate_set_parameter command.
>
> To switch from string_input_visitor to keyval_input_visitor simply put
> the parameter name and value into a dict and invoke
> visit_type_MigrationParameters().
>
> Note that it's not necessary to go through any of the keyval_* code
> because due to the nature of HMP, there's no parsing to do (no '=', no
> ',', etc).
>
> With this the migrate_set_parameters HMP commands will be
> automatically updated anytime a new migration parameter is added.
>
> Note that one parameter, "cpr-exec-command", takes the strList type so
> the HMP string needs to be first converted to a QList.
>
> A small side-effect improvement of this change is that by not using
> the MigrationParameter enum as a means to verify whether the
> user-provided key is a valid parameter name, the message when an
> invalid parameter is given now changes from "invalid value x" to
> "parameter x is unexpected", which is more accurate.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/migration-hmp-cmds.c | 187 ++++-------------------------
> tests/qtest/migration/misc-tests.c | 2 +-
> 2 files changed, 26 insertions(+), 163 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index b3422b848a..7491c79a2e 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -25,7 +25,10 @@
> #include "qapi/error.h"
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/qapi-visit-migration.h"
> +#include "qapi/qobject-input-visitor.h"
> #include "qobject/qdict.h"
> +#include "qobject/qlist.h"
> +#include "qobject/qstring.h"
> #include "qapi/string-input-visitor.h"
> #include "qapi/string-output-visitor.h"
> #include "qemu/cutils.h"
> @@ -605,183 +608,43 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const QDict *qdict)
> {
> const char *param = qdict_get_str(qdict, "parameter");
> const char *valuestr = qdict_get_str(qdict, "value");
> - Visitor *v = string_input_visitor_new(valuestr);
> - MigrationParameters *p = g_new0(MigrationParameters, 1);
> - uint64_t cache_size;
> + g_autoptr(QDict) input = qdict_new();
> + g_autoptr(MigrationParameters) p = NULL;
> + QObject *value;
> + Visitor *v;
> Error *err = NULL;
> - int val;
>
> - val = qapi_enum_parse(&MigrationParameter_lookup, param, -1, &err);
> - if (val < 0) {
> - goto cleanup;
> - }
> -
> - switch (val) {
> - case MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD:
> - p->has_throttle_trigger_threshold = true;
> - visit_type_uint8(v, param, &p->throttle_trigger_threshold, &err);
> - break;
> - case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
> - p->has_cpu_throttle_initial = true;
> - visit_type_uint8(v, param, &p->cpu_throttle_initial, &err);
> - break;
> - case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
> - p->has_cpu_throttle_increment = true;
> - visit_type_uint8(v, param, &p->cpu_throttle_increment, &err);
> - break;
> - case MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW:
> - p->has_cpu_throttle_tailslow = true;
> - visit_type_bool(v, param, &p->cpu_throttle_tailslow, &err);
> - break;
> - case MIGRATION_PARAMETER_MAX_CPU_THROTTLE:
> - p->has_max_cpu_throttle = true;
> - visit_type_uint8(v, param, &p->max_cpu_throttle, &err);
> - break;
> - case MIGRATION_PARAMETER_TLS_CREDS:
> - p->tls_creds = g_new0(StrOrNull, 1);
> - p->tls_creds->type = QTYPE_QSTRING;
> - visit_type_str(v, param, &p->tls_creds->u.s, &err);
> - break;
> - case MIGRATION_PARAMETER_TLS_HOSTNAME:
> - p->tls_hostname = g_new0(StrOrNull, 1);
> - p->tls_hostname->type = QTYPE_QSTRING;
> - visit_type_str(v, param, &p->tls_hostname->u.s, &err);
> - break;
> - case MIGRATION_PARAMETER_TLS_AUTHZ:
> - p->tls_authz = g_new0(StrOrNull, 1);
> - p->tls_authz->type = QTYPE_QSTRING;
> - visit_type_str(v, param, &p->tls_authz->u.s, &err);
> - break;
> - case MIGRATION_PARAMETER_MAX_BANDWIDTH:
> - p->has_max_bandwidth = true;
> - visit_type_size(v, param, &p->max_bandwidth, &err);
> - break;
> - case MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH:
> - p->has_avail_switchover_bandwidth = true;
> - visit_type_size(v, param, &p->avail_switchover_bandwidth, &err);
> - break;
> - case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
> - p->has_downtime_limit = true;
> - visit_type_size(v, param, &p->downtime_limit, &err);
> - break;
> - case MIGRATION_PARAMETER_X_CHECKPOINT_DELAY:
> - p->has_x_checkpoint_delay = true;
> - visit_type_uint32(v, param, &p->x_checkpoint_delay, &err);
> - break;
> - case MIGRATION_PARAMETER_MULTIFD_CHANNELS:
> - p->has_multifd_channels = true;
> - visit_type_uint8(v, param, &p->multifd_channels, &err);
> - break;
> - case MIGRATION_PARAMETER_MULTIFD_COMPRESSION:
> - p->has_multifd_compression = true;
> - visit_type_MultiFDCompression(v, param, &p->multifd_compression,
> - &err);
> - break;
> - case MIGRATION_PARAMETER_MULTIFD_ZLIB_LEVEL:
> - p->has_multifd_zlib_level = true;
> - visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
> - break;
> - case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
> - p->has_multifd_qatzip_level = true;
> - visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
> - break;
> - case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
> - p->has_multifd_zstd_level = true;
> - visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
> - break;
> - case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION:
> - p->has_zero_page_detection = true;
> - visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, &err);
> - break;
> - case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
> - p->has_xbzrle_cache_size = true;
> - if (!visit_type_size(v, param, &cache_size, &err)) {
> - break;
> - }
> - if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
> - error_setg(&err, "Invalid size %s", valuestr);
> - break;
> - }
> - p->xbzrle_cache_size = cache_size;
> - break;
> - case MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH:
> - p->has_max_postcopy_bandwidth = true;
> - visit_type_size(v, param, &p->max_postcopy_bandwidth, &err);
> - break;
> - case MIGRATION_PARAMETER_ANNOUNCE_INITIAL:
> - p->has_announce_initial = true;
> - visit_type_size(v, param, &p->announce_initial, &err);
> - break;
> - case MIGRATION_PARAMETER_ANNOUNCE_MAX:
> - p->has_announce_max = true;
> - visit_type_size(v, param, &p->announce_max, &err);
> - break;
> - case MIGRATION_PARAMETER_ANNOUNCE_ROUNDS:
> - p->has_announce_rounds = true;
> - visit_type_size(v, param, &p->announce_rounds, &err);
> - break;
> - case MIGRATION_PARAMETER_ANNOUNCE_STEP:
> - p->has_announce_step = true;
> - visit_type_size(v, param, &p->announce_step, &err);
> - break;
> - case MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING:
> - error_setg(&err, "The block-bitmap-mapping parameter can only be set "
> - "through QMP");
> - break;
> - case MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD:
> - p->has_x_vcpu_dirty_limit_period = true;
> - visit_type_size(v, param, &p->x_vcpu_dirty_limit_period, &err);
> - break;
> - case MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT:
> - p->has_vcpu_dirty_limit = true;
> - visit_type_size(v, param, &p->vcpu_dirty_limit, &err);
> - break;
> - case MIGRATION_PARAMETER_MODE:
> - p->has_mode = true;
> - visit_type_MigMode(v, param, &p->mode, &err);
> - break;
> - case MIGRATION_PARAMETER_DIRECT_IO:
> - p->has_direct_io = true;
> - visit_type_bool(v, param, &p->direct_io, &err);
> - break;
> - case MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE:
> - p->has_x_rdma_chunk_size = true;
> - visit_type_size(v, param, &p->x_rdma_chunk_size, &err);
> - break;
> - case MIGRATION_PARAMETER_CPR_EXEC_COMMAND: {
> - /*
> - * NOTE: g_autofree will only auto g_free() the strv array when
> - * needed, it will not free the strings within the array. It's
> - * intentional: when strv is set, the ownership of the strings will
> - * always be passed to p->cpr_exec_command via QAPI_LIST_APPEND().
> - */
> + if (g_str_equal(param, "block-bitmap-mapping")) {
> + error_setg(&err, "The %s parameter can only be set through QMP", param);
> + goto err;
> + } else if (g_str_equal(param, "cpr-exec-command")) {
> g_autofree char **strv = NULL;
Not this patch's doing: initialization looks superfluous.
> g_autoptr(GError) gerr = NULL;
> - strList **tail = &p->cpr_exec_command;
> + QList *list;
>
> if (!g_shell_parse_argv(valuestr, NULL, &strv, &gerr)) {
> error_setg(&err, "%s", gerr->message);
> - break;
> + goto err;
> }
> +
> + list = qlist_new();
> for (int i = 0; strv[i]; i++) {
> - QAPI_LIST_APPEND(tail, strv[i]);
> + qlist_append_str(list, strv[i]);
> + g_free(strv[i]);
> }
> - p->has_cpr_exec_command = true;
> - break;
> - }
> - default:
> - g_assert_not_reached();
> + value = QOBJECT(list);
> + } else {
> + value = QOBJECT(qstring_from_str(valuestr));
> }
>
> - if (err) {
> - goto cleanup;
> + qdict_put_obj(input, param, value);
> + v = qobject_input_visitor_new_keyval(QOBJECT(input));
> + if (visit_type_MigrationParameters(v, NULL, &p, &err)) {
> + qmp_migrate_set_parameters(p, &err);
> }
>
> - qmp_migrate_set_parameters(p, &err);
> -
> - cleanup:
> - qapi_free_MigrationParameters(p);
> visit_free(v);
> +err:
> hmp_handle_error(hmp, err);
> }
>
> diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
> index 182a8518a9..49280e5bc9 100644
> --- a/tests/qtest/migration/misc-tests.c
> +++ b/tests/qtest/migration/misc-tests.c
> @@ -148,7 +148,7 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
> HMPTestData test_cases[] = {
> TEST("", "", "migrate_set_parameter: string expected"),
> TEST("foo", "", "migrate_set_parameter: string expected"),
> - TEST("foo", "on", "Error: invalid parameter value: foo"),
> + TEST("foo", "on", "Error: Parameter 'foo' is unexpected"),
>
> /* bool */
> TEST("cpu-throttle-tailslow", "on", "on"),
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command
2026-09-17 11:08 ` Markus Armbruster
@ 2026-09-17 18:28 ` Peter Xu
2026-09-17 18:41 ` Markus Armbruster
0 siblings, 1 reply; 37+ messages in thread
From: Peter Xu @ 2026-09-17 18:28 UTC (permalink / raw)
To: Markus Armbruster
Cc: Fabiano Rosas, qemu-devel, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
On Thu, Sep 17, 2026 at 01:08:04PM +0200, Markus Armbruster wrote:
> > + } else if (g_str_equal(param, "cpr-exec-command")) {
> > g_autofree char **strv = NULL;
>
> Not this patch's doing: initialization looks superfluous.
I do see most of such cases in QEMU we initialize it to NULL. I think it's
a good pattern for g_autofree lines so we will never try to free garbage.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command
2026-09-17 18:28 ` Peter Xu
@ 2026-09-17 18:41 ` Markus Armbruster
0 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 18:41 UTC (permalink / raw)
To: Peter Xu
Cc: Fabiano Rosas, qemu-devel, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
Peter Xu <peterx@redhat.com> writes:
> On Thu, Sep 17, 2026 at 01:08:04PM +0200, Markus Armbruster wrote:
>> > + } else if (g_str_equal(param, "cpr-exec-command")) {
>> > g_autofree char **strv = NULL;
>>
>> Not this patch's doing: initialization looks superfluous.
>
> I do see most of such cases in QEMU we initialize it to NULL. I think it's
> a good pattern for g_autofree lines so we will never try to free garbage.
Actually necessary for g_autofree. Ignore me :)
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (12 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 13/20] migration: Use keyval input visitor in HMP set command Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-17 11:53 ` Markus Armbruster
2026-09-15 0:02 ` [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict Fabiano Rosas
` (6 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
The hmp_info_migrate_parameters function currently open-codes the
mon_printf calls for each migration parameter. As with the set command
in the last patch, this should not be necessary as the QAPI
infrastructure already has generated code that takes type and struct
member names into account, including converting _ from C into the '-'
character as part of parameter names strings.
The current code is also quite painful to rebase if a series has been
carried for a long time while parameters have been added in master.
Replace all of this with a conversion from MigrationParameters to
QDict using an output visitor and a loop over the QDict that prints
per-QAPI-type formatted strings.
Modelled after block/qapi.c:dump_qobject, but with some changes to
keep the migration command output formatting.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration-hmp-cmds.c | 234 +++++++++++----------------------
migration/options.c | 2 +-
migration/options.h | 1 +
3 files changed, 81 insertions(+), 156 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7491c79a2e..00d2d310af 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -26,8 +26,12 @@
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qobject/qbool.h"
#include "qobject/qdict.h"
+#include "qobject/qjson.h"
#include "qobject/qlist.h"
+#include "qobject/qnum.h"
#include "qobject/qstring.h"
#include "qapi/string-input-visitor.h"
#include "qapi/string-output-visitor.h"
@@ -318,171 +322,91 @@ void hmp_info_migrate_capabilities(MonitorHMP *hmp, const QDict *qdict)
qapi_free_MigrationCapabilityStatusList(caps);
}
-static void monitor_print_cpr_exec_command(MonitorHMP *hmp, strList *args)
+static void hmp_migrate_print_qobject(MonitorHMP *hmp, const char *label,
+ QObject *obj)
{
- monitor_hmp_printf(hmp, "%s:",
- MigrationParameter_str(MIGRATION_PARAMETER_CPR_EXEC_COMMAND));
+ const char *sep;
- while (args) {
- monitor_hmp_printf(hmp, " %s", args->value);
- args = args->next;
+ if (!obj) {
+ return;
+ }
+
+ /*
+ * Put a space after labels
+ * foo: bar
+ * ^
+ */
+ if (label && label[0] && label[strlen(label) - 1] == ':') {
+ sep = " ";
+ } else {
+ sep = "";
+ }
+
+ switch (qobject_type(obj)) {
+ case QTYPE_QNUM: {
+ g_autofree char *str = qnum_to_string(qobject_to(QNum, obj));
+
+ monitor_hmp_printf(hmp, "%s%s%s", label, sep, str);
+ break;
+ }
+ case QTYPE_QSTRING:
+ monitor_hmp_printf(hmp, "%s%s%s", label, sep,
+ qstring_get_str(qobject_to(QString, obj)));
+ break;
+ case QTYPE_QDICT: {
+ QDict *d = qobject_to(QDict, obj);
+ const QDictEntry *e;
+ int i = 0;
+
+ monitor_hmp_printf(hmp, "%s%s", label, sep);
+
+ for (e = qdict_first(d); e; e = qdict_next(d, e), i++) {
+ g_autofree char *l = g_strdup_printf("%s:", qdict_entry_key(e));
+ if (i) {
+ monitor_hmp_printf(hmp, " ");
+ }
+ hmp_migrate_print_qobject(hmp, l, qdict_entry_value(e));
+ }
+ break;
+ }
+ case QTYPE_QLIST: {
+ const QListEntry *e;
+
+ monitor_hmp_printf(hmp, "%s\n", label);
+ for (e = qlist_first(qobject_to(QList, obj)); e; e = qlist_next(e)) {
+ hmp_migrate_print_qobject(hmp, " ", e->value);
+ monitor_hmp_printf(hmp, "\n");
+ }
+
+ break;
+ }
+ case QTYPE_QBOOL:
+ monitor_hmp_printf(hmp, "%s%s%s", label, sep,
+ qbool_get_bool(qobject_to(QBool, obj)) ?
+ "on" : "off");
+ break;
+ case QTYPE_NONE:
+ case QTYPE_QNULL:
+ default:
+ g_assert_not_reached();
+ break;
}
- monitor_hmp_printf(hmp, "\n");
}
void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
{
- MigrationParameters *params;
- MigrationState *s = migrate_get_current();
+ MigrationParameters *params = qmp_query_migrate_parameters(NULL);
+ g_autoptr(QDict) d;
+ const QDictEntry *e;
- params = qmp_query_migrate_parameters(NULL);
+ assert(params);
- if (params) {
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_INITIAL),
- params->announce_initial);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_MAX),
- params->announce_max);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_ROUNDS),
- params->announce_rounds);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_STEP),
- params->announce_step);
- assert(params->has_throttle_trigger_threshold);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD),
- params->throttle_trigger_threshold);
- assert(params->has_cpu_throttle_initial);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL),
- params->cpu_throttle_initial);
- assert(params->has_cpu_throttle_increment);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
- params->cpu_throttle_increment);
- assert(params->has_cpu_throttle_tailslow);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
- params->cpu_throttle_tailslow ? "on" : "off");
- assert(params->has_max_cpu_throttle);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
- params->max_cpu_throttle);
- assert(params->tls_creds);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
- params->tls_creds->u.s);
- assert(params->tls_hostname);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_TLS_HOSTNAME),
- params->tls_hostname->u.s);
- assert(params->tls_authz);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
- params->tls_authz->u.s);
- assert(params->has_max_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH),
- params->max_bandwidth);
- assert(params->has_avail_switchover_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH),
- params->avail_switchover_bandwidth);
- assert(params->has_max_postcopy_bandwidth);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
- params->max_postcopy_bandwidth);
- assert(params->has_downtime_limit);
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT),
- params->downtime_limit);
- assert(params->has_x_checkpoint_delay);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
- params->x_checkpoint_delay);
- monitor_hmp_printf(hmp, "%s: %u\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
- params->multifd_channels);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
- MultiFDCompression_str(params->multifd_compression));
- assert(params->has_zero_page_detection);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
- qapi_enum_lookup(&ZeroPageDetection_lookup,
- params->zero_page_detection));
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
- params->xbzrle_cache_size);
+ d = migrate_params_to_dict(params, NULL);
+ for (e = qdict_first(d); e; e = qdict_next(d, e)) {
+ g_autofree char *label = g_strdup_printf("%s:", qdict_entry_key(e));
- if (s->has_block_bitmap_mapping) {
- BitmapMigrationNodeAliasList *nal;
- BitmapMigrationNodeAlias *na;
- BitmapMigrationBitmapAliasList *bal;
- BitmapMigrationBitmapAlias *ba;
- BitmapMigrationBitmapAliasTransform *bat;
-
- monitor_hmp_printf(hmp, "%s:\n",
- MigrationParameter_str(
- MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING));
-
- for (nal = params->block_bitmap_mapping; nal; nal = nal->next)
- {
- na = nal->value;
- monitor_hmp_printf(hmp, " bitmaps:\n");
- for (bal = na->bitmaps; bal; bal = bal->next) {
- ba = bal->value;
- bat = ba->transform;
-
- monitor_hmp_printf(hmp, " name: %s", ba->name);
- if (bat && bat->has_persistent) {
- monitor_hmp_printf(hmp, " transform:");
- if (bat->persistent) {
- monitor_hmp_printf(hmp, " persistent: on");
- } else {
- monitor_hmp_printf(hmp, " persistent: off");
- }
- }
- monitor_hmp_printf(hmp, " alias: %s\n", ba->alias);
- }
- monitor_hmp_printf(hmp, " node-name: %s alias: %s\n",
- na->node_name, na->alias);
- }
-
- monitor_hmp_printf(hmp, "\n");
- }
-
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
- params->x_vcpu_dirty_limit_period);
-
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT),
- params->vcpu_dirty_limit);
-
- assert(params->has_mode);
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(MIGRATION_PARAMETER_MODE),
- qapi_enum_lookup(&MigMode_lookup, params->mode));
-
- if (params->has_direct_io) {
- monitor_hmp_printf(hmp, "%s: %s\n",
- MigrationParameter_str(
- MIGRATION_PARAMETER_DIRECT_IO),
- params->direct_io ? "on" : "off");
- }
-
- if (params->has_x_rdma_chunk_size) {
- monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
- MigrationParameter_str(
- MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE),
- params->x_rdma_chunk_size);
- }
-
- assert(params->has_cpr_exec_command);
- monitor_print_cpr_exec_command(hmp, params->cpr_exec_command);
+ hmp_migrate_print_qobject(hmp, label, qdict_entry_value(e));
+ monitor_hmp_printf(hmp, "\n");
}
qapi_free_MigrationParameters(params);
diff --git a/migration/options.c b/migration/options.c
index 46d040fc3f..29b96f48c1 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1097,7 +1097,7 @@ static void tls_opt_to_str(StrOrNull *opt)
opt->u.s = g_strdup("");
}
-static QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
+QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
{
QObject *obj = NULL;
Visitor *v = qobject_output_visitor_new(&obj);
diff --git a/migration/options.h b/migration/options.h
index c7da2d0b5b..0d9f88c11e 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -94,4 +94,5 @@ uint64_t migrate_rdma_chunk_size(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
bool migrate_params_free(MigrationParameters *params, Error **errp);
+QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
#endif
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-15 0:02 ` [PATCH v3 14/20] migration: Use output visitor in info command Fabiano Rosas
@ 2026-09-17 11:53 ` Markus Armbruster
2026-09-17 11:58 ` Markus Armbruster
` (2 more replies)
0 siblings, 3 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 11:53 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Fabiano Rosas <farosas@suse.de> writes:
> The hmp_info_migrate_parameters function currently open-codes the
> mon_printf calls for each migration parameter. As with the set command
> in the last patch, this should not be necessary as the QAPI
> infrastructure already has generated code that takes type and struct
> member names into account, including converting _ from C into the '-'
> character as part of parameter names strings.
>
> The current code is also quite painful to rebase if a series has been
> carried for a long time while parameters have been added in master.
>
> Replace all of this with a conversion from MigrationParameters to
> QDict using an output visitor and a loop over the QDict that prints
> per-QAPI-type formatted strings.
>
> Modelled after block/qapi.c:dump_qobject, but with some changes to
> keep the migration command output formatting.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Old output of info migrate_parameters for me:
announce-initial: 50
announce-max: 550
announce-rounds: 5
announce-step: 100
throttle-trigger-threshold: 50
cpu-throttle-initial: 20
cpu-throttle-increment: 10
cpu-throttle-tailslow: off
max-cpu-throttle: 99
tls-creds:
tls-hostname:
tls-authz:
max-bandwidth: 134217728
avail-switchover-bandwidth: 0
max-postcopy-bandwidth: 0
downtime-limit: 300
x-checkpoint-delay: 20000
multifd-channels: 2
multifd-compression: none
zero-page-detection: multifd
xbzrle-cache-size: 67108864
x-vcpu-dirty-limit-period: 1000
vcpu-dirty-limit: 1
mode: normal
direct-io: off
x-rdma-chunk-size: 1048576
cpr-exec-command:
New output:
cpu-throttle-tailslow: off
xbzrle-cache-size: 67108864
cpu-throttle-initial: 20
announce-max: 550
direct-io: off
avail-switchover-bandwidth: 0
local: off
zero-page-detection: multifd
multifd-qatzip-level: 1
x-rdma-chunk-size: 1048576
multifd-channels: 2
mode: normal
multifd-zstd-level: 1
announce-initial: 50
downtime-limit: 300
tls-authz:
cpr-exec-command:
vcpu-dirty-limit: 1
multifd-compression: none
announce-rounds: 5
announce-step: 100
tls-creds:
x-vcpu-dirty-limit-period: 1000
multifd-zlib-level: 1
max-cpu-throttle: 99
max-postcopy-bandwidth: 0
tls-hostname:
throttle-trigger-threshold: 50
max-bandwidth: 134217728
x-checkpoint-delay: 20000
cpu-throttle-increment: 10
New output has an unwanted blank line after "cpr-exec-command:".
New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
@multifd-zstd-level. Commit message should point that out.
Order of parameters changes, and not for the better. Hmm.
> ---
> migration/migration-hmp-cmds.c | 234 +++++++++++----------------------
> migration/options.c | 2 +-
> migration/options.h | 1 +
> 3 files changed, 81 insertions(+), 156 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 7491c79a2e..00d2d310af 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -26,8 +26,12 @@
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/qapi-visit-migration.h"
> #include "qapi/qobject-input-visitor.h"
> +#include "qapi/qobject-output-visitor.h"
> +#include "qobject/qbool.h"
> #include "qobject/qdict.h"
> +#include "qobject/qjson.h"
> #include "qobject/qlist.h"
> +#include "qobject/qnum.h"
> #include "qobject/qstring.h"
> #include "qapi/string-input-visitor.h"
> #include "qapi/string-output-visitor.h"
> @@ -318,171 +322,91 @@ void hmp_info_migrate_capabilities(MonitorHMP *hmp, const QDict *qdict)
> qapi_free_MigrationCapabilityStatusList(caps);
> }
>
> -static void monitor_print_cpr_exec_command(MonitorHMP *hmp, strList *args)
> +static void hmp_migrate_print_qobject(MonitorHMP *hmp, const char *label,
> + QObject *obj)
> {
> - monitor_hmp_printf(hmp, "%s:",
> - MigrationParameter_str(MIGRATION_PARAMETER_CPR_EXEC_COMMAND));
> + const char *sep;
>
> - while (args) {
> - monitor_hmp_printf(hmp, " %s", args->value);
> - args = args->next;
> + if (!obj) {
> + return;
> + }
> +
> + /*
> + * Put a space after labels
> + * foo: bar
> + * ^
> + */
> + if (label && label[0] && label[strlen(label) - 1] == ':') {
> + sep = " ";
> + } else {
> + sep = "";
> + }
Awkward.
Could simply we pass "foo: " instead of "foo"? Hmm, see case
QTYPE_QLIST below.
> +
> + switch (qobject_type(obj)) {
> + case QTYPE_QNUM: {
> + g_autofree char *str = qnum_to_string(qobject_to(QNum, obj));
> +
> + monitor_hmp_printf(hmp, "%s%s%s", label, sep, str);
> + break;
> + }
> + case QTYPE_QSTRING:
> + monitor_hmp_printf(hmp, "%s%s%s", label, sep,
> + qstring_get_str(qobject_to(QString, obj)));
> + break;
> + case QTYPE_QDICT: {
> + QDict *d = qobject_to(QDict, obj);
> + const QDictEntry *e;
> + int i = 0;
> +
> + monitor_hmp_printf(hmp, "%s%s", label, sep);
> +
> + for (e = qdict_first(d); e; e = qdict_next(d, e), i++) {
Printing members in alphabetic order would be much better.
> + g_autofree char *l = g_strdup_printf("%s:", qdict_entry_key(e));
> + if (i) {
> + monitor_hmp_printf(hmp, " ");
> + }
> + hmp_migrate_print_qobject(hmp, l, qdict_entry_value(e));
> + }
> + break;
> + }
dump_qobject() prints nested stuff indented. This function doesn't. I
fear the output will be confusing. Can you show us some example output
with a non-empty QDict?
> + case QTYPE_QLIST: {
> + const QListEntry *e;
> +
> + monitor_hmp_printf(hmp, "%s\n", label);
Here we print @label and not @sep. Why? Can you show us some example
output with a non-empty QList?
> + for (e = qlist_first(qobject_to(QList, obj)); e; e = qlist_next(e)) {
> + hmp_migrate_print_qobject(hmp, " ", e->value);
> + monitor_hmp_printf(hmp, "\n");
> + }
> +
Recommend to scratch the blank line.
> + break;
This case's output ends with a newline, unlike the other cases.
hmp_info_migrate_parameters() adds another newline. That's where the
unwanted blank line mentioned above comes from.
> + }
> + case QTYPE_QBOOL:
> + monitor_hmp_printf(hmp, "%s%s%s", label, sep,
> + qbool_get_bool(qobject_to(QBool, obj)) ?
> + "on" : "off");
> + break;
> + case QTYPE_NONE:
> + case QTYPE_QNULL:
I'd scratch these two lines. Matter of taste.
> + default:
> + g_assert_not_reached();
> + break;
> }
> - monitor_hmp_printf(hmp, "\n");
> }
>
> void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
> {
> - MigrationParameters *params;
> - MigrationState *s = migrate_get_current();
> + MigrationParameters *params = qmp_query_migrate_parameters(NULL);
> + g_autoptr(QDict) d;
> + const QDictEntry *e;
>
> - params = qmp_query_migrate_parameters(NULL);
> + assert(params);
>
> - if (params) {
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_INITIAL),
> - params->announce_initial);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_MAX),
> - params->announce_max);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_ROUNDS),
> - params->announce_rounds);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_STEP),
> - params->announce_step);
> - assert(params->has_throttle_trigger_threshold);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD),
> - params->throttle_trigger_threshold);
> - assert(params->has_cpu_throttle_initial);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL),
> - params->cpu_throttle_initial);
> - assert(params->has_cpu_throttle_increment);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
> - params->cpu_throttle_increment);
> - assert(params->has_cpu_throttle_tailslow);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
> - params->cpu_throttle_tailslow ? "on" : "off");
> - assert(params->has_max_cpu_throttle);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
> - params->max_cpu_throttle);
> - assert(params->tls_creds);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
> - params->tls_creds->u.s);
> - assert(params->tls_hostname);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_HOSTNAME),
> - params->tls_hostname->u.s);
> - assert(params->tls_authz);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
> - params->tls_authz->u.s);
> - assert(params->has_max_bandwidth);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH),
> - params->max_bandwidth);
> - assert(params->has_avail_switchover_bandwidth);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH),
> - params->avail_switchover_bandwidth);
> - assert(params->has_max_postcopy_bandwidth);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
> - params->max_postcopy_bandwidth);
> - assert(params->has_downtime_limit);
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT),
> - params->downtime_limit);
> - assert(params->has_x_checkpoint_delay);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
> - params->x_checkpoint_delay);
> - monitor_hmp_printf(hmp, "%s: %u\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
> - params->multifd_channels);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
> - MultiFDCompression_str(params->multifd_compression));
> - assert(params->has_zero_page_detection);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
> - qapi_enum_lookup(&ZeroPageDetection_lookup,
> - params->zero_page_detection));
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
> - params->xbzrle_cache_size);
> + d = migrate_params_to_dict(params, NULL);
> + for (e = qdict_first(d); e; e = qdict_next(d, e)) {
> + g_autofree char *label = g_strdup_printf("%s:", qdict_entry_key(e));
>
> - if (s->has_block_bitmap_mapping) {
> - BitmapMigrationNodeAliasList *nal;
> - BitmapMigrationNodeAlias *na;
> - BitmapMigrationBitmapAliasList *bal;
> - BitmapMigrationBitmapAlias *ba;
> - BitmapMigrationBitmapAliasTransform *bat;
> -
> - monitor_hmp_printf(hmp, "%s:\n",
> - MigrationParameter_str(
> - MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING));
> -
> - for (nal = params->block_bitmap_mapping; nal; nal = nal->next)
> - {
> - na = nal->value;
> - monitor_hmp_printf(hmp, " bitmaps:\n");
> - for (bal = na->bitmaps; bal; bal = bal->next) {
> - ba = bal->value;
> - bat = ba->transform;
> -
> - monitor_hmp_printf(hmp, " name: %s", ba->name);
> - if (bat && bat->has_persistent) {
> - monitor_hmp_printf(hmp, " transform:");
> - if (bat->persistent) {
> - monitor_hmp_printf(hmp, " persistent: on");
> - } else {
> - monitor_hmp_printf(hmp, " persistent: off");
> - }
> - }
> - monitor_hmp_printf(hmp, " alias: %s\n", ba->alias);
> - }
> - monitor_hmp_printf(hmp, " node-name: %s alias: %s\n",
> - na->node_name, na->alias);
> - }
> -
> - monitor_hmp_printf(hmp, "\n");
> - }
> -
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
> - params->x_vcpu_dirty_limit_period);
> -
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT),
> - params->vcpu_dirty_limit);
> -
> - assert(params->has_mode);
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(MIGRATION_PARAMETER_MODE),
> - qapi_enum_lookup(&MigMode_lookup, params->mode));
> -
> - if (params->has_direct_io) {
> - monitor_hmp_printf(hmp, "%s: %s\n",
> - MigrationParameter_str(
> - MIGRATION_PARAMETER_DIRECT_IO),
> - params->direct_io ? "on" : "off");
> - }
> -
> - if (params->has_x_rdma_chunk_size) {
> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
> - MigrationParameter_str(
> - MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE),
> - params->x_rdma_chunk_size);
> - }
> -
> - assert(params->has_cpr_exec_command);
> - monitor_print_cpr_exec_command(hmp, params->cpr_exec_command);
> + hmp_migrate_print_qobject(hmp, label, qdict_entry_value(e));
> + monitor_hmp_printf(hmp, "\n");
> }
Why not something like
hmp_migrate_print_qobject(hmp, NULL, d)
?
>
> qapi_free_MigrationParameters(params);
> diff --git a/migration/options.c b/migration/options.c
> index 46d040fc3f..29b96f48c1 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1097,7 +1097,7 @@ static void tls_opt_to_str(StrOrNull *opt)
> opt->u.s = g_strdup("");
> }
>
> -static QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
> +QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
> {
> QObject *obj = NULL;
> Visitor *v = qobject_output_visitor_new(&obj);
> diff --git a/migration/options.h b/migration/options.h
> index c7da2d0b5b..0d9f88c11e 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -94,4 +94,5 @@ uint64_t migrate_rdma_chunk_size(void);
> bool migrate_params_check(MigrationParameters *params, Error **errp);
> void migrate_params_init(MigrationParameters *params);
> bool migrate_params_free(MigrationParameters *params, Error **errp);
> +QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
> #endif
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-17 11:53 ` Markus Armbruster
@ 2026-09-17 11:58 ` Markus Armbruster
2026-09-17 18:41 ` Peter Xu
2026-09-18 22:41 ` Fabiano Rosas
2 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 11:58 UTC (permalink / raw)
To: Markus Armbruster
Cc: Fabiano Rosas, qemu-devel, Peter Xu, Dr . David Alan Gilbert
Markus Armbruster <armbru@redhat.com> writes:
> Fabiano Rosas <farosas@suse.de> writes:
>
>> The hmp_info_migrate_parameters function currently open-codes the
>> mon_printf calls for each migration parameter. As with the set command
>> in the last patch, this should not be necessary as the QAPI
>> infrastructure already has generated code that takes type and struct
>> member names into account, including converting _ from C into the '-'
>> character as part of parameter names strings.
>>
>> The current code is also quite painful to rebase if a series has been
>> carried for a long time while parameters have been added in master.
>>
>> Replace all of this with a conversion from MigrationParameters to
>> QDict using an output visitor and a loop over the QDict that prints
>> per-QAPI-type formatted strings.
>>
>> Modelled after block/qapi.c:dump_qobject, but with some changes to
>> keep the migration command output formatting.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Old output of info migrate_parameters for me:
>
> announce-initial: 50
> announce-max: 550
> announce-rounds: 5
> announce-step: 100
> throttle-trigger-threshold: 50
> cpu-throttle-initial: 20
> cpu-throttle-increment: 10
> cpu-throttle-tailslow: off
> max-cpu-throttle: 99
> tls-creds:
> tls-hostname:
> tls-authz:
> max-bandwidth: 134217728
> avail-switchover-bandwidth: 0
> max-postcopy-bandwidth: 0
> downtime-limit: 300
> x-checkpoint-delay: 20000
> multifd-channels: 2
> multifd-compression: none
> zero-page-detection: multifd
> xbzrle-cache-size: 67108864
> x-vcpu-dirty-limit-period: 1000
> vcpu-dirty-limit: 1
> mode: normal
> direct-io: off
> x-rdma-chunk-size: 1048576
> cpr-exec-command:
>
> New output:
>
> cpu-throttle-tailslow: off
> xbzrle-cache-size: 67108864
> cpu-throttle-initial: 20
> announce-max: 550
> direct-io: off
> avail-switchover-bandwidth: 0
> local: off
> zero-page-detection: multifd
> multifd-qatzip-level: 1
> x-rdma-chunk-size: 1048576
> multifd-channels: 2
> mode: normal
> multifd-zstd-level: 1
> announce-initial: 50
> downtime-limit: 300
> tls-authz:
> cpr-exec-command:
>
> vcpu-dirty-limit: 1
> multifd-compression: none
> announce-rounds: 5
> announce-step: 100
> tls-creds:
> x-vcpu-dirty-limit-period: 1000
> multifd-zlib-level: 1
> max-cpu-throttle: 99
> max-postcopy-bandwidth: 0
> tls-hostname:
> throttle-trigger-threshold: 50
> max-bandwidth: 134217728
> x-checkpoint-delay: 20000
> cpu-throttle-increment: 10
>
> New output has an unwanted blank line after "cpr-exec-command:".
>
> New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
> @multifd-zstd-level. Commit message should point that out.
>
> Order of parameters changes, and not for the better. Hmm.
Neither shows @block-bitmap-mapping.
[...]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-17 11:53 ` Markus Armbruster
2026-09-17 11:58 ` Markus Armbruster
@ 2026-09-17 18:41 ` Peter Xu
2026-09-17 20:57 ` Dr. David Alan Gilbert
2026-09-18 22:41 ` Fabiano Rosas
2 siblings, 1 reply; 37+ messages in thread
From: Peter Xu @ 2026-09-17 18:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Fabiano Rosas, qemu-devel, Dr . David Alan Gilbert
On Thu, Sep 17, 2026 at 01:53:22PM +0200, Markus Armbruster wrote:
> Fabiano Rosas <farosas@suse.de> writes:
>
> > The hmp_info_migrate_parameters function currently open-codes the
> > mon_printf calls for each migration parameter. As with the set command
> > in the last patch, this should not be necessary as the QAPI
> > infrastructure already has generated code that takes type and struct
> > member names into account, including converting _ from C into the '-'
> > character as part of parameter names strings.
> >
> > The current code is also quite painful to rebase if a series has been
> > carried for a long time while parameters have been added in master.
> >
> > Replace all of this with a conversion from MigrationParameters to
> > QDict using an output visitor and a loop over the QDict that prints
> > per-QAPI-type formatted strings.
> >
> > Modelled after block/qapi.c:dump_qobject, but with some changes to
> > keep the migration command output formatting.
> >
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Old output of info migrate_parameters for me:
>
> announce-initial: 50
> announce-max: 550
> announce-rounds: 5
> announce-step: 100
> throttle-trigger-threshold: 50
> cpu-throttle-initial: 20
> cpu-throttle-increment: 10
> cpu-throttle-tailslow: off
> max-cpu-throttle: 99
> tls-creds:
> tls-hostname:
> tls-authz:
> max-bandwidth: 134217728
> avail-switchover-bandwidth: 0
> max-postcopy-bandwidth: 0
> downtime-limit: 300
> x-checkpoint-delay: 20000
> multifd-channels: 2
> multifd-compression: none
> zero-page-detection: multifd
> xbzrle-cache-size: 67108864
> x-vcpu-dirty-limit-period: 1000
> vcpu-dirty-limit: 1
> mode: normal
> direct-io: off
> x-rdma-chunk-size: 1048576
> cpr-exec-command:
>
> New output:
>
> cpu-throttle-tailslow: off
> xbzrle-cache-size: 67108864
> cpu-throttle-initial: 20
> announce-max: 550
> direct-io: off
> avail-switchover-bandwidth: 0
> local: off
> zero-page-detection: multifd
> multifd-qatzip-level: 1
> x-rdma-chunk-size: 1048576
> multifd-channels: 2
> mode: normal
> multifd-zstd-level: 1
> announce-initial: 50
> downtime-limit: 300
> tls-authz:
> cpr-exec-command:
>
> vcpu-dirty-limit: 1
> multifd-compression: none
> announce-rounds: 5
> announce-step: 100
> tls-creds:
> x-vcpu-dirty-limit-period: 1000
> multifd-zlib-level: 1
> max-cpu-throttle: 99
> max-postcopy-bandwidth: 0
> tls-hostname:
> throttle-trigger-threshold: 50
> max-bandwidth: 134217728
> x-checkpoint-delay: 20000
> cpu-throttle-increment: 10
>
> New output has an unwanted blank line after "cpr-exec-command:".
>
> New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
> @multifd-zstd-level. Commit message should point that out.
>
> Order of parameters changes, and not for the better. Hmm.
Good point.. it might start to be awkward to users looking for tls-*,
multifd-*, announce-* or cpu-throttle*, etc. after this change, expecting
them grouped.
Maybe we can sort them by name with inserting top level to a QList first
from the QDict.
We won't be able to keep all relevant fields together, for example, we used
to have max-cpu-throttle together with cpu-throttle-*, now also separate
even with alphabetical order, but it's a special case of bad name since the
start.. where it could have been "cpu-throttle-max". Meanwhile, as another
good side effect, we can finally have max-* fields together too.
--
Peter Xu
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-17 18:41 ` Peter Xu
@ 2026-09-17 20:57 ` Dr. David Alan Gilbert
2026-09-18 8:20 ` Markus Armbruster
0 siblings, 1 reply; 37+ messages in thread
From: Dr. David Alan Gilbert @ 2026-09-17 20:57 UTC (permalink / raw)
To: Peter Xu; +Cc: Markus Armbruster, Fabiano Rosas, qemu-devel
* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Sep 17, 2026 at 01:53:22PM +0200, Markus Armbruster wrote:
> > Fabiano Rosas <farosas@suse.de> writes:
> >
> > > The hmp_info_migrate_parameters function currently open-codes the
> > > mon_printf calls for each migration parameter. As with the set command
> > > in the last patch, this should not be necessary as the QAPI
> > > infrastructure already has generated code that takes type and struct
> > > member names into account, including converting _ from C into the '-'
> > > character as part of parameter names strings.
> > >
> > > The current code is also quite painful to rebase if a series has been
> > > carried for a long time while parameters have been added in master.
> > >
> > > Replace all of this with a conversion from MigrationParameters to
> > > QDict using an output visitor and a loop over the QDict that prints
> > > per-QAPI-type formatted strings.
> > >
> > > Modelled after block/qapi.c:dump_qobject, but with some changes to
> > > keep the migration command output formatting.
> > >
> > > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >
> > Old output of info migrate_parameters for me:
> >
> > announce-initial: 50
> > announce-max: 550
> > announce-rounds: 5
> > announce-step: 100
> > throttle-trigger-threshold: 50
> > cpu-throttle-initial: 20
> > cpu-throttle-increment: 10
> > cpu-throttle-tailslow: off
> > max-cpu-throttle: 99
> > tls-creds:
> > tls-hostname:
> > tls-authz:
> > max-bandwidth: 134217728
> > avail-switchover-bandwidth: 0
> > max-postcopy-bandwidth: 0
> > downtime-limit: 300
> > x-checkpoint-delay: 20000
> > multifd-channels: 2
> > multifd-compression: none
> > zero-page-detection: multifd
> > xbzrle-cache-size: 67108864
> > x-vcpu-dirty-limit-period: 1000
> > vcpu-dirty-limit: 1
> > mode: normal
> > direct-io: off
> > x-rdma-chunk-size: 1048576
> > cpr-exec-command:
> >
> > New output:
> >
> > cpu-throttle-tailslow: off
> > xbzrle-cache-size: 67108864
> > cpu-throttle-initial: 20
> > announce-max: 550
> > direct-io: off
> > avail-switchover-bandwidth: 0
> > local: off
> > zero-page-detection: multifd
> > multifd-qatzip-level: 1
> > x-rdma-chunk-size: 1048576
> > multifd-channels: 2
> > mode: normal
> > multifd-zstd-level: 1
> > announce-initial: 50
> > downtime-limit: 300
> > tls-authz:
> > cpr-exec-command:
> >
> > vcpu-dirty-limit: 1
> > multifd-compression: none
> > announce-rounds: 5
> > announce-step: 100
> > tls-creds:
> > x-vcpu-dirty-limit-period: 1000
> > multifd-zlib-level: 1
> > max-cpu-throttle: 99
> > max-postcopy-bandwidth: 0
> > tls-hostname:
> > throttle-trigger-threshold: 50
> > max-bandwidth: 134217728
> > x-checkpoint-delay: 20000
> > cpu-throttle-increment: 10
> >
> > New output has an unwanted blank line after "cpr-exec-command:".
> >
> > New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
> > @multifd-zstd-level. Commit message should point that out.
> >
> > Order of parameters changes, and not for the better. Hmm.
>
> Good point.. it might start to be awkward to users looking for tls-*,
> multifd-*, announce-* or cpu-throttle*, etc. after this change, expecting
> them grouped.
>
> Maybe we can sort them by name with inserting top level to a QList first
> from the QDict.
Are there any filter visitors in qemu's visitor system - e.g. something that
sorts?
Dave
> We won't be able to keep all relevant fields together, for example, we used
> to have max-cpu-throttle together with cpu-throttle-*, now also separate
> even with alphabetical order, but it's a special case of bad name since the
> start.. where it could have been "cpu-throttle-max". Meanwhile, as another
> good side effect, we can finally have max-* fields together too.
>
> --
> Peter Xu
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-17 20:57 ` Dr. David Alan Gilbert
@ 2026-09-18 8:20 ` Markus Armbruster
2026-09-18 22:32 ` Fabiano Rosas
0 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2026-09-18 8:20 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Peter Xu, Fabiano Rosas, qemu-devel
"Dr. David Alan Gilbert" <dave@treblig.org> writes:
> * Peter Xu (peterx@redhat.com) wrote:
>> On Thu, Sep 17, 2026 at 01:53:22PM +0200, Markus Armbruster wrote:
>> > Fabiano Rosas <farosas@suse.de> writes:
>> >
>> > > The hmp_info_migrate_parameters function currently open-codes the
>> > > mon_printf calls for each migration parameter. As with the set command
>> > > in the last patch, this should not be necessary as the QAPI
>> > > infrastructure already has generated code that takes type and struct
>> > > member names into account, including converting _ from C into the '-'
>> > > character as part of parameter names strings.
>> > >
>> > > The current code is also quite painful to rebase if a series has been
>> > > carried for a long time while parameters have been added in master.
>> > >
>> > > Replace all of this with a conversion from MigrationParameters to
>> > > QDict using an output visitor and a loop over the QDict that prints
>> > > per-QAPI-type formatted strings.
>> > >
>> > > Modelled after block/qapi.c:dump_qobject, but with some changes to
>> > > keep the migration command output formatting.
>> > >
>> > > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> >
>> > Old output of info migrate_parameters for me:
>> >
>> > announce-initial: 50
>> > announce-max: 550
>> > announce-rounds: 5
>> > announce-step: 100
>> > throttle-trigger-threshold: 50
>> > cpu-throttle-initial: 20
>> > cpu-throttle-increment: 10
>> > cpu-throttle-tailslow: off
>> > max-cpu-throttle: 99
>> > tls-creds:
>> > tls-hostname:
>> > tls-authz:
>> > max-bandwidth: 134217728
>> > avail-switchover-bandwidth: 0
>> > max-postcopy-bandwidth: 0
>> > downtime-limit: 300
>> > x-checkpoint-delay: 20000
>> > multifd-channels: 2
>> > multifd-compression: none
>> > zero-page-detection: multifd
>> > xbzrle-cache-size: 67108864
>> > x-vcpu-dirty-limit-period: 1000
>> > vcpu-dirty-limit: 1
>> > mode: normal
>> > direct-io: off
>> > x-rdma-chunk-size: 1048576
>> > cpr-exec-command:
>> >
>> > New output:
>> >
>> > cpu-throttle-tailslow: off
>> > xbzrle-cache-size: 67108864
>> > cpu-throttle-initial: 20
>> > announce-max: 550
>> > direct-io: off
>> > avail-switchover-bandwidth: 0
>> > local: off
>> > zero-page-detection: multifd
>> > multifd-qatzip-level: 1
>> > x-rdma-chunk-size: 1048576
>> > multifd-channels: 2
>> > mode: normal
>> > multifd-zstd-level: 1
>> > announce-initial: 50
>> > downtime-limit: 300
>> > tls-authz:
>> > cpr-exec-command:
>> >
>> > vcpu-dirty-limit: 1
>> > multifd-compression: none
>> > announce-rounds: 5
>> > announce-step: 100
>> > tls-creds:
>> > x-vcpu-dirty-limit-period: 1000
>> > multifd-zlib-level: 1
>> > max-cpu-throttle: 99
>> > max-postcopy-bandwidth: 0
>> > tls-hostname:
>> > throttle-trigger-threshold: 50
>> > max-bandwidth: 134217728
>> > x-checkpoint-delay: 20000
>> > cpu-throttle-increment: 10
>> >
>> > New output has an unwanted blank line after "cpr-exec-command:".
>> >
>> > New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
>> > @multifd-zstd-level. Commit message should point that out.
>> >
>> > Order of parameters changes, and not for the better. Hmm.
>>
>> Good point.. it might start to be awkward to users looking for tls-*,
>> multifd-*, announce-* or cpu-throttle*, etc. after this change, expecting
>> them grouped.
>>
>> Maybe we can sort them by name with inserting top level to a QList first
>> from the QDict.
>
> Are there any filter visitors in qemu's visitor system - e.g. something that
> sorts?
No. Visitors walk members in source order. Source order is good as
long as we care to make it good. But it doesn't help us here, because
we use the visitor only to convert to QObject, then walk that. This
loses order.
If QDict iterated in insertion order, we'd be fine. Python dict do.
Could be a fun little project for someone with time to spare.
The other solution is to iterate over sorted QDict keys, as Peter says.
> Dave
>
>> We won't be able to keep all relevant fields together, for example, we used
>> to have max-cpu-throttle together with cpu-throttle-*, now also separate
>> even with alphabetical order, but it's a special case of bad name since the
>> start.. where it could have been "cpu-throttle-max". Meanwhile, as another
>> good side effect, we can finally have max-* fields together too.
>>
>> --
>> Peter Xu
>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-18 8:20 ` Markus Armbruster
@ 2026-09-18 22:32 ` Fabiano Rosas
0 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:32 UTC (permalink / raw)
To: Markus Armbruster, Dr. David Alan Gilbert; +Cc: Peter Xu, qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> "Dr. David Alan Gilbert" <dave@treblig.org> writes:
>
>> * Peter Xu (peterx@redhat.com) wrote:
>>> On Thu, Sep 17, 2026 at 01:53:22PM +0200, Markus Armbruster wrote:
>>> > Fabiano Rosas <farosas@suse.de> writes:
>>> >
>>> > > The hmp_info_migrate_parameters function currently open-codes the
>>> > > mon_printf calls for each migration parameter. As with the set command
>>> > > in the last patch, this should not be necessary as the QAPI
>>> > > infrastructure already has generated code that takes type and struct
>>> > > member names into account, including converting _ from C into the '-'
>>> > > character as part of parameter names strings.
>>> > >
>>> > > The current code is also quite painful to rebase if a series has been
>>> > > carried for a long time while parameters have been added in master.
>>> > >
>>> > > Replace all of this with a conversion from MigrationParameters to
>>> > > QDict using an output visitor and a loop over the QDict that prints
>>> > > per-QAPI-type formatted strings.
>>> > >
>>> > > Modelled after block/qapi.c:dump_qobject, but with some changes to
>>> > > keep the migration command output formatting.
>>> > >
>>> > > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> >
>>> > Old output of info migrate_parameters for me:
>>> >
>>> > announce-initial: 50
>>> > announce-max: 550
>>> > announce-rounds: 5
>>> > announce-step: 100
>>> > throttle-trigger-threshold: 50
>>> > cpu-throttle-initial: 20
>>> > cpu-throttle-increment: 10
>>> > cpu-throttle-tailslow: off
>>> > max-cpu-throttle: 99
>>> > tls-creds:
>>> > tls-hostname:
>>> > tls-authz:
>>> > max-bandwidth: 134217728
>>> > avail-switchover-bandwidth: 0
>>> > max-postcopy-bandwidth: 0
>>> > downtime-limit: 300
>>> > x-checkpoint-delay: 20000
>>> > multifd-channels: 2
>>> > multifd-compression: none
>>> > zero-page-detection: multifd
>>> > xbzrle-cache-size: 67108864
>>> > x-vcpu-dirty-limit-period: 1000
>>> > vcpu-dirty-limit: 1
>>> > mode: normal
>>> > direct-io: off
>>> > x-rdma-chunk-size: 1048576
>>> > cpr-exec-command:
>>> >
>>> > New output:
>>> >
>>> > cpu-throttle-tailslow: off
>>> > xbzrle-cache-size: 67108864
>>> > cpu-throttle-initial: 20
>>> > announce-max: 550
>>> > direct-io: off
>>> > avail-switchover-bandwidth: 0
>>> > local: off
>>> > zero-page-detection: multifd
>>> > multifd-qatzip-level: 1
>>> > x-rdma-chunk-size: 1048576
>>> > multifd-channels: 2
>>> > mode: normal
>>> > multifd-zstd-level: 1
>>> > announce-initial: 50
>>> > downtime-limit: 300
>>> > tls-authz:
>>> > cpr-exec-command:
>>> >
>>> > vcpu-dirty-limit: 1
>>> > multifd-compression: none
>>> > announce-rounds: 5
>>> > announce-step: 100
>>> > tls-creds:
>>> > x-vcpu-dirty-limit-period: 1000
>>> > multifd-zlib-level: 1
>>> > max-cpu-throttle: 99
>>> > max-postcopy-bandwidth: 0
>>> > tls-hostname:
>>> > throttle-trigger-threshold: 50
>>> > max-bandwidth: 134217728
>>> > x-checkpoint-delay: 20000
>>> > cpu-throttle-increment: 10
>>> >
>>> > New output has an unwanted blank line after "cpr-exec-command:".
>>> >
>>> > New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
>>> > @multifd-zstd-level. Commit message should point that out.
>>> >
>>> > Order of parameters changes, and not for the better. Hmm.
>>>
>>> Good point.. it might start to be awkward to users looking for tls-*,
>>> multifd-*, announce-* or cpu-throttle*, etc. after this change, expecting
>>> them grouped.
>>>
>>> Maybe we can sort them by name with inserting top level to a QList first
>>> from the QDict.
>>
>> Are there any filter visitors in qemu's visitor system - e.g. something that
>> sorts?
>
> No. Visitors walk members in source order. Source order is good as
> long as we care to make it good. But it doesn't help us here, because
> we use the visitor only to convert to QObject, then walk that. This
> loses order.
>
> If QDict iterated in insertion order, we'd be fine. Python dict do.
> Could be a fun little project for someone with time to spare.
>
> The other solution is to iterate over sorted QDict keys, as Peter says.
>
Thanks, I didn't notice the output changed so drastically. The readline
completion suggestions are sorted, so I just assumed the parameters were
sorted too. I'll fix it up.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3 14/20] migration: Use output visitor in info command
2026-09-17 11:53 ` Markus Armbruster
2026-09-17 11:58 ` Markus Armbruster
2026-09-17 18:41 ` Peter Xu
@ 2026-09-18 22:41 ` Fabiano Rosas
2 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-18 22:41 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Markus Armbruster <armbru@redhat.com> writes:
> Fabiano Rosas <farosas@suse.de> writes:
>
>> The hmp_info_migrate_parameters function currently open-codes the
>> mon_printf calls for each migration parameter. As with the set command
>> in the last patch, this should not be necessary as the QAPI
>> infrastructure already has generated code that takes type and struct
>> member names into account, including converting _ from C into the '-'
>> character as part of parameter names strings.
>>
>> The current code is also quite painful to rebase if a series has been
>> carried for a long time while parameters have been added in master.
>>
>> Replace all of this with a conversion from MigrationParameters to
>> QDict using an output visitor and a loop over the QDict that prints
>> per-QAPI-type formatted strings.
>>
>> Modelled after block/qapi.c:dump_qobject, but with some changes to
>> keep the migration command output formatting.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Old output of info migrate_parameters for me:
>
> announce-initial: 50
> announce-max: 550
> announce-rounds: 5
> announce-step: 100
> throttle-trigger-threshold: 50
> cpu-throttle-initial: 20
> cpu-throttle-increment: 10
> cpu-throttle-tailslow: off
> max-cpu-throttle: 99
> tls-creds:
> tls-hostname:
> tls-authz:
> max-bandwidth: 134217728
> avail-switchover-bandwidth: 0
> max-postcopy-bandwidth: 0
> downtime-limit: 300
> x-checkpoint-delay: 20000
> multifd-channels: 2
> multifd-compression: none
> zero-page-detection: multifd
> xbzrle-cache-size: 67108864
> x-vcpu-dirty-limit-period: 1000
> vcpu-dirty-limit: 1
> mode: normal
> direct-io: off
> x-rdma-chunk-size: 1048576
> cpr-exec-command:
>
> New output:
>
> cpu-throttle-tailslow: off
> xbzrle-cache-size: 67108864
> cpu-throttle-initial: 20
> announce-max: 550
> direct-io: off
> avail-switchover-bandwidth: 0
> local: off
> zero-page-detection: multifd
> multifd-qatzip-level: 1
> x-rdma-chunk-size: 1048576
> multifd-channels: 2
> mode: normal
> multifd-zstd-level: 1
> announce-initial: 50
> downtime-limit: 300
> tls-authz:
> cpr-exec-command:
>
> vcpu-dirty-limit: 1
> multifd-compression: none
> announce-rounds: 5
> announce-step: 100
> tls-creds:
> x-vcpu-dirty-limit-period: 1000
> multifd-zlib-level: 1
> max-cpu-throttle: 99
> max-postcopy-bandwidth: 0
> tls-hostname:
> throttle-trigger-threshold: 50
> max-bandwidth: 134217728
> x-checkpoint-delay: 20000
> cpu-throttle-increment: 10
>
> New output has an unwanted blank line after "cpr-exec-command:".
>
> New output adds @local, @multifd-qatzip-level, @multifd-zlib-level,
> @multifd-zstd-level. Commit message should point that out.
>
> Order of parameters changes, and not for the better. Hmm.
>
>> ---
>> migration/migration-hmp-cmds.c | 234 +++++++++++----------------------
>> migration/options.c | 2 +-
>> migration/options.h | 1 +
>> 3 files changed, 81 insertions(+), 156 deletions(-)
>>
>> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
>> index 7491c79a2e..00d2d310af 100644
>> --- a/migration/migration-hmp-cmds.c
>> +++ b/migration/migration-hmp-cmds.c
>> @@ -26,8 +26,12 @@
>> #include "qapi/qapi-commands-migration.h"
>> #include "qapi/qapi-visit-migration.h"
>> #include "qapi/qobject-input-visitor.h"
>> +#include "qapi/qobject-output-visitor.h"
>> +#include "qobject/qbool.h"
>> #include "qobject/qdict.h"
>> +#include "qobject/qjson.h"
>> #include "qobject/qlist.h"
>> +#include "qobject/qnum.h"
>> #include "qobject/qstring.h"
>> #include "qapi/string-input-visitor.h"
>> #include "qapi/string-output-visitor.h"
>> @@ -318,171 +322,91 @@ void hmp_info_migrate_capabilities(MonitorHMP *hmp, const QDict *qdict)
>> qapi_free_MigrationCapabilityStatusList(caps);
>> }
>>
>> -static void monitor_print_cpr_exec_command(MonitorHMP *hmp, strList *args)
>> +static void hmp_migrate_print_qobject(MonitorHMP *hmp, const char *label,
>> + QObject *obj)
>> {
>> - monitor_hmp_printf(hmp, "%s:",
>> - MigrationParameter_str(MIGRATION_PARAMETER_CPR_EXEC_COMMAND));
>> + const char *sep;
>>
>> - while (args) {
>> - monitor_hmp_printf(hmp, " %s", args->value);
>> - args = args->next;
>> + if (!obj) {
>> + return;
>> + }
>> +
>> + /*
>> + * Put a space after labels
>> + * foo: bar
>> + * ^
>> + */
>> + if (label && label[0] && label[strlen(label) - 1] == ':') {
>> + sep = " ";
>> + } else {
>> + sep = "";
>> + }
>
> Awkward.
>
> Could simply we pass "foo: " instead of "foo"? Hmm, see case
> QTYPE_QLIST below.
>
This is to avoid a solitary "foo: " when the object turns out to be
empty. Maybe it can be done differently, I'll try it.
>> +
>> + switch (qobject_type(obj)) {
>> + case QTYPE_QNUM: {
>> + g_autofree char *str = qnum_to_string(qobject_to(QNum, obj));
>> +
>> + monitor_hmp_printf(hmp, "%s%s%s", label, sep, str);
>> + break;
>> + }
>> + case QTYPE_QSTRING:
>> + monitor_hmp_printf(hmp, "%s%s%s", label, sep,
>> + qstring_get_str(qobject_to(QString, obj)));
>> + break;
>> + case QTYPE_QDICT: {
>> + QDict *d = qobject_to(QDict, obj);
>> + const QDictEntry *e;
>> + int i = 0;
>> +
>> + monitor_hmp_printf(hmp, "%s%s", label, sep);
>> +
>> + for (e = qdict_first(d); e; e = qdict_next(d, e), i++) {
>
> Printing members in alphabetic order would be much better.
>
Yep, I'll improve this.
>> + g_autofree char *l = g_strdup_printf("%s:", qdict_entry_key(e));
>> + if (i) {
>> + monitor_hmp_printf(hmp, " ");
>> + }
>> + hmp_migrate_print_qobject(hmp, l, qdict_entry_value(e));
>> + }
>> + break;
>> + }
>
> dump_qobject() prints nested stuff indented. This function doesn't. I
> fear the output will be confusing. Can you show us some example output
> with a non-empty QDict?
>
>> + case QTYPE_QLIST: {
>> + const QListEntry *e;
>> +
>> + monitor_hmp_printf(hmp, "%s\n", label);
>
> Here we print @label and not @sep. Why? Can you show us some example
> output with a non-empty QList?
>
Both QList and QDict are represented on the output of
block-bitmap-mapping. All the seemingly extra spacing is to make the
list and dict look ok.
I'll construct an artificial object that is more complex and see how it
looks.
>> + for (e = qlist_first(qobject_to(QList, obj)); e; e = qlist_next(e)) {
>> + hmp_migrate_print_qobject(hmp, " ", e->value);
>> + monitor_hmp_printf(hmp, "\n");
>> + }
>> +
>
> Recommend to scratch the blank line.
>
>> + break;
>
> This case's output ends with a newline, unlike the other cases.
> hmp_info_migrate_parameters() adds another newline. That's where the
> unwanted blank line mentioned above comes from.
>> + }
>> + case QTYPE_QBOOL:
>> + monitor_hmp_printf(hmp, "%s%s%s", label, sep,
>> + qbool_get_bool(qobject_to(QBool, obj)) ?
>> + "on" : "off");
>> + break;
>> + case QTYPE_NONE:
>> + case QTYPE_QNULL:
>
> I'd scratch these two lines. Matter of taste.
>
>> + default:
>> + g_assert_not_reached();
>> + break;
>> }
>> - monitor_hmp_printf(hmp, "\n");
>> }
>>
>> void hmp_info_migrate_parameters(MonitorHMP *hmp, const QDict *qdict)
>> {
>> - MigrationParameters *params;
>> - MigrationState *s = migrate_get_current();
>> + MigrationParameters *params = qmp_query_migrate_parameters(NULL);
>> + g_autoptr(QDict) d;
>> + const QDictEntry *e;
>>
>> - params = qmp_query_migrate_parameters(NULL);
>> + assert(params);
>>
>> - if (params) {
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_INITIAL),
>> - params->announce_initial);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_MAX),
>> - params->announce_max);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_ROUNDS),
>> - params->announce_rounds);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_STEP),
>> - params->announce_step);
>> - assert(params->has_throttle_trigger_threshold);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD),
>> - params->throttle_trigger_threshold);
>> - assert(params->has_cpu_throttle_initial);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL),
>> - params->cpu_throttle_initial);
>> - assert(params->has_cpu_throttle_increment);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
>> - params->cpu_throttle_increment);
>> - assert(params->has_cpu_throttle_tailslow);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
>> - params->cpu_throttle_tailslow ? "on" : "off");
>> - assert(params->has_max_cpu_throttle);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
>> - params->max_cpu_throttle);
>> - assert(params->tls_creds);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
>> - params->tls_creds->u.s);
>> - assert(params->tls_hostname);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_HOSTNAME),
>> - params->tls_hostname->u.s);
>> - assert(params->tls_authz);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
>> - params->tls_authz->u.s);
>> - assert(params->has_max_bandwidth);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH),
>> - params->max_bandwidth);
>> - assert(params->has_avail_switchover_bandwidth);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH),
>> - params->avail_switchover_bandwidth);
>> - assert(params->has_max_postcopy_bandwidth);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
>> - params->max_postcopy_bandwidth);
>> - assert(params->has_downtime_limit);
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT),
>> - params->downtime_limit);
>> - assert(params->has_x_checkpoint_delay);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
>> - params->x_checkpoint_delay);
>> - monitor_hmp_printf(hmp, "%s: %u\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
>> - params->multifd_channels);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
>> - MultiFDCompression_str(params->multifd_compression));
>> - assert(params->has_zero_page_detection);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
>> - qapi_enum_lookup(&ZeroPageDetection_lookup,
>> - params->zero_page_detection));
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>> - params->xbzrle_cache_size);
>> + d = migrate_params_to_dict(params, NULL);
>> + for (e = qdict_first(d); e; e = qdict_next(d, e)) {
>> + g_autofree char *label = g_strdup_printf("%s:", qdict_entry_key(e));
>>
>> - if (s->has_block_bitmap_mapping) {
>> - BitmapMigrationNodeAliasList *nal;
>> - BitmapMigrationNodeAlias *na;
>> - BitmapMigrationBitmapAliasList *bal;
>> - BitmapMigrationBitmapAlias *ba;
>> - BitmapMigrationBitmapAliasTransform *bat;
>> -
>> - monitor_hmp_printf(hmp, "%s:\n",
>> - MigrationParameter_str(
>> - MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING));
>> -
>> - for (nal = params->block_bitmap_mapping; nal; nal = nal->next)
>> - {
>> - na = nal->value;
>> - monitor_hmp_printf(hmp, " bitmaps:\n");
>> - for (bal = na->bitmaps; bal; bal = bal->next) {
>> - ba = bal->value;
>> - bat = ba->transform;
>> -
>> - monitor_hmp_printf(hmp, " name: %s", ba->name);
>> - if (bat && bat->has_persistent) {
>> - monitor_hmp_printf(hmp, " transform:");
>> - if (bat->persistent) {
>> - monitor_hmp_printf(hmp, " persistent: on");
>> - } else {
>> - monitor_hmp_printf(hmp, " persistent: off");
>> - }
>> - }
>> - monitor_hmp_printf(hmp, " alias: %s\n", ba->alias);
>> - }
>> - monitor_hmp_printf(hmp, " node-name: %s alias: %s\n",
>> - na->node_name, na->alias);
>> - }
>> -
>> - monitor_hmp_printf(hmp, "\n");
>> - }
>> -
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
>> - params->x_vcpu_dirty_limit_period);
>> -
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT),
>> - params->vcpu_dirty_limit);
>> -
>> - assert(params->has_mode);
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(MIGRATION_PARAMETER_MODE),
>> - qapi_enum_lookup(&MigMode_lookup, params->mode));
>> -
>> - if (params->has_direct_io) {
>> - monitor_hmp_printf(hmp, "%s: %s\n",
>> - MigrationParameter_str(
>> - MIGRATION_PARAMETER_DIRECT_IO),
>> - params->direct_io ? "on" : "off");
>> - }
>> -
>> - if (params->has_x_rdma_chunk_size) {
>> - monitor_hmp_printf(hmp, "%s: %" PRIu64 "\n",
>> - MigrationParameter_str(
>> - MIGRATION_PARAMETER_X_RDMA_CHUNK_SIZE),
>> - params->x_rdma_chunk_size);
>> - }
>> -
>> - assert(params->has_cpr_exec_command);
>> - monitor_print_cpr_exec_command(hmp, params->cpr_exec_command);
>> + hmp_migrate_print_qobject(hmp, label, qdict_entry_value(e));
>> + monitor_hmp_printf(hmp, "\n");
>> }
>
> Why not something like
>
> hmp_migrate_print_qobject(hmp, NULL, d)
>
> ?
>
>>
>> qapi_free_MigrationParameters(params);
>> diff --git a/migration/options.c b/migration/options.c
>> index 46d040fc3f..29b96f48c1 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -1097,7 +1097,7 @@ static void tls_opt_to_str(StrOrNull *opt)
>> opt->u.s = g_strdup("");
>> }
>>
>> -static QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
>> +QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp)
>> {
>> QObject *obj = NULL;
>> Visitor *v = qobject_output_visitor_new(&obj);
>> diff --git a/migration/options.h b/migration/options.h
>> index c7da2d0b5b..0d9f88c11e 100644
>> --- a/migration/options.h
>> +++ b/migration/options.h
>> @@ -94,4 +94,5 @@ uint64_t migrate_rdma_chunk_size(void);
>> bool migrate_params_check(MigrationParameters *params, Error **errp);
>> void migrate_params_init(MigrationParameters *params);
>> bool migrate_params_free(MigrationParameters *params, Error **errp);
>> +QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
>> #endif
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (13 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 14/20] migration: Use output visitor in info command Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-17 12:02 ` Markus Armbruster
2026-09-15 0:02 ` [PATCH v3 16/20] migration: Add capabilities into MigrationParameters Fabiano Rosas
` (5 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
We're moving towards having no open-coded access to the
MigrationParameters members. Instances of parameters enumeration are
being removed.
The migrate_set_parameter_completion function is one of the last users
of the MigrationParameter enum. Rewrite that code using an output
visitor and QDict instead.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration-hmp-cmds.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 00d2d310af..c63d7f1ccb 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -693,14 +693,20 @@ void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
const char *str)
{
+ g_autoptr(QDict) d = NULL;
+ const QDictEntry *e;
size_t len;
+ /* Temporarily borrow the global parameters */
+ d = migrate_params_to_dict(&migrate_get_current()->parameters,
+ &error_abort);
+
len = strlen(str);
readline_set_completion_index(rs, len);
if (nb_args == 2) {
- int i;
- for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
- readline_add_completion_of(rs, str, MigrationParameter_str(i));
+ for (e = qdict_first(d); e; e = qdict_next(d, e)) {
+ const char *key = qdict_entry_key(e);
+ readline_add_completion_of(rs, str, key);
}
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict
2026-09-15 0:02 ` [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict Fabiano Rosas
@ 2026-09-17 12:02 ` Markus Armbruster
0 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 12:02 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Peter Xu, Dr . David Alan Gilbert
Fabiano Rosas <farosas@suse.de> writes:
> We're moving towards having no open-coded access to the
> MigrationParameters members. Instances of parameters enumeration are
> being removed.
>
> The migrate_set_parameter_completion function is one of the last users
> of the MigrationParameter enum. Rewrite that code using an output
> visitor and QDict instead.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/migration-hmp-cmds.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 00d2d310af..c63d7f1ccb 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -693,14 +693,20 @@ void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
> void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
> const char *str)
> {
> + g_autoptr(QDict) d = NULL;
> + const QDictEntry *e;
> size_t len;
>
> + /* Temporarily borrow the global parameters */
What do you mean by "borrow"?
> + d = migrate_params_to_dict(&migrate_get_current()->parameters,
> + &error_abort);
@d is unused unless @nb_args == 2. Move the call into the conditional?
> +
> len = strlen(str);
> readline_set_completion_index(rs, len);
> if (nb_args == 2) {
> - int i;
> - for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
> - readline_add_completion_of(rs, str, MigrationParameter_str(i));
> + for (e = qdict_first(d); e; e = qdict_next(d, e)) {
> + const char *key = qdict_entry_key(e);
> + readline_add_completion_of(rs, str, key);
> }
> }
> }
This relies on all parameters being present in the value of
migrate_get_current()->parameters. Worth a comment?
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 16/20] migration: Add capabilities into MigrationParameters
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (14 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-15 0:02 ` [PATCH v3 17/20] migration: Remove s->capabilities Fabiano Rosas
` (4 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert, Eric Blake
Add capabilities to MigrationParameters. This structure will hold all
migration options. Capabilities will go away in the next patch.
From this point on, both QMP and HMP versions of
migrate-set-parameters and query-migrate-parameters gain the ability
to work with capabilities.
With MigrationParameters now having members for each capability, the
migration capabilities commands (query-migrate-capabilities,
migrate-set-capabilities) will soon be deprecated. Add a set of
helpers to convert between the old MigrationCapability representation
and the new representation as members of MigrationParameters.
Acked-by: Markus Armbruster <armbru@redhat.com> #QAPI schema
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 8 +++
migration/options.c | 71 ++++++++++++++++++++++++
migration/options.h | 5 ++
qapi/migration.json | 123 ++++++++++++++++++++++++++++++++++++++++--
4 files changed, 203 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index fab7949fe1..36db60ad58 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4099,6 +4099,14 @@ static bool migration_object_check(MigrationState *ms, Error **errp)
return false;
}
+ /*
+ * FIXME: Temporarily while -global capabilties are still using
+ * s->capabilities. Will be gone by the end of the series.
+ */
+ for (int i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
+ migrate_capability_set_compat(&ms->parameters, i, ms->capabilities[i]);
+ }
+
return migrate_caps_check(old_caps, ms->capabilities, errp);
}
diff --git a/migration/options.c b/migration/options.c
index 29b96f48c1..432037aae0 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -783,6 +783,52 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
return true;
}
+static bool *migrate_capability_get_addr(MigrationParameters *params,
+ enum MigrationCapability i)
+{
+ bool *cap_addr[] = {
+ ¶ms->xbzrle, ¶ms->rdma_pin_all,
+ ¶ms->auto_converge, ¶ms->events, ¶ms->postcopy_ram,
+ ¶ms->x_colo, ¶ms->release_ram, ¶ms->return_path,
+ ¶ms->pause_before_switchover, ¶ms->multifd,
+ ¶ms->dirty_bitmaps, ¶ms->postcopy_blocktime,
+ ¶ms->late_block_activate, ¶ms->x_ignore_shared,
+ ¶ms->validate_uuid, ¶ms->background_snapshot,
+ ¶ms->zero_copy_send, ¶ms->postcopy_preempt,
+ ¶ms->switchover_ack, ¶ms->dirty_limit, ¶ms->mapped_ram,
+ };
+
+ assert(i >= 0 && i < MIGRATION_CAPABILITY__MAX);
+ return cap_addr[i];
+}
+
+/* Compatibility for code that reads capabilities in a loop */
+bool migrate_capability_get_compat(MigrationParameters *params, int i)
+{
+ return *(migrate_capability_get_addr(params, i));
+}
+
+/* Compatibility for code that writes capabilities in a loop */
+void migrate_capability_set_compat(MigrationParameters *params, int i, bool val)
+{
+ *(migrate_capability_get_addr(params, i)) = val;
+}
+
+/*
+ * Set capabilities for compatibility with the old
+ * migrate-set-capabilities command.
+ */
+void migrate_capabilities_set_compat(MigrationParameters *params,
+ MigrationCapabilityStatusList *caps)
+{
+ MigrationCapabilityStatusList *cap;
+
+ for (cap = caps; cap; cap = cap->next) {
+ migrate_capability_set_compat(params, cap->value->capability,
+ cap->value->state);
+ }
+}
+
MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
MigrationCapabilityStatusList *head = NULL, **tail = &head;
@@ -824,6 +870,8 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
for (cap = params; cap; cap = cap->next) {
s->capabilities[cap->value->capability] = cap->value->state;
}
+
+ migrate_capabilities_set_compat(&s->parameters, params);
}
/* parameters */
@@ -1147,6 +1195,15 @@ static void migrate_mark_all_params_present(MigrationParameters *p)
&p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
&p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
&p->has_x_rdma_chunk_size, &p->has_cpr_exec_command, &p->has_local,
+ &p->has_xbzrle, &p->has_rdma_pin_all,
+ &p->has_auto_converge, &p->has_events,
+ &p->has_postcopy_ram, &p->has_x_colo, &p->has_release_ram,
+ &p->has_return_path, &p->has_pause_before_switchover, &p->has_multifd,
+ &p->has_dirty_bitmaps, &p->has_postcopy_blocktime,
+ &p->has_late_block_activate, &p->has_x_ignore_shared,
+ &p->has_validate_uuid, &p->has_background_snapshot,
+ &p->has_zero_copy_send, &p->has_postcopy_preempt,
+ &p->has_switchover_ack, &p->has_dirty_limit, &p->has_mapped_ram,
};
len = ARRAY_SIZE(has_fields);
@@ -1422,6 +1479,20 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
tls_opt_to_str(input->tls_hostname);
tls_opt_to_str(input->tls_authz);
+ /*
+ * FIXME: Temporarily while migrate_caps_check is not
+ * converted to look at s->parameters. Will be gone the end of
+ * the series.
+ */
+ bool new_caps[MIGRATION_CAPABILITY__MAX] = { 0 };
+ for (int i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
+ new_caps[i] = migrate_capability_get_compat(cur, i);
+ }
+ if (!migrate_caps_check(migrate_get_current()->capabilities, new_caps,
+ errp)) {
+ return;
+ }
+
/* merge input on top of current */
if (!migrate_params_merge(cur, input, &new, errp)) {
return;
diff --git a/migration/options.h b/migration/options.h
index 0d9f88c11e..f7bf56e9ff 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -95,4 +95,9 @@ bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
bool migrate_params_free(MigrationParameters *params, Error **errp);
QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
+bool migrate_capability_get_compat(MigrationParameters *params, int i);
+void migrate_capability_set_compat(MigrationParameters *params, int i,
+ bool val);
+void migrate_capabilities_set_compat(MigrationParameters *params,
+ MigrationCapabilityStatusList *caps);
#endif
diff --git a/qapi/migration.json b/qapi/migration.json
index f7628b55e9..9ebd266fcf 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -831,7 +831,13 @@
'direct-io',
{ 'name': 'x-rdma-chunk-size', 'features': [ 'unstable' ] },
'cpr-exec-command',
- 'local'] }
+ 'local', 'xbzrle', 'rdma-pin-all', 'auto-converge', 'events',
+ 'postcopy-ram', 'x-colo', 'release-ram', 'return-path',
+ 'pause-before-switchover', 'multifd', 'dirty-bitmaps',
+ 'postcopy-blocktime', 'late-block-activate', 'x-ignore-shared',
+ 'validate-uuid', 'background-snapshot', 'zero-copy-send',
+ 'postcopy-preempt', 'switchover-ack', 'dirty-limit',
+ 'mapped-ram'] }
##
# @migrate-set-parameters:
@@ -1023,10 +1029,98 @@
# local migration optimizations. This will be documented against
# the device backends where it applies. (Since 11.2)
#
+# @xbzrle: Migration supports xbzrle (Xor Based Zero Run Length
+# Encoding). This feature allows us to minimize migration traffic
+# for certain work loads, by sending compressed difference of the
+# pages
+#
+# @rdma-pin-all: Controls whether or not the entire VM memory
+# footprint is mlock()'d on demand or all at once. Refer to
+# docs/rdma.txt for usage. Disabled by default. (since 2.0)
+#
+# @events: generate events for each migration state change (since 2.4)
+#
+# @auto-converge: If enabled, QEMU will automatically throttle down
+# the guest to speed up convergence of RAM migration. (since 1.6)
+#
+# @postcopy-ram: Start executing on the migration target before all of
+# RAM has been migrated, pulling the remaining pages along as
+# needed. The capacity must have the same setting on both source
+# and target or migration will not even start. **Note:** if the
+# migration fails during postcopy the VM will fail. (since 2.6)
+#
+# @x-colo: If enabled, migration will never end, and the state of the
+# VM on the primary side will be migrated continuously to the VM
+# on secondary side, this process is called COarse-Grain LOck
+# Stepping (COLO) for Non-stop Service. (since 2.8)
+#
+# @release-ram: if enabled, QEMU will free the migrated ram pages on
+# the source during postcopy-ram migration. (since 2.9)
+#
+# @return-path: If enabled, migration will use the return path even
+# for precopy. (since 2.10)
+#
+# @pause-before-switchover: Pause outgoing migration before
+# serialising device state and before disabling block IO
+# (since 2.11)
+#
+# @multifd: Use more than one fd for migration (since 4.0)
+#
+# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
+# (since 2.12)
+#
+# @postcopy-blocktime: Calculate downtime for postcopy live migration
+# (since 3.0)
+#
+# @late-block-activate: If enabled, the destination will not activate
+# block devices (and thus take locks) immediately at the end of
+# migration. (since 3.0)
+#
+# @x-ignore-shared: If enabled, QEMU will not migrate shared memory
+# that is accessible on the destination machine. (since 4.0)
+#
+# @validate-uuid: Send the UUID of the source to allow the destination
+# to ensure it is the same. (since 4.2)
+#
+# @background-snapshot: If enabled, the migration stream will be a
+# snapshot of the VM exactly at the point when the migration
+# procedure starts. The VM RAM is saved with running VM.
+# (since 6.0)
+#
+# @zero-copy-send: Controls behavior on sending memory pages on
+# migration. When true, enables a zero-copy mechanism for sending
+# memory pages, if host supports it. Requires that QEMU be
+# permitted to use locked memory for guest RAM pages. (since 7.1)
+#
+# @postcopy-preempt: If enabled, the migration process will allow
+# postcopy requests to preempt precopy stream, so postcopy
+# requests will be handled faster. This is a performance feature
+# and should not affect the correctness of postcopy migration.
+# (since 7.1)
+#
+# @switchover-ack: If enabled, migration will not stop the source VM
+# and complete the migration until the destination has
+# acknowledged that it is OK to switch over. The acknowledgement
+# may depend, for example, on some device's data being loaded in
+# the destination before doing switchover. This can reduce
+# downtime if devices that support this capability are present.
+# Capability @return-path must be enabled to use it. (since 8.1)
+#
+# @dirty-limit: If enabled, migration will throttle vCPUs as needed to
+# keep their dirty page rate within @vcpu-dirty-limit. This can
+# improve responsiveness of large guests during live migration,
+# and can result in more stable read performance. Requires KVM
+# with accelerator property "dirty-ring-size" set. (Since 8.1)
+#
+# @mapped-ram: Migrate using fixed offsets in the migration file for
+# each RAM page. Requires a migration URI that supports seeking,
+# such as a file. (since 9.0)
+#
# Features:
#
-# @unstable: Members @local, @x-checkpoint-delay, @x-rdma-chunk-size,
-# and @x-vcpu-dirty-limit-period are experimental.
+# @unstable: Members @x-checkpoint-delay, @x-rdma-chunk-size,
+# @x-vcpu-dirty-limit-period, @x-colo, @x-ignore-shared and are
+# experimental.
#
# Since: 2.4
##
@@ -1066,7 +1160,28 @@
'features': [ 'unstable' ] },
'*cpr-exec-command': [ 'str' ],
'*local': { 'type': 'bool',
- 'features': [ 'unstable' ] } } }
+ 'features': [ 'unstable' ] },
+ '*xbzrle': 'bool',
+ '*rdma-pin-all': 'bool',
+ '*auto-converge': 'bool',
+ '*events': 'bool',
+ '*postcopy-ram': 'bool',
+ '*x-colo': { 'type': 'bool', 'features': [ 'unstable' ] },
+ '*release-ram': 'bool',
+ '*return-path': 'bool',
+ '*pause-before-switchover': 'bool',
+ '*multifd': 'bool',
+ '*dirty-bitmaps': 'bool',
+ '*postcopy-blocktime': 'bool',
+ '*late-block-activate': 'bool',
+ '*x-ignore-shared': { 'type': 'bool', 'features': [ 'unstable' ] },
+ '*validate-uuid': 'bool',
+ '*background-snapshot': 'bool',
+ '*zero-copy-send': 'bool',
+ '*postcopy-preempt': 'bool',
+ '*switchover-ack': 'bool',
+ '*dirty-limit': 'bool',
+ '*mapped-ram': 'bool' } }
##
# @query-migrate-parameters:
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 17/20] migration: Remove s->capabilities
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (15 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 16/20] migration: Add capabilities into MigrationParameters Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-15 0:02 ` [PATCH v3 18/20] qapi/migration: Deprecate capabilities commands Fabiano Rosas
` (3 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Laurent Vivier, Paolo Bonzini
Last patch added capabilities to s->parameters. Now we can replace all
instances of s->capabilities with s->parameters:
- The -global properties now get set directly in s->parameters.
- Accessors from options.c now read from s->parameters.
- migrate_caps_check() now takes a MigrationParameters object. The
function is still kept around because migrate-set-capabilities will
still use it.
- The machinery for background-snapshot compatibility check goes
away. We can check each capability by name (if s->parameters.cap ...)
- savevm uses the helper functions introduced in the last patch to do
validation of capabilities found on the migration stream.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 22 +-
migration/migration.h | 2 +-
migration/options.c | 318 ++++++++++++-----------------
migration/options.h | 19 +-
migration/savevm.c | 8 +-
tests/qtest/migration/misc-tests.c | 145 +++++++++++++
6 files changed, 289 insertions(+), 225 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 36db60ad58..fcfb82ad57 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -255,9 +255,10 @@ static bool
migration_capabilities_and_transport_compatible(MigrationAddress *addr,
Error **errp)
{
+ MigrationState *s = migrate_get_current();
+
if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
- return migrate_rdma_caps_check(migrate_get_current()->capabilities,
- errp);
+ return migrate_rdma_caps_check(&s->parameters, errp);
}
if (migrate_local() && migrate_mode() != MIG_MODE_CPR_EXEC &&
@@ -4092,22 +4093,7 @@ static void migration_instance_init(Object *obj)
*/
static bool migration_object_check(MigrationState *ms, Error **errp)
{
- /* Assuming all off */
- bool old_caps[MIGRATION_CAPABILITY__MAX] = { 0 };
-
- if (!migrate_params_check(&ms->parameters, errp)) {
- return false;
- }
-
- /*
- * FIXME: Temporarily while -global capabilties are still using
- * s->capabilities. Will be gone by the end of the series.
- */
- for (int i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
- migrate_capability_set_compat(&ms->parameters, i, ms->capabilities[i]);
- }
-
- return migrate_caps_check(old_caps, ms->capabilities, errp);
+ return migrate_params_check(&ms->parameters, errp);
}
static const TypeInfo migration_type = {
diff --git a/migration/migration.h b/migration/migration.h
index 683bc7bdd5..7f005bd12c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -357,7 +357,7 @@ struct MigrationState {
/* Timestamp when VM is down (ms) to migrate the last stuff */
int64_t downtime_start;
int64_t downtime;
- bool capabilities[MIGRATION_CAPABILITY__MAX];
+ int64_t expected_downtime;
int64_t setup_time;
/*
diff --git a/migration/options.c b/migration/options.c
index 432037aae0..02b9c64ae6 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -87,9 +87,6 @@
#define DEFAULT_MIGRATE_ANNOUNCE_ROUNDS 5
#define DEFAULT_MIGRATE_ANNOUNCE_STEP 100
-#define DEFINE_PROP_MIG_CAP(name, x) \
- DEFINE_PROP_BOOL(name, MigrationState, capabilities[x], false)
-
const PropertyInfo qdev_prop_StrOrNull;
#define DEFINE_PROP_STR_OR_NULL(_name, _state, _field) \
DEFINE_PROP(_name, _state, _field, qdev_prop_StrOrNull, StrOrNull *, \
@@ -197,32 +194,42 @@ const Property migration_properties[] = {
parameters.x_rdma_chunk_size,
DEFAULT_MIGRATE_X_RDMA_CHUNK_SIZE),
- /* Migration capabilities */
- DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
- DEFINE_PROP_MIG_CAP("x-rdma-pin-all", MIGRATION_CAPABILITY_RDMA_PIN_ALL),
- DEFINE_PROP_MIG_CAP("x-auto-converge", MIGRATION_CAPABILITY_AUTO_CONVERGE),
- DEFINE_PROP_MIG_CAP("x-events", MIGRATION_CAPABILITY_EVENTS),
- DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
- DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
- MIGRATION_CAPABILITY_POSTCOPY_PREEMPT),
- DEFINE_PROP_MIG_CAP("postcopy-blocktime",
- MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME),
- DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
- DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
- DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
- DEFINE_PROP_MIG_CAP("x-multifd", MIGRATION_CAPABILITY_MULTIFD),
- DEFINE_PROP_MIG_CAP("x-background-snapshot",
- MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT),
+ DEFINE_PROP_BOOL("x-xbzrle",
+ MigrationState, parameters.xbzrle, false),
+ DEFINE_PROP_BOOL("x-rdma-pin-all",
+ MigrationState, parameters.rdma_pin_all, false),
+ DEFINE_PROP_BOOL("x-auto-converge",
+ MigrationState, parameters.auto_converge, false),
+ DEFINE_PROP_BOOL("x-events",
+ MigrationState, parameters.events, false),
+ DEFINE_PROP_BOOL("x-postcopy-ram",
+ MigrationState, parameters.postcopy_ram, false),
+ DEFINE_PROP_BOOL("x-postcopy-preempt",
+ MigrationState, parameters.postcopy_preempt, false),
+ DEFINE_PROP_BOOL("postcopy-blocktime",
+ MigrationState, parameters.postcopy_blocktime, false),
+ DEFINE_PROP_BOOL("x-colo",
+ MigrationState, parameters.x_colo, false),
+ DEFINE_PROP_BOOL("x-release-ram",
+ MigrationState, parameters.release_ram, false),
+ DEFINE_PROP_BOOL("x-return-path",
+ MigrationState, parameters.return_path, false),
+ DEFINE_PROP_BOOL("x-multifd",
+ MigrationState, parameters.multifd, false),
+ DEFINE_PROP_BOOL("x-background-snapshot",
+ MigrationState, parameters.background_snapshot, false),
#ifdef CONFIG_LINUX
- DEFINE_PROP_MIG_CAP("x-zero-copy-send",
- MIGRATION_CAPABILITY_ZERO_COPY_SEND),
+ DEFINE_PROP_BOOL("x-zero-copy-send",
+ MigrationState, parameters.zero_copy_send, false),
#endif
- DEFINE_PROP_MIG_CAP("x-switchover-ack",
- MIGRATION_CAPABILITY_SWITCHOVER_ACK),
- DEFINE_PROP_MIG_CAP("x-dirty-limit", MIGRATION_CAPABILITY_DIRTY_LIMIT),
- DEFINE_PROP_MIG_CAP("mapped-ram", MIGRATION_CAPABILITY_MAPPED_RAM),
- DEFINE_PROP_MIG_CAP("x-ignore-shared",
- MIGRATION_CAPABILITY_X_IGNORE_SHARED),
+ DEFINE_PROP_BOOL("x-switchover-ack",
+ MigrationState, parameters.switchover_ack, false),
+ DEFINE_PROP_BOOL("x-dirty-limit",
+ MigrationState, parameters.dirty_limit, false),
+ DEFINE_PROP_BOOL("mapped-ram",
+ MigrationState, parameters.mapped_ram, false),
+ DEFINE_PROP_BOOL("x-ignore-shared",
+ MigrationState, parameters.x_ignore_shared, false),
};
const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
@@ -305,7 +312,7 @@ bool migrate_auto_converge(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_AUTO_CONVERGE];
+ return s->parameters.auto_converge;
}
bool migrate_send_switchover_start(void)
@@ -319,42 +326,42 @@ bool migrate_background_snapshot(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT];
+ return s->parameters.background_snapshot;
}
bool migrate_colo(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
+ return s->parameters.x_colo;
}
bool migrate_dirty_bitmaps(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_DIRTY_BITMAPS];
+ return s->parameters.dirty_bitmaps;
}
bool migrate_dirty_limit(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_DIRTY_LIMIT];
+ return s->parameters.dirty_limit;
}
bool migrate_events(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_EVENTS];
+ return s->parameters.events;
}
bool migrate_mapped_ram(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
+ return s->parameters.mapped_ram;
}
bool migrate_local(void)
@@ -367,102 +374,100 @@ bool migrate_ignore_shared(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_X_IGNORE_SHARED];
+ return s->parameters.x_ignore_shared;
}
bool migrate_late_block_activate(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_LATE_BLOCK_ACTIVATE];
+ return s->parameters.late_block_activate;
}
bool migrate_multifd(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
+ return s->parameters.multifd;
}
bool migrate_pause_before_switchover(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_PAUSE_BEFORE_SWITCHOVER];
+ return s->parameters.pause_before_switchover;
}
bool migrate_postcopy_blocktime(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME];
+ return s->parameters.postcopy_blocktime;
}
bool migrate_postcopy_preempt(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT];
+ return s->parameters.postcopy_preempt;
}
bool migrate_postcopy_ram(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_POSTCOPY_RAM];
+ return s->parameters.postcopy_ram;
}
bool migrate_rdma_pin_all(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
+ return s->parameters.rdma_pin_all;
}
bool migrate_release_ram(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_RELEASE_RAM];
+ return s->parameters.release_ram;
}
bool migrate_return_path(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_RETURN_PATH];
+ return s->parameters.return_path;
}
bool migrate_switchover_ack(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_SWITCHOVER_ACK];
+ return s->parameters.switchover_ack;
}
bool migrate_validate_uuid(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_VALIDATE_UUID];
+ return s->parameters.validate_uuid;
}
bool migrate_xbzrle(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_XBZRLE];
+ return s->parameters.xbzrle;
}
bool migrate_zero_copy_send(void)
{
MigrationState *s = migrate_get_current();
- return s->capabilities[MIGRATION_CAPABILITY_ZERO_COPY_SEND];
+ return s->parameters.zero_copy_send;
}
-/* pseudo capabilities */
-
bool migrate_multifd_flush_after_each_section(void)
{
MigrationState *s = migrate_get_current();
@@ -514,44 +519,6 @@ WriteTrackingSupport migrate_query_write_tracking(void)
return WT_SUPPORT_COMPATIBLE;
}
-/* Migration capabilities set */
-struct MigrateCapsSet {
- int size; /* Capability set size */
- MigrationCapability caps[]; /* Variadic array of capabilities */
-};
-typedef struct MigrateCapsSet MigrateCapsSet;
-
-/* Define and initialize MigrateCapsSet */
-#define INITIALIZE_MIGRATE_CAPS_SET(_name, ...) \
- MigrateCapsSet _name = { \
- .size = sizeof((int []) { __VA_ARGS__ }) / sizeof(int), \
- .caps = { __VA_ARGS__ } \
- }
-
-/* Background-snapshot compatibility check list */
-static const
-INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
- MIGRATION_CAPABILITY_POSTCOPY_RAM,
- MIGRATION_CAPABILITY_DIRTY_BITMAPS,
- MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME,
- MIGRATION_CAPABILITY_LATE_BLOCK_ACTIVATE,
- MIGRATION_CAPABILITY_RETURN_PATH,
- MIGRATION_CAPABILITY_MULTIFD,
- MIGRATION_CAPABILITY_PAUSE_BEFORE_SWITCHOVER,
- MIGRATION_CAPABILITY_AUTO_CONVERGE,
- MIGRATION_CAPABILITY_RELEASE_RAM,
- MIGRATION_CAPABILITY_RDMA_PIN_ALL,
- MIGRATION_CAPABILITY_XBZRLE,
- MIGRATION_CAPABILITY_X_COLO,
- MIGRATION_CAPABILITY_VALIDATE_UUID,
- MIGRATION_CAPABILITY_ZERO_COPY_SEND);
-
-/* Snapshot compatibility check list */
-static const
-INITIALIZE_MIGRATE_CAPS_SET(check_caps_savevm,
- MIGRATION_CAPABILITY_MULTIFD,
-);
-
static bool migrate_incoming_started(void)
{
return !!migration_incoming_get_current()->transport_data;
@@ -560,34 +527,28 @@ static bool migrate_incoming_started(void)
bool migrate_can_snapshot(Error **errp)
{
MigrationState *s = migrate_get_current();
- int i;
- for (i = 0; i < check_caps_savevm.size; i++) {
- int incomp_cap = check_caps_savevm.caps[i];
-
- if (s->capabilities[incomp_cap]) {
- error_setg(errp,
- "Snapshots are not compatible with %s",
- MigrationCapability_str(incomp_cap));
- return false;
- }
+ if (migrate_capability_get_compat(
+ &s->parameters, MIGRATION_CAPABILITY_MULTIFD)) {
+ error_setg(errp,
+ "Snapshots are not compatible with multifd");
+ return false;
}
return true;
}
-
-bool migrate_rdma_caps_check(bool *caps, Error **errp)
+bool migrate_rdma_caps_check(MigrationParameters *params, Error **errp)
{
- if (caps[MIGRATION_CAPABILITY_XBZRLE]) {
+ if (params->xbzrle) {
error_setg(errp, "RDMA and XBZRLE can't be used together");
return false;
}
- if (caps[MIGRATION_CAPABILITY_MULTIFD]) {
+ if (params->multifd) {
error_setg(errp, "RDMA and multifd can't be used together");
return false;
}
- if (caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+ if (params->postcopy_ram) {
error_setg(errp, "RDMA and postcopy-ram can't be used together");
return false;
}
@@ -595,30 +556,23 @@ bool migrate_rdma_caps_check(bool *caps, Error **errp)
return true;
}
-/**
- * @migration_caps_check - check capability compatibility
- *
- * @old_caps: old capability list
- * @new_caps: new capability list
- * @errp: set *errp if the check failed, with reason
- *
- * Returns true if check passed, otherwise false.
- */
-bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
+bool migrate_caps_check(MigrationParameters *new, Error **errp)
{
- ERRP_GUARD();
+ MigrationState *s = migrate_get_current();
MigrationIncomingState *mis = migration_incoming_get_current();
+ bool postcopy_already_on = s->parameters.postcopy_ram;
+ ERRP_GUARD();
#ifdef CONFIG_REPLICATION
- if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
- if (!new_caps[MIGRATION_CAPABILITY_RETURN_PATH]) {
+ if (new->x_colo) {
+ if (!new->return_path) {
error_setg(errp, "Capability 'x-colo' requires capability "
"'return-path'");
return false;
}
}
#else
- if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
+ if (new->x_colo) {
error_setg(errp, "QEMU compiled without replication module"
" can't enable COLO");
error_append_hint(errp, "Please enable replication before COLO.\n");
@@ -626,27 +580,27 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
}
#endif
- if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+ if (new->postcopy_ram) {
/* This check is reasonably expensive, so only when it's being
* set the first time, also it's only the destination that needs
* special support.
*/
- if (!old_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] &&
+ if (!postcopy_already_on &&
runstate_check(RUN_STATE_INMIGRATE) &&
!postcopy_ram_supported_by_host(mis, errp)) {
error_prepend(errp, "Postcopy is not supported: ");
return false;
}
- if (new_caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED]) {
+ if (new->x_ignore_shared) {
error_setg(errp, "Postcopy is not compatible with ignore-shared");
return false;
}
}
- if (new_caps[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT]) {
+ if (new->background_snapshot) {
WriteTrackingSupport wt_support;
- int idx;
+
/*
* Check if 'background-snapshot' capability is supported by
* host kernel and compatible with guest memory configuration.
@@ -662,41 +616,45 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
return false;
}
- /*
- * Check if there are any migration capabilities
- * incompatible with 'background-snapshot'.
- */
- for (idx = 0; idx < check_caps_background_snapshot.size; idx++) {
- int incomp_cap = check_caps_background_snapshot.caps[idx];
- if (new_caps[incomp_cap]) {
- error_setg(errp,
- "Background-snapshot is not compatible with %s",
- MigrationCapability_str(incomp_cap));
- return false;
- }
+ if (new->postcopy_ram ||
+ new->dirty_bitmaps ||
+ new->postcopy_blocktime ||
+ new->late_block_activate ||
+ new->return_path ||
+ new->multifd ||
+ new->pause_before_switchover ||
+ new->auto_converge ||
+ new->release_ram ||
+ new->rdma_pin_all ||
+ new->xbzrle ||
+ new->x_colo ||
+ new->validate_uuid ||
+ new->zero_copy_send) {
+ error_setg(errp,
+ "Background-snapshot is not compatible with "
+ "currently set capabilities");
+ return false;
}
}
#ifdef CONFIG_LINUX
- if (new_caps[MIGRATION_CAPABILITY_ZERO_COPY_SEND] &&
- (!new_caps[MIGRATION_CAPABILITY_MULTIFD] ||
- new_caps[MIGRATION_CAPABILITY_XBZRLE] ||
- migrate_multifd_compression() ||
- migrate_tls())) {
+ if (new->zero_copy_send &&
+ (!new->multifd || new->xbzrle ||
+ migrate_multifd_compression() || migrate_tls())) {
error_setg(errp,
"Zero copy only available for non-compressed non-TLS multifd migration");
return false;
}
#else
- if (new_caps[MIGRATION_CAPABILITY_ZERO_COPY_SEND]) {
+ if (new->zero_copy_send) {
error_setg(errp,
"Zero copy currently only available on Linux");
return false;
}
#endif
- if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT]) {
- if (!new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+ if (new->postcopy_preempt) {
+ if (!new->postcopy_ram) {
error_setg(errp, "Postcopy preempt requires postcopy-ram");
return false;
}
@@ -708,22 +666,22 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
}
}
- if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
+ if (new->multifd) {
if (!migrate_multifd() && migrate_incoming_started()) {
error_setg(errp, "Multifd must be set before incoming starts");
return false;
}
}
- if (new_caps[MIGRATION_CAPABILITY_SWITCHOVER_ACK]) {
- if (!new_caps[MIGRATION_CAPABILITY_RETURN_PATH]) {
+ if (new->switchover_ack) {
+ if (!new->return_path) {
error_setg(errp, "Capability 'switchover-ack' requires capability "
"'return-path'");
return false;
}
}
- if (new_caps[MIGRATION_CAPABILITY_DIRTY_LIMIT]) {
- if (new_caps[MIGRATION_CAPABILITY_AUTO_CONVERGE]) {
+ if (new->dirty_limit) {
+ if (new->auto_converge) {
error_setg(errp, "dirty-limit conflicts with auto-converge"
" either of then available currently");
return false;
@@ -736,30 +694,29 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
}
}
- if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
- if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) {
+ if (new->multifd) {
+ if (new->xbzrle) {
error_setg(errp, "Multifd is not compatible with xbzrle");
return false;
}
}
- if (new_caps[MIGRATION_CAPABILITY_MAPPED_RAM]) {
- if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) {
+ if (new->mapped_ram) {
+ if (new->xbzrle) {
error_setg(errp,
"Mapped-ram migration is incompatible with xbzrle");
return false;
}
}
- if (new_caps[MIGRATION_CAPABILITY_MAPPED_RAM] &&
- new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
- if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
+ if (new->mapped_ram && new->postcopy_ram) {
+ if (new->multifd) {
error_setg(errp,
"Multifd is not supported with fast snapshot load");
return false;
}
- if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT]) {
+ if (new->postcopy_preempt) {
error_setg(
errp,
"Postcopy Preempt is incompatible with fast snapshot load");
@@ -777,7 +734,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
* On destination side, check the cases that capability is being set
* after incoming thread has started.
*/
- if (migrate_rdma() && !migrate_rdma_caps_check(new_caps, errp)) {
+ if (migrate_rdma() && !migrate_rdma_caps_check(new, errp)) {
return false;
}
return true;
@@ -839,39 +796,37 @@ MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
caps = g_malloc0(sizeof(*caps));
caps->capability = i;
- caps->state = s->capabilities[i];
+ caps->state = migrate_capability_get_compat(&s->parameters, i);
QAPI_LIST_APPEND(tail, caps);
}
return head;
}
-void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
+void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *capabilities,
Error **errp)
{
MigrationState *s = migrate_get_current();
- MigrationCapabilityStatusList *cap;
- bool new_caps[MIGRATION_CAPABILITY__MAX];
+ g_autoptr(MigrationParameters) params = NULL;
if (migration_is_running() || migration_in_colo_state()) {
error_setg(errp, "There's a migration process in progress");
return;
}
- memcpy(new_caps, s->capabilities, sizeof(new_caps));
- for (cap = params; cap; cap = cap->next) {
- new_caps[cap->value->capability] = cap->value->state;
- }
+ /*
+ * Capabilities validation needs to first copy from s->parameters
+ * in case the incoming capabilities have a capability that
+ * conflicts with another that's already set.
+ */
+ params = QAPI_CLONE(MigrationParameters, &s->parameters);
+ migrate_capabilities_set_compat(params, capabilities);
- if (!migrate_caps_check(s->capabilities, new_caps, errp)) {
+ if (!migrate_caps_check(params, errp)) {
return;
}
- for (cap = params; cap; cap = cap->next) {
- s->capabilities[cap->value->capability] = cap->value->state;
- }
-
- migrate_capabilities_set_compat(&s->parameters, params);
+ migrate_capabilities_set_compat(&s->parameters, capabilities);
}
/* parameters */
@@ -932,9 +887,8 @@ bool migrate_direct_io(void)
* isolated to the main migration thread while multifd channels
* process the aligned data with O_DIRECT enabled.
*/
- return s->parameters.direct_io &&
- s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM] &&
- s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
+ return s->parameters.direct_io && s->parameters.mapped_ram &&
+ s->parameters.multifd;
}
uint64_t migrate_downtime_limit(void)
@@ -1459,6 +1413,10 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (!migrate_caps_check(params, errp)) {
+ return false;
+ }
+
return true;
}
@@ -1479,20 +1437,6 @@ void qmp_migrate_set_parameters(MigrationParameters *input, Error **errp)
tls_opt_to_str(input->tls_hostname);
tls_opt_to_str(input->tls_authz);
- /*
- * FIXME: Temporarily while migrate_caps_check is not
- * converted to look at s->parameters. Will be gone the end of
- * the series.
- */
- bool new_caps[MIGRATION_CAPABILITY__MAX] = { 0 };
- for (int i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
- new_caps[i] = migrate_capability_get_compat(cur, i);
- }
- if (!migrate_caps_check(migrate_get_current()->capabilities, new_caps,
- errp)) {
- return;
- }
-
/* merge input on top of current */
if (!migrate_params_merge(cur, input, &new, errp)) {
return;
diff --git a/migration/options.h b/migration/options.h
index f7bf56e9ff..28cf762ace 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -1,5 +1,5 @@
/*
- * QEMU migration capabilities
+ * QEMU migration options
*
* Copyright (c) 2012-2023 Red Hat Inc
*
@@ -23,8 +23,6 @@
extern const Property migration_properties[];
extern const size_t migration_properties_count;
-/* capabilities */
-
bool migrate_auto_converge(void);
bool migrate_colo(void);
bool migrate_dirty_bitmaps(void);
@@ -43,22 +41,12 @@ bool migrate_validate_uuid(void);
bool migrate_xbzrle(void);
bool migrate_zero_copy_send(void);
-/*
- * pseudo capabilities
- *
- * These are functions that are used in a similar way to capabilities
- * check, but they are not a capability.
- */
-
bool migrate_multifd_flush_after_each_section(void);
bool migrate_postcopy(void);
bool migrate_rdma(void);
bool migrate_tls(void);
-/* capabilities helpers */
-
-bool migrate_rdma_caps_check(bool *caps, Error **errp);
-bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp);
+bool migrate_rdma_caps_check(MigrationParameters *config, Error **errp);
bool migrate_can_snapshot(Error **errp);
/* parameters */
@@ -89,8 +77,6 @@ uint64_t migrate_xbzrle_cache_size(void);
ZeroPageDetection migrate_zero_page_detection(void);
uint64_t migrate_rdma_chunk_size(void);
-/* parameters helpers */
-
bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
bool migrate_params_free(MigrationParameters *params, Error **errp);
@@ -100,4 +86,5 @@ void migrate_capability_set_compat(MigrationParameters *params, int i,
bool val);
void migrate_capabilities_set_compat(MigrationParameters *params,
MigrationCapabilityStatusList *caps);
+bool migrate_caps_check(MigrationParameters *new, Error **errp);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 4b590ea672..27e68a4c7b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -296,7 +296,8 @@ static uint32_t get_validatable_capabilities_count(void)
uint32_t result = 0;
int i;
for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
- if (should_validate_capability(i) && s->capabilities[i]) {
+ if (should_validate_capability(i) &&
+ migrate_capability_get_compat(&s->parameters, i)) {
result++;
}
}
@@ -318,7 +319,8 @@ static bool configuration_pre_save(void *opaque, Error **errp)
state->capabilities = g_renew(MigrationCapability, state->capabilities,
state->caps_count);
for (i = j = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
- if (should_validate_capability(i) && s->capabilities[i]) {
+ if (should_validate_capability(i) &&
+ migrate_capability_get_compat(&s->parameters, i)) {
state->capabilities[j++] = i;
}
}
@@ -367,7 +369,7 @@ static bool configuration_validate_capabilities(SaveState *state)
continue;
}
source_state = test_bit(i, source_caps_bm);
- target_state = s->capabilities[i];
+ target_state = migrate_capability_get_compat(&s->parameters, i);
if (source_state != target_state) {
error_report("Capability %s is %s, but received capability is %s",
MigrationCapability_str(i),
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 49280e5bc9..ef94a47b83 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -27,8 +27,11 @@ static char *tmpfs;
#ifdef CONFIG_HMP
static int test_case_line;
+#define TEST_IF(i1, i2, e, c) { i1, i2, e , .line = __LINE__, .skip = !c }
#define TEST(i1, i2, e) { i1, i2, e , .line = __LINE__, }
#define SKIP(i1, i2, e) { i1, i2, e , .skip = true, }
+#define BG_SNAP_MSG ("Error: Background-snapshot is not compatible with " \
+ "currently set capabilities")
typedef struct HMPTestData {
const char *input1;
@@ -136,6 +139,7 @@ static void assert_hmp_success(const char *str)
static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
{
+ MigrationTestEnv *env = migration_get_env();
QTestState *qts;
/*
* .input1: string to be used as parameter name
@@ -154,6 +158,147 @@ static void test_hmp_migration_parameters(char *name, MigrateCommon *args)
TEST("cpu-throttle-tailslow", "on", "on"),
TEST("direct-io", "on", "on"),
TEST("local", "on", "on"),
+ TEST("events", "on", "on"),
+
+ /* bool, with dependencies */
+
+ /*
+ * background-snapshot:
+ * rejects dirty-bitmaps
+ * rejects postcopy-blocktime
+ * rejects late-block-activate
+ * rejects multifd
+ * rejects pause-before-switchover
+ * rejects auto-converge
+ * rejects release-ram
+ * rejects rdma-pin-all
+ * rejects validate-uuid
+ * rejects zero-copy-send
+ * rejects postcopy-ram
+ */
+ TEST_IF("background-snapshot", "on", "on", env->has_uffd),
+ TEST_IF("dirty-bitmaps", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("postcopy-blocktime", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("late-block-activate", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("multifd", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("pause-before-switchover", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("auto-converge", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("release-ram", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("rdma-pin-all", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("validate-uuid", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("zero-copy-send", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST_IF("postcopy-ram", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST("background-snapshot", "off", "off"),
+
+ TEST("dirty-bitmaps", "on", "on"),
+ TEST("postcopy-blocktime", "on", "on"),
+ TEST("late-block-activate", "on", "on"),
+ TEST("pause-before-switchover", "on", "on"),
+ TEST("auto-converge", "on", "on"),
+ TEST("release-ram", "on", "on"),
+ TEST("rdma-pin-all", "on", "on"),
+ TEST("validate-uuid", "on", "on"),
+
+ /*
+ * postcopy-preempt
+ * requires postcopy-ram:
+ */
+ TEST("postcopy-preempt", "on",
+ "Error: Postcopy preempt requires postcopy-ram"),
+
+ /*
+ * postcopy-ram:
+ * required by postcopy-preempt
+ * rejected by x-ignore-shared
+ * rejected by background-snapshot
+ * rejected by mapped-ram
+ */
+ TEST("postcopy-ram", "on", "on"),
+ TEST("postcopy-preempt", "on", "on"),
+ TEST("x-ignore-shared", "on",
+ "Error: Postcopy is not compatible with ignore-shared"),
+ TEST_IF("background-snapshot", "on", BG_SNAP_MSG, env->has_uffd),
+ TEST("mapped-ram", "on",
+ "Error: Postcopy Preempt is incompatible with fast snapshot load"),
+ TEST("postcopy-ram", "off",
+ "Error: Postcopy preempt requires postcopy-ram"),
+ TEST("postcopy-preempt", "off", "off"),
+ TEST("postcopy-ram", "off", "off"),
+
+ /*
+ * x-ignore-shared:
+ * rejected by postcopy-ram
+ */
+ TEST("x-ignore-shared", "on", "on"),
+ TEST("postcopy-ram", "on",
+ "Error: Postcopy is not compatible with ignore-shared"),
+ TEST("x-ignore-shared", "off", "off"),
+
+ /*
+ * return-path:
+ * required by x-colo
+ * required by switchover-ack
+ * rejected by background-snapshot
+ */
+ TEST("return-path", "on", "on"),
+ TEST("x-colo", "on", "on"),
+ TEST("switchover-ack", "on", "on"),
+ TEST_IF("background-snapshot", "on", BG_SNAP_MSG, env->has_uffd),
+
+ TEST("return-path", "off",
+ "Error: Capability 'x-colo' requires capability 'return-path'"),
+ TEST("x-colo", "off", "off"),
+
+ TEST("return-path", "off",
+ "Error: Capability 'switchover-ack' requires capability "
+ "'return-path'"),
+ TEST("switchover-ack", "off", "off"),
+ TEST("return-path", "off", "off"),
+
+ TEST("x-colo", "on",
+ "Error: Capability 'x-colo' requires capability 'return-path'"),
+ TEST("switchover-ack", "on", "Error: Capability 'switchover-ack' "
+ "requires capability 'return-path'"),
+
+ /*
+ * xbzrle:
+ * rejected by multifd
+ */
+ TEST("xbzrle", "on", "on"),
+ TEST("multifd", "on", "Error: Multifd is not compatible with xbzrle"),
+ TEST("xbzrle", "off", "off"),
+
+ /*
+ * multifd:
+ * rejected by xbzrle
+ * required by zero-copy-send
+ */
+ TEST("multifd", "on", "on"),
+ TEST("xbzrle", "on", "Error: Multifd is not compatible with xbzrle"),
+ TEST("zero-copy-send", "on", "on"),
+ TEST("multifd", "off", "Error: Zero copy only available for "
+ "non-compressed non-TLS multifd migration"),
+ TEST("zero-copy-send", "off", "off"),
+ TEST("multifd", "off", "off"),
+
+ /*
+ * auto-converge:
+ * rejected by dirty-limit
+ */
+ TEST("auto-converge", "on", "on"),
+ TEST("dirty-limit", "on",
+ "Error: dirty-limit conflicts with auto-converge "
+ "either of then available currently"),
+ TEST("auto-converge", "off", "off"),
+
+ /*
+ * dirty-limit:
+ * rejected by auto-converge
+ * requires KVM acceleration
+ */
+ TEST("dirty-limit", "on",
+ "Error: dirty-limit requires KVM with accelerator "
+ "property 'dirty-ring-size' set"),
/* uint64_t */
TEST("announce-initial", "60", "60"),
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 18/20] qapi/migration: Deprecate capabilities commands
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (16 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 17/20] migration: Remove s->capabilities Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-15 0:02 ` [PATCH v3 19/20] migration: Set the parameters present flag with a clone visitor Fabiano Rosas
` (2 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Pierrick Bouvier, Eric Blake
The concept of capabilities is being merged into the concept of
parameters. From now on, the commands that handle capabilities are
deprecated in favor of the commands that handle parameters.
Affected commands:
- migrate-set-capabilities
- query-migrate-capabilities
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
docs/about/deprecated.rst | 13 +++++++++++++
migration/migration-hmp-cmds.c | 6 ++++++
qapi/migration.json | 16 ++++++++++++++--
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ccbc421949..910455df46 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -524,3 +524,16 @@ If the user requests a modern x86 CPU model (i.e. not one of ``486``,
``athlon``, ``kvm32``, ``pentium``, ``pentium2``, ``pentium3``or ``qemu32``)
a warning will be displayed until a future QEMU version when such CPUs will
be rejected.
+
+Migration
+---------
+
+``migrate-set-capabilities`` command (since 11.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``migrate-set-parameters`` instead.
+
+``query-migrate-capabilities`` command (since 11.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``query-migrate-parameters`` instead.
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index c63d7f1ccb..9f530e3c57 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -309,6 +309,9 @@ void hmp_info_migrate_capabilities(MonitorHMP *hmp, const QDict *qdict)
{
MigrationCapabilityStatusList *caps, *cap;
+ warn_report("info migrate_capabilities is deprecated;"
+ " use info migrate_parameters instead");
+
caps = qmp_query_migrate_capabilities(NULL);
if (caps) {
@@ -512,6 +515,9 @@ void hmp_migrate_set_capability(MonitorHMP *hmp, const QDict *qdict)
MigrationCapabilityStatus *value;
int val;
+ warn_report("migrate_set_capability is deprecated;"
+ " use migrate_set_parameter instead");
+
val = qapi_enum_parse(&MigrationCapability_lookup, cap, -1, &err);
if (val < 0) {
goto end;
diff --git a/qapi/migration.json b/qapi/migration.json
index 9ebd266fcf..b303e91456 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -564,6 +564,11 @@
#
# @capabilities: json array of capability modifications to make
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use migrate-set-parameters
+# instead.
+#
# Since: 1.2
#
# .. qmp-example::
@@ -573,13 +578,19 @@
# <- { "return": {} }
##
{ 'command': 'migrate-set-capabilities',
- 'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
+ 'data': { 'capabilities': ['MigrationCapabilityStatus'] },
+ 'features': ['deprecated'] }
##
# @query-migrate-capabilities:
#
# Return information about the current migration capabilities status
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use
+# query-migrate-parameters instead.
+#
# Since: 1.2
#
# .. qmp-example::
@@ -594,7 +605,8 @@
# {"state": false, "capability": "x-colo"}
# ]}
##
-{ 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
+{ 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus'],
+ 'features': ['deprecated'] }
##
# @MultiFDCompression:
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v3 19/20] migration: Set the parameters present flag with a clone visitor
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (17 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 18/20] qapi/migration: Deprecate capabilities commands Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-17 14:10 ` Markus Armbruster
2026-09-15 0:02 ` [PATCH v3 20/20] qapi: Remove MigrationParameter Fabiano Rosas
2026-09-22 12:44 ` [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
20 siblings, 1 reply; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert,
Michael Roth
The MigrationParameters object is defined by the QAPI. It is also used
by the migration code as its internal store of parameters.
Due to a historic coupling of the migration code with qdev properties,
when a migration parameter has its initial value set by qdev, the QAPI
present flag for that parameter is kept unset. This is fine as long as
the MigrationParameters object is not used to interact with the QAPI
(e.g. as input to query-migrate) or the visitors infrastructure
(e.g. using QAPI_CLONE to duplicate an object).
Recent changes have removed code duplication and complexity in the
aforementioned interactions by adding a helper that attempts to ensure
consistency by setting all of the present flags for a (manually kept)
list of parameters. See commit f55f4ef632 ("migration: Extract code to
mark all parameters as present").
There are still some drawbacks to this scheme such as the need to keep
the list of parameters updated.
Replace the logic with a walk from a custom visitor that force sets
each present flag and dispenses with all manual updates as it uses the
QAPI generated code and will not become out of sync with
MigrationParameters.
This also allows the removal in the next patch of the
MigrationParameter enum which is the last place where migration
parameter names need to be replicated.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/qapi/clone-visitor.h | 1 +
migration/migration.c | 2 +-
migration/options.c | 77 ++++++++++++------------------------
migration/options.h | 2 +-
qapi/qapi-clone-visitor.c | 13 ++++++
5 files changed, 41 insertions(+), 54 deletions(-)
diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
index ebc182b034..04cd1a88d1 100644
--- a/include/qapi/clone-visitor.h
+++ b/include/qapi/clone-visitor.h
@@ -22,6 +22,7 @@
typedef struct QapiCloneVisitor QapiCloneVisitor;
Visitor *qapi_clone_visitor_new(void);
+Visitor *qapi_clone_visitor_new_all(void);
Visitor *qapi_clone_members_visitor_new(void);
/*
diff --git a/migration/migration.c b/migration/migration.c
index fcfb82ad57..f92d13234f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4076,7 +4076,7 @@ static void migration_instance_init(Object *obj)
qemu_event_init(&ms->pause_event, false);
qemu_mutex_init(&ms->error_mutex);
- migrate_params_init(&ms->parameters);
+ migrate_params_init(ms);
qemu_sem_init(&ms->postcopy_pause_sem, 0);
qemu_sem_init(&ms->rp_state.rp_sem, 0);
diff --git a/migration/options.c b/migration/options.c
index 02b9c64ae6..8d869209ee 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -233,6 +233,31 @@ const Property migration_properties[] = {
};
const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
+/*
+ * After qdev sets the defaults for the migration parameters using the
+ * migration_properties above, s->parameters is left inconsistent from
+ * QAPI point of view because the parameters' present flag is not
+ * set. Use the custom-built qapi_clone_visitor_all() to copy
+ * s->parameters back to itself while setting all present flags. The
+ * resulting object is fit to be used with QAPI and the visitor
+ * infrastructure.
+ *
+ * WARNING: missing entries in the migration_properties array in
+ * respect to MigrationParameters will still have their corresponding
+ * struct member marked as present.
+ */
+void migrate_params_init(MigrationState *s)
+{
+ Visitor *v = qapi_clone_visitor_new_all();
+ g_autoptr(MigrationParameters) p = &s->parameters;
+
+ visit_type_MigrationParameters(v, NULL, &p, &error_abort);
+ visit_free(v);
+ migrate_params_free(&s->parameters, &error_abort);
+ QAPI_CLONE_MEMBERS(MigrationParameters, &s->parameters, p);
+}
+
+
static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -1121,53 +1146,6 @@ static MigrationParameters *migrate_params_from_dict(QDict *d, Error **errp)
return tmp;
}
-/*
- * query-migrate-parameters expects all members of MigrationParameters
- * to be present, but we cannot mark them non-optional in QAPI because
- * the structure is also used for migrate-set-parameters, which needs
- * the optionality. Force all parameters to be seen as present
- * now. Note that this depends on some form of default being set for
- * every member of MigrationParameters, currently done during qdev
- * init using migration_properties defined in this file. The TLS
- * options are a special case because they don't have a default and
- * need to be normalized before use.
- */
-static void migrate_mark_all_params_present(MigrationParameters *p)
-{
- int len, n_str_args = 3; /* tls-creds, tls-hostname, tls-authz */
- bool *has_fields[] = {
- &p->has_throttle_trigger_threshold, &p->has_cpu_throttle_initial,
- &p->has_cpu_throttle_increment, &p->has_cpu_throttle_tailslow,
- &p->has_max_bandwidth, &p->has_avail_switchover_bandwidth,
- &p->has_downtime_limit, &p->has_x_checkpoint_delay,
- &p->has_multifd_channels, &p->has_multifd_compression,
- &p->has_multifd_zlib_level, &p->has_multifd_qatzip_level,
- &p->has_multifd_zstd_level, &p->has_xbzrle_cache_size,
- &p->has_max_postcopy_bandwidth, &p->has_max_cpu_throttle,
- &p->has_announce_initial, &p->has_announce_max, &p->has_announce_rounds,
- &p->has_announce_step, &p->has_block_bitmap_mapping,
- &p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
- &p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
- &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command, &p->has_local,
- &p->has_xbzrle, &p->has_rdma_pin_all,
- &p->has_auto_converge, &p->has_events,
- &p->has_postcopy_ram, &p->has_x_colo, &p->has_release_ram,
- &p->has_return_path, &p->has_pause_before_switchover, &p->has_multifd,
- &p->has_dirty_bitmaps, &p->has_postcopy_blocktime,
- &p->has_late_block_activate, &p->has_x_ignore_shared,
- &p->has_validate_uuid, &p->has_background_snapshot,
- &p->has_zero_copy_send, &p->has_postcopy_preempt,
- &p->has_switchover_ack, &p->has_dirty_limit, &p->has_mapped_ram,
- };
-
- len = ARRAY_SIZE(has_fields);
- assert(len + n_str_args == MIGRATION_PARAMETER__MAX);
-
- for (int i = 0; i < len; i++) {
- *has_fields[i] = true;
- }
-}
-
MigrationParameters *qmp_query_migrate_parameters(Error **errp)
{
MigrationState *s = migrate_get_current();
@@ -1188,11 +1166,6 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
return params;
}
-void migrate_params_init(MigrationParameters *params)
-{
- migrate_mark_all_params_present(params);
-}
-
static void migrate_post_update_params(MigrationParameters *new, Error **errp)
{
MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 28cf762ace..7abd527610 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -78,7 +78,7 @@ ZeroPageDetection migrate_zero_page_detection(void);
uint64_t migrate_rdma_chunk_size(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
-void migrate_params_init(MigrationParameters *params);
+void migrate_params_init(MigrationState *s);
bool migrate_params_free(MigrationParameters *params, Error **errp);
QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
bool migrate_capability_get_compat(MigrationParameters *params, int i);
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index 30997638de..8e2135b78e 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -174,6 +174,19 @@ Visitor *qapi_clone_visitor_new(void)
return &v->visitor;
}
+static void qapi_clone_optional_all(Visitor *v, const char *name, bool *present)
+{
+ *present = true;
+}
+
+Visitor *qapi_clone_visitor_new_all(void)
+{
+ Visitor *v = qapi_clone_visitor_new();
+
+ v->optional = qapi_clone_optional_all;
+ return v;
+}
+
Visitor *qapi_clone_members_visitor_new(void)
{
Visitor *v = qapi_clone_visitor_new();
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 19/20] migration: Set the parameters present flag with a clone visitor
2026-09-15 0:02 ` [PATCH v3 19/20] migration: Set the parameters present flag with a clone visitor Fabiano Rosas
@ 2026-09-17 14:10 ` Markus Armbruster
0 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2026-09-17 14:10 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Peter Xu, Dr . David Alan Gilbert, Michael Roth
Fabiano Rosas <farosas@suse.de> writes:
> The MigrationParameters object is defined by the QAPI. It is also used
> by the migration code as its internal store of parameters.
>
> Due to a historic coupling of the migration code with qdev properties,
> when a migration parameter has its initial value set by qdev, the QAPI
> present flag for that parameter is kept unset. This is fine as long as
> the MigrationParameters object is not used to interact with the QAPI
> (e.g. as input to query-migrate) or the visitors infrastructure
> (e.g. using QAPI_CLONE to duplicate an object).
>
> Recent changes have removed code duplication and complexity in the
> aforementioned interactions by adding a helper that attempts to ensure
> consistency by setting all of the present flags for a (manually kept)
> list of parameters. See commit f55f4ef632 ("migration: Extract code to
> mark all parameters as present").
>
> There are still some drawbacks to this scheme such as the need to keep
> the list of parameters updated.
>
> Replace the logic with a walk from a custom visitor that force sets
> each present flag and dispenses with all manual updates as it uses the
> QAPI generated code and will not become out of sync with
> MigrationParameters.
>
> This also allows the removal in the next patch of the
> MigrationParameter enum which is the last place where migration
> parameter names need to be replicated.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
I think this is better than manually setting all the has_FOO. It's not
without footguns, though.
> ---
> include/qapi/clone-visitor.h | 1 +
> migration/migration.c | 2 +-
> migration/options.c | 77 ++++++++++++------------------------
> migration/options.h | 2 +-
> qapi/qapi-clone-visitor.c | 13 ++++++
> 5 files changed, 41 insertions(+), 54 deletions(-)
>
> diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
> index ebc182b034..04cd1a88d1 100644
> --- a/include/qapi/clone-visitor.h
> +++ b/include/qapi/clone-visitor.h
> @@ -22,6 +22,7 @@
> typedef struct QapiCloneVisitor QapiCloneVisitor;
>
> Visitor *qapi_clone_visitor_new(void);
> +Visitor *qapi_clone_visitor_new_all(void);
> Visitor *qapi_clone_members_visitor_new(void);
>
> /*
> diff --git a/migration/migration.c b/migration/migration.c
> index fcfb82ad57..f92d13234f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -4076,7 +4076,7 @@ static void migration_instance_init(Object *obj)
> qemu_event_init(&ms->pause_event, false);
> qemu_mutex_init(&ms->error_mutex);
>
> - migrate_params_init(&ms->parameters);
> + migrate_params_init(ms);
>
> qemu_sem_init(&ms->postcopy_pause_sem, 0);
> qemu_sem_init(&ms->rp_state.rp_sem, 0);
> diff --git a/migration/options.c b/migration/options.c
> index 02b9c64ae6..8d869209ee 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -233,6 +233,31 @@ const Property migration_properties[] = {
> };
> const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
>
> +/*
> + * After qdev sets the defaults for the migration parameters using the
> + * migration_properties above, s->parameters is left inconsistent from
> + * QAPI point of view because the parameters' present flag is not
> + * set. Use the custom-built qapi_clone_visitor_all() to copy
> + * s->parameters back to itself while setting all present flags. The
> + * resulting object is fit to be used with QAPI and the visitor
> + * infrastructure.
> + *
> + * WARNING: missing entries in the migration_properties array in
> + * respect to MigrationParameters will still have their corresponding
> + * struct member marked as present.
> + */
> +void migrate_params_init(MigrationState *s)
> +{
> + Visitor *v = qapi_clone_visitor_new_all();
This is a variation of the clone visitor.
The clone visitor replaces all allocated member values by deep copies of
themselves. The variation additionally makes all optional members
present. I'll discuss this in more detail at the end. You might want
to jump there now, and then back here.
The comment in clone-visitor advises "The clone visitor is for direct
use only by the QAPI_CLONE() macro". We're ignoring that here. Hmm.
> + g_autoptr(MigrationParameters) p = &s->parameters;
Arrange to call qapi_free_MigrationParameters(&s->parameters) on return
from this function. But &s->parameters does not point to a
heap-allocated object, it points *into* a struct MigrationState! You'll
change it shortly to point to a heap-allocated object, but this is
super-confusing.
Anyway, @p points to s->parameters now. Returning from the function
would be catastrophic.
> +
> + visit_type_MigrationParameters(v, NULL, &p, &error_abort);
Replace @p by a deep clone. Returning from the function is safe again.
> + visit_free(v);
> + migrate_params_free(&s->parameters, &error_abort);
Free all the members of s->parameters.
> + QAPI_CLONE_MEMBERS(MigrationParameters, &s->parameters, p);
Copy deep clones of the members of @p to s->parameters.
Free @p on return.
> +}
Please avoid the confusing g_autoptr() in this function.
All the cloning is useless. Could we reduce that? Idea at the end.
> +
> +
> static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -1121,53 +1146,6 @@ static MigrationParameters *migrate_params_from_dict(QDict *d, Error **errp)
> return tmp;
> }
>
> -/*
> - * query-migrate-parameters expects all members of MigrationParameters
> - * to be present, but we cannot mark them non-optional in QAPI because
> - * the structure is also used for migrate-set-parameters, which needs
> - * the optionality. Force all parameters to be seen as present
> - * now. Note that this depends on some form of default being set for
> - * every member of MigrationParameters, currently done during qdev
> - * init using migration_properties defined in this file. The TLS
> - * options are a special case because they don't have a default and
> - * need to be normalized before use.
> - */
> -static void migrate_mark_all_params_present(MigrationParameters *p)
> -{
> - int len, n_str_args = 3; /* tls-creds, tls-hostname, tls-authz */
> - bool *has_fields[] = {
> - &p->has_throttle_trigger_threshold, &p->has_cpu_throttle_initial,
> - &p->has_cpu_throttle_increment, &p->has_cpu_throttle_tailslow,
> - &p->has_max_bandwidth, &p->has_avail_switchover_bandwidth,
> - &p->has_downtime_limit, &p->has_x_checkpoint_delay,
> - &p->has_multifd_channels, &p->has_multifd_compression,
> - &p->has_multifd_zlib_level, &p->has_multifd_qatzip_level,
> - &p->has_multifd_zstd_level, &p->has_xbzrle_cache_size,
> - &p->has_max_postcopy_bandwidth, &p->has_max_cpu_throttle,
> - &p->has_announce_initial, &p->has_announce_max, &p->has_announce_rounds,
> - &p->has_announce_step, &p->has_block_bitmap_mapping,
> - &p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
> - &p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
> - &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command, &p->has_local,
> - &p->has_xbzrle, &p->has_rdma_pin_all,
> - &p->has_auto_converge, &p->has_events,
> - &p->has_postcopy_ram, &p->has_x_colo, &p->has_release_ram,
> - &p->has_return_path, &p->has_pause_before_switchover, &p->has_multifd,
> - &p->has_dirty_bitmaps, &p->has_postcopy_blocktime,
> - &p->has_late_block_activate, &p->has_x_ignore_shared,
> - &p->has_validate_uuid, &p->has_background_snapshot,
> - &p->has_zero_copy_send, &p->has_postcopy_preempt,
> - &p->has_switchover_ack, &p->has_dirty_limit, &p->has_mapped_ram,
> - };
> -
> - len = ARRAY_SIZE(has_fields);
> - assert(len + n_str_args == MIGRATION_PARAMETER__MAX);
> -
> - for (int i = 0; i < len; i++) {
> - *has_fields[i] = true;
> - }
> -}
> -
> MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> {
> MigrationState *s = migrate_get_current();
> @@ -1188,11 +1166,6 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> return params;
> }
>
> -void migrate_params_init(MigrationParameters *params)
> -{
> - migrate_mark_all_params_present(params);
> -}
> -
> static void migrate_post_update_params(MigrationParameters *new, Error **errp)
> {
> MigrationState *s = migrate_get_current();
> diff --git a/migration/options.h b/migration/options.h
> index 28cf762ace..7abd527610 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -78,7 +78,7 @@ ZeroPageDetection migrate_zero_page_detection(void);
> uint64_t migrate_rdma_chunk_size(void);
>
> bool migrate_params_check(MigrationParameters *params, Error **errp);
> -void migrate_params_init(MigrationParameters *params);
> +void migrate_params_init(MigrationState *s);
> bool migrate_params_free(MigrationParameters *params, Error **errp);
> QDict *migrate_params_to_dict(MigrationParameters *p, Error **errp);
> bool migrate_capability_get_compat(MigrationParameters *params, int i);
> diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
> index 30997638de..8e2135b78e 100644
> --- a/qapi/qapi-clone-visitor.c
> +++ b/qapi/qapi-clone-visitor.c
> @@ -174,6 +174,19 @@ Visitor *qapi_clone_visitor_new(void)
> return &v->visitor;
> }
>
> +static void qapi_clone_optional_all(Visitor *v, const char *name, bool *present)
> +{
> + *present = true;
> +}
From visit_optional()'s contract:
* Input visitors set *@present according to input; other visitors
* leave it unchanged. In either case, return *@present for
* convenience.
This is a clone visitor, so it's supposed to leave *present unchanged.
It doesn't.
> +
> +Visitor *qapi_clone_visitor_new_all(void)
> +{
> + Visitor *v = qapi_clone_visitor_new();
> +
> + v->optional = qapi_clone_optional_all;
> + return v;
> +}
> +
> Visitor *qapi_clone_members_visitor_new(void)
> {
> Visitor *v = qapi_clone_visitor_new();
Let's have closer look at how this contraption works.
The ordinary clone visitor only visits members that are present. This
visitor visits all members, thanks to its peculiar optional() method.
What could go wrong? Let's see:
* A member M of numeric of bool type T is encoded as
bool has_M;
T M;
We set has_M to true and visit M. The visit does nothing (see
qapi_clone_type_int64() & friends). Okay.
* A member M of type array of T is encoded as
bool has_M;
MList *M;
We set has_M to true and visit M. The visit replaces the value of M
by a deep copy, which does nothing when the array is empty. Okay.
* A member M of type str is encoded as
char *M;
We visit M. The visit replaces the value of M by a copy, except it
replaces null by "" (see qapi_clone_type_str()). Okay, sort of.
* A member M of object or alternate type T is encoded as
T *M;
We visit M. The visit replaces a non-null value by a deep copy. What
about null value?
static bool qapi_clone_start_struct(Visitor *v, const char *name, void **obj,
size_t size, Error **errp)
{
QapiCloneVisitor *qcv = to_qcv(v);
if (!obj) {
---> assert(qcv->depth);
/* Only possible when visiting an alternate's object
* branch. Nothing further to do here, since the earlier
* visit_start_alternate() already copied memory. */
return true;
}
*obj = g_memdup(*obj, size);
qcv->depth++;
return true;
}
We're messing with design assumptions here.
I guess shit can't happen the way you use this thing:
MigrationParameters has sensible values before you pass it to
visit_type_MigrationParameters().
Hmm, does it? Old migrate_mark_all_params_present() sets the has_M that
are directly in MigrationParameters. It doesn't recurse into M.. This
clone visitor does. What if it runs into nested absent values? Will
they have sensible values?
Should we make qapi_clone_optional_all() check v->depth?
I think this beast needs scary comments. Hiding it in migration/ seems
advisable.
Is it wise to shanghai the clone visitor for this purpose? What about a
new visitor that does pretty much nothing, just sets *present in
optional() when v->depth is 1?
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3 20/20] qapi: Remove MigrationParameter
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (18 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 19/20] migration: Set the parameters present flag with a clone visitor Fabiano Rosas
@ 2026-09-15 0:02 ` Fabiano Rosas
2026-09-17 12:06 ` Markus Armbruster
2026-09-22 12:44 ` [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
20 siblings, 1 reply; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-15 0:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert, Eric Blake
The MigrationParameter enum is now unused, remove it.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
qapi/migration.json | 43 -------------------------------------------
1 file changed, 43 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index b303e91456..9f3b9b718d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -808,49 +808,6 @@
'bitmaps': [ 'BitmapMigrationBitmapAlias' ]
} }
-##
-# @MigrationParameter:
-#
-# Migration parameters enumeration. The enumeration values mirror the
-# members of @MigrationParameters.
-#
-# Features:
-#
-# @unstable: Members @x-checkpoint-delay, @x-rdma-chunk-size, and
-# @x-vcpu-dirty-limit-period are experimental.
-#
-# Since: 2.4
-##
-{ 'enum': 'MigrationParameter',
- 'data': ['announce-initial', 'announce-max',
- 'announce-rounds', 'announce-step',
- 'throttle-trigger-threshold',
- 'cpu-throttle-initial', 'cpu-throttle-increment',
- 'cpu-throttle-tailslow',
- 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
- 'avail-switchover-bandwidth', 'downtime-limit',
- { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
- 'multifd-channels',
- 'xbzrle-cache-size', 'max-postcopy-bandwidth',
- 'max-cpu-throttle', 'multifd-compression',
- 'multifd-zlib-level', 'multifd-zstd-level',
- 'multifd-qatzip-level',
- 'block-bitmap-mapping',
- { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
- 'vcpu-dirty-limit',
- 'mode',
- 'zero-page-detection',
- 'direct-io',
- { 'name': 'x-rdma-chunk-size', 'features': [ 'unstable' ] },
- 'cpr-exec-command',
- 'local', 'xbzrle', 'rdma-pin-all', 'auto-converge', 'events',
- 'postcopy-ram', 'x-colo', 'release-ram', 'return-path',
- 'pause-before-switchover', 'multifd', 'dirty-bitmaps',
- 'postcopy-blocktime', 'late-block-activate', 'x-ignore-shared',
- 'validate-uuid', 'background-snapshot', 'zero-copy-send',
- 'postcopy-preempt', 'switchover-ack', 'dirty-limit',
- 'mapped-ram'] }
-
##
# @migrate-set-parameters:
#
--
2.53.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v3 00/20] migration: MigrationParameters changes
2026-09-15 0:01 [PATCH v3 00/20] migration: MigrationParameters changes Fabiano Rosas
` (19 preceding siblings ...)
2026-09-15 0:02 ` [PATCH v3 20/20] qapi: Remove MigrationParameter Fabiano Rosas
@ 2026-09-22 12:44 ` Fabiano Rosas
20 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2026-09-22 12:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr . David Alan Gilbert
Fabiano Rosas <farosas@suse.de> writes:
> since v2:
>
> - Better handling the cpr-exec-command special case by converting the
> string input to a QList.
>
> - New clone visitor to replace the manual setting of has_* fields. It
> now happens once at instance_init before s->parameters is even
> available to the rest of the code.
>
> - Removed MigrationParameter.
>
> - Added some line breaks and spacing to the block-bitmap-mapping
> output (and lists in general).
>
> - Simplified the completion test by using g_ptr_array.
>
> - Updated the parameters test to call query-migrate since there's no
> MigrateParameter_lookup to list the parameters names anymore.
>
> - Rebased on master and picked-up the new 'local' parameter.
>
> (apologies if I might have missed some r-bs, my 'b4 trailers' is
> broken at the moment)
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2848741815
>
> v2:
> https://lore.kernel.org/r/20260909214509.237309-1-farosas@suse.de
> v1:
> https://lore.kernel.org/r/20260902221547.1812481-1-farosas@suse.de
>
> Fabiano Rosas (20):
> checkpatch: Fix checking of newlines in error messages
> migration/options.c: Don't export migrate_tls_opts_free
> migration: Rename variables in qmp_migrate_set_parameters
> migration: Use QAPI_CLONE_MEMBERS in migrate_params_apply
> migration: Merge parameter structs instead of assigning one by one
> migration: Open code migrate_params_apply
> migration: Stop freeing s->parameters members individually
> migration: Use migrate_params_free during finalize
> tests/qtest/migration: Add a test for HMP
> tests/qtest/migration: Add a test for HMP completion
> migration: HMP: Change bandwidth parameters expected input unit
> migration: Change HMP 'info migrate_parameters' output
> migration: Use keyval input visitor in HMP set command
> migration: Use output visitor in info command
> migration: Rewrite migrate_set_parameter_completion using QDict
> migration: Add capabilities into MigrationParameters
> migration: Remove s->capabilities
> qapi/migration: Deprecate capabilities commands
> migration: Set the parameters present flag with a clone visitor
> qapi: Remove MigrationParameter
>
> docs/about/deprecated.rst | 13 +
> include/qapi/clone-visitor.h | 1 +
> migration/migration-hmp-cmds.c | 445 +++++-----------
> migration/migration.c | 19 +-
> migration/migration.h | 2 +-
> migration/options.c | 789 +++++++++++------------------
> migration/options.h | 29 +-
> migration/savevm.c | 8 +-
> qapi/migration.json | 168 ++++--
> qapi/qapi-clone-visitor.c | 13 +
> scripts/checkpatch.pl | 11 +-
> tests/qemu-iotests/300 | 20 +-
> tests/qtest/migration/misc-tests.c | 560 ++++++++++++++++++++
> 13 files changed, 1173 insertions(+), 905 deletions(-)
I queued patches 2-8 for the next pull request. I'll leave
migration-next to marinate for a day or two. If anyone needs more time
with those patches, let me know and I can drop them no problem.
Thanks
^ permalink raw reply [flat|nested] 37+ messages in thread