From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50450) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHv5A-00080s-US for qemu-devel@nongnu.org; Tue, 28 Jun 2016 11:37:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHv55-0000u4-Se for qemu-devel@nongnu.org; Tue, 28 Jun 2016 11:37:47 -0400 Date: Tue, 28 Jun 2016 17:37:36 +0200 From: Kevin Wolf Message-ID: <20160628153736.GK6800@noname.redhat.com> References: <20160628123734.500a3114@fiorina> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160628123734.500a3114@fiorina> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] curl: Operate on zero-length file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?B?VG9tw6HFoSBHb2xlbWJpb3Zza8O9?= Cc: qemu-devel@nongnu.org, qemu-trivial@nongnu.org, qemu-block@nongnu.org Am 28.06.2016 um 12:37 hat Tom=C3=A1=C5=A1 Golembiovsk=C3=BD geschrieben: > This is an attempt to fix small bug 1596870. >=20 > 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: >=20 > qemu-img: disk.qcow2: CURL: Error opening file: >=20 > 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. >=20 > Signed-off-by: Tom=C3=A1=C5=A1 Golembiovsk=C3=BD > --- > block/curl.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) >=20 > 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 =3D (size_t)d; > - else if(!s->len) > + if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOA= D, &d)) { > goto out; > + } > + if (d < 0) { > + pstrcpy(state->errmsg, CURL_ERROR_SIZE, > + "Received invalid file length."); > + goto out; > + } Actually, it seems that d =3D=3D -1 means that there is no information ab= out 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 =3D (size_t)d; > + > if ((!strncasecmp(s->url, "http://", strlen("http://")) > || !strncasecmp(s->url, "https://", strlen("https://"))) > && !s->accept_range) { Kevin