[Qemu-devel] [PATCH 6/6] curl: Check curl_multi_add_handle()'s return code

Max Reitz posted 6 patches 6 years, 2 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH 6/6] curl: Check curl_multi_add_handle()'s return code
Posted by Max Reitz 6 years, 2 months ago
If we had done that all along, debugging would have been much simpler.
(Also, I/O errors are better than hangs.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/curl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 5e0cca601d..4a7aff02a6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -894,7 +894,13 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
     trace_curl_setup_preadv(acb->bytes, start, state->range);
     curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
 
-    curl_multi_add_handle(s->multi, state->curl);
+    if (curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) {
+        state->acb[0] = NULL;
+        acb->ret = -EIO;
+
+        curl_clean_state(state);
+        goto out;
+    }
 
     /* Tell curl it needs to kick things off */
     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
-- 
2.21.0


Re: [Qemu-devel] [Qemu-block] [PATCH 6/6] curl: Check curl_multi_add_handle()'s return code
Posted by John Snow 6 years, 2 months ago

On 8/27/19 12:34 PM, Max Reitz wrote:
> If we had done that all along, debugging would have been much simpler.
> (Also, I/O errors are better than hangs.)
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/curl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 5e0cca601d..4a7aff02a6 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -894,7 +894,13 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
>      trace_curl_setup_preadv(acb->bytes, start, state->range);
>      curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
>  
> -    curl_multi_add_handle(s->multi, state->curl);
> +    if (curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) {
> +        state->acb[0] = NULL;
> +        acb->ret = -EIO;
> +
> +        curl_clean_state(state);
> +        goto out;
> +    }
>  
>      /* Tell curl it needs to kick things off */
>      curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
> 

And, well, looks fine. I think there's just a refactoring typo and maybe
explaining some curiosities to me that I am fairly sure are fine, actually.

--js