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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature