* Daniel P. Berrange (berra...@redhat.com) wrote:
> The MigrateState struct uses an array for storing migration
> parameters. This presumes that all future parameters will
> be integers too, which is not going to be the case. There
> is no functional reason why an array is used, if anything
> it makes the code less clear. The QAPI schema already
> defines a struct - MigrationParameters - capable of storing
> all the individual parameters, so just use that instead of
> an array.
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
(You could have done this as a separate patch rather than put
it in this big series)
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> include/migration/migration.h | 5 +++-
> migration/migration.c | 56
> +++++++++++++++++++------------------------
> migration/ram.c | 6 ++---
> 3 files changed, 30 insertions(+), 37 deletions(-)
>
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 5d44b07..999a5ee 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -133,9 +133,12 @@ struct MigrationState
> QemuThread thread;
> QEMUBH *cleanup_bh;
> QEMUFile *to_dst_file;
> - int parameters[MIGRATION_PARAMETER__MAX];
> +
> + /* New style params from 'migrate-set-parameters' */
> + MigrationParameters parameters;
>
> int state;
> + /* Old style params from 'migrate' command */
> MigrationParams params;
>
> /* State related to return path */
> diff --git a/migration/migration.c b/migration/migration.c
> index e90a14f..b3bdc31 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -82,16 +82,14 @@ MigrationState *migrate_get_current(void)
> .bandwidth_limit = MAX_THROTTLE,
> .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
> .mbps = -1,
> - .parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] =
> - DEFAULT_MIGRATE_COMPRESS_LEVEL,
> - .parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
> - DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
> - .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
> - DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
> - .parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
> - DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL,
> - .parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
> - DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT,
> + .parameters = {
> + .compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
> + .compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
> + .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
> + .x_cpu_throttle_initial = DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL,
> + .x_cpu_throttle_increment =
> + DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT,
> + },
> };
>
> if (!once) {
> @@ -525,15 +523,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> MigrationState *s = migrate_get_current();
>
> params = g_malloc0(sizeof(*params));
> - params->compress_level =
> s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
> - params->compress_threads =
> - s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
> - params->decompress_threads =
> - s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
> - params->x_cpu_throttle_initial =
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
> - params->x_cpu_throttle_increment =
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
> + params->compress_level = s->parameters.compress_level;
> + params->compress_threads = s->parameters.compress_threads;
> + params->decompress_threads = s->parameters.decompress_threads;
> + params->x_cpu_throttle_initial = s->parameters.x_cpu_throttle_initial;
> + params->x_cpu_throttle_increment =
> s->parameters.x_cpu_throttle_increment;
>
> return params;
> }
> @@ -733,7 +727,8 @@ void qmp_migrate_set_parameters(bool has_compress_level,
> bool has_x_cpu_throttle_initial,
> int64_t x_cpu_throttle_initial,
> bool has_x_cpu_throttle_increment,
> - int64_t x_cpu_throttle_increment, Error
> **errp)
> + int64_t x_cpu_throttle_increment,
> + Error **errp)
> {
> MigrationState *s = migrate_get_current();
>
> @@ -770,26 +765,23 @@ void qmp_migrate_set_parameters(bool has_compress_level,
> }
>
> if (has_compress_level) {
> - s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level;
> + s->parameters.compress_level = compress_level;
> }
> if (has_compress_threads) {
> - s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
> compress_threads;
> + s->parameters.compress_threads = compress_threads;
> }
> if (has_decompress_threads) {
> - s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
> - decompress_threads;
> + s->parameters.decompress_threads = decompress_threads;
> }
> if (has_x_cpu_throttle_initial) {
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
> - x_cpu_throttle_initial;
> + s->parameters.x_cpu_throttle_initial = x_cpu_throttle_initial;
> }
> -
> if (has_x_cpu_throttle_increment) {
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
> - x_cpu_throttle_increment;
> + s->parameters.x_cpu_throttle_increment = x_cpu_throttle_increment;
> }
> }
>
> +
> void qmp_migrate_start_postcopy(Error **errp)
> {
> MigrationState *s = migrate_get_current();
> @@ -1173,7 +1165,7 @@ int migrate_compress_level(void)
>
> s = migrate_get_current();
>
> - return s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
> + return s->parameters.compress_level;
> }
>
> int migrate_compress_threads(void)
> @@ -1182,7 +1174,7 @@ int migrate_compress_threads(void)
>
> s = migrate_get_current();
>
> - return s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
> + return s->parameters.compress_threads;
> }
>
> int migrate_decompress_threads(void)
> @@ -1191,7 +1183,7 @@ int migrate_decompress_threads(void)
>
> s = migrate_get_current();
>
> - return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
> + return s->parameters.decompress_threads;
> }
>
> bool migrate_use_events(void)
> diff --git a/migration/ram.c b/migration/ram.c
> index 704f6a9..42957bd 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -426,10 +426,8 @@ static size_t save_page_header(QEMUFile *f, RAMBlock
> *block, ram_addr_t offset)
> static void mig_throttle_guest_down(void)
> {
> MigrationState *s = migrate_get_current();
> - uint64_t pct_initial =
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
> - uint64_t pct_icrement =
> - s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
> + uint64_t pct_initial = s->parameters.x_cpu_throttle_initial;
> + uint64_t pct_icrement = s->parameters.x_cpu_throttle_increment;
>
> /* We have not started throttling yet. Let's start it. */
> if (!cpu_throttle_active()) {
> --
> 2.5.0
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
