Re: [Qemu-block] [PATCH] qemu-img / curl: When fetching Content-Size use GET instead of HEAD.

[John Snow]

On 12/07/2015 04:23 PM, Boris Schrijver wrote:
> Hi all,
> 

Hi!

> I was testing out the "qemu-img info/convert" options in combination with
> "http/https" when I stumbled upon this issue. When "qemu-img info/convert" 
> tries
> to collect the file info it will first try to fetch the Content-Size of the
> remote file. It does a HEAD request and after a GET request for the correct
> range.
> 
> The HEAD request is an issue. Because when you've got a pre-signed url, for
> example from S3, which INCLUDES the REQUEST METHOD in it's signature, you'll 
> get
> a 403 Forbidden.
> 
> It's is therefore better to use only the GET request method, and discard the
> body at the first call.
> 

How big is the body? Won't this introduce a really large overhead?

> Please review! I'll be ready for answers!
> 

Please use the git format-patch format for sending patch emails; see
http://qemu-project.org/Contribute/SubmitAPatch#Use_git_format-patch --
and remember to include a Signed-off-by line.

> [PATCH] qemu-img / curl: When fetching Content-Size use GET instead of HEAD.
> 
> A server can respond different to both methods, or can block one of the two.
> ---
>  block/curl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 8994182..2e74c32 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -594,11 +594,11 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options,
> int flags,
>      // Get file size
>  
>      s->accept_range = false;
> -    curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
> +    curl_easy_setopt(state->curl, CURLOPT_HTTPGET, 1);
>      curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
>                       curl_header_cb);
>      curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
> -    if (curl_easy_perform(state->curl))
> +    if (curl_easy_perform(state->curl) != 23)

We go from making sure there were no errors to enforcing that we *do*
get CURLE_WRITE_ERROR? Can you explain why this change doesn't break
error handling scenarios for all other cases?

>          goto out;
>      curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d);
>      if (d)
>