Re: [Qemu-devel] [PATCH v3 2/5] migration: Parameters for auto-converge cpu throttling

[Dr. David Alan Gilbert]

* Jason J. Herne (address@hidden) wrote:
> Add migration parameters to allow the user to adjust the parameters
> that control cpu throttling when auto-converge is in effect. The added
> parameters are as follows:
> 
> x-cpu-throttle-initial : Initial percantage of time guest cpus are throttled
> when migration auto-converge is activated.
> 
> x-cpu-throttle-increment: throttle percantage increase each time
> auto-converge detects that migration is not making progress.
> 
> Signed-off-by: Jason J. Herne <address@hidden>

Other than your spelling of 'percantage':

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> ---
>  hmp.c                 | 16 ++++++++++++++++
>  migration/migration.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json      | 33 ++++++++++++++++++++++++++++++---
>  3 files changed, 91 insertions(+), 4 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index e17852d..eb65998 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -269,6 +269,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const 
> QDict *qdict)
>          monitor_printf(mon, " %s: %" PRId64,
>              
> MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
>              params->decompress_threads);
> +        monitor_printf(mon, " %s: %" PRId64,
> +            
> MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL],
> +            params->x_cpu_throttle_initial);
> +        monitor_printf(mon, " %s: %" PRId64,
> +            
> MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT],
> +            params->x_cpu_throttle_increment);
>          monitor_printf(mon, "\n");
>      }
>  
> @@ -1216,6 +1222,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>      bool has_compress_level = false;
>      bool has_compress_threads = false;
>      bool has_decompress_threads = false;
> +    bool has_x_cpu_throttle_initial = false;
> +    bool has_x_cpu_throttle_increment = false;
>      int i;
>  
>      for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) {
> @@ -1230,10 +1238,18 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>              case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
>                  has_decompress_threads = true;
>                  break;
> +            case MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL:
> +                has_x_cpu_throttle_initial = true;
> +                break;
> +            case MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT:
> +                has_x_cpu_throttle_increment = true;
> +                break;
>              }
>              qmp_migrate_set_parameters(has_compress_level, value,
>                                         has_compress_threads, value,
>                                         has_decompress_threads, value,
> +                                       has_x_cpu_throttle_initial, value,
> +                                       has_x_cpu_throttle_increment, value,
>                                         &err);
>              break;
>          }
> diff --git a/migration/migration.c b/migration/migration.c
> index 732d229..05790e9 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -40,6 +40,9 @@
>  #define DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT 2
>  /*0: means nocompress, 1: best speed, ... 9: best compress ratio */
>  #define DEFAULT_MIGRATE_COMPRESS_LEVEL 1
> +/* Define default autoconverge cpu throttle migration parameters */
> +#define DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL 20
> +#define DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT 10
>  
>  /* Migration XBZRLE default cache size */
>  #define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
> @@ -66,6 +69,10 @@ MigrationState *migrate_get_current(void)
>                  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,
>      };
>  
>      return &current_migration;
> @@ -199,6 +206,10 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
> **errp)
>              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];
>  
>      return params;
>  }
> @@ -321,7 +332,11 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>                                  bool has_compress_threads,
>                                  int64_t compress_threads,
>                                  bool has_decompress_threads,
> -                                int64_t decompress_threads, Error **errp)
> +                                int64_t decompress_threads,
> +                                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)
>  {
>      MigrationState *s = migrate_get_current();
>  
> @@ -344,6 +359,18 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>                    "is invalid, it should be in the range of 1 to 255");
>          return;
>      }
> +    if (has_x_cpu_throttle_initial &&
> +            (x_cpu_throttle_initial < 1 || x_cpu_throttle_initial > 99)) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> +                  "x_cpu_throttle_initial",
> +                  "an integer in the range of 1 to 99");
> +    }
> +    if (has_x_cpu_throttle_increment &&
> +            (x_cpu_throttle_increment < 1 || x_cpu_throttle_increment > 99)) 
> {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> +                  "x_cpu_throttle_increment",
> +                  "an integer in the range of 1 to 99");
> +    }
>  
>      if (has_compress_level) {
>          s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level;
> @@ -355,6 +382,15 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>          s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
>                                                      decompress_threads;
>      }
> +    if (has_x_cpu_throttle_initial) {
> +        s->parameters[MIGRATION_PARAMETER_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;
> +    }
>  }
>  
>  /* shared migration helpers */
> @@ -470,6 +506,10 @@ static MigrationState *migrate_init(const 
> MigrationParams *params)
>              s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
>      int decompress_thread_count =
>              s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
> +    int x_cpu_throttle_initial =
> +            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
> +    int x_cpu_throttle_increment =
> +            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
>  
>      memcpy(enabled_capabilities, s->enabled_capabilities,
>             sizeof(enabled_capabilities));
> @@ -485,6 +525,10 @@ static MigrationState *migrate_init(const 
> MigrationParams *params)
>                 compress_thread_count;
>      s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
>                 decompress_thread_count;
> +    s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
> +                x_cpu_throttle_initial;
> +    s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
> +                x_cpu_throttle_increment;
>      s->bandwidth_limit = bandwidth_limit;
>      s->state = MIGRATION_STATUS_SETUP;
>      trace_migrate_set_state(MIGRATION_STATUS_SETUP);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f97ffa1..7dd324e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -587,10 +587,18 @@
>  #          compression, so set the decompress-threads to the number about 1/4
>  #          of compress-threads is adequate.
>  #
> +# @x-cpu-throttle-initial: Initial percantage of time guest cpus are 
> throttled
> +#                          when migration auto-converge is activated.
> +#                          (Since 2.4)
> +#
> +# @x-cpu-throttle-increment: throttle percantage increase each time
> +#                            auto-converge detects that migration is not 
> making
> +#                            progress. (Since 2.4)
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> -  'data': ['compress-level', 'compress-threads', 'decompress-threads'] }
> +  'data': ['compress-level', 'compress-threads', 'decompress-threads',
> +           'x-cpu-throttle-initial', 'x-cpu-throttle-increment'] }
>  
>  #
>  # @migrate-set-parameters
> @@ -603,12 +611,22 @@
>  #
>  # @decompress-threads: decompression thread count
>  #
> +# @x-cpu-throttle-initial: Initial percantage of time guest cpus are 
> throttled
> +#                          when migration auto-converge is activated.
> +#                          (Since 2.4)
> +#
> +# @x-cpu-throttle-increment: throttle percantage increase each time
> +#                            auto-converge detects that migration is not 
> making
> +#                            progress. (Since 2.4)
> +#
>  # Since: 2.4
>  ##
>  { 'command': 'migrate-set-parameters',
>    'data': { '*compress-level': 'int',
>              '*compress-threads': 'int',
> -            '*decompress-threads': 'int'} }
> +            '*decompress-threads': 'int',
> +            '*x-cpu-throttle-initial': 'int',
> +            '*x-cpu-throttle-increment': 'int'} }
>  
>  #
>  # @MigrationParameters
> @@ -618,13 +636,22 @@
>  # @compress-threads: compression thread count
>  #
>  # @decompress-threads: decompression thread count
> +# @x-cpu-throttle-initial: Initial percantage of time guest cpus are 
> throttled
> +#                          when migration auto-converge is activated.
> +#                          (Since 2.4)
> +#
> +# @x-cpu-throttle-increment: throttle percantage increase each time
> +#                            auto-converge detects that migration is not 
> making
> +#                            progress. (Since 2.4)
>  #
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
>    'data': { 'compress-level': 'int',
>              'compress-threads': 'int',
> -            'decompress-threads': 'int'} }
> +            'decompress-threads': 'int',
> +            'x-cpu-throttle-initial': 'int',
> +            'x-cpu-throttle-increment': 'int'} }
>  ##
>  # @query-migrate-parameters
>  #
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK