From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: Amit Shah <amit.shah@redhat.com>,
Luiz Capitulino <lcapitulino@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
ashijeetacharya@gmail.com, quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH 3/3] migrate: Use boxed qapi for migrate-set-parameters
Date: Fri, 09 Sep 2016 09:08:59 +0000 [thread overview]
Message-ID: <CAJ+F1CKuC4b8Tifsh64+4Ye7mr03H+RdLE2VhWv9x6VbGDwwFw@mail.gmail.com> (raw)
In-Reply-To: <1473390856-4502-4-git-send-email-eblake@redhat.com>
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
next prev parent reply other threads:[~2016-09-09 9:09 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-09 3:14 [Qemu-devel] [PATCH 0/3] migrate: simplify migrate-set-parameters Eric Blake
2016-09-09 3:14 ` [Qemu-devel] [PATCH 1/3] migrate: Fix cpu-throttle-increment regression in HMP Eric Blake
2016-09-09 8:06 ` Marc-André Lureau
2016-10-05 9:12 ` Juan Quintela
2016-09-09 3:14 ` [Qemu-devel] [PATCH 2/3] migrate: Share common MigrationParameters struct Eric Blake
2016-09-09 9:08 ` Marc-André Lureau
2016-10-05 9:23 ` Juan Quintela
2016-10-17 21:31 ` Eric Blake
2016-09-09 3:14 ` [Qemu-devel] [PATCH 3/3] migrate: Use boxed qapi for migrate-set-parameters Eric Blake
2016-09-09 9:08 ` Marc-André Lureau [this message]
2016-10-05 9:26 ` Juan Quintela
2016-10-05 9:37 ` [Qemu-devel] [PATCH 0/3] migrate: simplify migrate-set-parameters Juan Quintela
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ+F1CKuC4b8Tifsh64+4Ye7mr03H+RdLE2VhWv9x6VbGDwwFw@mail.gmail.com \
--to=marcandre.lureau@gmail.com \
--cc=amit.shah@redhat.com \
--cc=armbru@redhat.com \
--cc=ashijeetacharya@gmail.com \
--cc=eblake@redhat.com \
--cc=lcapitulino@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.