Skip to content

Commit

Permalink
throttle: Check current timers before updating any_timer_armed[]
Browse files Browse the repository at this point in the history
Calling throttle_group_config() cancels all timers from a particular
BlockDriverState, so any_timer_armed[] should be updated accordingly.

However, with the current code it may happen that a timer is armed in
a different BlockDriverState from the same group, so any_timer_armed[]
would be set to false in a situation where there is still a timer
armed.

The consequence is that we might end up with two timers armed. This
should not have any noticeable impact however, since all accesses to
the ThrottleGroup are protected by a lock, and the situation would
become normal again shortly thereafter as soon as all timers have been
fired.

The correct way to solve this is to check that we're actually
cancelling a timer before updating any_timer_armed[].

Signed-off-by: Alberto Garcia <[email protected]>
Message-id: [email protected]
Signed-off-by: Stefan Hajnoczi <[email protected]>
  • Loading branch information
bertogg authored and stefanhaRH committed Jun 23, 2015
1 parent f406c03 commit 2f388b9
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions block/throttle-groups.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,14 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
ThrottleState *ts = bs->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
throttle_config(ts, tt, cfg);
/* throttle_config() cancels the timers */
tg->any_timer_armed[0] = tg->any_timer_armed[1] = false;
if (timer_pending(tt->timers[0])) {
tg->any_timer_armed[0] = false;
}
if (timer_pending(tt->timers[1])) {
tg->any_timer_armed[1] = false;
}
throttle_config(ts, tt, cfg);
qemu_mutex_unlock(&tg->lock);
}

Expand Down

0 comments on commit 2f388b9

Please sign in to comment.