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

[Fam Zheng]

On Wed, 04/01 17:18, Alberto Garcia wrote:
> 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?

Yes.

> What would happen if all members of the group are removed?

The automatic group created from "bps=" should be anonymous, which is attached
by the only one BDS. If it is removed, refcount should release the group
automatically.

> And what would happen if it's a group created with -object?

The -object has a refcount, so it should stay even when all members are
removed, unless "(QMP) object-del".
> 
> My first impression is that the idea is feasible, but I'm unsure at
> the moment of its complexity or possible side effects.

I think it is a cleaner model and easier to understand. What complexity are you
referring to?

> 
> > > @@ -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 :)

:D

Fam