Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] block/curl: Fix return value from curl_read_cb

[Richard W.M. Jones]

On Tue, Oct 25, 2016 at 04:54:29AM +0200, Max Reitz wrote:
> While commit 38bbc0a580f9f10570b1d1b5d3e92f0e6feb2970 is correct in that
> the callback is supposed to return the number of bytes handled; what it
> does not mention is that libcurl will throw an error if the callback did
> not "handle" all of the data passed to it.
> 
> Therefore, if the callback receives some data that it cannot handle
> (either because the receive buffer has not been set up yet or because it
> would not fit into the receive buffer) and we have to ignore it, we
> still have to report that the data has been handled.
> 
> Obviously, this should not happen normally. But it does happen at least
> for FTP connections where some data (that we do not expect) may be
> generated when the connection is established.
> 
> Cc: address@hidden
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/curl.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 12afa15..095ffda 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -212,12 +212,13 @@ static size_t curl_read_cb(void *ptr, size_t size, 
> size_t nmemb, void *opaque)
>  
>      DPRINTF("CURL: Just reading %zd bytes\n", realsize);
>  
> -    if (!s || !s->orig_buf)
> -        return 0;
> +    if (!s || !s->orig_buf) {
> +        goto read_end;
> +    }
>  
>      if (s->buf_off >= s->buf_len) {
>          /* buffer full, read nothing */
> -        return 0;
> +        goto read_end;
>      }
>      realsize = MIN(realsize, s->buf_len - s->buf_off);
>      memcpy(s->orig_buf + s->buf_off, ptr, realsize);
> @@ -238,7 +239,9 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t 
> nmemb, void *opaque)
>          }
>      }
>  
> -    return realsize;
> +read_end:
> +    /* curl will error out if we do not return this value */
> +    return size * nmemb;
>  }

Luckily I documented my test case last time:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg04883.html

I went back and tried a similar test case with just this patch on top
of current qemu from git, and it doesn't appear to break anything for
the http case.

So it's fine as far as I can see.

Rich.

$ http_proxy= LIBGUESTFS_BACKEND=direct 
LIBGUESTFS_HV=~/d/qemu/x86_64-softmmu/qemu-system-x86_64 guestfish -a 
http://onuma.home.annexia.org/media/installers/Fedora-23-Cloud-x86_64/Fedora-Cloud-Base-23-20151030.x86_64.qcow2
 --ro -i

Welcome to guestfish, the guest filesystem shell for
editing virtual machine filesystems and disk images.

Type: 'help' for help on commands
      'man' to read the manual
      'quit' to quit the shell

Operating system: Fedora 23 (Cloud Edition)
/dev/sda1 mounted on /

><fs> ll /
total 84
dr-xr-xr-x. 18 root root  4096 Oct 30  2015 .
drwxr-xr-x  19 root root  4096 Oct 26 09:36 ..
lrwxrwxrwx.  1 root root     7 Sep 10  2015 bin -> usr/bin
dr-xr-xr-x.  5 root root  4096 Oct 30  2015 boot
drwxr-xr-x.  2 root root  4096 Oct 30  2015 dev
drwxr-xr-x. 68 root root  4096 Oct 30  2015 etc
drwxr-xr-x.  2 root root  4096 Oct 30  2015 home
lrwxrwxrwx.  1 root root     7 Sep 10  2015 lib -> usr/lib
lrwxrwxrwx.  1 root root     9 Sep 10  2015 lib64 -> usr/lib64
drwx------.  2 root root 16384 Oct 30  2015 lost+found
drwxr-xr-x.  2 root root  4096 Sep 10  2015 media
drwxr-xr-x.  2 root root  4096 Sep 10  2015 mnt
drwxr-xr-x.  2 root root  4096 Sep 10  2015 opt
drwxr-xr-x.  2 root root  4096 Oct 30  2015 proc
dr-xr-x---.  2 root root  4096 Oct 30  2015 root
drwxr-xr-x.  2 root root  4096 Oct 30  2015 run
lrwxrwxrwx.  1 root root     8 Sep 10  2015 sbin -> usr/sbin
drwxr-xr-x.  2 root root  4096 Sep 10  2015 srv
drwxr-xr-x.  2 root root  4096 Oct 30  2015 sys
drwxrwxrwt.  7 root root  4096 Oct 30  2015 tmp
drwxr-xr-x. 12 root root  4096 Oct 30  2015 usr
drwxr-xr-x. 18 root root  4096 Oct 30  2015 var

><fs> find / | wc -l
26532
><fs> 


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html