[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2] throttle: fix a qemu crash problem when call
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [PATCH v2] throttle: fix a qemu crash problem when calling blk_delete |
Date: |
Fri, 10 Nov 2017 14:42:36 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Thu 09 Nov 2017 06:12:10 PM CET, Stefan Hajnoczi wrote:
>> 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.
I think so too.
And as I said in a previous e-mail I don't know if we should have a
valid blk->public.throttle_group_member.throttle_state with no timers at
all. I'd rather attach the default AioContext while the BDS is removed.
I'll try to prepare a patch with all of this together.
Berto