On Tue, Oct 24, 2017 at 11:33:51AM +0800, sochin jiang wrote: > commit 7ca7f0 moves the throttling related part of the BDS life cycle > management to BlockBackend, adds call to > throttle_timers_detach_aio_context in blk_remove_bs. commit 1606e > remove a block device from its throttle group in blk_delete by calling > blk_io_limits_disable, this fix an easily reproducible qemu crash. But > delete a BB without a BDS inserted could easily cause a qemu crash too > by calling bdrv_drained_begin in blk_io_limits_disable. Say, a simply > drive_add and then a drive_del command. > > This patch removes draining BDS by calling throttle_group_unregister_tgm > directly instead of blk_io_limits_disable, leaves draining operation to > blk_remove_bs in case that there is no BDS inserted. Futhermore, make sure > throttle timers are initialized or attached before throttle_timers_destroy > is called in throttle_group_unregister_tgm. > > Signed-off-by: sochin jiang <[email protected]> > --- > block/block-backend.c | 2 +- > block/throttle-groups.c | 4 +++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index 45d9101..39c7cca 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -341,7 +341,7 @@ static void blk_delete(BlockBackend *blk) > assert(!blk->name); > assert(!blk->dev); > if (blk->public.throttle_group_member.throttle_state) { > - blk_io_limits_disable(blk); > + throttle_group_unregister_tgm(&blk->public.throttle_group_member);
The following assertions fail without the drain when there are pending
requests:
void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
{
ThrottleState *ts = tgm->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
ThrottleGroupMember *token;
int i;
if (!ts) {
/* Discard already unregistered 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]));
A safer approach is making blk_io_limits_disable(blk) skip the draining
when blk_bs(blk) == NULL. That is the only case where we are 100% sure
that there are no pending requests.
signature.asc
Description: PGP signature
