See inline! Thanks for your response! --
Met vriendelijke groet / Kind regards, Boris Schrijver PCextreme B.V. http://www.pcextreme.nl/contact Tel direct: +31 (0) 118 700 215 > On December 8, 2015 at 8:40 PM John Snow <js...@redhat.com> wrote: > > > > > 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? The body is "worst-case" the size of the Ethernet v2 frame, around 1500 bytes. > > > 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. > Ok, will do! > > [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? > We're enforcing the CURLE_WRITE_ERROR here. We receive data, but don't want to save it anywhere -> We only want the header. CURLE_WRITE_ERROR implicitly means the connection is successful, because we received a response body! Any other error will not be CURLE_WRITE_ERROR and thus fail. Please run the following command: curl -v -X GET -I http://qemu.org/ It will at the last line read: * Excess found in a non pipelined read: excess = 279 url = / (zero-length body) That is the body we're discarding. Libcurl basically doesn't provide a nice way to handle this. That's why I've implemented in this fashion. > > goto out; > > curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d); > > if (d) > > [PATCH] commit ec8d3ef01eaca9264d97e9ad757fe536e0dc037b Author: Boris Schrijver <bo...@pcextreme.nl> Date: Mon Dec 7 22:01:59 2015 +0100 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. Signed-off-by: Boris Schrijver <bo...@pcextreme.nl> 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) goto out; curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d); if (d)
