[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/7] throttle: Add throttle group support
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 4/7] throttle: Add throttle group support |
Date: |
Thu, 2 Apr 2015 11:26:30 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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