[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] migrate: Use boxed qapi for migrate-set-par
|
From: |
Marc-André Lureau |
|
Subject: |
Re: [Qemu-devel] [PATCH 3/3] migrate: Use boxed qapi for migrate-set-parameters |
|
Date: |
Fri, 09 Sep 2016 09:08:59 +0000 |
On Fri, Sep 9, 2016 at 7:16 AM Eric Blake <address@hidden> wrote:
> Now that QAPI makes it easy to pass a struct around, we don't
> have to declare as many parameters or local variables.
>
> Signed-off-by: Eric Blake <address@hidden>
>
Reviewed-by: Marc-André Lureau <address@hidden>
> ---
> qapi-schema.json | 2 +-
> hmp.c | 40 ++++++++++++++-----------------
> migration/migration.c | 65
> +++++++++++++++++++++------------------------------
> 3 files changed, 46 insertions(+), 61 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4a51e5b..88b4888 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -651,7 +651,7 @@
> #
> # Since: 2.4
> ##
> -{ 'command': 'migrate-set-parameters',
> +{ 'command': 'migrate-set-parameters', 'boxed': true,
> 'data': 'MigrationParameters' }
>
> #
> diff --git a/hmp.c b/hmp.c
> index 1e4094a..0893b8e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1260,44 +1260,40 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> const char *valuestr = qdict_get_str(qdict, "value");
> long valueint = 0;
> Error *err = NULL;
> - bool has_compress_level = false;
> - bool has_compress_threads = false;
> - bool has_decompress_threads = false;
> - bool has_cpu_throttle_initial = false;
> - bool has_cpu_throttle_increment = false;
> - bool has_tls_creds = false;
> - bool has_tls_hostname = false;
> bool use_int_value = false;
> int i;
>
> for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
> if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
> + MigrationParameters p = { 0 };
> switch (i) {
> case MIGRATION_PARAMETER_COMPRESS_LEVEL:
> - has_compress_level = true;
> + p.has_compress_level = true;
> use_int_value = true;
> break;
> case MIGRATION_PARAMETER_COMPRESS_THREADS:
> - has_compress_threads = true;
> + p.has_compress_threads = true;
> use_int_value = true;
> break;
> case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
> - has_decompress_threads = true;
> + p.has_decompress_threads = true;
> use_int_value = true;
> break;
> case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
> - has_cpu_throttle_initial = true;
> + p.has_cpu_throttle_initial = true;
> use_int_value = true;
> break;
> case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
> - has_cpu_throttle_increment = true;
> + p.has_cpu_throttle_increment = true;
> use_int_value = true;
> break;
> case MIGRATION_PARAMETER_TLS_CREDS:
> - has_tls_creds = true;
> + p.has_tls_creds = true;
> + p.tls_creds = (char *) valuestr;
> break;
> case MIGRATION_PARAMETER_TLS_HOSTNAME:
> - has_tls_hostname = true;
> + p.has_tls_hostname = true;
> + p.tls_hostname = (char *) valuestr;
> break;
> }
>
> @@ -1307,16 +1303,16 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> valuestr);
> goto cleanup;
> }
> + /* Set all integers; only one has_FOO will be set, and
> + * the code ignores the remaining values */
> + p.compress_level = valueint;
> + p.compress_threads = valueint;
> + p.decompress_threads = valueint;
> + p.cpu_throttle_initial = valueint;
> + p.cpu_throttle_increment = valueint;
> }
>
> - qmp_migrate_set_parameters(has_compress_level, valueint,
> - has_compress_threads, valueint,
> - has_decompress_threads, valueint,
> - has_cpu_throttle_initial, valueint,
> - has_cpu_throttle_increment,
> valueint,
> - has_tls_creds, valuestr,
> - has_tls_hostname, valuestr,
> - &err);
> + qmp_migrate_set_parameters(&p, &err);
> break;
> }
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index 1a8f26b..42336e3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -766,78 +766,67 @@ void
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> }
> }
>
> -void qmp_migrate_set_parameters(bool has_compress_level,
> - int64_t compress_level,
> - bool has_compress_threads,
> - int64_t compress_threads,
> - bool has_decompress_threads,
> - int64_t decompress_threads,
> - bool has_cpu_throttle_initial,
> - int64_t cpu_throttle_initial,
> - bool has_cpu_throttle_increment,
> - int64_t cpu_throttle_increment,
> - bool has_tls_creds,
> - const char *tls_creds,
> - bool has_tls_hostname,
> - const char *tls_hostname,
> - Error **errp)
> +void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
> {
> MigrationState *s = migrate_get_current();
>
> - if (has_compress_level && (compress_level < 0 || compress_level > 9))
> {
> + if (params->has_compress_level &&
> + (params->compress_level < 0 || params->compress_level > 9)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
> "is invalid, it should be in the range of 0 to 9");
> return;
> }
> - if (has_compress_threads &&
> - (compress_threads < 1 || compress_threads > 255)) {
> + if (params->has_compress_threads &&
> + (params->compress_threads < 1 || params->compress_threads > 255))
> {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> "compress_threads",
> "is invalid, it should be in the range of 1 to 255");
> return;
> }
> - if (has_decompress_threads &&
> - (decompress_threads < 1 || decompress_threads > 255)) {
> + if (params->has_decompress_threads &&
> + (params->decompress_threads < 1 || params->decompress_threads >
> 255)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> "decompress_threads",
> "is invalid, it should be in the range of 1 to 255");
> return;
> }
> - if (has_cpu_throttle_initial &&
> - (cpu_throttle_initial < 1 || cpu_throttle_initial > 99)) {
> + if (params->has_cpu_throttle_initial &&
> + (params->cpu_throttle_initial < 1 ||
> + params->cpu_throttle_initial > 99)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> "cpu_throttle_initial",
> "an integer in the range of 1 to 99");
> }
> - if (has_cpu_throttle_increment &&
> - (cpu_throttle_increment < 1 || cpu_throttle_increment > 99)) {
> + if (params->has_cpu_throttle_increment &&
> + (params->cpu_throttle_increment < 1 ||
> + params->cpu_throttle_increment > 99)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> "cpu_throttle_increment",
> "an integer in the range of 1 to 99");
> }
>
> - if (has_compress_level) {
> - s->parameters.compress_level = compress_level;
> + if (params->has_compress_level) {
> + s->parameters.compress_level = params->compress_level;
> }
> - if (has_compress_threads) {
> - s->parameters.compress_threads = compress_threads;
> + if (params->has_compress_threads) {
> + s->parameters.compress_threads = params->compress_threads;
> }
> - if (has_decompress_threads) {
> - s->parameters.decompress_threads = decompress_threads;
> + if (params->has_decompress_threads) {
> + s->parameters.decompress_threads = params->decompress_threads;
> }
> - if (has_cpu_throttle_initial) {
> - s->parameters.cpu_throttle_initial = cpu_throttle_initial;
> + if (params->has_cpu_throttle_initial) {
> + s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
> }
> - if (has_cpu_throttle_increment) {
> - s->parameters.cpu_throttle_increment = cpu_throttle_increment;
> + if (params->has_cpu_throttle_increment) {
> + s->parameters.cpu_throttle_increment =
> params->cpu_throttle_increment;
> }
> - if (has_tls_creds) {
> + if (params->has_tls_creds) {
> g_free(s->parameters.tls_creds);
> - s->parameters.tls_creds = g_strdup(tls_creds);
> + s->parameters.tls_creds = g_strdup(params->tls_creds);
> }
> - if (has_tls_hostname) {
> + if (params->has_tls_hostname) {
> g_free(s->parameters.tls_hostname);
> - s->parameters.tls_hostname = g_strdup(tls_hostname);
> + s->parameters.tls_hostname = g_strdup(params->tls_hostname);
> }
> }
>
> --
> 2.7.4
>
>
> --
Marc-André Lureau