Convert the code in migrate_params_test_apply() from an open-coded
copy of every migration parameter to a copy using visitors. The
current code has conditionals for each parameter's has_* field, which
is exactly what the visitors do.
This hides the details of QAPI from the migration code and avoids the
need to update migrate_params_test_apply() every time a new migration
parameter is added. Both were very confusing and while the visitor
code can become a bit involved, there is no need for new contributors
to ever touch it.
Change the name of the function to a more direct reference of what it
does: merging the user params with the temporary copy.
Move the QAPI_CLONE_MEMBERS into the caller, so QAPI_CLONE can be used
and there's no need to allocate memory in the migration
code. Similarly, turn 'tmp' into a pointer so the proper qapi_free_
routine can be used.
An extra call to migrate_mark_all_params_present() is now needed
because the visitors update the has_ field for non-present fields, but
we actually want them all set so migrate_params_apply() can copy all
of them.
Signed-off-by: Fabiano Rosas <faro...@suse.de>
---
migration/options.c | 157 +++++++++++++++-----------------------------
1 file changed, 54 insertions(+), 103 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 6619b5f21a..695bec5b8f 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -20,6 +20,10 @@
#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 "qapi/visitor.h"
+#include "qobject/qdict.h"
#include "qobject/qnull.h"
#include "system/runstate.h"
#include "migration/colo.h"
@@ -1223,120 +1227,63 @@ bool migrate_params_check(MigrationParameters *params,
Error **errp)
return true;
}
-static void migrate_params_test_apply(MigrationParameters *params,
- MigrationParameters *dest)
+static bool migrate_params_merge(MigrationParameters *dst,
+ MigrationParameters *src, Error **errp)
{
- MigrationState *s = migrate_get_current();
+ QObject *ret_out = NULL;
+ Visitor *v;
+ bool ok;
- 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;
+ /* free memory from pointers that are about to be assigned */
+ if (src->has_block_bitmap_mapping) {
+ qapi_free_BitmapMigrationNodeAliasList(dst->block_bitmap_mapping);
}
- if (params->has_cpu_throttle_tailslow) {
- dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+ if (src->tls_creds) {
+ qapi_free_StrOrNull(dst->tls_creds);
}
- if (params->tls_creds) {
- qapi_free_StrOrNull(dest->tls_creds);
- dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
+ if (src->tls_hostname) {
+ qapi_free_StrOrNull(dst->tls_hostname);
}
- if (params->tls_hostname) {
- qapi_free_StrOrNull(dest->tls_hostname);
- dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname);
+ if (src->tls_authz) {
+ qapi_free_StrOrNull(dst->tls_authz);
}
- if (params->tls_authz) {
- qapi_free_StrOrNull(dest->tls_authz);
- dest->tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
+ /* read in from src */
+ v = qobject_output_visitor_new(&ret_out);
+ ok = visit_type_MigrationParameters(v, NULL, &src, errp);
+ if (!ok) {
+ goto out;
}
+ visit_complete(v, &ret_out);
+ visit_free(v);
- if (params->has_max_bandwidth) {
- dest->max_bandwidth = params->max_bandwidth;
+ /*
+ * Write to dst but leave existing fields intact (except for has_*
+ * which will be updated according to their presence in src).
+ */
+ v = qobject_input_visitor_new(ret_out);
+ ok = visit_start_struct(v, NULL, NULL, 0, errp);
+ if (!ok) {
+ goto out;
}
-
- if (params->has_avail_switchover_bandwidth) {
- dest->avail_switchover_bandwidth = params->avail_switchover_bandwidth;
+ ok = visit_type_MigrationParameters_members(v, dst, errp);
+ if (!ok) {
+ goto out;
}
-
- if (params->has_downtime_limit) {
- dest->downtime_limit = params->downtime_limit;
+ ok = visit_check_struct(v, errp);
+ visit_end_struct(v, NULL);
+ if (!ok) {
+ goto out;
}
- if (params->has_x_checkpoint_delay) {
- dest->x_checkpoint_delay = params->x_checkpoint_delay;
- }
+out:
+ visit_free(v);
+ qobject_unref(ret_out);
- 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) {
- dest->block_bitmap_mapping = 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;
- }
+ return ok;
}
static void migrate_params_apply(MigrationParameters *params)
@@ -1353,7 +1300,9 @@ static void migrate_params_apply(MigrationParameters
*params)
void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
{
- MigrationParameters tmp;
+ MigrationState *s = migrate_get_current();
+ g_autoptr(MigrationParameters) tmp = QAPI_CLONE(MigrationParameters,
+ &s->parameters);
/*
* Convert QTYPE_QNULL and NULL to the empty string (""). Even
@@ -1367,7 +1316,9 @@ void qmp_migrate_set_parameters(MigrationParameters
*params, Error **errp)
tls_opt_to_str(¶ms->tls_hostname);
tls_opt_to_str(¶ms->tls_authz);
- migrate_params_test_apply(params, &tmp);
+ if (!migrate_params_merge(tmp, params, errp)) {
+ return;
+ }
/*
* Mark block_bitmap_mapping as present now while we have the
@@ -1377,10 +1328,10 @@ void qmp_migrate_set_parameters(MigrationParameters
*params, Error **errp)
migrate_get_current()->has_block_bitmap_mapping = true;
}
- if (migrate_params_check(&tmp, errp)) {
- migrate_params_apply(&tmp);
+ if (migrate_params_check(tmp, errp)) {
+ /* mark all present, so they're all copied */
+ migrate_mark_all_params_present(tmp);
+ migrate_params_apply(tmp);
migrate_post_update_params(params, errp);
}
-
- migrate_tls_opts_free(&tmp);
}
--
2.35.3
