Re: [PATCH 2/2] mirror: Don't let an operation wait for itself

[Eric Blake]

On 1/28/20 9:17 AM, Kevin Wolf wrote:
> mirror_wait_for_free_in_flight_slot() just picks a random operation to
> wait for. However, when mirror_co_read() waits for free slots, its
> MirrorOp is already in s->ops_in_flight, so if not enough slots are
> immediately available, an operation can end up waiting for itself to
> complete, which results in a hang.

Eww. That can't have been fun to debug.

> Fix this by passing the current MirrorOp and skipping this operation
> when picking an operation to wait for.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1794692
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>   block/mirror.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 8959e4255f..cacbc70014 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -283,11 +283,14 @@ static int mirror_cow_align(MirrorBlockJob *s, int64_t 
> *offset,
>   }
>   
>   static inline void coroutine_fn
> -mirror_wait_for_any_operation(MirrorBlockJob *s, bool active)
> +mirror_wait_for_any_operation(MirrorBlockJob *s, MirrorOp *self, bool active)
>   {
>       MirrorOp *op;
>   
>       QTAILQ_FOREACH(op, &s->ops_in_flight, next) {
> +        if (self == op) {
> +            continue;
> +        }

But this is the obvious fix.

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org