* Fabiano Rosas ([email protected]) wrote:
> The max-bandwidth and avail-switchover-bandwidth parameters are
> declared as bytes/second in migration.json and interpreted as such
> throughout the code, except in HMP where they're expected to be
> provided as mebibytes.
> 
> Fix the HMP code to parse the values as bytes.
> 
> Signed-off-by: Fabiano Rosas <[email protected]>

Makes sense, and shockingly, it means that 'max-bandwidth' and 
'max-postcopy-bandwidth'
now have the same units!

Please point this out in the release changelog.

Reviewed-by: Dr. David Alan Gilbert <[email protected]>

> ---
>  migration/migration-hmp-cmds.c     | 23 +++--------------------
>  tests/qtest/migration/misc-tests.c |  4 ++--
>  2 files changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 27b38d0e5f..67ea2eea15 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -596,10 +596,9 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const 
> QDict *qdict)
>      const char *valuestr = qdict_get_str(qdict, "value");
>      Visitor *v = string_input_visitor_new(valuestr);
>      MigrationParameters *p = g_new0(MigrationParameters, 1);
> -    uint64_t valuebw = 0;
>      uint64_t cache_size;
>      Error *err = NULL;
> -    int val, ret;
> +    int val;
>  
>      val = qapi_enum_parse(&MigrationParameter_lookup, param, -1, &err);
>      if (val < 0) {
> @@ -644,27 +643,11 @@ void hmp_migrate_set_parameter(MonitorHMP *hmp, const 
> QDict *qdict)
>          break;
>      case MIGRATION_PARAMETER_MAX_BANDWIDTH:
>          p->has_max_bandwidth = true;
> -        /*
> -         * Can't use visit_type_size() here, because it
> -         * defaults to Bytes rather than Mebibytes.
> -         */
> -        ret = qemu_strtosz_MiB(valuestr, NULL, &valuebw);
> -        if (ret < 0 || valuebw > INT64_MAX
> -            || (size_t)valuebw != valuebw) {
> -            error_setg(&err, "Invalid size %s", valuestr);
> -            break;
> -        }
> -        p->max_bandwidth = valuebw;
> +        visit_type_size(v, param, &p->max_bandwidth, &err);
>          break;
>      case MIGRATION_PARAMETER_AVAIL_SWITCHOVER_BANDWIDTH:
>          p->has_avail_switchover_bandwidth = true;
> -        ret = qemu_strtosz_MiB(valuestr, NULL, &valuebw);
> -        if (ret < 0 || valuebw > INT64_MAX
> -            || (size_t)valuebw != valuebw) {
> -            error_setg(&err, "Invalid size %s", valuestr);
> -            break;
> -        }
> -        p->avail_switchover_bandwidth = valuebw;
> +        visit_type_size(v, param, &p->avail_switchover_bandwidth, &err);
>          break;
>      case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
>          p->has_downtime_limit = true;
> diff --git a/tests/qtest/migration/misc-tests.c 
> b/tests/qtest/migration/misc-tests.c
> index 04f46d5e81..447fa5e232 100644
> --- a/tests/qtest/migration/misc-tests.c
> +++ b/tests/qtest/migration/misc-tests.c
> @@ -62,8 +62,8 @@ HMPTestData test_cases[] = {
>      TEST("announce-rounds", "6", "6"),
>      TEST("announce-step", "15", "15 ms"),
>      TEST("downtime-limit", "400", "400 ms"),
> -    TEST("avail-switchover-bandwidth", "2097152", "2199023255552 
> bytes/second"),
> -    TEST("max-bandwidth", "9876543", "10356305952768 bytes/second"),
> +    TEST("avail-switchover-bandwidth", "2097152", "2097152 bytes/second"),
> +    TEST("max-bandwidth", "9876543", "9876543 bytes/second"),
>      TEST("max-postcopy-bandwidth", "1048576", "1048576 bytes/second"),
>      TEST("vcpu-dirty-limit", "20", "20 MB/s"),
>      TEST("x-rdma-chunk-size", "1048576", "1048576 bytes"),
> -- 
> 2.53.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Reply via email to