| Message ID | 52D7065F.9020203@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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.
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
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
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
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!