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 <sochin.ji...@huawei.com>
> ---
> 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
