[libvirt] [PATCH 2/3] esx: Extend esxVI_CURL_Download for partial downloads

[Matthias Bolte]

2012/7/3 Doug Goldstein <cardoe at gentoo.org>:
> On Mon, Jul 2, 2012 at 4:44 PM, Matthias Bolte
> <matthias.bolte at googlemail.com> wrote:
>> ---
>>  src/esx/esx_driver.c |    2 +-
>>  src/esx/esx_vi.c     |   27 +++++++++++++++++++++++++--
>>  src/esx/esx_vi.h     |    3 ++-
>>  3 files changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
>> index db2144c..95b9286 100644
>> --- a/src/esx/esx_driver.c
>> +++ b/src/esx/esx_driver.c
>> @@ -2802,7 +2802,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
>>
>>      url = virBufferContentAndReset(&buffer);
>>
>> -    if (esxVI_CURL_Download(priv->primary->curl, url, &vmx) < 0) {
>> +    if (esxVI_CURL_Download(priv->primary->curl, url, &vmx, 0, NULL) < 0) {
>>          goto cleanup;
>>      }
>>
>> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
>> index 48718b6..3f8d745 100644
>> --- a/src/esx/esx_vi.c
>> +++ b/src/esx/esx_vi.c
>> @@ -355,8 +355,10 @@ esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri)
>>  }
>>
>>  int
>> -esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content)
>> +esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content,
>> +                    unsigned long long offset, unsigned long long *length)
>>  {
>> +    char *range = NULL;
>>      virBuffer buffer = VIR_BUFFER_INITIALIZER;
>>      int responseCode = 0;
>>
>> @@ -365,9 +367,22 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content)
>>          return -1;
>>      }
>>
>> +    if (length != NULL && *length > 0) {
>> +        if (virAsprintf(&range, "%llu-%llu", offset, offset + *length - 1) < 0) {
>> +            virReportOOMError();
>> +            goto cleanup;
>> +        }
>> +    } else if (offset > 0) {
>> +        if (virAsprintf(&range, "%llu-", offset) < 0) {
>> +            virReportOOMError();
>> +            goto cleanup;
>> +        }
>> +    }
>> +
>>      virMutexLock(&curl->lock);
>>
>>      curl_easy_setopt(curl->handle, CURLOPT_URL, url);
>> +    curl_easy_setopt(curl->handle, CURLOPT_RANGE, range);
>>      curl_easy_setopt(curl->handle, CURLOPT_WRITEDATA, &buffer);
>>      curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 0);
>>      curl_easy_setopt(curl->handle, CURLOPT_HTTPGET, 1);
>> @@ -378,7 +393,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content)
>>
>>      if (responseCode < 0) {
>>          goto cleanup;
>> -    } else if (responseCode != 200) {
>> +    } else if (responseCode != 200 && responseCode != 206) {
>>          ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR,
>>                       _("HTTP response code %d for download from '%s'"),
>>                       responseCode, url);
>> @@ -390,9 +405,15 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content)
>>          goto cleanup;
>>      }
>>
>> +    if (length != NULL) {
>> +        *length = virBufferUse(&buffer);
>> +    }
>> +
>>      *content = virBufferContentAndReset(&buffer);
>>
>>    cleanup:
>> +    VIR_FREE(range);
>> +
>>      if (*content == NULL) {
>>          virBufferFreeAndReset(&buffer);
>>          return -1;
>> @@ -414,6 +435,7 @@ esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content)
>>      virMutexLock(&curl->lock);
>>
>>      curl_easy_setopt(curl->handle, CURLOPT_URL, url);
>> +    curl_easy_setopt(curl->handle, CURLOPT_RANGE, NULL);
>>      curl_easy_setopt(curl->handle, CURLOPT_READDATA, &content);
>>      curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 1);
>>      curl_easy_setopt(curl->handle, CURLOPT_INFILESIZE, strlen(content));
>> @@ -1231,6 +1253,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
>>      virMutexLock(&ctx->curl->lock);
>>
>>      curl_easy_setopt(ctx->curl->handle, CURLOPT_URL, ctx->url);
>> +    curl_easy_setopt(ctx->curl->handle, CURLOPT_RANGE, NULL);
>>      curl_easy_setopt(ctx->curl->handle, CURLOPT_WRITEDATA, &buffer);
>>      curl_easy_setopt(ctx->curl->handle, CURLOPT_UPLOAD, 0);
>>      curl_easy_setopt(ctx->curl->handle, CURLOPT_POSTFIELDS, request);
>> diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h
>> index 9560bd2..49b7ca2 100644
>> --- a/src/esx/esx_vi.h
>> +++ b/src/esx/esx_vi.h
>> @@ -167,7 +167,8 @@ struct _esxVI_CURL {
>>  int esxVI_CURL_Alloc(esxVI_CURL **curl);
>>  void esxVI_CURL_Free(esxVI_CURL **curl);
>>  int esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri);
>> -int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content);
>> +int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content,
>> +                        unsigned long long offset, unsigned long long *length);
>>  int esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content);
>>
>>
>> --
>> 1.7.4.1
>
> This patch technically allows you to operate with ranges larger than
> an unsigned int due to the use of unsigned long long. But virBuffer
> can only work with unsigned ints. So there's a very real possibility
> to overflow the type here when performing a download over the 4GB
> mark. Since the content can only be in 1 file, this is a very real
> possibility.

You're right. But the problem was already there before this patch.
I'll address it in a v2.

-- 
Matthias Bolte
http://photron.blogspot.com