From: Eric Blake <eblake@redhat.com>
To: Alberto Garcia <berto@igalia.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 9/9] throttle: add name of ThrottleGroup to BlockDeviceInfo
Date: Tue, 03 Mar 2015 10:53:05 -0700 [thread overview]
Message-ID: <54F5F501.1020802@redhat.com> (raw)
In-Reply-To: <20150226135606.GA11033@igalia.com>
[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2015-03-03 17:53 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-13 16:06 [Qemu-devel] [PATCH v2 0/9] Block Throttle Group Support Alberto Garcia
2015-02-13 16:06 ` [Qemu-devel] [PATCH 1/9] throttle: Extract timers from ThrottleState into a separate ThrottleTimers structure Alberto Garcia
2015-03-03 16:03 ` Stefan Hajnoczi
2015-02-13 16:06 ` [Qemu-devel] [PATCH 2/9] throttle: Add throttle group infrastructure Alberto Garcia
2015-03-03 16:38 ` Stefan Hajnoczi
2015-03-04 10:18 ` Alberto Garcia
2015-03-04 16:02 ` Stefan Hajnoczi
2015-02-13 16:06 ` [Qemu-devel] [PATCH 3/9] throttle: Add throttle group infrastructure tests Alberto Garcia
2015-02-13 16:06 ` [Qemu-devel] [PATCH 4/9] throttle: Prepare to have multiple timers for one ThrottleState Alberto Garcia
2015-03-03 16:50 ` Stefan Hajnoczi
2015-02-13 16:06 ` [Qemu-devel] [PATCH 5/9] throttle: Add a way to know if throttle_schedule_timer had armed a timer Alberto Garcia
2015-02-13 16:06 ` [Qemu-devel] [PATCH 6/9] throttle: Add a way to fire one of the timers asap like a bottom half Alberto Garcia
2015-03-03 17:08 ` Stefan Hajnoczi
2015-02-13 16:06 ` [Qemu-devel] [PATCH 7/9] throttle: Add throttle group support Alberto Garcia
2015-02-24 16:45 ` Eric Blake
2015-02-24 16:47 ` Eric Blake
2015-03-03 21:00 ` Stefan Hajnoczi
2015-03-04 13:53 ` Alberto Garcia
2015-03-04 16:04 ` Stefan Hajnoczi
2015-03-04 16:16 ` Alberto Garcia
2015-03-05 17:41 ` Stefan Hajnoczi
2015-02-13 16:06 ` [Qemu-devel] [PATCH 8/9] throttle: Update throttle infrastructure copyright Alberto Garcia
2015-02-24 16:49 ` Eric Blake
2015-02-24 20:21 ` Benoît Canet
2015-02-13 16:06 ` [Qemu-devel] [PATCH 9/9] throttle: add name of ThrottleGroup to BlockDeviceInfo Alberto Garcia
2015-02-24 16:54 ` Eric Blake
2015-02-25 10:56 ` Alberto Garcia
2015-02-25 15:23 ` Eric Blake
2015-02-25 15:37 ` Alberto Garcia
2015-02-26 13:56 ` Alberto Garcia
2015-03-03 17:53 ` Eric Blake [this message]
2015-03-04 7:09 ` Markus Armbruster
2015-03-04 7:20 ` Alberto Garcia
2015-03-03 21:07 ` [Qemu-devel] [PATCH v2 0/9] Block Throttle Group Support Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54F5F501.1020802@redhat.com \
--to=eblake@redhat.com \
--cc=berto@igalia.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.