| Message ID | 20160628123734.500a3114@fiorina |
|---|---|
| State | New |
| Headers | show |
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ý <tgolembi@redhat.com> > --- > 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
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; + } + + s->len = (size_t)d; + if ((!strncasecmp(s->url, "http://", strlen("http://")) || !strncasecmp(s->url, "https://", strlen("https://"))) && !s->accept_range) {
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ý <tgolembi@redhat.com> --- block/curl.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)