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
