qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Proposal] I/O throttling through new -object interface


From: Manos Pitsidianakis
Subject: Re: [Qemu-devel] [Proposal] I/O throttling through new -object interface
Date: Fri, 9 Jun 2017 19:06:09 +0300
User-agent: NeoMutt/20170113 (1.7.2)

On Fri, Jun 09, 2017 at 03:42:41PM +0200, Alberto Garcia wrote:
Hey Manos, thanks for your proposal and for the good work :)

On Thu 08 Jun 2017 08:21:13 PM CEST, Manos Pitsidianakis wrote:

As part of my GSoC project, IO throttling is moved to a block filter
driver, which means multiple filter nodes sharing ThrottleGroup
instances, and also, chaining filter nodes. The old interface will be
retained as much as possible. By defining ThrottleGroups with QOM the
following interface can be used:
    -drive file=foo.qcow2,throttling.group=bar \
    -object throttle-group,iops-total=100,id=bar

It should be --object

Both seem to have the result in my qemu build.

, and I guess the name would be throttling-group?

Hm, I thought throttle-group would be nicer because it's shorter, but I can change it, of course. I am not certain if there's a semantic difference (because I am not a native speaker).


'block_set_io_throttle' command can be extended with a "group"
argument, that when defined instead of "device", will find and edit
the object in question.

Note that block_set_io_throttle() already has a 'group' argument. Its
current semantics are:

  - 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.

where 'device' is mandatory and 'group' defaults to the value of
'device' if unset.

If I get it right, what you propose is that 'device' is optional and in
that case 'group' is mandatory and the command updates the throttling
group parameters.

Note that you can do that already if you select a device that is already
in the group. These new semantics would allow modifying a group that is
not being used by any block device.

That was my thinking as well.

I'm not a big fan of overloading block_set_io_throttle() even more, but
this proposal does actually sound fine.

Users can hotplug ThrottleGroups with object-add in QMP/HMP, then
attach drives to a throttle group (ie adding a filter node in the
drive path that points to that throttle group) with an
'attach-throttle' and detach with 'detach-throttle', each taking the
drive and throttle group name as arguments. An alternative (but
uglier) approach would be to use 'block_set_io_throttle' for this as
well.

Yeah I agree that the block_set_io_throttle() approach would be uglier.

Something that's also sound is a 'block_set_throttle_groups' that
takes a list of groups and a drive as arguments.

Would that simply be a foreach(g, groups) { attach(device, g); } ?

Something like
foreach(g, groups) { attach(device, g); }
foreach(g, device.groups - groups) { detach(device, g); } #where '-' is the set minus operation)

This would mean you would have to redefine previous groups as well everytime you use 'block_set_throttle_groups' otherwise they would get removed from the device. At least that's a way to avoid using more commands for attaching/detaching.

Regarding 'query-block'. This case is complicated by the fact that it
would be possible for a drive to have many nodes, thus the values
printed by 'query-block' might be inaccurate if they only report one
of the node configurations.  The current approach is to report only
the first filter node in the BDS tree. A user with advanced throttling
configurations might have no need for this command.

I guess the user can use 'query-block' and walk the backing chain
looking for throttling filter nodes?

I think it would be trivial to do this. Isn't the BDS tree always "short"? Otherwise the filter nodes should be cached in BlockBackend for performance, if it can be argued that it's needed.

Berto

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]