1. Patchew
  2. QEMU
  3. View series

[PATCH] curl: remove compatibility code

Paolo Bonzini posted 1 patch 5 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201117113850.64108-1-pbonzini@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
block/curl.c | 28 ----------------------------
1 file changed, 28 deletions(-)
[PATCH] curl: remove compatibility code
Posted by Paolo Bonzini 5 years, 2 months ago 
cURL 7.16.0 was released in October 2006.  Just remove code that is
in all likelihood not being used anywhere.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/curl.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 4f907c47be..b77bfe12e7 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -37,26 +37,6 @@
 
 // #define DEBUG_VERBOSE
 
-#if LIBCURL_VERSION_NUM >= 0x071000
-/* The multi interface timer callback was introduced in 7.16.0 */
-#define NEED_CURL_TIMER_CALLBACK
-#define HAVE_SOCKET_ACTION
-#endif
-
-#ifndef HAVE_SOCKET_ACTION
-/* If curl_multi_socket_action isn't available, define it statically here in
- * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is
- * less efficient but still safe. */
-static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
-                                            curl_socket_t sockfd,
-                                            int ev_bitmask,
-                                            int *running_handles)
-{
-    return curl_multi_socket(multi_handle, sockfd, running_handles);
-}
-#define curl_multi_socket_action __curl_multi_socket_action
-#endif
-
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
                    CURLPROTO_FTP | CURLPROTO_FTPS)
 
@@ -140,7 +120,6 @@ typedef struct BDRVCURLState {
 static void curl_clean_state(CURLState *s);
 static void curl_multi_do(void *arg);
 
-#ifdef NEED_CURL_TIMER_CALLBACK
 /* Called from curl_multi_do_locked, with s->mutex held.  */
 static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
 {
@@ -156,7 +135,6 @@ static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
     }
     return 0;
 }
-#endif
 
 /* Called from curl_multi_do_locked, with s->mutex held.  */
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
@@ -433,7 +411,6 @@ static void curl_multi_do(void *arg)
 
 static void curl_multi_timeout_do(void *arg)
 {
-#ifdef NEED_CURL_TIMER_CALLBACK
     BDRVCURLState *s = (BDRVCURLState *)arg;
     int running;
 
@@ -446,9 +423,6 @@ static void curl_multi_timeout_do(void *arg)
 
     curl_multi_check_completion(s);
     qemu_mutex_unlock(&s->mutex);
-#else
-    abort();
-#endif
 }
 
 /* Called with s->mutex held.  */
@@ -598,10 +572,8 @@ static void curl_attach_aio_context(BlockDriverState *bs,
     s->multi = curl_multi_init();
     s->aio_context = new_context;
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
-#ifdef NEED_CURL_TIMER_CALLBACK
     curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
-#endif
 }
 
 static QemuOptsList runtime_opts = {
-- 
2.28.0
Re: [PATCH] curl: remove compatibility code
Posted by Daniel P. Berrangé 5 years, 2 months ago 
On Tue, Nov 17, 2020 at 12:38:50PM +0100, Paolo Bonzini wrote:
> cURL 7.16.0 was released in October 2006.  Just remove code that is
> in all likelihood not being used anywhere.

Rather than assuming that, we should be picking our minimum version
and enforcing that in configure/meson.

Currently, we have a manual code compile check for curl_multi_setopt,
after doing a pkg-config check with no min version.

We should set a min version in pkg-config and drop the compile check
in configure.

Based on repology.org and our platform support matrix, RHEL-7 looks
like the oldest curl that we need to care about, 7.29.0

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/curl.c | 28 ----------------------------
>  1 file changed, 28 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 4f907c47be..b77bfe12e7 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -37,26 +37,6 @@
>  
>  // #define DEBUG_VERBOSE
>  
> -#if LIBCURL_VERSION_NUM >= 0x071000
> -/* The multi interface timer callback was introduced in 7.16.0 */
> -#define NEED_CURL_TIMER_CALLBACK
> -#define HAVE_SOCKET_ACTION
> -#endif
> -
> -#ifndef HAVE_SOCKET_ACTION
> -/* If curl_multi_socket_action isn't available, define it statically here in
> - * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is
> - * less efficient but still safe. */
> -static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
> -                                            curl_socket_t sockfd,
> -                                            int ev_bitmask,
> -                                            int *running_handles)
> -{
> -    return curl_multi_socket(multi_handle, sockfd, running_handles);
> -}
> -#define curl_multi_socket_action __curl_multi_socket_action
> -#endif
> -
>  #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
>                     CURLPROTO_FTP | CURLPROTO_FTPS)
>  
> @@ -140,7 +120,6 @@ typedef struct BDRVCURLState {
>  static void curl_clean_state(CURLState *s);
>  static void curl_multi_do(void *arg);
>  
> -#ifdef NEED_CURL_TIMER_CALLBACK
>  /* Called from curl_multi_do_locked, with s->mutex held.  */
>  static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
>  {
> @@ -156,7 +135,6 @@ static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
>      }
>      return 0;
>  }
> -#endif
>  
>  /* Called from curl_multi_do_locked, with s->mutex held.  */
>  static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
> @@ -433,7 +411,6 @@ static void curl_multi_do(void *arg)
>  
>  static void curl_multi_timeout_do(void *arg)
>  {
> -#ifdef NEED_CURL_TIMER_CALLBACK
>      BDRVCURLState *s = (BDRVCURLState *)arg;
>      int running;
>  
> @@ -446,9 +423,6 @@ static void curl_multi_timeout_do(void *arg)
>  
>      curl_multi_check_completion(s);
>      qemu_mutex_unlock(&s->mutex);
> -#else
> -    abort();
> -#endif
>  }
>  
>  /* Called with s->mutex held.  */
> @@ -598,10 +572,8 @@ static void curl_attach_aio_context(BlockDriverState *bs,
>      s->multi = curl_multi_init();
>      s->aio_context = new_context;
>      curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
> -#ifdef NEED_CURL_TIMER_CALLBACK
>      curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
>      curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
> -#endif
>  }
>  
>  static QemuOptsList runtime_opts = {
> -- 
> 2.28.0
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] curl: remove compatibility code
Posted by Paolo Bonzini 5 years, 2 months ago 
On 17/11/20 12:46, Daniel P. Berrangé wrote:
> On Tue, Nov 17, 2020 at 12:38:50PM +0100, Paolo Bonzini wrote:
>> cURL 7.16.0 was released in October 2006.  Just remove code that is
>> in all likelihood not being used anywhere.
> 
> Rather than assuming that, we should be picking our minimum version
> and enforcing that in configure/meson.
> 
> Currently, we have a manual code compile check for curl_multi_setopt,
> after doing a pkg-config check with no min version.
> 
> We should set a min version in pkg-config and drop the compile check
> in configure.
> 
> Based on repology.org and our platform support matrix, RHEL-7 looks
> like the oldest curl that we need to care about, 7.29.0

That is complicated a bit by the fact that curl is detected with both 
pkg-config and curl-config.  That is probably unnecessary too, since we 
do not need any of the options that are exclusive to curl-config, such 
as --ca.  If we can drop curl-config, moving the detection to meson is 
much easier.

Paolo

>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   block/curl.c | 28 ----------------------------
>>   1 file changed, 28 deletions(-)
>>
>> diff --git a/block/curl.c b/block/curl.c
>> index 4f907c47be..b77bfe12e7 100644
>> --- a/block/curl.c
>> +++ b/block/curl.c
>> @@ -37,26 +37,6 @@
>>   
>>   // #define DEBUG_VERBOSE
>>   
>> -#if LIBCURL_VERSION_NUM >= 0x071000
>> -/* The multi interface timer callback was introduced in 7.16.0 */
>> -#define NEED_CURL_TIMER_CALLBACK
>> -#define HAVE_SOCKET_ACTION
>> -#endif
>> -
>> -#ifndef HAVE_SOCKET_ACTION
>> -/* If curl_multi_socket_action isn't available, define it statically here in
>> - * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is
>> - * less efficient but still safe. */
>> -static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
>> -                                            curl_socket_t sockfd,
>> -                                            int ev_bitmask,
>> -                                            int *running_handles)
>> -{
>> -    return curl_multi_socket(multi_handle, sockfd, running_handles);
>> -}
>> -#define curl_multi_socket_action __curl_multi_socket_action
>> -#endif
>> -
>>   #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
>>                      CURLPROTO_FTP | CURLPROTO_FTPS)
>>   
>> @@ -140,7 +120,6 @@ typedef struct BDRVCURLState {
>>   static void curl_clean_state(CURLState *s);
>>   static void curl_multi_do(void *arg);
>>   
>> -#ifdef NEED_CURL_TIMER_CALLBACK
>>   /* Called from curl_multi_do_locked, with s->mutex held.  */
>>   static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
>>   {
>> @@ -156,7 +135,6 @@ static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
>>       }
>>       return 0;
>>   }
>> -#endif
>>   
>>   /* Called from curl_multi_do_locked, with s->mutex held.  */
>>   static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
>> @@ -433,7 +411,6 @@ static void curl_multi_do(void *arg)
>>   
>>   static void curl_multi_timeout_do(void *arg)
>>   {
>> -#ifdef NEED_CURL_TIMER_CALLBACK
>>       BDRVCURLState *s = (BDRVCURLState *)arg;
>>       int running;
>>   
>> @@ -446,9 +423,6 @@ static void curl_multi_timeout_do(void *arg)
>>   
>>       curl_multi_check_completion(s);
>>       qemu_mutex_unlock(&s->mutex);
>> -#else
>> -    abort();
>> -#endif
>>   }
>>   
>>   /* Called with s->mutex held.  */
>> @@ -598,10 +572,8 @@ static void curl_attach_aio_context(BlockDriverState *bs,
>>       s->multi = curl_multi_init();
>>       s->aio_context = new_context;
>>       curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
>> -#ifdef NEED_CURL_TIMER_CALLBACK
>>       curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
>>       curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
>> -#endif
>>   }
>>   
>>   static QemuOptsList runtime_opts = {
>> -- 
>> 2.28.0
>>
>>
> 
> Regards,
> Daniel
>
Re: [PATCH] curl: remove compatibility code
Posted by Daniel P. Berrangé 5 years, 2 months ago 
On Tue, Nov 17, 2020 at 01:40:56PM +0100, Paolo Bonzini wrote:
> On 17/11/20 12:46, Daniel P. Berrangé wrote:
> > On Tue, Nov 17, 2020 at 12:38:50PM +0100, Paolo Bonzini wrote:
> > > cURL 7.16.0 was released in October 2006.  Just remove code that is
> > > in all likelihood not being used anywhere.
> > 
> > Rather than assuming that, we should be picking our minimum version
> > and enforcing that in configure/meson.
> > 
> > Currently, we have a manual code compile check for curl_multi_setopt,
> > after doing a pkg-config check with no min version.
> > 
> > We should set a min version in pkg-config and drop the compile check
> > in configure.
> > 
> > Based on repology.org and our platform support matrix, RHEL-7 looks
> > like the oldest curl that we need to care about, 7.29.0
> 
> That is complicated a bit by the fact that curl is detected with both
> pkg-config and curl-config.  That is probably unnecessary too, since we do
> not need any of the options that are exclusive to curl-config, such as --ca.
> If we can drop curl-config, moving the detection to meson is much easier.

That's just another bit of historical cruft - use of curl-config dates
back to when you introduced pkg-config for curl in 2010, to cope with
distros that didn't ship pkg-config files yet. Our supported platforms
today will all have pkg-config for curl, so we can drop curl-config too.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.