Re: [Qemu-block] [PATCH v3 5/7] block/curl: check error return of curl_global_init()

[Richard W.M. Jones]

On Tue, Nov 07, 2017 at 05:27:22PM -0500, Jeff Cody wrote:
> If curl_global_init() fails, per the documentation no other curl
> functions may be called, so make sure to check the return value.
> 
> Also, some minor changes to the initialization latch variable 'inited':
> 
> - Make it static in the file, for clarity
> - Change the name for clarity
> - Make it a bool
> 
> Signed-off-by: Jeff Cody <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>

This is just a simple evolution of the previous code, so:

Reviewed-by: Richard W.M. Jones <address@hidden>

Rich.

>  block/curl.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 2a244e2..00a9879 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -89,6 +89,8 @@ static CURLMcode __curl_multi_socket_action(CURLM 
> *multi_handle,
>  
>  struct BDRVCURLState;
>  
> +static bool libcurl_initialized;
> +
>  typedef struct CURLAIOCB {
>      Coroutine *co;
>      QEMUIOVector *qiov;
> @@ -686,14 +688,23 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      double d;
>      const char *secretid;
>      const char *protocol_delimiter;
> +    int ret;
>  
> -    static int inited = 0;
>  
>      if (flags & BDRV_O_RDWR) {
>          error_setg(errp, "curl block device does not support writes");
>          return -EROFS;
>      }
>  
> +    if (!libcurl_initialized) {
> +        ret = curl_global_init(CURL_GLOBAL_ALL);
> +        if (ret) {
> +            error_setg(errp, "libcurl initialization failed with %d", ret);
> +            return -EIO;
> +        }
> +        libcurl_initialized = true;
> +    }
> +
>      qemu_mutex_init(&s->mutex);
>      opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>      qemu_opts_absorb_qdict(opts, options, &local_err);
> @@ -772,11 +783,6 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options, int flags,
>          }
>      }
>  
> -    if (!inited) {
> -        curl_global_init(CURL_GLOBAL_ALL);
> -        inited = 1;
> -    }
> -
>      DPRINTF("CURL: Opening %s\n", file);
>      QSIMPLEQ_INIT(&s->free_state_waitq);
>      s->aio_context = bdrv_get_aio_context(bs);
> -- 
> 2.9.5

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW