Re: [Qemu-devel] [Qemu-block] [PATCH 4/7] curl: split curl_find_state/curl_init_state

[Max Reitz <mreitz@redhat.com>]

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

On 10.05.2017 16:32, Paolo Bonzini wrote:
> If curl_easy_init fails, a CURLState is left with s->in_use = 1.  Split
> curl_init_state in two, so that we can distinguish the two failures and
> call curl_clean_state if needed.
> 
> While at it, simplify curl_find_state, removing a dummy loop.  The
> aio_poll loop is moved to the sole caller that needs it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/curl.c | 52 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index b18e79bf54..4b4d5a2389 100644
> --- a/block/curl.c
> +++ b/block/curl.c

[...]

> @@ -856,8 +854,18 @@ static void curl_readv_bh_cb(void *p)
>      }
>  
>      // No cache found, so let's start a new request
> -    state = curl_init_state(acb->common.bs, s);
> -    if (!state) {
> +    for (;;) {
> +        state = curl_find_state(s);
> +        if (state) {
> +            break;
> +        }
> +        qemu_mutex_unlock(&s->mutex);
> +        aio_poll(bdrv_get_aio_context(bs), true);
> +        qemu_mutex_lock(&s->mutex);
> +    }
> +
> +    if (curl_init_state(s, state) < 0) {
> +        curl_clean_state(state);

We could also initialize state to NULL and call curl_clean_state() under
out: if state != NULL. Then again, it would only save us two
curl_clean_state() instances...

So I'll leave it to you and unconditionally give a

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

>          ret = -EIO;
>          goto out;
>      }
> 



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