Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

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 <sochin.jiang@huawei.com>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/block-backend.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]