Re: [Qemu-trivial] [Qemu-block] [PATCH] curl: Operate on zero-length file

[Kevin Wolf]

Am 28.06.2016 um 12:37 hat Tomáš Golembiovský geschrieben:
> This is an attempt to fix small bug 1596870.
> 
> When creating new disk backed by remote file accessed via HTTPS and the
> backing file has zero length, qemu-img terminates with uniformative
> error message:
> 
>     qemu-img: disk.qcow2: CURL: Error opening file:
> 
> While it may not make much sense to operate on empty file, other block
> backends (e.g. raw backend for regular files) seem to allow it. This
> patch fixes it for the curl backend.
> 
> Signed-off-by: Tomáš Golembiovský <address@hidden>
> ---
>  block/curl.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index da9f5e8..ee6c5ea 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -675,11 +675,17 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
>      if (curl_easy_perform(state->curl))
>          goto out;
> -    curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d);
> -    if (d)
> -        s->len = (size_t)d;
> -    else if(!s->len)
> +    if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, 
> &d)) {
>          goto out;
> +    }
> +    if (d < 0) {
> +        pstrcpy(state->errmsg, CURL_ERROR_SIZE,
> +                "Received invalid file length.");
> +        goto out;
> +    }

Actually, it seems that d == -1 means that there is no information about
the file length, so maybe the error message could be a bit more precise.


A possible problem with the patch is that curl changed its behaviour in
version 7.19.4. Before this version, it used to return 0 for a missing
length, so we can't distinguish these cases on older versions. It's
probably more common not to have the file length than having a
zero-length file.

Do we care about older versions? Commit 8a8f584 suggests that we did
three years ago, it added an #if for a feature added in 7.19.4. Not sure
if we do any more. RHEL 6 seems to be new enough.

But if we do care, I guess we could do a similar #if that enables your
code for newer curl versions and keeps erroring out for older versions.

Any opinions on whether we still care about curl versions that old?

> +    s->len = (size_t)d;
> +
>      if ((!strncasecmp(s->url, "http://";, strlen("http://";))
>          || !strncasecmp(s->url, "https://";, strlen("https://";)))
>          && !s->accept_range) {

Kevin