[PATCH] esx: Bump minimal version of curl

Michal Privoznik posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/31c27271cc5770fe0960cc829d119f7f69760f7d.1613547247.git.mprivozn@redhat.com
meson.build          | 2 +-
src/esx/esx_stream.c | 8 --------
src/esx/esx_vi.c     | 7 -------
3 files changed, 1 insertion(+), 16 deletions(-)
[PATCH] esx: Bump minimal version of curl
Posted by Michal Privoznik 3 years, 2 months ago
According to meson.build the minimal version of curl needed is
7.18.0 which was released in January 2008. If the minimal version
is bumped to 7.19.1 (released in November 2008) we can drop some
workarounds because this newer version provides APIs we need.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Since the new version is still ~12 years old, I did not check whether it
is available in all supported distros. If it isn't then we should
re-evaluate support of running libvirt from git on that distro.

 meson.build          | 2 +-
 src/esx/esx_stream.c | 8 --------
 src/esx/esx_vi.c     | 7 -------
 3 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/meson.build b/meson.build
index 766bc06b61..369548f127 100644
--- a/meson.build
+++ b/meson.build
@@ -1001,7 +1001,7 @@ if capng_dep.found()
   conf.set('WITH_CAPNG', 1)
 endif
 
-curl_version = '7.18.0'
+curl_version = '7.19.1'
 curl_dep = dependency('libcurl', version: '>=' + curl_version, required: get_option('curl'))
 if curl_dep.found()
   # XXX as of libcurl-devel-7.20.1-3.fc13.x86_64, curl ships a version
diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
index 131fbc100b..e4e67a01bb 100644
--- a/src/esx/esx_stream.c
+++ b/src/esx/esx_stream.c
@@ -396,7 +396,6 @@ esxStreamOpen(virStreamPtr stream, esxPrivate *priv, const char *url,
     int result = -1;
     esxStreamPrivate *streamPriv;
     g_autofree char *range = NULL;
-    g_autofree char *userpwd = NULL;
     esxVI_MultiCURL *multi = NULL;
 
     /* FIXME: Although there is already some code in place to deal with
@@ -438,17 +437,10 @@ esxStreamOpen(virStreamPtr stream, esxPrivate *priv, const char *url,
     curl_easy_setopt(streamPriv->curl->handle, CURLOPT_URL, url);
     curl_easy_setopt(streamPriv->curl->handle, CURLOPT_RANGE, range);
 
-#if LIBCURL_VERSION_NUM >= 0x071301 /* 7.19.1 */
     curl_easy_setopt(streamPriv->curl->handle, CURLOPT_USERNAME,
                      priv->primary->username);
     curl_easy_setopt(streamPriv->curl->handle, CURLOPT_PASSWORD,
                      priv->primary->password);
-#else
-    userpwd = g_strdup_printf("%s:%s", priv->primary->username,
-                              priv->primary->password);
-
-    curl_easy_setopt(streamPriv->curl->handle, CURLOPT_USERPWD, userpwd);
-#endif
 
     if (esxVI_MultiCURL_Alloc(&multi) < 0 ||
         esxVI_MultiCURL_Add(multi, streamPriv->curl) < 0)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index e1c1a15ab6..e535b28484 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -230,9 +230,7 @@ esxVI_CURL_Perform(esxVI_CURL *curl, const char *url)
 {
     CURLcode errorCode;
     long responseCode = 0;
-#if LIBCURL_VERSION_NUM >= 0x071202 /* 7.18.2 */
     const char *redirectUrl = NULL;
-#endif
 
     errorCode = curl_easy_perform(curl->handle);
 
@@ -262,7 +260,6 @@ esxVI_CURL_Perform(esxVI_CURL *curl, const char *url)
     }
 
     if (responseCode == 301) {
-#if LIBCURL_VERSION_NUM >= 0x071202 /* 7.18.2 */
         errorCode = curl_easy_getinfo(curl->handle, CURLINFO_REDIRECT_URL,
                                       &redirectUrl);
 
@@ -277,10 +274,6 @@ esxVI_CURL_Perform(esxVI_CURL *curl, const char *url)
                            _("The server redirects from '%s' to '%s'"), url,
                            redirectUrl);
         }
-#else
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("The server redirects from '%s'"), url);
-#endif
 
         return -1;
     }
-- 
2.26.2

Re: [PATCH] esx: Bump minimal version of curl
Posted by Ján Tomko 3 years, 2 months ago
On a Wednesday in 2021, Michal Privoznik wrote:
>According to meson.build the minimal version of curl needed is
>7.18.0 which was released in January 2008. If the minimal version
>is bumped to 7.19.1 (released in November 2008) we can drop some
>workarounds because this newer version provides APIs we need.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>Since the new version is still ~12 years old, I did not check whether it
>is available in all supported distros. If it isn't then we should
>re-evaluate support of running libvirt from git on that distro.
>
> meson.build          | 2 +-
> src/esx/esx_stream.c | 8 --------
> src/esx/esx_vi.c     | 7 -------
> 3 files changed, 1 insertion(+), 16 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] esx: Bump minimal version of curl
Posted by Daniel P. Berrangé 3 years, 2 months ago
On Wed, Feb 17, 2021 at 08:36:27AM +0100, Michal Privoznik wrote:
> According to meson.build the minimal version of curl needed is
> 7.18.0 which was released in January 2008. If the minimal version
> is bumped to 7.19.1 (released in November 2008) we can drop some
> workarounds because this newer version provides APIs we need.

RHEL-7 has 7.29.0, so I think you should bump the version
right up to that.


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] esx: Bump minimal version of curl
Posted by Michal Privoznik 3 years, 2 months ago
On 2/17/21 11:06 AM, Daniel P. Berrangé wrote:
> On Wed, Feb 17, 2021 at 08:36:27AM +0100, Michal Privoznik wrote:
>> According to meson.build the minimal version of curl needed is
>> 7.18.0 which was released in January 2008. If the minimal version
>> is bumped to 7.19.1 (released in November 2008) we can drop some
>> workarounds because this newer version provides APIs we need.
> 
> RHEL-7 has 7.29.0, so I think you should bump the version
> right up to that.
> 

While we could do that, it doesn't matter really - after this commit we 
don't check for curl version anywhere in our code so I'd say 7.19.1 is 
okay - it doesn't put any burden on us. But if we still want to bump the 
version then please post a patch and I will review it.

Michal