[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when rem
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend |
Date: |
Fri, 10 Nov 2017 21:27:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 2017-11-10 19:54, Alberto Garcia wrote:
> If a BlockBackend has I/O limits set then its ThrottleGroupMember
> structure uses the AioContext from its attached BlockDriverState.
> Those two contexts must be kept in sync manually. This is not
> ideal and will be fixed in the future by removing the throttling
> configuration from the BlockBackend and storing it in an implicit
> filter node instead, but for now we have to live with this.
>
> When you remove the BlockDriverState from the backend then the
> throttle timers are destroyed. If a new BlockDriverState is later
> inserted then they are created again using the new AioContext.
>
> There'a a couple of problems with this:
>
> a) The code manipulates the timers directly, leaving the
> ThrottleGroupMember.aio_context field in an inconsisent state.
>
> b) If you remove the I/O limits (e.g by destroying the backend)
> when the timers are gone then throttle_group_unregister_tgm()
> will attempt to destroy them again, crashing QEMU.
>
> While b) could be fixed easily by allowing the timers to be freed
> twice, this would result in a situation in which we can no longer
> guarantee that a valid ThrottleState has a valid AioContext and
> timers.
>
> This patch ensures that the timers and AioContext are always valid
> when I/O limits are set, regardless of whether the BlockBackend has a
> BlockDriverState inserted or not.
>
> Reported-by: sochin jiang <address@hidden>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
> block/block-backend.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Max Reitz <address@hidden>
signature.asc
Description: OpenPGP digital signature