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

[Alberto Garcia <berto@igalia.com>]

On Tue, Mar 03, 2015 at 03:00:06PM -0600, Stefan Hajnoczi wrote:

> > +    throttle_group_lock(bs->throttle_state);
> > +    bdrv_throttle_group_remove(bs);
> > +    throttle_group_unlock(bs->throttle_state);
> > +
> > +    throttle_group_unref(bs->throttle_state);
> > +    bs->throttle_state = NULL;
> > +
> >      throttle_timers_destroy(&bs->throttle_timers);
> >  }
> >  
> >  static void bdrv_throttle_read_timer_cb(void *opaque)
> >  {
> >      BlockDriverState *bs = opaque;
> > -    throttle_timer_fired(&bs->throttle_state, false);
> > +
> > +    throttle_group_lock(bs->throttle_state);
> > +    throttle_timer_fired(bs->throttle_state, false);
> > +    throttle_group_unlock(bs->throttle_state);
> 
> 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/).

Berto