Re: [Qemu-devel] [PATCH 9/9] throttle: add name of ThrottleGroup to BlockDeviceInfo

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 02/26/2015 06:56 AM, Alberto Garcia wrote:
>> On Wed, Feb 25, 2015 at 08:23:10AM -0700, Eric Blake wrote:
>> 
>>> How about query-block-throttle, returning an array of dicts.
>>>
>>> => { "execute":"query-block-throttle" }
>>> <= { "return": [
>>>      { "name": "throttle1", "bps_max": 100000,
>>>        "nodes": [ "block0", "block1" ] },
>>>      { "name": "throttle2", "iops_max": 10000,
>>>        "nodes": [ "block2" ] }
>>>    ] }
>> 
>> Ok I wrote it, however I have some doubts about what to put exactly in
>> the 'nodes' array.
>> 
>> We can just put node names as you suggest, however at the moment not
>> all nodes have a name so we could end up with a list of empty strings.
>
> At one point, we were considering a patch from Jeff Cody that guarantees
> ALL nodes have a name.  Maybe that's still worthwhile.
>
>> 
>> I think the easiest solution in terms of lines of code and simplicity
>> of the return type is this one:
>> 
>> { 'type': 'ThrottleGroupInfo',
>>   'data': { 'name': 'str', 'nodes': ['BlockDeviceInfo'] } }
>
> That's a bit more verbose, but also a bit more usable (all the block
> data is available, rather than just a name that has to be looked up via
> another command), so that could work.
>
>> 
>> { 'command': 'query-block-throttle',
>>   'returns': ['ThrottleGroupInfo'] }
>> 
>> All the information is there, the code to fill the BlockDeviceInfo
>> structure is already written so I can just make use of it.
>> 
>> The throttling settings themselves are not present in
>> ThrottleGroupInfo, but since all nodes from a group have the same
>> settings, you can get them from any of them. It also keeps the
>> ThrottleGroupInfo structure simpler.
>> 
>> What do you think? If you're ok with this solution I can submit the
>> patch series again.
>
> Sure, sounds like it's worth that solution, since it reused code and
> made for less work on your part.

We need a coherent set of block queries.  What we have is an ad hoc
mess.  Alberto proposes to add to the mess.  Digs us deeper into the
hole.  But asking him to solve the complete block query problem so he
can have his little feature is hardly fair.