All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Richard W.M. Jones" <rjones@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Jeff Cody <jcody@redhat.com>,
	qemu-stable@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/4] block/curl: Fix return value from curl_read_cb
Date: Wed, 26 Oct 2016 10:38:18 +0100	[thread overview]
Message-ID: <20161026093818.GD27578@redhat.com> (raw)
In-Reply-To: <20161025025431.24714-3-mreitz@redhat.com>

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: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  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

  parent reply	other threads:[~2016-10-26  9:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-25  2:54 [Qemu-devel] [PATCH 0/4] block/curl: Fix FTP Max Reitz
2016-10-25  2:54 ` [Qemu-devel] [PATCH 1/4] block/curl: Use BDRV_SECTOR_SIZE Max Reitz
2016-10-25 18:31   ` Eric Blake
2016-10-26 14:39     ` Max Reitz
2016-10-26  9:31   ` Richard W.M. Jones
2016-10-25  2:54 ` [Qemu-devel] [PATCH 2/4] block/curl: Fix return value from curl_read_cb Max Reitz
2016-10-25 18:37   ` Eric Blake
2016-10-26  9:17     ` Kevin Wolf
2016-11-01  9:58       ` Matthew Booth
2016-10-26 14:43     ` Max Reitz
2016-10-26  9:38   ` Richard W.M. Jones [this message]
2016-10-25  2:54 ` [Qemu-devel] [PATCH 3/4] block/curl: Remember all sockets Max Reitz
2016-10-25  2:54 ` [Qemu-devel] [PATCH 4/4] block/curl: Do not wait for data beyond EOF Max Reitz
2016-10-26  9:44 ` [Qemu-devel] [PATCH 0/4] block/curl: Fix FTP Richard W.M. Jones
2016-11-15  3:47   ` Jeff Cody

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161026093818.GD27578@redhat.com \
    --to=rjones@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.