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

[Stefan Hajnoczi]

On Wed, Mar 04, 2015 at 05:16:51PM +0100, Alberto Garcia wrote:
> On Wed, Mar 04, 2015 at 10:04:27AM -0600, Stefan Hajnoczi wrote:
> 
> > > > This pattern suggests throttle_timer_fired() should acquire the
> > > > lock internally instead.
> > > 
> > > The idea is that the ThrottleState code itself doesn't know
> > > anything about locks or groups. As I understood it Benoît
> > > designed the ThrottleState code to be independent from the block
> > > layer and reusable for other things (that's why it's in util/).
> > 
> > Then ThrottleGroup could offer an API throttle_group_timer_fired()
> > that does the locking.
> > 
> > The advantage of encapsulating locking in ThrottleGroup is that
> > callers don't have to remember the take the lock.  (But they must
> > still be careful about sequences of calls which will not be atomic.)
> 
> No other code in ThrottleGroup takes the lock directly, so making this
> an exception could be confusing.

It shouldn't be an exception.  I'm proposing that the ThrottleGroup API
should handle locking internally instead of requiring callers to do it.

Stefan

pgpGlUuspM4cF.pgp
Description: PGP signature