Sizes and byte rates should use QAPI type 'size' (uint64_t).
BlockIOThrottle and BlockDeviceInfo members @bps, @bps_rd, @bps_wr,
@bps_max, @bps_rd_max, @bps_wr_max, @iops_size are 'int' (int64_t).
qmp_block_set_io_throttle() and bdrv_block_device_info() copy @bps,
@bps_rd, @bps_wr to / from LeakyBucket member @avg (implicit
conversion to / from double), @bps_max, @bps_rd_max, @bps_wr_max to /
from LeakyBucket member @max (implicit conversion to / from double),
and @iops_size to / from ThrottleConfig member op_size (implicit
conversion to / from uint64_t).

Change these BlockIOThrottle and BlockDeviceInfo members to 'size'.

block_set_io_throttle now accepts sizes and rates between 2^63 and
2^64-1.  It accepts negative values as before, because that's how the
QObject input visitor works for backward compatibility.

Doing the same for HMP's block_set_io_throttle deserves its own commit
(the next one).

query-block and query-named-block-nodes now report sizes and rates
above 2^63-1 correctly instead of their (negative) two's complement.

So does HMP's "info block".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hmp.c                | 12 ++++++------
 qapi/block-core.json | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hmp.c b/hmp.c
index 9bcdcb3..3253674 100644
--- a/hmp.c
+++ b/hmp.c
@@ -468,17 +468,17 @@ static void print_block_info(Monitor *mon, BlockInfo *info,
     if (inserted->bps  || inserted->bps_rd  || inserted->bps_wr  ||
         inserted->iops || inserted->iops_rd || inserted->iops_wr)
     {
-        monitor_printf(mon, "    I/O throttling:   bps=%" PRId64
-                        " bps_rd=%" PRId64  " bps_wr=%" PRId64
-                        " bps_max=%" PRId64
-                        " bps_rd_max=%" PRId64
-                        " bps_wr_max=%" PRId64
+        monitor_printf(mon, "    I/O throttling:   bps=%" PRIu64
+                        " bps_rd=%" PRIu64  " bps_wr=%" PRIu64
+                        " bps_max=%" PRIu64
+                        " bps_rd_max=%" PRIu64
+                        " bps_wr_max=%" PRIu64
                         " iops=%" PRId64 " iops_rd=%" PRId64
                         " iops_wr=%" PRId64
                         " iops_max=%" PRId64
                         " iops_rd_max=%" PRId64
                         " iops_wr_max=%" PRId64
-                        " iops_size=%" PRId64
+                        " iops_size=%" PRIu64
                         " group=%s\n",
                         inserted->bps,
                         inserted->bps_rd,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9e96590..1e26cb0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -356,16 +356,16 @@
             '*backing_file': 'str', 'backing_file_depth': 'int',
             'encrypted': 'bool', 'encryption_key_missing': 'bool',
             'detect_zeroes': 'BlockdevDetectZeroesOptions',
-            'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
+            'bps': 'size', 'bps_rd': 'size', 'bps_wr': 'size',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             'image': 'ImageInfo',
-            '*bps_max': 'int', '*bps_rd_max': 'int',
-            '*bps_wr_max': 'int', '*iops_max': 'int',
+            '*bps_max': 'size', '*bps_rd_max': 'size',
+            '*bps_wr_max': 'size', '*iops_max': 'int',
             '*iops_rd_max': 'int', '*iops_wr_max': 'int',
             '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
             '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
             '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-            '*iops_size': 'int', '*group': 'str', 'cache': 'BlockdevCacheInfo',
+            '*iops_size': 'size', '*group': 'str', 'cache': 'BlockdevCacheInfo',
             'write_threshold': 'size' } }
 
 ##
@@ -1866,15 +1866,15 @@
 # Since: 1.1
 ##
 { 'struct': 'BlockIOThrottle',
-  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-            'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-            '*bps_max': 'int', '*bps_rd_max': 'int',
-            '*bps_wr_max': 'int', '*iops_max': 'int',
-            '*iops_rd_max': 'int', '*iops_wr_max': 'int',
+  'data': { '*device': 'str', '*id': 'str',
+            'bps': 'size', 'bps_rd': 'size', 'bps_wr': 'size',
+            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+            '*bps_max': 'size', '*bps_rd_max': 'size', '*bps_wr_max': 'size',
+            '*iops_max': 'int', '*iops_rd_max': 'int', '*iops_wr_max': 'int',
             '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
             '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
             '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-            '*iops_size': 'int', '*group': 'str' } }
+            '*iops_size': 'size', '*group': 'str' } }
 
 ##
 # @block-stream:
-- 
2.7.5

On Mon 07 Aug 2017 04:45:35 PM CEST, Markus Armbruster wrote:
> Sizes and byte rates should use QAPI type 'size' (uint64_t).
> BlockIOThrottle and BlockDeviceInfo members @bps, @bps_rd, @bps_wr,
> @bps_max, @bps_rd_max, @bps_wr_max, @iops_size are 'int' (int64_t).
> qmp_block_set_io_throttle() and bdrv_block_device_info() copy @bps,
> @bps_rd, @bps_wr to / from LeakyBucket member @avg (implicit
> conversion to / from double), @bps_max, @bps_rd_max, @bps_wr_max to /
> from LeakyBucket member @max (implicit conversion to / from double),
> and @iops_size to / from ThrottleConfig member op_size (implicit
> conversion to / from uint64_t).
>
> Change these BlockIOThrottle and BlockDeviceInfo members to 'size'.
>
> block_set_io_throttle now accepts sizes and rates between 2^63 and
> 2^64-1.  It accepts negative values as before, because that's how the
> QObject input visitor works for backward compatibility.
>
> Doing the same for HMP's block_set_io_throttle deserves its own commit
> (the next one).
>
> query-block and query-named-block-nodes now report sizes and rates
> above 2^63-1 correctly instead of their (negative) two's complement.
>
> So does HMP's "info block".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

This is fine because all those parameters are limited to [0, 10^15], so
changing int64_t -> uint64_t is not a problem.

I have already sent a patch that changes the fields in the data
structure in throttle.h from double to uint64_t

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Alberto Garcia <berto@igalia.com> writes:

> On Mon 07 Aug 2017 04:45:35 PM CEST, Markus Armbruster wrote:
>> Sizes and byte rates should use QAPI type 'size' (uint64_t).
>> BlockIOThrottle and BlockDeviceInfo members @bps, @bps_rd, @bps_wr,
>> @bps_max, @bps_rd_max, @bps_wr_max, @iops_size are 'int' (int64_t).
>> qmp_block_set_io_throttle() and bdrv_block_device_info() copy @bps,
>> @bps_rd, @bps_wr to / from LeakyBucket member @avg (implicit
>> conversion to / from double), @bps_max, @bps_rd_max, @bps_wr_max to /
>> from LeakyBucket member @max (implicit conversion to / from double),
>> and @iops_size to / from ThrottleConfig member op_size (implicit
>> conversion to / from uint64_t).
>>
>> Change these BlockIOThrottle and BlockDeviceInfo members to 'size'.
>>
>> block_set_io_throttle now accepts sizes and rates between 2^63 and
>> 2^64-1.  It accepts negative values as before, because that's how the
>> QObject input visitor works for backward compatibility.
>>
>> Doing the same for HMP's block_set_io_throttle deserves its own commit
>> (the next one).
>>
>> query-block and query-named-block-nodes now report sizes and rates
>> above 2^63-1 correctly instead of their (negative) two's complement.
>>
>> So does HMP's "info block".
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> This is fine because all those parameters are limited to [0, 10^15], so
> changing int64_t -> uint64_t is not a problem.
>
> I have already sent a patch that changes the fields in the data
> structure in throttle.h from double to uint64_t

I expect v2 to be based on your patch.  Might simplify the commit
message.

> Reviewed-by: Alberto Garcia <berto@igalia.com>

Thanks!