Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure

[Manos Pitsidianakis <el13635@mail.ntua.gr>]

[-- Attachment #1: Type: text/plain, Size: 5534 bytes --]

On Thu, Sep 14, 2017 at 06:40:06AM -0400, Pradeep Jagadeesh wrote:
>This patch factors out code to use the ThrottleLimits
>strurcture.
>
>Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>Reviewed-by: Greg Kurz <groug@kaod.org>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>Reviewed-by: Alberto Garcia <berto@igalia.com>
>Reviewed-by: Markus Armbruster <armbru@redhat.com>
>---
> qapi/block-core.json | 78 +++-------------------------------------------------
> 1 file changed, 4 insertions(+), 74 deletions(-)
>
>diff --git a/qapi/block-core.json b/qapi/block-core.json
>index bb11815..d0ccfda 100644
>--- a/qapi/block-core.json
>+++ b/qapi/block-core.json
>@@ -1826,84 +1826,13 @@
> #
> # @device: Block device name (deprecated, use @id instead)
> #
>-# @id: The name or QOM path of the guest device (since: 2.8)
>-#
>-# @bps: total throughput limit in bytes per second
>-#
>-# @bps_rd: read throughput limit in bytes per second
>-#
>-# @bps_wr: write throughput limit in bytes per second
>-#
>-# @iops: total I/O operations per second
>-#
>-# @iops_rd: read I/O operations per second
>-#
>-# @iops_wr: write I/O operations per second
>-#
>-# @bps_max: total throughput limit during bursts,
>-#                     in bytes (Since 1.7)
>-#
>-# @bps_rd_max: read throughput limit during bursts,
>-#                        in bytes (Since 1.7)
>-#
>-# @bps_wr_max: write throughput limit during bursts,
>-#                        in bytes (Since 1.7)
>-#
>-# @iops_max: total I/O operations per second during bursts,
>-#                      in bytes (Since 1.7)
>-#
>-# @iops_rd_max: read I/O operations per second during bursts,
>-#                         in bytes (Since 1.7)
>-#
>-# @iops_wr_max: write I/O operations per second during bursts,
>-#                         in bytes (Since 1.7)
>-#
>-# @bps_max_length: maximum length of the @bps_max burst
>-#                            period, in seconds. It must only
>-#                            be set if @bps_max is set as well.
>-#                            Defaults to 1. (Since 2.6)
>-#
>-# @bps_rd_max_length: maximum length of the @bps_rd_max
>-#                               burst period, in seconds. It must only
>-#                               be set if @bps_rd_max is set as well.
>-#                               Defaults to 1. (Since 2.6)
>-#
>-# @bps_wr_max_length: maximum length of the @bps_wr_max
>-#                               burst period, in seconds. It must only
>-#                               be set if @bps_wr_max is set as well.
>-#                               Defaults to 1. (Since 2.6)
>-#
>-# @iops_max_length: maximum length of the @iops burst
>-#                             period, in seconds. It must only
>-#                             be set if @iops_max is set as well.
>-#                             Defaults to 1. (Since 2.6)
>-#
>-# @iops_rd_max_length: maximum length of the @iops_rd_max
>-#                                burst period, in seconds. It must only
>-#                                be set if @iops_rd_max is set as well.
>-#                                Defaults to 1. (Since 2.6)
>-#
>-# @iops_wr_max_length: maximum length of the @iops_wr_max
>-#                                burst period, in seconds. It must only
>-#                                be set if @iops_wr_max is set as well.
>-#                                Defaults to 1. (Since 2.6)
>-#
>-# @iops_size: an I/O size in bytes (Since 1.7)
>-#
> # @group: throttle group name (Since 2.4)

BlockIOThrottle and ThrottleLimits are not directly compatible, for 
example here we have iops_rd_max_length as a name,

> #
> # 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',
>-            '*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' } }
>+  'base': 'ThrottleLimits',
>+  'data': { '*device': 'str', '*group': 'str' } }
>
> ##
> # @ThrottleLimits:
>@@ -1913,6 +1842,7 @@
> # transaction. All fields are optional. When setting limits, if a field is
> # missing the current value is not changed.
> #
>+# @id:                     device id
> # @iops-total:             limit total I/O operations per second
> # @iops-total-max:         I/O operations burst
> # @iops-total-max-length:  length of the iops-total-max burst period, in seconds
>@@ -1942,7 +1872,7 @@
> # Since: 2.11
> ##
> { 'struct': 'ThrottleLimits',
>-  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>+  'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
>             '*iops-total-max-length' : 'int', '*iops-read' : 'int',
>             '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
>             '*iops-write' : 'int', '*iops-write-max' : 'int',

And here it's iops-read-max-length. This breaks the block layer's old 
throttling API. Do we want the old API replaced for 2.11? I don't know 
which maintainer is responsible for this, so I CC'd armbru.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]