throttle-groups: Don't allow timers without throttled requests

Commit 6fccbb475b fixed a bug caused by
QEMU attempting to remove a throttle group member with no pending
requests but an active timer set. This was the result of a previous
bdrv_drained_begin() call processing the throttled requests but
leaving the timer untouched.

Although the commit does solve the problem, the situation shouldn't
happen in the first place. If we try to drain a throttle group member
which has a timer set, we should cancel the timer instead of ignoring
it.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Alberto Garcia 2018-08-02 17:50:26 +03:00 committed by Kevin Wolf
parent 3db3e9c621
commit 25b8e4db7f

View file

@ -36,6 +36,7 @@
static void throttle_group_obj_init(Object *obj);
static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);
static void timer_cb(ThrottleGroupMember *tgm, bool is_write);
/* The ThrottleGroup structure (with its ThrottleState) is shared
* among different ThrottleGroupMembers and it's independent from
@ -424,15 +425,31 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
rd->tgm = tgm;
rd->is_write = is_write;
/* This function is called when a timer is fired or when
* throttle_group_restart_tgm() is called. Either way, there can
* be no timer pending on this tgm at this point */
assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
aio_co_enter(tgm->aio_context, co);
}
void throttle_group_restart_tgm(ThrottleGroupMember *tgm)
{
int i;
if (tgm->throttle_state) {
throttle_group_restart_queue(tgm, 0);
throttle_group_restart_queue(tgm, 1);
for (i = 0; i < 2; i++) {
QEMUTimer *t = tgm->throttle_timers.timers[i];
if (timer_pending(t)) {
/* If there's a pending timer on this tgm, fire it now */
timer_del(t);
timer_cb(tgm, i);
} else {
/* Else run the next request from the queue manually */
throttle_group_restart_queue(tgm, i);
}
}
}
}
@ -567,16 +584,11 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
return;
}
assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);
assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));
assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
if (timer_pending(tgm->throttle_timers.timers[i])) {
tg->any_timer_armed[i] = false;
schedule_next_request(tgm, i);
}
assert(tgm->pending_reqs[i] == 0);
assert(qemu_co_queue_empty(&tgm->throttled_reqs[i]));
assert(!timer_pending(tgm->throttle_timers.timers[i]));
if (tg->tokens[i] == tgm) {
token = throttle_group_next_tgm(tgm);
/* Take care of the case where this is the last tgm in the group */