[Qemu-devel] Throttling groups vs filter nodes

[Qemu-devel] Throttling groups vs filter nodes

[Stefan Hajnoczi]

Throttling groups allow multiple drives to share the same throttling
state (i.e. budget) between them.  Manos is working on moving the
throttling code into a block filter driver so it is no longer
hardcoded into the I/O code path.

Throttling groups are not defined explicitly using -object syntax.
Instead they are brought into existence by referring to them by name:
-drive throttling.group=group0.

A quirk in the current implementation is that the throttling limits
for the group are overwritten by each -drive throttling.group=group0.
Limits for all but the last -drive in a group are ignored.

There is no way to associate with an existing throttling group while
keeping current limits in place.  The caller must pass in desired
limits with at least the last -drive (and with every hotplugged
drive).

The new throttling filter node could do things differently:
If *no* limits were specified (i.e. iops, bps, etc) then keep existing
limits for the group in place.

These semantics are more convenient - especially for hotplugging
drives after the guest has launched.

Manos: I suggest implementing this new behavior when you write the
throttling filter driver.  The code needs to check that all throttle
cfg fields are 0.  There are no backwards compatibility concerns since
the throttle filter is new and existing users don't rely on it.

Thoughts?

Stefan

Re: [Qemu-devel] Throttling groups vs filter nodes

[Alberto Garcia]

On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote:
> A quirk in the current implementation is that the throttling limits
> for the group are overwritten by each -drive throttling.group=group0.
> Limits for all but the last -drive in a group are ignored.
>
> There is no way to associate with an existing throttling group while
> keeping current limits in place.  The caller must pass in desired
> limits with at least the last -drive (and with every hotplugged
> drive).

I actually just realized that passing throttling.group=group0 but no
actual limits is allowed but does nothing.

> The new throttling filter node could do things differently: If *no*
> limits were specified (i.e. iops, bps, etc) then keep existing limits
> for the group in place.

Yes, that sounds good to me. The problem here is block_set_io_throttle
(the QMP command), becuase the bps / iops parameters are mandatory, and
setting them to 0 disables the I/O limits for that device.

We can of course make them optional and if all limits are unset then we
can add a device to a throttling group. I'm not sure if that would make
the command a bit too complicated, as there are already different ways
to use it:

   - bps or iops != 0   -> set the I/O limits of a throttling group. The
                           selected device is moved to that group if it
                           wasn't there yet.

   - bps and iops == 0  -> remove a device from a throttling group
                           without touching that group's I/O limits.

and a new one would be:

   - bps and iops unset -> add a device to a throttling group, without
                           touching that group's I/O limits.

Berto

Re: [Qemu-devel] Throttling groups vs filter nodes

[Kevin Wolf]

Am 27.05.2017 um 09:56 hat Stefan Hajnoczi geschrieben:
> Throttling groups allow multiple drives to share the same throttling
> state (i.e. budget) between them.  Manos is working on moving the
> throttling code into a block filter driver so it is no longer
> hardcoded into the I/O code path.
> 
> Throttling groups are not defined explicitly using -object syntax.
> Instead they are brought into existence by referring to them by name:
> -drive throttling.group=group0.

We could still add -object for throttling groups if that allows us to
use a cleaner syntax.

> A quirk in the current implementation is that the throttling limits
> for the group are overwritten by each -drive throttling.group=group0.
> Limits for all but the last -drive in a group are ignored.
> 
> There is no way to associate with an existing throttling group while
> keeping current limits in place.  The caller must pass in desired
> limits with at least the last -drive (and with every hotplugged
> drive).
> 
> The new throttling filter node could do things differently:
> If *no* limits were specified (i.e. iops, bps, etc) then keep existing
> limits for the group in place.
> 
> These semantics are more convenient - especially for hotplugging
> drives after the guest has launched.
> 
> Manos: I suggest implementing this new behavior when you write the
> throttling filter driver.  The code needs to check that all throttle
> cfg fields are 0.  There are no backwards compatibility concerns since
> the throttle filter is new and existing users don't rely on it.

If we implement things this way, we shouldn't test that all fields are
0, but that the limits are simply not given. In QAPI, I think we get
something like this then:

{ 'struct': 'BlockdevOptionsThrottle',
  'data': { 'image': 'BlockdevRef',
            '*limits': 'ThrottleLimits',
            '*group': 'str' } }

Callers must either pass 'limits' or 'group', but formally they are both
optional. The first time that a group is referenced, giving limits as
well is mandatory, afterwards it is forbidden.

If we use a separate QOM object with -object, it always becomes either
limits or groups, i.e. we could use a QAPI union here.


Another interesting question is how the limits are updated after
creating the first throttle node of the group. With -object, I guess
this would simply become qom-set commands.

Without it, we would probably want to use bdrv_reopen() - with some
strange effects like bdrv_reopen() of one throttle node affecting other
throttle nodes, even though their bs->options/explicit_options don't
represent this. Another bdrv_reopen() on a second throttling node, even
if it just wants to update some unrelated option, say 'read-only', could
end up switching throttling back to old configuration values as they are
recorded in that other node's bs->options. Requiring that limits can
only be changed via the node that initially created the group isn't a
solution either because it could have been closed while the throttle
group is still in use by different images.

After writing this, my gut feeling is that -object might well be worth
it.

Kevin

Re: [Qemu-devel] Throttling groups vs filter nodes

[Manos Pitsidianakis]

On Mon, May 29, 2017 at 05:05:17PM +0200, Alberto Garcia wrote:
>On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote:
>> A quirk in the current implementation is that the throttling limits
>> for the group are overwritten by each -drive throttling.group=group0.
>> Limits for all but the last -drive in a group are ignored.
>   - bps or iops != 0   -> set the I/O limits of a throttling group. The
>                           selected device is moved to that group if it
>                           wasn't there yet.
>
>   - bps and iops == 0  -> remove a device from a throttling group
>                           without touching that group's I/O limits.

These are very unintuitive. However, even without considering backwards 
compatibility, I think that using -object notation (eg "object-add 
throttle-group,id=foo,iops=...) is intuitive in the case of groups, but 
not when you need individual limits for each device as the syntax would 
be too verbose.  Of course the old interface covers that.  

In any case, is having multiple interfaces a problem or not? And, is 
using QOM straightforward implementation-wise?

Re: [Qemu-devel] Throttling groups vs filter nodes

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 187 bytes --]

On Mon, May 29, 2017 at 05:50:36PM +0200, Kevin Wolf wrote:
> After writing this, my gut feeling is that -object might well be worth
> it.

Yes, a real -object would be cleanest.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] Throttling groups vs filter nodes

[Kevin Wolf]

[ Cc: qemu-block - noticed only now that it's missing ]

Am 29.05.2017 um 22:57 hat Manos Pitsidianakis geschrieben:
> On Mon, May 29, 2017 at 05:05:17PM +0200, Alberto Garcia wrote:
> >On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote:
> >>A quirk in the current implementation is that the throttling limits
> >>for the group are overwritten by each -drive throttling.group=group0.
> >>Limits for all but the last -drive in a group are ignored.
> >  - bps or iops != 0   -> set the I/O limits of a throttling group. The
> >                          selected device is moved to that group if it
> >                          wasn't there yet.
> >
> >  - bps and iops == 0  -> remove a device from a throttling group
> >                          without touching that group's I/O limits.
> 
> These are very unintuitive.

I agree, this is not an interface to extend, but one to get rid of. (Of
course, we'll have to keep it around for a while because compatibility,
but we should try to offer something better.)

> However, even without considering backwards compatibility, I think
> that using -object notation (eg "object-add
> throttle-group,id=foo,iops=...) is intuitive in the case of groups,
> but not when you need individual limits for each device as the syntax
> would be too verbose.  Of course the old interface covers that.
> 
> In any case, is having multiple interfaces a problem or not? And, is
> using QOM straightforward implementation-wise?

We can have an interface for the throttling node that requires that you
specify either a throttle group object name or the limits, but never
both. If you specify the limits, this would just be a convenience
function that creates the right QOM object internally.

As for the implementation, QOM tends to be a bit heavy on boilerplate
code, but I think it's not too bad otherwise.

Kevin

Re: [Qemu-devel] Throttling groups vs filter nodes

[Alberto Garcia]

On Mon 29 May 2017 10:57:37 PM CEST, Manos Pitsidianakis wrote:
>>> A quirk in the current implementation is that the throttling limits
>>> for the group are overwritten by each -drive throttling.group=group0.
>>> Limits for all but the last -drive in a group are ignored.
>>   - bps or iops != 0   -> set the I/O limits of a throttling group. The
>>                           selected device is moved to that group if it
>>                           wasn't there yet.
>>
>>   - bps and iops == 0  -> remove a device from a throttling group
>>                           without touching that group's I/O limits.
>
> These are very unintuitive. However, even without considering
> backwards compatibility, I think that using -object notation (eg
> "object-add throttle-group,id=foo,iops=...) is intuitive in the case
> of groups, but not when you need individual limits for each device as
> the syntax would be too verbose.  Of course the old interface covers
> that.

Yes, that's clear. I think that when adding drives, the command-line UI
-drive throttling.iops=XXX notation is good enough for individual
devices and the -object notation is good for groups.

My question was about the block_set_io_throttle QMP command, which you
can use to change the limits of existing drives.

But I think Kevin is right and we should probably think of an
alternative interface for that.

Berto

Re: [Qemu-devel] Throttling groups vs filter nodes

[Alberto Garcia]

On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote:
> Throttling groups allow multiple drives to share the same throttling
> state (i.e. budget) between them.  Manos is working on moving the
> throttling code into a block filter driver so it is no longer
> hardcoded into the I/O code path.

Now that we're discussing changes to the throttling code, I'll take the
opportunity to describe a feature that I'd like to work on at some point
in the future.

This is of course out of the scope of the work that Manos is going to
do, but I'd like to discuss it briefly to see what people think of this
idea and whether it's compatible with the changes that we are now
proposing.

Currently each block device can have either zero or one set of I/O
limits, and that set of limits can be shared among several drives using
throttling groups.

I have the following use case that cannot be handled with the current
code:

   - The provider offers the user different storage types, and each one
     of them can have a different storage backend and a different set of
     I/O limits.

   - Inside each storage type, the user can have several volumes, each
     one with its own set of I/O limits.

   - So the user could have drive A, B and C, each one of them with its
     own separate limits, and in addition to that the combination of all
     three would have a limit specific to the storage type.

I believe that this could be achieved by simply having two filter nodes
per drive, one of them would be attached to a throttling object specific
to that drive, and the other one would be attached to a throttling
object that would be shared by all drives (i.e. it would be a 3-drive
throttling group).

I think this should be possible with the -object approach that we are
discussing.

Thoughts?

Berto

Re: [Qemu-devel] Throttling groups vs filter nodes

[Kevin Wolf]

Am 30.05.2017 um 16:29 hat Alberto Garcia geschrieben:
> On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote:
> > Throttling groups allow multiple drives to share the same throttling
> > state (i.e. budget) between them.  Manos is working on moving the
> > throttling code into a block filter driver so it is no longer
> > hardcoded into the I/O code path.
> 
> Now that we're discussing changes to the throttling code, I'll take the
> opportunity to describe a feature that I'd like to work on at some point
> in the future.
> 
> This is of course out of the scope of the work that Manos is going to
> do, but I'd like to discuss it briefly to see what people think of this
> idea and whether it's compatible with the changes that we are now
> proposing.
> 
> Currently each block device can have either zero or one set of I/O
> limits, and that set of limits can be shared among several drives using
> throttling groups.
> 
> I have the following use case that cannot be handled with the current
> code:
> 
>    - The provider offers the user different storage types, and each one
>      of them can have a different storage backend and a different set of
>      I/O limits.
> 
>    - Inside each storage type, the user can have several volumes, each
>      one with its own set of I/O limits.
> 
>    - So the user could have drive A, B and C, each one of them with its
>      own separate limits, and in addition to that the combination of all
>      three would have a limit specific to the storage type.
> 
> I believe that this could be achieved by simply having two filter nodes
> per drive, one of them would be attached to a throttling object specific
> to that drive, and the other one would be attached to a throttling
> object that would be shared by all drives (i.e. it would be a 3-drive
> throttling group).
> 
> I think this should be possible with the -object approach that we are
> discussing.

Yes, that should be possible. In fact, I don't think it's out of scope
for Manos' work, but it should just natually fall out of it.

That you can put throttle filter nodes anywhere in the graph (and
therefore also stack multiple throttle filter nodes on top of each
other) is the whole point of doing this work, so I would consider this a
very basic requirement for any configuration interface that we could
come up wih.

Kevin