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 <[email protected]> --- migration/options.c | 137 +++++++------------------------------------- 1 file changed, 22 insertions(+), 115 deletions(-) diff --git a/migration/options.c b/migration/options.c index 03e63c918c..36feaa5fe6 100644 --- a/migration/options.c +++ b/migration/options.c @@ -20,6 +20,7 @@ #include "qapi/qapi-commands-migration.h" #include "qapi/qapi-visit-migration.h" #include "qapi/qmp/qerror.h" +#include "qapi/type-helpers.h" #include "qobject/qnull.h" #include "system/runstate.h" #include "migration/colo.h" @@ -1260,124 +1261,28 @@ bool migrate_params_check(MigrationParameters *params, Error **errp) return true; } -static void migrate_params_test_apply(MigrationParameters *params, - MigrationParameters *dest) +static void migrate_params_merge(MigrationParameters *dst, + MigrationParameters *src) { - 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; + /* free memory from pointers that are about to be assigned */ + if (src->has_block_bitmap_mapping) { + g_clear_pointer(&dst->block_bitmap_mapping, + qapi_free_BitmapMigrationNodeAliasList); } - if (params->has_cpu_throttle_tailslow) { - dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow; + if (src->tls_creds) { + g_clear_pointer(&dst->tls_creds, qapi_free_StrOrNull); } - if (params->tls_creds) { - qapi_free_StrOrNull(dest->tls_creds); - dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds); + if (src->tls_hostname) { + g_clear_pointer(&dst->tls_hostname, qapi_free_StrOrNull); } - if (params->tls_hostname) { - qapi_free_StrOrNull(dest->tls_hostname); - dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname); + if (src->tls_authz) { + g_clear_pointer(&dst->tls_authz, qapi_free_StrOrNull); } - 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) { - 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; - } - - if (params->has_cpr_exec_command) { - dest->cpr_exec_command = params->cpr_exec_command; - } + QAPI_MERGE(MigrationParameters, dst, src); } /* @@ -1399,7 +1304,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 @@ -1413,7 +1320,7 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp) tls_opt_to_str(params->tls_hostname); tls_opt_to_str(params->tls_authz); - migrate_params_test_apply(params, &tmp); + migrate_params_merge(tmp, params); /* * Mark block_bitmap_mapping as present now while we have the @@ -1423,10 +1330,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.51.0
