From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, andrey.gruzdev@virtuozzo.com,
berrange@redhat.com, gaojinhao@huawei.com, armbru@redhat.com,
mst@redhat.com, philmd@redhat.com, wainersm@redhat.com
Subject: [PULL 10/27] migration: Fix migrate-set-parameters argument validation
Date: Thu, 4 Feb 2021 16:39:42 +0000 [thread overview]
Message-ID: <20210204163959.377618-11-dgilbert@redhat.com> (raw)
In-Reply-To: <20210204163959.377618-1-dgilbert@redhat.com>
From: Markus Armbruster <armbru@redhat.com>
Commit 741d4086c8 "migration: Use proper types in json" (v2.12.0)
switched MigrationParameters to narrower integer types, and removed
the simplified qmp_migrate_set_parameters()'s argument checking
accordingly.
Good idea, except qmp_migrate_set_parameters() takes
MigrateSetParameters, not MigrationParameters. Its job is updating
migrate_get_current()->parameters (which *is* of type
MigrationParameters) according to its argument. The integers now get
truncated silently. Reproducer:
---> {'execute': 'query-migrate-parameters'}
<--- {"return": {[...] "compress-threads": 8, [...]}}
---> {"execute": "migrate-set-parameters", "arguments": {"compress-threads": 257}}
<--- {"return": {}}
---> {'execute': 'query-migrate-parameters'}
<--- {"return": {[...] "compress-threads": 1, [...]}}
Fix by resynchronizing MigrateSetParameters with MigrationParameters.
Fixes: 741d4086c856320807a2575389d7c0505578270b
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210202141734.2488076-2-armbru@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
monitor/hmp-cmds.c | 24 ++++++++++++------------
qapi/migration.json | 28 ++++++++++++++--------------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a48bc1e904..509d6b01ee 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1294,11 +1294,11 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
switch (val) {
case MIGRATION_PARAMETER_COMPRESS_LEVEL:
p->has_compress_level = true;
- visit_type_int(v, param, &p->compress_level, &err);
+ visit_type_uint8(v, param, &p->compress_level, &err);
break;
case MIGRATION_PARAMETER_COMPRESS_THREADS:
p->has_compress_threads = true;
- visit_type_int(v, param, &p->compress_threads, &err);
+ visit_type_uint8(v, param, &p->compress_threads, &err);
break;
case MIGRATION_PARAMETER_COMPRESS_WAIT_THREAD:
p->has_compress_wait_thread = true;
@@ -1306,19 +1306,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
p->has_decompress_threads = true;
- visit_type_int(v, param, &p->decompress_threads, &err);
+ visit_type_uint8(v, param, &p->decompress_threads, &err);
break;
case MIGRATION_PARAMETER_THROTTLE_TRIGGER_THRESHOLD:
p->has_throttle_trigger_threshold = true;
- visit_type_int(v, param, &p->throttle_trigger_threshold, &err);
+ visit_type_uint8(v, param, &p->throttle_trigger_threshold, &err);
break;
case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
p->has_cpu_throttle_initial = true;
- visit_type_int(v, param, &p->cpu_throttle_initial, &err);
+ visit_type_uint8(v, param, &p->cpu_throttle_initial, &err);
break;
case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
p->has_cpu_throttle_increment = true;
- visit_type_int(v, param, &p->cpu_throttle_increment, &err);
+ visit_type_uint8(v, param, &p->cpu_throttle_increment, &err);
break;
case MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW:
p->has_cpu_throttle_tailslow = true;
@@ -1326,7 +1326,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_MAX_CPU_THROTTLE:
p->has_max_cpu_throttle = true;
- visit_type_int(v, param, &p->max_cpu_throttle, &err);
+ visit_type_uint8(v, param, &p->max_cpu_throttle, &err);
break;
case MIGRATION_PARAMETER_TLS_CREDS:
p->has_tls_creds = true;
@@ -1362,11 +1362,11 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
p->has_downtime_limit = true;
- visit_type_int(v, param, &p->downtime_limit, &err);
+ visit_type_size(v, param, &p->downtime_limit, &err);
break;
case MIGRATION_PARAMETER_X_CHECKPOINT_DELAY:
p->has_x_checkpoint_delay = true;
- visit_type_int(v, param, &p->x_checkpoint_delay, &err);
+ visit_type_uint32(v, param, &p->x_checkpoint_delay, &err);
break;
case MIGRATION_PARAMETER_BLOCK_INCREMENTAL:
p->has_block_incremental = true;
@@ -1374,7 +1374,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_MULTIFD_CHANNELS:
p->has_multifd_channels = true;
- visit_type_int(v, param, &p->multifd_channels, &err);
+ visit_type_uint8(v, param, &p->multifd_channels, &err);
break;
case MIGRATION_PARAMETER_MULTIFD_COMPRESSION:
p->has_multifd_compression = true;
@@ -1383,11 +1383,11 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
break;
case MIGRATION_PARAMETER_MULTIFD_ZLIB_LEVEL:
p->has_multifd_zlib_level = true;
- visit_type_int(v, param, &p->multifd_zlib_level, &err);
+ visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
break;
case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
p->has_multifd_zstd_level = true;
- visit_type_int(v, param, &p->multifd_zstd_level, &err);
+ visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
diff --git a/qapi/migration.json b/qapi/migration.json
index 6c12b368aa..37026643ab 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -890,28 +890,28 @@
'*announce-max': 'size',
'*announce-rounds': 'size',
'*announce-step': 'size',
- '*compress-level': 'int',
- '*compress-threads': 'int',
+ '*compress-level': 'uint8',
+ '*compress-threads': 'uint8',
'*compress-wait-thread': 'bool',
- '*decompress-threads': 'int',
- '*throttle-trigger-threshold': 'int',
- '*cpu-throttle-initial': 'int',
- '*cpu-throttle-increment': 'int',
+ '*decompress-threads': 'uint8',
+ '*throttle-trigger-threshold': 'uint8',
+ '*cpu-throttle-initial': 'uint8',
+ '*cpu-throttle-increment': 'uint8',
'*cpu-throttle-tailslow': 'bool',
'*tls-creds': 'StrOrNull',
'*tls-hostname': 'StrOrNull',
'*tls-authz': 'StrOrNull',
- '*max-bandwidth': 'int',
- '*downtime-limit': 'int',
- '*x-checkpoint-delay': 'int',
+ '*max-bandwidth': 'size',
+ '*downtime-limit': 'uint64',
+ '*x-checkpoint-delay': 'uint32',
'*block-incremental': 'bool',
- '*multifd-channels': 'int',
+ '*multifd-channels': 'uint8',
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
- '*max-cpu-throttle': 'int',
+ '*max-cpu-throttle': 'uint8',
'*multifd-compression': 'MultiFDCompression',
- '*multifd-zlib-level': 'int',
- '*multifd-zstd-level': 'int',
+ '*multifd-zlib-level': 'uint8',
+ '*multifd-zstd-level': 'uint8',
'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
##
@@ -1098,7 +1098,7 @@
'*max-bandwidth': 'size',
'*downtime-limit': 'uint64',
'*x-checkpoint-delay': 'uint32',
- '*block-incremental': 'bool' ,
+ '*block-incremental': 'bool',
'*multifd-channels': 'uint8',
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
--
2.29.2
next prev parent reply other threads:[~2021-02-04 17:28 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-04 16:39 [PULL 00/27] migration queue Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 01/27] spapr_pci: Fix memory leak of vmstate_spapr_pci Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 02/27] savevm: Fix memory leak of vmstate_configuration Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 03/27] vmstate: Fix memory leak in vmstate_handle_alloc() Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 04/27] migration/qemu-file: Fix maybe uninitialized on qemu_get_buffer_in_place() Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 05/27] migration: introduce 'background-snapshot' migration capability Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 06/27] migration: introduce UFFD-WP low-level interface helpers Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 07/27] migration: support UFFD write fault processing in ram_save_iterate() Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 08/27] migration: implementation of background snapshot thread Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 09/27] migration: introduce 'userfaultfd-wrlat.py' script Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` Dr. David Alan Gilbert (git) [this message]
2021-02-04 16:39 ` [PULL 11/27] migration: Clean up signed vs. unsigned XBZRLE cache-size Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 12/27] migration: Fix cache_init()'s "Failed to allocate" error messages Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 13/27] migration: Fix a few absurdly defective " Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 14/27] migration: Add blocker information Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 15/27] migration: Display the migration blockers Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 16/27] block: push error reporting into bdrv_all_*_snapshot functions Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 17/27] migration: Make save_snapshot() return bool, not 0/-1 Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 18/27] migration: stop returning errno from load_snapshot() Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 19/27] block: add ability to specify list of blockdevs during snapshot Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 20/27] block: allow specifying name of block device for vmstate storage Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 21/27] block: rename and alter bdrv_all_find_snapshot semantics Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 22/27] migration: control whether snapshots are ovewritten Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 23/27] migration: wire up support for snapshot device selection Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 24/27] migration: introduce a delete_snapshot wrapper Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 25/27] iotests: add support for capturing and matching QMP events Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 26/27] iotests: fix loading of common.config from tests/ subdir Dr. David Alan Gilbert (git)
2021-02-04 16:39 ` [PULL 27/27] migration: introduce snapshot-{save, load, delete} QMP commands Dr. David Alan Gilbert (git)
2021-02-04 19:48 ` [PULL 00/27] migration queue Peter Maydell
2021-02-04 19:51 ` Dr. David Alan Gilbert
2021-02-08 10:42 ` Dr. David Alan Gilbert
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=20210204163959.377618-11-dgilbert@redhat.com \
--to=dgilbert@redhat.com \
--cc=andrey.gruzdev@virtuozzo.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=gaojinhao@huawei.com \
--cc=mst@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=wainersm@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).