Re: [Qemu-block] [PATCH v2 7/7] curl: Check curl_multi_add_handle()'s return code

2019-09-10 Thread Maxim Levitsky
On Tue, 2019-09-10 at 14:41 +0200, 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 
> ---
>  block/curl.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index c343c7ed3d..f86299378e 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -882,7 +882,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, );

Checking the return values is always a very good idea.
I would myself make this patch #1 in the series, since it doesn't
depend on others and it itself a bugfix.
But this is my style, so I don't mind if you leave this as is.

Reviewed-by: Maxim Levitsky 

Best regards,
Maxim Levitsky




[Qemu-block] [PATCH v2 7/7] curl: Check curl_multi_add_handle()'s return code

2019-09-10 Thread Max Reitz
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 
---
 block/curl.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index c343c7ed3d..f86299378e 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -882,7 +882,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, );
-- 
2.21.0