On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote:
>On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote:
>> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote:
>>>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote:
>>>>>> So basically if we have anonymous groups, we accept limits in the
>>>>>> driver options but only without a group-name.
>>>>>
>>>>>In the commit message you do however have limits and a group name, is
>>>>>that a mistake?
>>>>>
>>>>>    -drive driver=throttle,file.filename=foo.qcow2, \
>>>>>           limits.iops-total=...,throttle-group=bar
>>>>
>>>> Sorry this wasn't clear, I'm actually proposing to remove limits from
>>>> the throttle driver options and only create/config throttle groups via
>>>> -object/object-add.
>>>
>>>Sorry I think it was me who misunderstood :-) Anyway in the new
>>>command-line API I would be more inclined to have limits defined using
>>>"-object throttle-group" and -drive would only reference the group id.
>>>
>>>I understand that this implies that it wouldn't be possible to create
>>>anonymous groups (at least not from the command line), is that a
>>>problem?
>>
>> We can accept anonymous groups if a user specifies limits but not a
>> group name in the throttle driver. (The only case where limits would
>> be acccepted)
>
>Yeah but that's only if we have the limits.iops-total=... options in the
>throttle driver. If we "remove limits from the throttle driver options
>and only create/config throttle groups via -object/object-add" we 
>cannot
>do that.

We can check that groups is not defined at the same time as limits,
>
>> Not creating eponymous throttle groups via the throttle driver means
>> we don't need throttle_groups anymore, since even anonymous ones don't
>> need to be accounted for in a list.
>
>I don't follow you here, how else do you get a group by its name?

If all eponymous groups are managed by the QOM tree, we should be able 
to iterate over the object root container for all ThrottleGroups just 
like qmp_query_iothreads() in iothread.c