| Message ID | 1473390856-4502-4-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Sep 9, 2016 at 7:16 AM Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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
Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> NIIIICEEEEEEE I hated that function parameters list. Thanks very much.
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); } }
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 <eblake@redhat.com> --- qapi-schema.json | 2 +- hmp.c | 40 ++++++++++++++----------------- migration/migration.c | 65 +++++++++++++++++++++------------------------------ 3 files changed, 46 insertions(+), 61 deletions(-)