Re: [Qemu-block] [PATCH v9 4/6] block: convert ThrottleGroup to object with QOM

[Kevin Wolf]

Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
> ThrottleGroup is converted to an object. This will allow the future
> throttle block filter drive easy creation and configuration of throttle
> groups in QMP and cli.
> 
> A new QAPI struct, ThrottleLimits, is introduced to provide a shared
> struct for all throttle configuration needs in QMP.
> 
> ThrottleGroups can be created via CLI as
> -object throttle-group,id=foo,x-iops-total=100,x-..
> where x-* are individual limit properties. Since we can't add non-scalar
> properties in -object this interface must be used instead. However,
> setting these properties must be disabled after initialization because
> certain combinations of limits are forbidden and thus configuration
> changes should be done in one transaction. The individual properties
> will go away when support for non-scalar values in CLI is implemented
> and thus are marked as experimental.
> 
> ThrottleGroup also has a `limits` property that uses the ThrottleLimits
> struct.  It can be used to create ThrottleGroups or set the
> configuration in existing groups as follows:
> 
> { "execute": "object-add",
>   "arguments": {
> "qom-type": "throttle-group",
> "id": "foo",
> "props" : {
>   "limits": {
>   "iops-total": 100
>   }
> }
>   }
> }
> { "execute" : "qom-set",
> "arguments" : {
> "path" : "foo",
> "property" : "limits",
> "value" : {
> "iops-total" : 99
> }
> }
> }
> 
> This also means a group's configuration can be fetched with qom-get.
> 
> Signed-off-by: Manos Pitsidianakis 

> +static bool throttle_group_can_be_deleted(UserCreatable *uc, Error **errp)
> +{
> +return OBJECT(uc)->ref == 1;
> +}
> +
> +static void throttle_group_obj_class_init(ObjectClass *klass, void 
> *class_data)
> +{
> +size_t i = 0;
> +UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
> +
> +ucc->complete = throttle_group_obj_complete;
> +ucc->can_be_deleted = throttle_group_can_be_deleted;

block/throttle-groups.c: In function 'throttle_group_obj_class_init':
block/throttle-groups.c:880:25: error: assignment from incompatible pointer 
type [-Werror=incompatible-pointer-types]
 ucc->can_be_deleted = throttle_group_can_be_deleted;

This is from a semantic conflict with commit 3beacfb9 ('qom: Remove
unused errp parameter from can_be_deleted()'). If the rest of the series
is okay, I'll just remove the **errp parameter while applying.

Kevin

Re: [Qemu-block] [PATCH v9 4/6] block: convert ThrottleGroup to object with QOM

[Stefan Hajnoczi]

On Fri, Aug 25, 2017 at 04:20:26PM +0300, Manos Pitsidianakis wrote:
> ThrottleGroup is converted to an object. This will allow the future
> throttle block filter drive easy creation and configuration of throttle
> groups in QMP and cli.
> 
> A new QAPI struct, ThrottleLimits, is introduced to provide a shared
> struct for all throttle configuration needs in QMP.
> 
> ThrottleGroups can be created via CLI as
> -object throttle-group,id=foo,x-iops-total=100,x-..
> where x-* are individual limit properties. Since we can't add non-scalar
> properties in -object this interface must be used instead. However,
> setting these properties must be disabled after initialization because
> certain combinations of limits are forbidden and thus configuration
> changes should be done in one transaction. The individual properties
> will go away when support for non-scalar values in CLI is implemented
> and thus are marked as experimental.
> 
> ThrottleGroup also has a `limits` property that uses the ThrottleLimits
> struct.  It can be used to create ThrottleGroups or set the
> configuration in existing groups as follows:
> 
> { "execute": "object-add",
>   "arguments": {
> "qom-type": "throttle-group",
> "id": "foo",
> "props" : {
>   "limits": {
>   "iops-total": 100
>   }
> }
>   }
> }
> { "execute" : "qom-set",
> "arguments" : {
> "path" : "foo",
> "property" : "limits",
> "value" : {
> "iops-total" : 99
> }
> }
> }
> 
> This also means a group's configuration can be fetched with qom-get.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  qapi/block-core.json|  48 +
>  include/block/throttle-groups.h |   3 +
>  include/qemu/throttle-options.h |  59 --
>  include/qemu/throttle.h |   3 +
>  block/throttle-groups.c | 422 
> 
>  tests/test-throttle.c   |   1 +
>  util/throttle.c | 151 ++
>  7 files changed, 627 insertions(+), 60 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-block] [PATCH v9 4/6] block: convert ThrottleGroup to object with QOM

[Alberto Garcia]

On Fri 25 Aug 2017 03:20:26 PM CEST, Manos Pitsidianakis wrote:
> ThrottleGroup is converted to an object. This will allow the future
> throttle block filter drive easy creation and configuration of throttle
> groups in QMP and cli.

Reviewed-by: Alberto Garcia 

Berto

[Qemu-block] [PATCH v9 4/6] block: convert ThrottleGroup to object with QOM

[Manos Pitsidianakis]

ThrottleGroup is converted to an object. This will allow the future
throttle block filter drive easy creation and configuration of throttle
groups in QMP and cli.

A new QAPI struct, ThrottleLimits, is introduced to provide a shared
struct for all throttle configuration needs in QMP.

ThrottleGroups can be created via CLI as
-object throttle-group,id=foo,x-iops-total=100,x-..
where x-* are individual limit properties. Since we can't add non-scalar
properties in -object this interface must be used instead. However,
setting these properties must be disabled after initialization because
certain combinations of limits are forbidden and thus configuration
changes should be done in one transaction. The individual properties
will go away when support for non-scalar values in CLI is implemented
and thus are marked as experimental.

ThrottleGroup also has a `limits` property that uses the ThrottleLimits
struct.  It can be used to create ThrottleGroups or set the
configuration in existing groups as follows:

{ "execute": "object-add",
  "arguments": {
"qom-type": "throttle-group",
"id": "foo",
"props" : {
  "limits": {
  "iops-total": 100
  }
}
  }
}
{ "execute" : "qom-set",
"arguments" : {
"path" : "foo",
"property" : "limits",
"value" : {
"iops-total" : 99
}
}
}

This also means a group's configuration can be fetched with qom-get.

Signed-off-by: Manos Pitsidianakis 
---
 qapi/block-core.json|  48 +
 include/block/throttle-groups.h |   3 +
 include/qemu/throttle-options.h |  59 --
 include/qemu/throttle.h |   3 +
 block/throttle-groups.c | 422 
 tests/test-throttle.c   |   1 +
 util/throttle.c | 151 ++
 7 files changed, 627 insertions(+), 60 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 833c602150..0bdc69aa5f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1905,6 +1905,54 @@
 '*iops_size': 'int', '*group': 'str' } }
 
 ##
+# @ThrottleLimits:
+#
+# Limit parameters for throttling.
+# Since some limit combinations are illegal, limits should always be set in one
+# transaction. All fields are optional. When setting limits, if a field is
+# missing the current value is not changed.
+#
+# @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
+#  It must only be set if @iops-total-max is set as 
well.
+# @iops-read:  limit read operations per second
+# @iops-read-max:  I/O operations read burst
+# @iops-read-max-length:   length of the iops-read-max burst period, in seconds
+#  It must only be set if @iops-read-max is set as 
well.
+# @iops-write: limit write operations per second
+# @iops-write-max: I/O operations write burst
+# @iops-write-max-length:  length of the iops-write-max burst period, in 
seconds
+#  It must only be set if @iops-write-max is set as 
well.
+# @bps-total:  limit total bytes per second
+# @bps-total-max:  total bytes burst
+# @bps-total-max-length:   length of the bps-total-max burst period, in 
seconds.
+#  It must only be set if @bps-total-max is set as 
well.
+# @bps-read:   limit read bytes per second
+# @bps-read-max:   total bytes read burst
+# @bps-read-max-length:length of the bps-read-max burst period, in seconds
+#  It must only be set if @bps-read-max is set as well.
+# @bps-write:  limit write bytes per second
+# @bps-write-max:  total bytes write burst
+# @bps-write-max-length:   length of the bps-write-max burst period, in seconds
+#  It must only be set if @bps-write-max is set as 
well.
+# @iops-size:  when limiting by iops max size of an I/O in bytes
+#
+# Since: 2.11
+##
+{ 'struct': 'ThrottleLimits',
+  'data': { '*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',
+'*iops-write-max-length' : 'int', '*bps-total' : 'int',
+'*bps-total-max' : 'int', '*bps-total-max-length' : 'int',
+'*bps-read' : 'int', '*bps-read-max' : 'int',
+'*bps-read-max-length' : 'int', '*bps-write' : 'int',
+'*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
+'*iops-size' : 'int' } }
+
+##
 # @block-stream:
 #
 # Copy data from a backing file into a block device.
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index