[Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Peter Maydell]

libcurl versions 7.16.0 and later have a timer callback interface which
must be implemented in order for libcurl to make forward progress (it
will sometimes rely on being called back on the timeout if there are
no file descriptors registered). Implement the callback, and use a
QEMU AIO timer to ensure we prod libcurl again when it asks us to.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This fixes the problem I was seeing where trying to use the curl block
backend just hung. I'm not sure whether all libcurl versions that provide
the timer callback API require its use, but it shouldn't hurt.

This is probably a candidate for stable if it passes code review.

 block/curl.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index a603936..3c4c5fc 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -34,6 +34,11 @@
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
+#if LIBCURL_VERSION_NUM >= 0x071000
+/* The multi interface timer callback was introduced in 7.16.0 */
+#define NEED_CURL_TIMER_CALLBACK
+#endif
+
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
                    CURLPROTO_FTP | CURLPROTO_FTPS | \
                    CURLPROTO_TFTP)
@@ -77,6 +82,7 @@ typedef struct CURLState
 
 typedef struct BDRVCURLState {
     CURLM *multi;
+    QEMUTimer timer;
     size_t len;
     CURLState states[CURL_NUM_STATES];
     char *url;
@@ -87,6 +93,23 @@ typedef struct BDRVCURLState {
 static void curl_clean_state(CURLState *s);
 static void curl_multi_do(void *arg);
 
+#ifdef NEED_CURL_TIMER_CALLBACK
+static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
+{
+    BDRVCURLState *s = opaque;
+
+    DPRINTF("CURL: timer callback timeout_ms %ld\n", timeout_ms);
+    if (timeout_ms == -1) {
+        timer_del(&s->timer);
+    } else {
+        int64_t timeout_ns = (int64_t)timeout_ms * 1000 * 1000;
+        timer_mod(&s->timer,
+                  qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ns);
+    }
+    return 0;
+}
+#endif
+
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
                         void *s, void *sp)
 {
@@ -473,12 +496,20 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     curl_easy_cleanup(state->curl);
     state->curl = NULL;
 
+    aio_timer_init(bdrv_get_aio_context(bs), &s->timer,
+                   QEMU_CLOCK_REALTIME, SCALE_NS,
+                   curl_multi_do, s);
+
     // Now we know the file exists and its size, so let's
     // initialize the multi interface!
 
     s->multi = curl_multi_init();
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
+#ifdef NEED_CURL_TIMER_CALLBACK
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
+#endif
     curl_multi_do(s);
 
     qemu_opts_del(opts);
@@ -597,6 +628,9 @@ static void curl_close(BlockDriverState *bs)
     }
     if (s->multi)
         curl_multi_cleanup(s->multi);
+
+    timer_del(&s->timer);
+
     g_free(s->url);
 }
 
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Richard W.M. Jones]

On Wed, Jan 15, 2014 at 05:23:58PM +0000, Peter Maydell wrote:
> libcurl versions 7.16.0 and later have a timer callback interface which
> must be implemented in order for libcurl to make forward progress (it
> will sometimes rely on being called back on the timeout if there are
> no file descriptors registered). Implement the callback, and use a
> QEMU AIO timer to ensure we prod libcurl again when it asks us to.

Somewhat off topic, but does the curl driver work for you at all?
Especially if you have a disk image on a remote server, but one with
relatively low latency (eg. on a LAN but not localhost).

We have had endless problems with it, including upstream discussions
with curl people, summarised in this bug:

https://bugzilla.redhat.com/show_bug.cgi?id=971790

Just now I tried the latest qemu from git with and without your patch
and still cannot make it work, although the bugginess is now different
from RHBZ#971790.  I opened a new bug about what I'm seeing today
(https://bugs.launchpad.net/qemu/+bug/1269606).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Peter Maydell]

On 15 January 2014 21:37, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Wed, Jan 15, 2014 at 05:23:58PM +0000, Peter Maydell wrote:
>> libcurl versions 7.16.0 and later have a timer callback interface which
>> must be implemented in order for libcurl to make forward progress (it
>> will sometimes rely on being called back on the timeout if there are
>> no file descriptors registered). Implement the callback, and use a
>> QEMU AIO timer to ensure we prod libcurl again when it asks us to.
>
> Somewhat off topic, but does the curl driver work for you at all?
> Especially if you have a disk image on a remote server, but one with
> relatively low latency (eg. on a LAN but not localhost).

I haven't attempted to use it in anger. I just found that it was hanging
completely when I tried a test case for the modules code, investigated
a bit, found that libcurl requires this timer callback, implemented it
and determined that my test case at least started to boot from the remote
image.

> We have had endless problems with it, including upstream discussions
> with curl people, summarised in this bug:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=971790

Looking through the thread from upstream, I see they basically
said "you need to implement the timer callback" :-)

> Just now I tried the latest qemu from git with and without your patch
> and still cannot make it work, although the bugginess is now different
> from RHBZ#971790.  I opened a new bug about what I'm seeing today
> (https://bugs.launchpad.net/qemu/+bug/1269606).

This should be relatively easy to debug, because that "Error opening file"
case happens very early on, in curl_open(), before we even try to use the
curl-multi interface or do anything event driven or AIO based, only for
the case where something went wrong during the initial "HEAD"
operation to get the size and confirm the server supports HTTP ranges.
You should be able to just throw a pile of debugging at curl_open() to
see what's actually failing.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Paolo Bonzini]

Il 15/01/2014 18:23, Peter Maydell ha scritto:
> libcurl versions 7.16.0 and later have a timer callback interface which
> must be implemented in order for libcurl to make forward progress (it
> will sometimes rely on being called back on the timeout if there are
> no file descriptors registered). Implement the callback, and use a
> QEMU AIO timer to ensure we prod libcurl again when it asks us to.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This fixes the problem I was seeing where trying to use the curl block
> backend just hung. I'm not sure whether all libcurl versions that provide
> the timer callback API require its use, but it shouldn't hurt.

It still hangs here, but the adding the following patch on top fixes
curl on Fedora for me!

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/block/curl.c b/block/curl.c
index 5238961..e0cf138 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -232,20 +232,10 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
     return FIND_RET_NONE;
 }
 
-static void curl_multi_do(void *arg)
+static void curl_multi_read(BDRVCURLState *s)
 {
-    BDRVCURLState *s = (BDRVCURLState *)arg;
-    int running;
-    int r;
     int msgs_in_queue;
 
-    if (!s->multi)
-        return;
-
-    do {
-        r = curl_multi_socket_all(s->multi, &running);
-    } while(r == CURLM_CALL_MULTI_PERFORM);
-
     /* Try to find done transfers, so we can free the easy
      * handle again. */
     do {
@@ -289,6 +279,37 @@ static void curl_multi_do(void *arg)
     } while(msgs_in_queue);
 }
 
+static void curl_multi_do(void *arg)
+{
+    BDRVCURLState *s = (BDRVCURLState *)arg;
+    int running;
+    int r;
+
+    if (!s->multi) {
+        return;
+    }
+
+    do {
+        r = curl_multi_socket_all(s->multi, &running);
+    } while(r == CURLM_CALL_MULTI_PERFORM);
+
+    curl_multi_read(s);
+}
+
+static void curl_multi_timeout_do(void *arg)
+{
+    BDRVCURLState *s = (BDRVCURLState *)arg;
+    int running;
+
+    if (!s->multi) {
+        return;
+    }
+
+    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
+
+    curl_multi_read(s);
+}
+
 static CURLState *curl_init_state(BDRVCURLState *s)
 {
     CURLState *state = NULL;
@@ -498,7 +519,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
 
     aio_timer_init(bdrv_get_aio_context(bs), &s->timer,
                    QEMU_CLOCK_REALTIME, SCALE_NS,
-                   curl_multi_do, s);
+                   curl_multi_timeout_do, s);
 
     // Now we know the file exists and its size, so let's
     // initialize the multi interface!

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Peter Maydell]

On 15 January 2014 22:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 15/01/2014 18:23, Peter Maydell ha scritto:
>> libcurl versions 7.16.0 and later have a timer callback interface which
>> must be implemented in order for libcurl to make forward progress (it
>> will sometimes rely on being called back on the timeout if there are
>> no file descriptors registered). Implement the callback, and use a
>> QEMU AIO timer to ensure we prod libcurl again when it asks us to.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> This fixes the problem I was seeing where trying to use the curl block
>> backend just hung. I'm not sure whether all libcurl versions that provide
>> the timer callback API require its use, but it shouldn't hurt.
>
> It still hangs here, but the adding the following patch on top fixes
> curl on Fedora for me!

Hrm. I wonder why I didn't need to do this bit...

> +    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);

The libcurl docs say "This function was added in libcurl 7.15.4, and
 is deemed stable since 7.16.0. " So if we want to keep supporting
pre-7.16 libcurl then we need to retain the multi_socket_all codepath.

On the other hand 7.16 was released in October 2006. What's
the oldest version we actually care about?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Paolo Bonzini]

Il 15/01/2014 23:15, Peter Maydell ha scritto:
> 
>> > +    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
> The libcurl docs say "This function was added in libcurl 7.15.4, and
>  is deemed stable since 7.16.0. " So if we want to keep supporting
> pre-7.16 libcurl then we need to retain the multi_socket_all codepath.
> 
> On the other hand 7.16 was released in October 2006. What's
> the oldest version we actually care about?

I say 7.16 :)

Paolo

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Paolo Bonzini]

Il 15/01/2014 22:56, Peter Maydell ha scritto:
> > We have had endless problems with it, including upstream discussions
> > with curl people, summarised in this bug:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=971790
> 
> Looking through the thread from upstream, I see they basically
> said "you need to implement the timer callback" :-)

Which was hard to do only 6 months ago.  But now that Alex Bligh
implemented all the infrastructure for aio_timer_init, it is fairly easy
as Peter's patch and mine show.

Paolo

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Richard W.M. Jones]

On Wed, Jan 15, 2014 at 11:06:23PM +0100, Paolo Bonzini wrote:
> Il 15/01/2014 18:23, Peter Maydell ha scritto:
> > libcurl versions 7.16.0 and later have a timer callback interface which
> > must be implemented in order for libcurl to make forward progress (it
> > will sometimes rely on being called back on the timeout if there are
> > no file descriptors registered). Implement the callback, and use a
> > QEMU AIO timer to ensure we prod libcurl again when it asks us to.
> > 
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > This fixes the problem I was seeing where trying to use the curl block
> > backend just hung. I'm not sure whether all libcurl versions that provide
> > the timer callback API require its use, but it shouldn't hurt.
> 
> It still hangs here, but the adding the following patch on top fixes
> curl on Fedora for me!

Can you share what exact test you're doing?

Because with both patches, this still fails for me.  My test is below.

Rich.

(1) Apache with cirros disk image.  Tried both remote Apache
and localhost Apache.

(2) Locally compiled qemu + both patches.

(3) Tried with Fedora curl and upstream curl from git.

(4) Run:

http_proxy= \
LIBGUESTFS_BACKEND=direct \
LIBGUESTFS_HV=/home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64 \
guestfish -v --ro -a http://127.0.0.1/%7erjones/cirros-0.3.1-x86_64-disk.img run

(5) Output:

libguestfs: launch: program=guestfish
libguestfs: launch: version=1.25.20fedora=21,release=1.fc21,libvirt
libguestfs: launch: backend registered: unix
libguestfs: launch: backend registered: uml
libguestfs: launch: backend registered: libvirt
libguestfs: launch: backend registered: direct
libguestfs: launch: backend=direct
libguestfs: launch: tmpdir=/tmp/libguestfsMqoJ8L
libguestfs: launch: umask=0002
libguestfs: launch: euid=1000
libguestfs: command: run: /usr/bin/supermin-helper
libguestfs: command: run: \ --verbose
libguestfs: command: run: \ -f checksum
libguestfs: command: run: \ --host-cpu x86_64
libguestfs: command: run: \ /usr/lib64/guestfs/supermin.d
supermin helper [00000ms] whitelist = (not specified)
supermin helper [00000ms] host_cpu = x86_64
supermin helper [00000ms] dtb_wildcard = (not specified)
supermin helper [00000ms] inputs:
supermin helper [00000ms] inputs[0] = /usr/lib64/guestfs/supermin.d
supermin helper [00000ms] outputs:
supermin helper [00000ms] kernel = (none)
supermin helper [00000ms] dtb = (none)
supermin helper [00000ms] initrd = (none)
supermin helper [00000ms] appliance = (none)
checking modpath /lib/modules/3.12.5-302.fc20.x86_64 is a directory
checking modpath /lib/modules/3.11.9-200.fc19.x86_64 is a directory
checking modpath /lib/modules/3.11.10-200.fc19.x86_64 is a directory
checking modpath /lib/modules/3.11.4-201.fc19.x86_64 is a directory
picked kernel vmlinuz-3.12.5-302.fc20.x86_64
supermin helper [00000ms] finished creating kernel
supermin helper [00000ms] visiting /usr/lib64/guestfs/supermin.d
supermin helper [00000ms] visiting /usr/lib64/guestfs/supermin.d/base.img.gz
supermin helper [00000ms] visiting /usr/lib64/guestfs/supermin.d/daemon.img.gz
supermin helper [00000ms] visiting /usr/lib64/guestfs/supermin.d/hostfiles
supermin helper [00039ms] visiting /usr/lib64/guestfs/supermin.d/init.img
supermin helper [00039ms] visiting /usr/lib64/guestfs/supermin.d/udev-rules.img
supermin helper [00039ms] adding kernel modules
supermin helper [00059ms] finished creating appliance
libguestfs: checksum of existing appliance: 2017df18eaeee7c45b87139c9bd80be2216d655a1513322c47f58a7a3668cd1f
libguestfs: [00105ms] begin testing qemu features
libguestfs: command: run: /home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64
libguestfs: command: run: \ -display none
libguestfs: command: run: \ -help
libguestfs: command: run: /home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64
libguestfs: command: run: \ -display none
libguestfs: command: run: \ -version
libguestfs: qemu version 1.7
libguestfs: command: run: /home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64
libguestfs: command: run: \ -display none
libguestfs: command: run: \ -machine accel=kvm:tcg
libguestfs: command: run: \ -device ?
libguestfs: [00183ms] finished testing qemu features
[00189ms] /home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64 \
    -global virtio-blk-pci.scsi=off \
    -nodefconfig \
    -enable-fips \
    -nodefaults \
    -display none \
    -machine accel=kvm:tcg \
    -cpu host,+kvmclock \
    -m 500 \
    -no-reboot \
    -no-hpet \
    -kernel /var/tmp/.guestfs-1000/kernel.2215 \
    -initrd /var/tmp/.guestfs-1000/initrd.2215 \
    -device virtio-scsi-pci,id=scsi \
    -drive file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,snapshot=on,cache=writeback,id=hd0,if=none \
    -device scsi-hd,drive=hd0 \
    -drive file=/var/tmp/.guestfs-1000/root.2215,snapshot=on,id=appliance,cache=unsafe,if=none \
    -device scsi-hd,drive=appliance \
    -device virtio-serial-pci \
    -serial stdio \
    -device sga \
    -chardev socket,path=/tmp/libguestfsMqoJ8L/guestfsd.sock,id=channel0 \
    -device virtserialport,chardev=channel0,name=org.libguestfs.channel.0 \
    -append 'panic=1 console=ttyS0 udevtimeout=600 no_timer_check acpi=off printk.time=1 cgroup_disable=memory root=/dev/sdb selinux=0 guestfs_verbose=1 TERM=xterm-256color'
CURL: Opening http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img
* About to connect() to 127.0.0.1 port 80 (#0)
*   Trying 127.0.0.1...
* Adding handle: conn: 0x7f29f02d3380
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x7f29f02d3380) send_pipe: 1, recv_pipe: 0
* Connected to 127.0.0.1 (127.0.0.1) port 80 (#0)
> HEAD /~rjones/cirros-0.3.1-x86_64-disk.img HTTP/1.1
Host: 127.0.0.1
Accept: */*

< HTTP/1.1 200 OK
< Date: Thu, 16 Jan 2014 09:10:19 GMT
* Server Apache/2.4.6 (Fedora) PHP/5.5.7 mod_wsgi/3.4 Python/2.7.5 is not blacklisted
< Server: Apache/2.4.6 (Fedora) PHP/5.5.7 mod_wsgi/3.4 Python/2.7.5
< Last-Modified: Thu, 16 Jan 2014 09:06:27 GMT
< ETag: "c89e00-4f012bd9965f4"
< Accept-Ranges: bytes
< Content-Length: 13147648
< Content-Type: application/octet-stream
< 
* Connection #0 to host 127.0.0.1 left intact
CURL: Size = 13147648
CURL (AIO): Reading 2048 at 0 (0-264191)
CURL: timer callback timeout_ms 1
* About to connect() to 127.0.0.1 port 80 (#1)
*   Trying 127.0.0.1...
* Adding handle: conn: 0x7f29f02cf2e0
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 1 (0x7f29f02cf2e0) send_pipe: 1, recv_pipe: 0
* Connected to 127.0.0.1 (127.0.0.1) port 80 (#1)
> GET /~rjones/cirros-0.3.1-x86_64-disk.img HTTP/1.1
Range: bytes=0-264191
Host: 127.0.0.1
Accept: */*

CURL: timer callback timeout_ms 1
CURL (AIO): Sock action 1 on fd 9
< HTTP/1.1 206 Partial Content
< Date: Thu, 16 Jan 2014 09:10:19 GMT
* Server Apache/2.4.6 (Fedora) PHP/5.5.7 mod_wsgi/3.4 Python/2.7.5 is not blacklisted
< Server: Apache/2.4.6 (Fedora) PHP/5.5.7 mod_wsgi/3.4 Python/2.7.5
< Last-Modified: Thu, 16 Jan 2014 09:06:27 GMT
< ETag: "c89e00-4f012bd9965f4"
< Accept-Ranges: bytes
< Content-Length: 264192
< Content-Range: bytes 0-264191/13147648
< Content-Type: application/octet-stream
< 
CURL: Just reading 16046 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 16384 bytes
CURL: Just reading 2386 bytes
* Connection #1 to host 127.0.0.1 left intact
CURL (AIO): Sock action 4 on fd 9
CURL: timer callback timeout_ms -1
CURL: Close
qemu-system-x86_64: -drive file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,snapshot=on,cache=writeback,id=hd0,if=none: could not open disk image http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img: Could not open backing file: Could not open 'http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img': No such file or directory
libguestfs: error: appliance closed the connection unexpectedly, see earlier error messages
libguestfs: child_cleanup: 0x7f1df123fd90: child process died
libguestfs: sending SIGTERM to process 2224
libguestfs: error: /home/rjones/d/qemu/x86_64-softmmu/qemu-system-x86_64 exited with error status 1, see debug messages above
libguestfs: error: guestfs_launch failed, see earlier error messages
libguestfs: closing guestfs handle 0x7f1df123fd90 (state 0)
libguestfs: command: run: rm
libguestfs: command: run: \ -rf /tmp/libguestfsMqoJ8L

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Richard W.M. Jones]

On further investigation, the "No such file or directory" error occurs
when using snapshot=on.

ie:

This fails:

./x86_64-softmmu/qemu-system-x86_64 -drive 'file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,if=virtio,snapshot=on'

This works:

./x86_64-softmmu/qemu-system-x86_64 -drive 'file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,if=virtio,readonly=on'

Also, creating a qcow2 file which has the http://... path as its
backing file also works (which should have the same effect as
snapshot=on, but I understand the implementation is a bit different).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Kevin Wolf]

Am 16.01.2014 um 10:24 hat Richard W.M. Jones geschrieben:
> 
> On further investigation, the "No such file or directory" error occurs
> when using snapshot=on.
> 
> ie:
> 
> This fails:
> 
> ./x86_64-softmmu/qemu-system-x86_64 -drive 'file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,if=virtio,snapshot=on'
> 
> This works:
> 
> ./x86_64-softmmu/qemu-system-x86_64 -drive 'file=http://127.0.0.1/~rjones/cirros-0.3.1-x86_64-disk.img,if=virtio,readonly=on'

Whoops, sorry, I think it was me who broke this when I rewrote
snapshot=on support to fit better in the whole blockdev-add
infrastructure (commit 9fd3171a).

The reason is that instead of writing the file name into the backing
file header field of the temporary image, it now uses the file.filename
runtime option, which doesn't parse protocol names from the filename.

I believe Max was going to look into some related issues (basically how
to get rid of the "magic" filenames in the core code without breaking
compatibility), perhaps a fix comes out naturally from such changes.

Workaround for now: Add an explicit file.driver=http

Kevin

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Peter Maydell]

On 16 January 2014 08:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 15/01/2014 23:15, Peter Maydell ha scritto:
>>
>>> > +    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
>> The libcurl docs say "This function was added in libcurl 7.15.4, and
>>  is deemed stable since 7.16.0. " So if we want to keep supporting
>> pre-7.16 libcurl then we need to retain the multi_socket_all codepath.
>>
>> On the other hand 7.16 was released in October 2006. What's
>> the oldest version we actually care about?
>
> I say 7.16 :)

What dos RHEL5 ship? That's usually our benchmark for
"oldest thing we need to support". Ubuntu 10.04 LTS (lucid)
and Debian oldstable (squeeze) both ship something more
recent than 7.16, so we're OK there.

We should probably update the configure test to check for
curl_multi_socket_action() rather than curl_multi_setopt().

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Paolo Bonzini]

Il 16/01/2014 10:55, Peter Maydell ha scritto:
> On 16 January 2014 08:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 15/01/2014 23:15, Peter Maydell ha scritto:
>>>
>>>>> +    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
>>> The libcurl docs say "This function was added in libcurl 7.15.4, and
>>>  is deemed stable since 7.16.0. " So if we want to keep supporting
>>> pre-7.16 libcurl then we need to retain the multi_socket_all codepath.
>>>
>>> On the other hand 7.16 was released in October 2006. What's
>>> the oldest version we actually care about?
>>
>> I say 7.16 :)
> 
> What dos RHEL5 ship? That's usually our benchmark for
> "oldest thing we need to support". Ubuntu 10.04 LTS (lucid)
> and Debian oldstable (squeeze) both ship something more
> recent than 7.16, so we're OK there.
> 
> We should probably update the configure test to check for
> curl_multi_socket_action() rather than curl_multi_setopt().

It ships 7.15.5.  But curl_multi_socket_action is used only if there is
a timeouts, and curl_multi_timeout_do will never be called before
7.16.0.  Your patch calls aio_timer_init unconditionally, but the timer
will never be activated with timer_mod (which I think is a fine thing to
do).

Paolo

[Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Paolo Bonzini]

From: Peter Maydell <peter.maydell@linaro.org>

libcurl versions 7.16.0 and later have a timer callback interface which
must be implemented in order for libcurl to make forward progress (it
will sometimes rely on being called back on the timeout if there are
no file descriptors registered). Implement the callback, and use a
QEMU AIO timer to ensure we prod libcurl again when it asks us to.

Based on Peter's original patch plus my fix to add curl_multi_timeout_do.
Should compile just fine even on older versions of libcurl.

I also tried copy-on-read and streaming:

    $ ./qemu-img create -f qcow2 -o \
         backing_file=http://download.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso \
         foo.qcow2 1G
    $ x86_64-softmmu/qemu-system-x86_64 \
         -drive if=none,file=foo.qcow2,copy-on-read=on,id=cd \
         -device ide-cd,drive=cd --enable-kvm -m 1024

Direct http usage is probably too slow, but with copy-on-read ultimately
the image does boot!

After some time, streaming gets canceled by an EIO, which needs further
investigation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/curl.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 11 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index a603936..a807584 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -34,6 +34,11 @@
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
+#if LIBCURL_VERSION_NUM >= 0x071000
+/* The multi interface timer callback was introduced in 7.16.0 */
+#define NEED_CURL_TIMER_CALLBACK
+#endif
+
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
                    CURLPROTO_FTP | CURLPROTO_FTPS | \
                    CURLPROTO_TFTP)
@@ -77,6 +82,7 @@ typedef struct CURLState
 
 typedef struct BDRVCURLState {
     CURLM *multi;
+    QEMUTimer timer;
     size_t len;
     CURLState states[CURL_NUM_STATES];
     char *url;
@@ -87,6 +93,23 @@ typedef struct BDRVCURLState {
 static void curl_clean_state(CURLState *s);
 static void curl_multi_do(void *arg);
 
+#ifdef NEED_CURL_TIMER_CALLBACK
+static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
+{
+    BDRVCURLState *s = opaque;
+
+    DPRINTF("CURL: timer callback timeout_ms %ld\n", timeout_ms);
+    if (timeout_ms == -1) {
+        timer_del(&s->timer);
+    } else {
+        int64_t timeout_ns = (int64_t)timeout_ms * 1000 * 1000;
+        timer_mod(&s->timer,
+                  qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ns);
+    }
+    return 0;
+}
+#endif
+
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
                         void *s, void *sp)
 {
@@ -209,20 +232,10 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
     return FIND_RET_NONE;
 }
 
-static void curl_multi_do(void *arg)
+static void curl_multi_read(BDRVCURLState *s)
 {
-    BDRVCURLState *s = (BDRVCURLState *)arg;
-    int running;
-    int r;
     int msgs_in_queue;
 
-    if (!s->multi)
-        return;
-
-    do {
-        r = curl_multi_socket_all(s->multi, &running);
-    } while(r == CURLM_CALL_MULTI_PERFORM);
-
     /* Try to find done transfers, so we can free the easy
      * handle again. */
     do {
@@ -266,6 +279,41 @@ static void curl_multi_do(void *arg)
     } while(msgs_in_queue);
 }
 
+static void curl_multi_do(void *arg)
+{
+    BDRVCURLState *s = (BDRVCURLState *)arg;
+    int running;
+    int r;
+
+    if (!s->multi) {
+        return;
+    }
+
+    do {
+        r = curl_multi_socket_all(s->multi, &running);
+    } while(r == CURLM_CALL_MULTI_PERFORM);
+
+    curl_multi_read(s);
+}
+
+static void curl_multi_timeout_do(void *arg)
+{
+#ifdef NEED_CURL_TIMER_CALLBACK
+    BDRVCURLState *s = (BDRVCURLState *)arg;
+    int running;
+
+    if (!s->multi) {
+        return;
+    }
+
+    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
+
+    curl_multi_read(s);
+#else
+    abort();
+#endif
+}
+
 static CURLState *curl_init_state(BDRVCURLState *s)
 {
     CURLState *state = NULL;
@@ -473,12 +521,20 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     curl_easy_cleanup(state->curl);
     state->curl = NULL;
 
+    aio_timer_init(bdrv_get_aio_context(bs), &s->timer,
+                   QEMU_CLOCK_REALTIME, SCALE_NS,
+                   curl_multi_timeout_do, s);
+
     // Now we know the file exists and its size, so let's
     // initialize the multi interface!
 
     s->multi = curl_multi_init();
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
+#ifdef NEED_CURL_TIMER_CALLBACK
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
+#endif
     curl_multi_do(s);
 
     qemu_opts_del(opts);
@@ -597,6 +653,9 @@ static void curl_close(BlockDriverState *bs)
     }
     if (s->multi)
         curl_multi_cleanup(s->multi);
+
+    timer_del(&s->timer);
+
     g_free(s->url);
 }
 
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH] block/curl: Implement the libcurl timer callback interface

[Kevin Wolf]

Am 24.01.2014 um 14:56 hat Paolo Bonzini geschrieben:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> libcurl versions 7.16.0 and later have a timer callback interface which
> must be implemented in order for libcurl to make forward progress (it
> will sometimes rely on being called back on the timeout if there are
> no file descriptors registered). Implement the callback, and use a
> QEMU AIO timer to ensure we prod libcurl again when it asks us to.
> 
> Based on Peter's original patch plus my fix to add curl_multi_timeout_do.
> Should compile just fine even on older versions of libcurl.
> 
> I also tried copy-on-read and streaming:
> 
>     $ ./qemu-img create -f qcow2 -o \
>          backing_file=http://download.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso \
>          foo.qcow2 1G
>     $ x86_64-softmmu/qemu-system-x86_64 \
>          -drive if=none,file=foo.qcow2,copy-on-read=on,id=cd \
>          -device ide-cd,drive=cd --enable-kvm -m 1024
> 
> Direct http usage is probably too slow, but with copy-on-read ultimately
> the image does boot!
> 
> After some time, streaming gets canceled by an EIO, which needs further
> investigation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied to the block branch.

Kevin