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)
#
# 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',
--
1.8.3.1
On Thu, 14 Sep 2017 06:40:06 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> 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 +++-------------------------------------------------
Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
patch before...
> 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)
> #
> # 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',
On 9/14/2017 1:19 PM, Greg Kurz wrote:
> On Thu, 14 Sep 2017 06:40:06 -0400
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> 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 +++-------------------------------------------------
>
> Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
> patch before...
I did rebase the code on 2.10
Now I am using the ThrottleLimits structure that was introduced IN 2.10.
This is same as the IOThrottle structure.
So, many things got changed in this version.
-Pradeep
>
>> 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)
>> #
>> # 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',
>
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.
On 9/18/2017 6:04 PM, Manos Pitsidianakis wrote:
> 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.
I do not think its going to break. It just exist we neither try to set
that or to query.
-Pradeep
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)
> #
> # 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',
>--
>1.8.3.1
>
Why is id needed? ThrottleLimits is not per group, not per device. And
as I mentioned in another reply, this changes the block_set_io_throttle
command which is not simple refactoring.
On 9/18/2017 6:25 PM, Manos Pitsidianakis wrote:
> 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)
>> #
>> # 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',
>> --
>> 1.8.3.1
>>
>
> Why is id needed? ThrottleLimits is not per group, not per device. And
> as I mentioned in another reply, this changes the block_set_io_throttle
> command which is not simple refactoring.
When you use qmp/hmp interface the id is used to set/get throttle
configuration. So, I thought if id is part of the throttle config, it
will be useful. Now I will try to move it to just for fsdeviothrottle.
-Pradeep
On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote: > This patch factors out code to use the ThrottleLimits > strurcture. s/strurcture/structure/ > > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> > Reviewed-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Eric Blake <eblake@redhat.com> Echoing the sentiments made elsewhere - this is a significant rewrite from the earlier version, so you WANT reviewers to look at it fresh rather than assuming that previous reviews still apply. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 9/18/2017 7:10 PM, Eric Blake wrote: > On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote: >> This patch factors out code to use the ThrottleLimits >> strurcture. > > s/strurcture/structure/ > >> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >> Reviewed-by: Greg Kurz <groug@kaod.org> >> Reviewed-by: Eric Blake <eblake@redhat.com> > > Echoing the sentiments made elsewhere - this is a significant rewrite > from the earlier version, so you WANT reviewers to look at it fresh > rather than assuming that previous reviews still apply. > OK, but shall I send them as V12 or V01 again? -Pradeep
On 09/19/2017 05:06 AM, Pradeep Jagadeesh wrote: > On 9/18/2017 7:10 PM, Eric Blake wrote: >> On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote: >>> This patch factors out code to use the ThrottleLimits >>> strurcture. >> >> s/strurcture/structure/ >> >>> >>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >>> Reviewed-by: Greg Kurz <groug@kaod.org> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >> >> Echoing the sentiments made elsewhere - this is a significant rewrite >> from the earlier version, so you WANT reviewers to look at it fresh >> rather than assuming that previous reviews still apply. >> > OK, but shall I send them as V12 or V01 again? v12 is fine - you really are revising the patches a twelfth time. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, 19 Sep 2017 12:06:26 +0200
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:
> On 9/18/2017 7:10 PM, Eric Blake wrote:
> > On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote:
> >> This patch factors out code to use the ThrottleLimits
> >> strurcture.
> >
> > s/strurcture/structure/
> >
> >>
> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> >> Reviewed-by: Greg Kurz <groug@kaod.org>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >
> > Echoing the sentiments made elsewhere - this is a significant rewrite
> > from the earlier version, so you WANT reviewers to look at it fresh
> > rather than assuming that previous reviews still apply.
> >
> OK, but shall I send them as V12 or V01 again?
>
The version is rather tied to the whole patchset than to each
individual patch. The next round will be the 12th, so all
patches should have v12 in the Subject.
> -Pradeep
>
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/LTC http://www.ibm.com
Tel 33-5-6218-1607
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
© 2016 - 2026 Red Hat, Inc.