From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M23f0-0000if-HO for qemu-devel@nongnu.org; Thu, 07 May 2009 09:29:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M23ev-0000hG-PG for qemu-devel@nongnu.org; Thu, 07 May 2009 09:29:14 -0400 Received: from [199.232.76.173] (port=33343 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M23ev-0000hD-JX for qemu-devel@nongnu.org; Thu, 07 May 2009 09:29:09 -0400 Received: from qw-out-1920.google.com ([74.125.92.144]:59354) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M23ev-00008D-5O for qemu-devel@nongnu.org; Thu, 07 May 2009 09:29:09 -0400 Received: by qw-out-1920.google.com with SMTP id 4so508648qwk.4 for ; Thu, 07 May 2009 06:29:08 -0700 (PDT) Message-ID: <4A02E221.4030209@codemonkey.ws> Date: Thu, 07 May 2009 08:29:05 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1241702428-26376-1-git-send-email-alex@csgraf.de> In-Reply-To: <1241702428-26376-1-git-send-email-alex@csgraf.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] Add HTTP protocol using curl v4 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: alex@csgraf.de Cc: nolan@sigbus.net, qemu-devel@nongnu.org, Alexander Graf alex@csgraf.de wrote: > From: Alexander Graf > > Currently Qemu can read from posix I/O and NBD. This patch adds a > third protocol to the game: HTTP. > > In certain situations it can be useful to access HTTP data directly, > for example if you want to try out an http provided OS image, but > don't know if you want to download it yet. > > Using this patch you can now try it on on the fly. Just use it like: > > qemu -cdrom http://host/path/my.iso > > In order to not reinvent the wheel, this patch uses libcurl. > > v2 changes: > > - fix the segfault (yay!) > - implement AIO > > v3 changes: > > - remove synchronous API > - implement caching > > v4 changes: > > - enable other protocols (HTTPS, FTP, FTPS, TFTP, SFTP, SCP) > (I only tested FTP so far, but they _should_ work) > So now all of the references to http are a big fat lie? :-) Perhaps we should s/http/curl/g. > diff --git a/block-http.c b/block-http.c > new file mode 100644 > index 0000000..3045cbb > --- /dev/null > +++ b/block-http.c > @@ -0,0 +1,534 @@ > +/* > + * QEMU Block driver for HTTP images > + * > + * Copyright (c) 2009 Alexander Graf > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > +#include "qemu-common.h" > +#include "block_int.h" > +#include > + > +#define DEBUG > +#define DEBUG_VERBOSE > + > +#ifdef DEBUG > +#define printd printf > +#else > +#define printd(x, ...) > +#endif > I'd prefer dprintf(). Also, the standard way to do this is: //#define DEBUG_CURL #ifdef DEBUG_CURL #define dprintf(fmt, ...) do { printf(fmt, ## __VA_ARGS__); } while (0) #else #define dprintf(fmt, ...) do { } while (0) #endif Also, make sure DEBUG_CURL is off by default (you have debug enabled in this patch). > +#define HTTP_NUM_STATES 8 > + > +static size_t http_size_cb(void *ptr, size_t size, size_t nmemb, void *opaque) > +{ > + HTTPState *s = ((HTTPState*)opaque); > + size_t realsize = size * nmemb; > + uint64_t fsize; > + > + if(sscanf(ptr, "Content-Length: %lld", &fsize) == 1) > + s->s->len = fsize; > How does this work with other protocols? Certainly, there isn't a Content-Length header with scp? > +static int http_find_buf(BDRVHTTPState *s, long long start, long long len, > + HTTPAIOCB *acb) > +{ > + int i; > + long long end = start + len; > size_t is the right type to represent lengths/offsets in a memory buffer. > +static void http_close(BlockDriverState *bs) > +{ > + BDRVHTTPState *s = bs->opaque; > + int i; > You strdup() url but don't free it. I'd suggest building qemu-io and running it under valgrind to verify that you aren't leaking other places too. > +BlockDriver bdrv_http = { > + .format_name = "http", > + .protocol_name = "http", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_https = { > + .format_name = "https", > + .protocol_name = "https", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_ftp = { > + .format_name = "ftp", > + .protocol_name = "ftp", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_ftps = { > + .format_name = "ftps", > + .protocol_name = "ftps", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_sftp = { > + .format_name = "sftp", > + .protocol_name = "sftp", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_scp = { > + .format_name = "scp", > + .protocol_name = "scp", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > + > +BlockDriver bdrv_tftp = { > + .format_name = "tftp", > + .protocol_name = "tftp", > + > + .instance_size = sizeof(BDRVHTTPState), > + .bdrv_open = http_open, > + .bdrv_close = http_close, > + .bdrv_getlength = http_getlength, > + > + .aiocb_size = sizeof(HTTPAIOCB), > + .bdrv_aio_readv = http_aio_readv, > + .bdrv_aio_cancel = http_aio_cancel, > +}; > It's rather slick to get all of these for free. Regards, Anthony Liguori