Re: [Qemu-devel] [PATCH 4/7] throttle: Add throttle group support

[Alberto Garcia]

On Wed, Apr 01, 2015 at 10:44:51PM +0800, Fam Zheng wrote:

> > +# @group: #optional throttle group name (Since 2.3)
> 
> We should probably elaborate (here, and at other places of @group
> appearances): @device is used as group name. This is useful since
> other devices could use this device name as the group name, for
> exmple the following -drive definition:
> 
> ...
> -drive file=foo,id=foo,bps=100 \
> -drive file=bar,id=foo,bps=200,group=foo \
> ...
> 
> will put both devices into the same group.

That's right, thanks for pointing it out. I can update the
documentation. Alternatively we could use some other mechanism for
auto-generating group names, rather than using the name of the device,
but either way I think it deserves being documented.

> Also, as the two bps values don't match, I assume the last one
> is used? Is there a warning when ignoring a config from previous
> definitions?

Everytime a new device is added to a group it sets new values for the
whole group, because values are not attached to any particular device.
For the same reason if one device is removed the rest will continue
working with the existing configuration.

> Thinking about this, I'd slightly prefer a canonical throttle group
> definition rather than patching the existing parameters:
> 
> -object throttle-group,id=tg0,bps=100,iops=200,iops-max=1000 \
> -drive file=foo,id=foo,throttle-group=tg0 \
> -drive file=bar,id=bar,throttle-group=tg0 \
> 
> and error out if "bps=" etc are specified together with
> "throttle-group=" in -drive.
> 
> And in QMP, add block_set_io_throttle_group which works together
> with object-add.

And then you would create a group automatically for drives that have
bps= specified? What would happen if all members of the group are
removed? And what would happen if it's a group created with -object?

My first impression is that the idea is feasible, but I'm unsure at
the moment of its complexity or possible side effects.

> > @@ -1756,6 +1756,7 @@ Arguments:
> >  - "iops_rd_max":  read I/O operations max (json-int)
> >  - "iops_wr_max":  write I/O operations max (json-int)
> >  - "iops_size":  I/O size in bytes when limiting (json-int)
> 
> Why removing these three?

There's nothing removed, that's just a list of bullets, note the
column where the dashes appear :)

Berto